[Qemu-devel] [STAGING]: Block migration segfaults

[Luiz Capitulino <lcapitulino@redhat.com>]

 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.

 You probably didn't note it before because the lowest-level monitor
print function would just return if the 'mon' parameter was NULL.

 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'?

 Anyways, the following patch adds the old behavior back just in case
you want to see it working...

commit 3575196202d4e54c1fc63a631ca5bd1a7778e30d
Author: Luiz Capitulino <lcapitulino@redhat.com>
Date:   Thu Dec 3 15:49:16 2009 -0200

    monitor: Fix block migration segfault
    
    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 <lcapitulino@redhat.com>

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)