From 9af6c67d89eeeae941ceab1cb16a8bf320680243 Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Thu, 7 Nov 2013 13:56:40 +0100 Subject: [PATCH 2/2] monitor: monitor_puts(): bail-out when mon=NULL RH-Author: Luiz Capitulino Message-id: <20131107085640.018867f3@redhat.com> Patchwork-id: 55602 O-Subject: [RHEL-6.5 qemu-kvm PATCH 0day] monitor: monitor_puts(): bail-out when mon=NULL Bugzilla: 1015979 RH-Acked-by: Paolo Bonzini RH-Acked-by: Laszlo Ersek RH-Acked-by: Orit Wasserman RH-Acked-by: Amit Shah Bugzilla: 1015979 Try this with latest qemu-kvm-rhel6: 1. Start a destination guest for migration: ./qemu-rhel6 [...] -incoming tcp:0:4444 2. From a running guest, try to migrate w/o shared storage: (qemu) migrate -d -b tcp:0:4444 Segmentation fault GDB says: Core was generated by `./qemu-rhel6 -drive file=disks/test-ide.img,if=ide,cache=writeback,aio=native,b'. Program terminated with signal 11, Segmentation fault. #0 monitor_flush (mon=mon@entry=0x0) at /home/lcapitulino/work/src/rh-internal/qemu-kvm-rhel6/monitor.c:283 283 buf = qstring_get_str(mon->outbuf); This is caused by the following sequence: 1. tcp_start_outgoing_migration() sets (FDMigrateState *) s->mon=NULL 2. When the target connects to the destination, we run: tcp_wait_for_connect() migrate_fd_connect(s) qemu_savevm_state_begin(s->mon, ...) block_save_live(s->mon) blk_mig_save_bulked_block(mon=NULL) monitor_printf(mon) monitor_flush(mon) (NOTE: mon=NULL for all calls due to step 1) The call to monitor_printf() is fine, because monitor_printf() checks for !mon and bails-out. monitor_flush() used to do it too but this was changed by commit 5f66f886b8412dc6976ee49327121590232eb8e2. Now the first thing monitor_flush() does is: buf = qstring_get_str(mon->outbuf); Kaput. There are two ways to fix this: 1. Zap the 'mon' object from migration code. This is what was done upstream by commit 539de1246d355d3b8aa33fb7cde732352d8827c7 2. Add the check for !mon back to monitor_flush(), as it used to be before commit 5f66f886 Option 1 is the Right Thing, but that commit is *a* conflict magnet and I'm not entirely sure it's backportable because it was done in the context of a larger refactoring upstream (which was to separate HMP and QMP). This commit does option 2, which is quite simple, is what was done before 5f66f886, and protects us from any other possible surprise due to mon=NULL. Signed-off-by: Luiz Capitulino --- monitor.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) Signed-off-by: Miroslav Rezanina --- monitor.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/monitor.c b/monitor.c index b096c7f..34dedc3 100644 --- a/monitor.c +++ b/monitor.c @@ -280,10 +280,14 @@ void monitor_flush(Monitor *mon) size_t len; const char *buf; + if (!mon) { + return; + } + buf = qstring_get_str(mon->outbuf); len = qstring_get_length(mon->outbuf); - if (mon && len && !mon->mux_out) { + if (len && !mon->mux_out) { rc = qemu_chr_fe_write(mon->chr, (const uint8_t *) buf, len); if (rc == len) { /* all flushed */ -- 1.7.1