[libvirt] [PATCH] qemu: wait for SPICE to migrate

Recently, there have been some improvements made to qemu so it supports seamless migration or something very close to it. However, it requires libvirt interaction. Once qemu is migrated, the SPICE server needs to send its internal state to the destination. Once it's done, it fires SPICE_MIGRATE_COMPLETED event and this fact is advertised in 'query-spice' output as well. We must not kill qemu until SPICE server finishes the transfer. --- Okay, I am ignoring the event completely in this patch. I am just asking for 'query-spice' until after 'query-migrate' reports 'completed'. It's simple and small. However, if there is a wider consensus I should catch the event as well, e.g. so we don't have to enter the monitor and thus have smaller overhead, I can rewrite this patch. src/qemu/qemu_migration.c | 32 ++++++++++++++++++++++--- src/qemu/qemu_monitor.c | 22 +++++++++++++++++ src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 52 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 ++ 5 files changed, 108 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 99fc8ce..e4ec66d 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -891,11 +891,13 @@ static int qemuMigrationUpdateJobStatus(struct qemud_driver *driver, virDomainObjPtr vm, const char *job, - enum qemuDomainAsyncJob asyncJob) + enum qemuDomainAsyncJob asyncJob, + bool wait_for_spice) { qemuDomainObjPrivatePtr priv = vm->privateData; int ret; int status; + bool spice_migrated = true; unsigned long long memProcessed; unsigned long long memRemaining; unsigned long long memTotal; @@ -910,6 +912,13 @@ qemuMigrationUpdateJobStatus(struct qemud_driver *driver, &memProcessed, &memRemaining, &memTotal); + + /* If qemu says migrated, check spice */ + if (wait_for_spice && (ret == 0) && + (status == QEMU_MONITOR_MIGRATION_STATUS_COMPLETED)) + ret = qemuMonitorGetSpiceMigrationStatus(priv->mon, + &spice_migrated); + qemuDomainObjExitMonitorWithDriver(driver, vm); if (ret < 0 || virTimeMillisNow(&priv->job.info.timeElapsed) < 0) { @@ -939,7 +948,8 @@ qemuMigrationUpdateJobStatus(struct qemud_driver *driver, break; case QEMU_MONITOR_MIGRATION_STATUS_COMPLETED: - priv->job.info.type = VIR_DOMAIN_JOB_COMPLETED; + if (spice_migrated) + priv->job.info.type = VIR_DOMAIN_JOB_COMPLETED; ret = 0; break; @@ -967,6 +977,12 @@ qemuMigrationWaitForCompletion(struct qemud_driver *driver, virDomainObjPtr vm, { qemuDomainObjPrivatePtr priv = vm->privateData; const char *job; + bool wait_for_spice; + + /* If guest uses SPICE we have to hold up migration finish + * until SPICE server transfers its data */ + wait_for_spice = (vm->def->ngraphics == 1) && + (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE); switch (priv->job.asyncJob) { case QEMU_ASYNC_JOB_MIGRATION_OUT: @@ -988,7 +1004,8 @@ qemuMigrationWaitForCompletion(struct qemud_driver *driver, virDomainObjPtr vm, /* Poll every 50ms for progress & to allow cancellation */ struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull }; - if (qemuMigrationUpdateJobStatus(driver, vm, job, asyncJob) < 0) + if (qemuMigrationUpdateJobStatus(driver, vm, job, + asyncJob, wait_for_spice) < 0) goto cleanup; if (dconn && virConnectIsAlive(dconn) <= 0) { @@ -1840,6 +1857,7 @@ qemuMigrationRun(struct qemud_driver *driver, int fd = -1; unsigned long migrate_speed = resource ? resource : priv->migMaxBandwidth; virErrorPtr orig_err = NULL; + bool wait_for_spice; VIR_DEBUG("driver=%p, vm=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, flags=%lx, resource=%lu, " @@ -1848,6 +1866,11 @@ qemuMigrationRun(struct qemud_driver *driver, cookieout, cookieoutlen, flags, resource, spec, spec->destType, spec->fwdType); + /* If guest uses SPICE we have to hold up migration finish + * until SPICE server transfers its data */ + wait_for_spice = (vm->def->ngraphics == 1) && + (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE); + if (virLockManagerPluginUsesState(driver->lockManager) && !cookieout) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1946,7 +1969,8 @@ qemuMigrationRun(struct qemud_driver *driver, * connection from qemu which may never be initiated. */ if (qemuMigrationUpdateJobStatus(driver, vm, _("migration job"), - QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) + QEMU_ASYNC_JOB_MIGRATION_OUT, + wait_for_spice) < 0) goto cancel; while ((fd = accept(spec->dest.unix_socket.sock, NULL, NULL)) < 0) { diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b43b15e..20bb333 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1844,6 +1844,28 @@ int qemuMonitorGetMigrationStatus(qemuMonitorPtr mon, return ret; } +int qemuMonitorGetSpiceMigrationStatus(qemuMonitorPtr mon, + bool *spice_migrated) +{ + int ret; + VIR_DEBUG("mon=%p", mon); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (mon->json) { + ret = qemuMonitorJSONGetSpiceMigrationStatus(mon, spice_migrated); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return ret; +} int qemuMonitorMigrateToFd(qemuMonitorPtr mon, unsigned int flags, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f206d49..d3a0877 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -346,6 +346,9 @@ int qemuMonitorGetMigrationStatus(qemuMonitorPtr mon, unsigned long long *remaining, unsigned long long *total); +int qemuMonitorGetSpiceMigrationStatus(qemuMonitorPtr mon, + bool *spice_migrated); + typedef enum { QEMU_MONITOR_MIGRATE_BACKGROUND = 1 << 0, QEMU_MONITOR_MIGRATE_NON_SHARED_DISK = 1 << 1, /* migration with non-shared storage with full disk copy */ diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 8842817..63dbd25 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2487,6 +2487,58 @@ int qemuMonitorJSONGetMigrationStatus(qemuMonitorPtr mon, } +static int +qemuMonitorJSONSpiceGetMigrationStatusReply(virJSONValuePtr reply, + bool *spice_migrated) +{ + virJSONValuePtr ret; + const char *migrated_str; + + if (!(ret = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-spice reply was missing return data")); + return -1; + } + + if (!(migrated_str = virJSONValueObjectGetString(ret, "migrated"))) { + /* Deliberately don't report error here as we are + * probably dealing with older qemu which doesn't + * report this yet. Pretend spice is migrated. */ + *spice_migrated = true; + } else { + *spice_migrated = STREQ(migrated_str, "true"); + } + + return 0; +} + + +int qemuMonitorJSONGetSpiceMigrationStatus(qemuMonitorPtr mon, + bool *spice_migrated) +{ + int ret; + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-spice", + NULL); + virJSONValuePtr reply = NULL; + + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + if (ret == 0) + ret = qemuMonitorJSONSpiceGetMigrationStatusReply(reply, + spice_migrated); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + int qemuMonitorJSONMigrate(qemuMonitorPtr mon, unsigned int flags, const char *uri) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index f5d515f..88edb73 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -132,6 +132,9 @@ int qemuMonitorJSONGetMigrationStatus(qemuMonitorPtr mon, unsigned long long *remaining, unsigned long long *total); +int qemuMonitorJSONGetSpiceMigrationStatus(qemuMonitorPtr mon, + bool *spice_migrated); + int qemuMonitorJSONMigrate(qemuMonitorPtr mon, unsigned int flags, const char *uri); -- 1.7.8.6

On 09/13/2012 11:06 AM, Michal Privoznik wrote:
Recently, there have been some improvements made to qemu so it supports seamless migration or something very close to it. However, it requires libvirt interaction. Once qemu is migrated, the SPICE server needs to send its internal state to the destination. Once it's done, it fires SPICE_MIGRATE_COMPLETED event and this fact is advertised in 'query-spice' output as well. We must not kill qemu until SPICE server finishes the transfer. ---
Okay, I am ignoring the event completely in this patch. I am just asking for 'query-spice' until after 'query-migrate' reports 'completed'. It's simple and small. However, if there is a wider consensus I should catch the event as well, e.g. so we don't have to enter the monitor and thus have smaller overhead, I can rewrite this patch.
I think this approach is incomplete. As I understand it from the qemu side of things, qemu commit 8c95705: The seamless-migration flag is required in order to identify whether libvirt supports the new QEVENT_SPICE_MIGRATE_COMPLETED or not (by default the flag is off). New libvirt versions that wait for QEVENT_SPICE_MIGRATE_COMPLETED should turn on this flag. When this flag is off, spice fallbacks to its old migration method, which can result in data loss. That is, only new qemu supports seamless migration, but it only does so if both the sending side has been notified to send the event, and the receiving side has been told to receive in seamless mode; both sides have to be in the same mode. Therefore, I think you are missing a change to qemu_capabilities.[hc] to probe whether the qemu instance supports the new spice migration abilities (check on the source, because it must be enabled at the time qemu is started, which also impacts qemu_command.c; and check on the destination, since we must fail migration if the destination does not support the same mode as the host). Hmm, in just writing that, I wonder if we need even more - if we KNOW that we will want to migrate to a destination that does not support seamless migration, then we want to disable the seamless-migration option on the source at the time we start the VM; sounds like we need to enhance our XML for <graphics> to allow the user to control whether seamless migration is mandatory (source and dest must be new qemu), forbidden (can mix old and new qemu on either end), or defaults to the abilities of the source (if source is new, dest must be new; but source can be old). As to your question about whethe libvirt should wait for the new event: waiting is nicer than polling, but since events can be missed, your approach of polling query-spice seems decent enough (I'm not quite sure how this interacts with restarting libvirtd while a migration is in progress, but polling a monitor command can be resumed when libvirtd restarts, while a missed event will never be seen again). I haven't looked at the code itself, because I'm worried we need the XML design added first, which turns this into a patch series. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Michal Privoznik