From patchwork Mon Aug 1 09:24:14 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= X-Patchwork-Id: 9254053 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 3021E6075F for ; Mon, 1 Aug 2016 09:26:00 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1E72D2848E for ; Mon, 1 Aug 2016 09:26:00 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 12C2C284A7; Mon, 1 Aug 2016 09:26:00 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 5746B2848E for ; Mon, 1 Aug 2016 09:25:59 +0000 (UTC) Received: from localhost ([::1]:48662 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bU9Ty-0004rx-Cj for patchwork-qemu-devel@patchwork.kernel.org; Mon, 01 Aug 2016 05:25:58 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40540) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bU9SP-0003sM-LN for qemu-devel@nongnu.org; Mon, 01 Aug 2016 05:24:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bU9SM-0004Ox-6o for qemu-devel@nongnu.org; Mon, 01 Aug 2016 05:24:21 -0400 Received: from mx4-phx2.redhat.com ([209.132.183.25]:54991) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bU9SL-0004Os-Pl for qemu-devel@nongnu.org; Mon, 01 Aug 2016 05:24:18 -0400 Received: from zmail17.collab.prod.int.phx2.redhat.com (zmail17.collab.prod.int.phx2.redhat.com [10.5.83.19]) by mx4-phx2.redhat.com (8.13.8/8.13.8) with ESMTP id u719OFro026498; Mon, 1 Aug 2016 05:24:15 -0400 Date: Mon, 1 Aug 2016 05:24:14 -0400 (EDT) From: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau To: Paolo Bonzini Message-ID: <1621526280.10219655.1470043454030.JavaMail.zimbra@redhat.com> In-Reply-To: References: <1468953718-27661-1-git-send-email-armbru@redhat.com> <1468953718-27661-3-git-send-email-armbru@redhat.com> MIME-Version: 1.0 X-Originating-IP: [10.3.116.73] X-Mailer: Zimbra 8.0.6_GA_5922 (ZimbraWebClient - FF47 (Linux)/8.0.6_GA_5922) Thread-Topic: qapi: change QmpInputVisitor to QSLIST Thread-Index: oZqQWxaTNOTpGfZVqs7ejQZWPzS6bQ== X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 209.132.183.25 Subject: Re: [Qemu-devel] [PULL v2 for-2.7 02/15] qapi: change QmpInputVisitor to QSLIST X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Laszlo Ersek , qemu-devel@nongnu.org, Markus Armbruster , Gerd Hoffmann , =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP Hi ----- Original Message ----- > > > On 27/07/2016 23:37, Laszlo Ersek wrote: > > It seems to me that QEMU deadlocks when it tries to emit the > > SPICE_DISCONNECTED event. > > > > (Note that I can't find "handle SPICE_DISCONNECTED" in the libvirtd log > > even in the successful case (i.e., when QEMU is built at the parent of > > 3d344c2aabb7).) > > > > Apparently audio_atexit() is triggered when QEMU is returning from > > main() -- or calling exit() --, which somehow results in QEMU trying to > > send a SPICE_DISCONNECTED event through the monitor? I guess the monitor > > has been long dead by then. > > > > Hmmm, this gives me an idea... What happens if I remove the following > > fragment from my domain XML? > > > > > >
> function='0x0'/> > > > > > > Yeah, the hang disappears, shutdown works just fine. It's a spice audio > > bug after all, apparently. Sorry for reporting it in this thread! :) I'm > > adding Gerd to the address list. > > > > To reiterate: this patch (commit 3d344c2aabb7) does *not* cause the > > symptom, it only exposes an independent bug that causes the symptom. > > And, I can work around that for now, by removing sound devices. > > I think the issue here is that the monitor is gone by the time > audio_atexit is called. It is caused by commit > c1111a24a3358ecd2f17be7c8b117cfe8bc5e5f8. > > The fix is to move the audio_atexit call to main before > qemu_chr_cleanup, but I'm not sure how to deal with coreaudio_atexit. alternatively, cleanup the monitor before cleaning up the chardev? I was just looking at that, see wip patch attached. From 6d8de90eaf12883a87721fa67b67cecfa9a67450 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Mon, 1 Aug 2016 13:16:55 +0400 Subject: [PATCH] monitor: fix crash when leaving qemu with spice audio MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since aa5cb7f5e, the chardevs are being cleaned up when leaving qemu. However, the monitor has still references to them, which may lead to crashes when running atexit() and trying to send monitor events: #0 0x00007fffdb18f6f5 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54 #1 0x00007fffdb1912fa in __GI_abort () at abort.c:89 #2 0x0000555555c263e7 in error_exit (err=22, msg=0x555555d47980 <__func__.13537> "qemu_mutex_lock") at util/qemu-thread-posix.c:39 #3 0x0000555555c26488 in qemu_mutex_lock (mutex=0x5555567a2420) at util/qemu-thread-posix.c:66 #4 0x00005555558c52db in qemu_chr_fe_write (s=0x5555567a2420, buf=0x55555740dc40 "{\"timestamp\": {\"seconds\": 1470041716, \"microseconds\": 989699}, \"event\": \"SPICE_DISCONNECTED\", \"data\": {\"server\": {\"port\": \"5900\", \"family\": \"ipv4\", \"host\": \"127.0.0.1\"}, \"client\": {\"port\": \"40272\", \"f"..., len=240) at qemu-char.c:280 #5 0x0000555555787cad in monitor_flush_locked (mon=0x5555567bd9e0) at /home/elmarco/src/qemu/monitor.c:311 #6 0x0000555555787e46 in monitor_puts (mon=0x5555567bd9e0, str=0x5555567a44ef "") at /home/elmarco/src/qemu/monitor.c:353 #7 0x00005555557880fe in monitor_json_emitter (mon=0x5555567bd9e0, data=0x5555567c73a0) at /home/elmarco/src/qemu/monitor.c:401 #8 0x00005555557882d2 in monitor_qapi_event_emit (event=QAPI_EVENT_SPICE_DISCONNECTED, qdict=0x5555567c73a0) at /home/elmarco/src/qemu/monitor.c:472 #9 0x000055555578838f in monitor_qapi_event_queue (event=QAPI_EVENT_SPICE_DISCONNECTED, qdict=0x5555567c73a0, errp=0x7fffffffca88) at /home/elmarco/src/qemu/monitor.c:497 #10 0x0000555555c15541 in qapi_event_send_spice_disconnected (server=0x5555571139d0, client=0x5555570d0db0, errp=0x5555566c0428 ) at qapi-event.c:1038 #11 0x0000555555b11bc6 in channel_event (event=3, info=0x5555570d6c00) at ui/spice-core.c:248 #12 0x00007fffdcc9983a in adapter_channel_event (event=3, info=0x5555570d6c00) at reds.c:120 #13 0x00007fffdcc99a25 in reds_handle_channel_event (reds=0x5555567a9d60, event=3, info=0x5555570d6c00) at reds.c:324 #14 0x00007fffdcc7d4c4 in main_dispatcher_self_handle_channel_event (self=0x5555567b28b0, event=3, info=0x5555570d6c00) at main-dispatcher.c:175 #15 0x00007fffdcc7d5b1 in main_dispatcher_channel_event (self=0x5555567b28b0, event=3, info=0x5555570d6c00) at main-dispatcher.c:194 #16 0x00007fffdcca7674 in reds_stream_push_channel_event (s=0x5555570d9910, event=3) at reds-stream.c:354 #17 0x00007fffdcca749b in reds_stream_free (s=0x5555570d9910) at reds-stream.c:323 #18 0x00007fffdccb5dad in snd_disconnect_channel (channel=0x5555576a89a0) at sound.c:229 #19 0x00007fffdccb9e57 in snd_detach_common (worker=0x555557739720) at sound.c:1589 #20 0x00007fffdccb9f0e in snd_detach_playback (sin=0x5555569fe3f8) at sound.c:1602 #21 0x00007fffdcca3373 in spice_server_remove_interface (sin=0x5555569fe3f8) at reds.c:3387 #22 0x00005555558ff6e2 in line_out_fini (hw=0x5555569fe370) at audio/spiceaudio.c:152 #23 0x00005555558f909e in audio_atexit () at audio/audio.c:1754 #24 0x00007fffdb1941e8 in __run_exit_handlers (status=0, listp=0x7fffdb5175d8 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true) at exit.c:82 #25 0x00007fffdb194235 in __GI_exit (status=) at exit.c:104 #26 0x00007fffdb17b738 in __libc_start_main (main=0x5555558d7874
, argc=67, argv=0x7fffffffcf48, init=, fini=, rtld_fini=, stack_end=0x7fffffffcf38) at ../csu/libc-start.c:323 Add a monitor_cleanup() functions to remove all the monitors before cleaning up the chardev. Note that we are "losing" some events that used to be sent during atexit(). Signed-off-by: Marc-André Lureau --- monitor.c | 20 ++++++++++++++++++++ include/monitor/monitor.h | 1 + vl.c | 1 + 3 files changed, 22 insertions(+) diff --git a/monitor.c b/monitor.c index 5d68a5d..5c00373 100644 --- a/monitor.c +++ b/monitor.c @@ -635,6 +635,13 @@ static void monitor_data_init(Monitor *mon) static void monitor_data_destroy(Monitor *mon) { + if (mon->chr) { + qemu_chr_add_handlers(mon->chr, NULL, NULL, NULL, NULL); + } + if (monitor_is_qmp(mon)) { + json_message_parser_destroy(&mon->qmp.parser); + } + g_free(mon->rs); QDECREF(mon->outbuf); qemu_mutex_destroy(&mon->out_lock); } @@ -4196,6 +4203,19 @@ void monitor_init(CharDriverState *chr, int flags) qemu_mutex_unlock(&monitor_lock); } +void monitor_cleanup(void) +{ + Monitor *mon, *next; + + qemu_mutex_lock(&monitor_lock); + QLIST_FOREACH_SAFE(mon, &mon_list, entry, next) { + QLIST_REMOVE(mon, entry); + monitor_data_destroy(mon); + g_free(mon); + } + qemu_mutex_unlock(&monitor_lock); +} + static void bdrv_password_cb(void *opaque, const char *password, void *readline_opaque) { diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h index c5c9ea2..a714d8e 100644 --- a/include/monitor/monitor.h +++ b/include/monitor/monitor.h @@ -17,6 +17,7 @@ extern Monitor *cur_mon; bool monitor_cur_is_qmp(void); void monitor_init(CharDriverState *chr, int flags); +void monitor_cleanup(void); int monitor_suspend(Monitor *mon); void monitor_resume(Monitor *mon); diff --git a/vl.c b/vl.c index e7c2c62..a14c438 100644 --- a/vl.c +++ b/vl.c @@ -4612,6 +4612,7 @@ int main(int argc, char **argv, char **envp) /* vhost-user must be cleaned up before chardevs. */ net_cleanup(); + monitor_cleanup(); qemu_chr_cleanup(); return 0; -- 2.9.0