Basic live migration was broken by the commit that added non-shared block support in two ways: 1) It added a virCheckFlags() to doNativeMigrate(). Besides the fact that typical usage of virCheckFlags() is in driver entry points, and doNativeMigrate() is not an entry point, it was missing important flags like VIR_MIGRATE_LIVE. Move the virCheckFlags to the top-level qemuDomainMigratePrepare2 and friends. 2) It also added a memory leak in qemuMonitorTextMigrate() by not freeing the memory used by virBufferContentAndReset(). This is fixed by storing the pointer in a temporary variable and freeing it at the end. With this patch in place, normal live migration works again. v3: Instead of the churn for virCheckFlagsUI and UL, instead always promote flags to an unsigned long and always use %lx for the fprintf. v2: Add back flags check, which required adding virCheckFlagsUI and virCheckFlagsUL Signed-off-by: Chris Lalancette <clalance(a)redhat.com&gt; --- src/internal.h | 7 ++++--- src/qemu/qemu_driver.c | 30 +++++++++++++++++++++++++++--- src/qemu/qemu_monitor_text.c | 10 +++++++--- 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/src/internal.h b/src/internal.h index c3c5311..fab3e11 100644 --- a/src/internal.h +++ b/src/internal.h @@ -212,15 +212,16 @@ */ # define virCheckFlags(supported, retval) \ do { \ - if ((flags & ~(supported))) { \ + unsigned long __unsuppflags = flags & ~(supported); \ + if (__unsuppflags) { \ virReportErrorHelper(NULL, \ VIR_FROM_THIS, \ VIR_ERR_INVALID_ARG, \ __FILE__, \ __FUNCTION__, \ __LINE__, \ - _("%s: unsupported flags (0x%x)"), \ - __FUNCTION__, flags & ~(supported)); \ + _("%s: unsupported flags (0x%lx)"), \ + __FUNCTION__, __unsuppflags); \ return retval; \ } \ } while (0) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 77e71cc..941b482 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10175,6 +10175,15 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, int ret = -1; int internalret; + virCheckFlags(VIR_MIGRATE_LIVE | + VIR_MIGRATE_PEER2PEER | + VIR_MIGRATE_TUNNELLED | + VIR_MIGRATE_PERSIST_DEST | + VIR_MIGRATE_UNDEFINE_SOURCE | + VIR_MIGRATE_PAUSED | + VIR_MIGRATE_NON_SHARED_DISK | + VIR_MIGRATE_NON_SHARED_INC, -1); + *uri_out = NULL; qemuDriverLock(driver); @@ -10356,9 +10365,6 @@ static int doNativeMigrate(struct qemud_driver *driver, qemuDomainObjPrivatePtr priv = vm->privateData; unsigned int background_flags = 0; - virCheckFlags(VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC, - -1); - /* Issue the migrate command. */ if (STRPREFIX(uri, "tcp:") && !STRPREFIX(uri, "tcp://")) { /* HACK: source host generates bogus URIs, so fix them up */ @@ -10779,6 +10785,15 @@ qemudDomainMigratePerform (virDomainPtr dom, int resume = 0; qemuDomainObjPrivatePtr priv; + virCheckFlags(VIR_MIGRATE_LIVE | + VIR_MIGRATE_PEER2PEER | + VIR_MIGRATE_TUNNELLED | + VIR_MIGRATE_PERSIST_DEST | + VIR_MIGRATE_UNDEFINE_SOURCE | + VIR_MIGRATE_PAUSED | + VIR_MIGRATE_NON_SHARED_DISK | + VIR_MIGRATE_NON_SHARED_INC, -1); + qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { @@ -10882,6 +10897,15 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn, virErrorPtr orig_err; int newVM = 1; + virCheckFlags(VIR_MIGRATE_LIVE | + VIR_MIGRATE_PEER2PEER | + VIR_MIGRATE_TUNNELLED | + VIR_MIGRATE_PERSIST_DEST | + VIR_MIGRATE_UNDEFINE_SOURCE | + VIR_MIGRATE_PAUSED | + VIR_MIGRATE_NON_SHARED_DISK | + VIR_MIGRATE_NON_SHARED_INC, NULL); + /* Migration failed. Save the current error so nothing squashes it */ orig_err = virSaveLastError(); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 5511550..4b1e2ec 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -1144,6 +1144,7 @@ static int qemuMonitorTextMigrate(qemuMonitorPtr mon, int ret = -1; char *safedest = qemuMonitorEscapeArg(dest); virBuffer extra = VIR_BUFFER_INITIALIZER; + char *extrastr = NULL; if (!safedest) { virReportOOMError(); @@ -1159,10 +1160,12 @@ static int qemuMonitorTextMigrate(qemuMonitorPtr mon, if (virBufferError(&extra)) { virBufferFreeAndReset(&extra); virReportOOMError(); - free(safedest); - return -1; + goto cleanup; } - if (virAsprintf(&cmd, "migrate %s\"%s\"", virBufferContentAndReset(&extra), safedest) < 0) { + + extrastr = virBufferContentAndReset(&extra); + if (virAsprintf(&cmd, "migrate %s\"%s\"", extrastr ? extrastr : "", + safedest) < 0) { virReportOOMError(); goto cleanup; } @@ -1191,6 +1194,7 @@ static int qemuMonitorTextMigrate(qemuMonitorPtr mon, ret = 0; cleanup: + VIR_FREE(extrastr); VIR_FREE(safedest); VIR_FREE(info); VIR_FREE(cmd); -- 1.6.6.1