From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NGGXQ-000368-62 for qemu-devel@nongnu.org; Thu, 03 Dec 2009 13:36:24 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NGGXL-00035Y-7C for qemu-devel@nongnu.org; Thu, 03 Dec 2009 13:36:23 -0500 Received: from [199.232.76.173] (port=37438 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NGGXL-00035V-28 for qemu-devel@nongnu.org; Thu, 03 Dec 2009 13:36:19 -0500 Received: from david.siemens.de ([192.35.17.14]:21748) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1NGGXK-0005Iw-Iy for qemu-devel@nongnu.org; Thu, 03 Dec 2009 13:36:18 -0500 Message-ID: <4B18051F.4040207@siemens.com> Date: Thu, 03 Dec 2009 19:36:15 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <20091203162121.67d9c120@doriath> In-Reply-To: <20091203162121.67d9c120@doriath> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [STAGING]: Block migration segfaults List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: "qemu-devel@nongnu.org" , "lirans@il.ibm.com" Luiz Capitulino wrote: > Hi there, > > Got this while testing block migration in staging: > > """ > Program terminated with signal 11, Segmentation fault. > #0 0x0000000000410cf9 in monitor_vprintf (mon=0x0, fmt=0x5ae5e7 "Start full migration for %s\n", > ap=0x7fff1f830a40) at /home/lcapitulino/src/aliguori-queue/monitor.c:192 > 192 if (mon->mc && !mon->mc->print_enabled) { > """ > > The problem here is that init_blk_migration() calls monitor_printf() with > a NULL 'mon' and the backtrace shows that this is true for the entire call > chain. What is the backtrace? And how did you start the migration? > > You probably didn't note it before because the lowest-level monitor > print function would just return if the 'mon' parameter was NULL. I was aware that mon might be NULL, but the existing code handled this gracefully. > > A patch from me (4a29a in staging) changes a higher level monitor > function to touch 'mon' before passing it down and here's the segfault. > > Now, the point is: I could give the old behavior back but I think we're > hiding a bug there. Why would you call monitor_printf() with a NULL 'mon'? If there is no monitor associated with the current context, it can be NULL. This is mostly the case during early startup. One may also use this to suppress output (though I don't recall any real case ATM). > > Anyways, the following patch adds the old behavior back just in case > you want to see it working... Yes, better restore the check. Still, your call stack would be interesting. Maybe there is actual another bug behind it. > > commit 3575196202d4e54c1fc63a631ca5bd1a7778e30d > Author: Luiz Capitulino > Date: Thu Dec 3 15:49:16 2009 -0200 > > monitor: Fix block migration segfault Let's call it by it true name: Catch printing to non-existent monitor. > > The monitor_vprintf() function now touches the 'mon' variable > before calling monitor_puts(), this causes block migration > to segfault as its functions call monitor_printf() with a > NULL 'mon'. > > This is probably hiding the real bug, but for some reason this > has been the behavior for a long time. > > We also change monitor_print_object() to use monitor_print(), > so that monitor_puts() is only called by monitor_vprintf(). > > Signed-off-by: Luiz Capitulino > > diff --git a/monitor.c b/monitor.c > index b035e0b..6d0b1dd 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -171,9 +171,6 @@ static void monitor_puts(Monitor *mon, const char *str) > { > char c; > > - if (!mon) > - return; > - > for(;;) { > c = *str++; > if (c == '\0') > @@ -189,6 +186,9 @@ static void monitor_puts(Monitor *mon, const char *str) > > void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) > { > + if (!mon) > + return; > + > if (mon->mc && !mon->mc->print_enabled) { > qemu_error_new(QERR_UNDEFINED_ERROR); > } else { > @@ -269,7 +269,7 @@ static void monitor_print_qobject(Monitor *mon, const QObject *data) > break; > } > > - monitor_puts(mon, "\n"); > + monitor_printf(mon, "\n"); > } > > static void monitor_json_emitter(Monitor *mon, const QObject *data) Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux