On Thu, Sep 20, 2012 at 01:29:21PM +0200, 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.
---
src/qemu/qemu_capabilities.c | 4 +++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 8 ++++++
src/qemu/qemu_migration.c | 33 ++++++++++++++++++++++++--
src/qemu/qemu_monitor.c | 24 +++++++++++++++++++
src/qemu/qemu_monitor.h | 2 +
src/qemu/qemu_monitor_json.c | 52 ++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 3 ++
8 files changed, 124 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 278b550..26364ec 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -180,6 +180,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
"ide-drive.wwn",
"scsi-disk.wwn",
"seccomp-sandbox",
+
+ "seamless-migration", /* 110 */
);
struct _qemuCaps {
@@ -1146,6 +1148,8 @@ qemuCapsComputeCmdFlags(const char *help,
}
if (strstr(help, "-spice"))
qemuCapsSet(caps, QEMU_CAPS_SPICE);
+ if (strstr(help, "seamless-migration="))
+ qemuCapsSet(caps, QEMU_CAPS_SEAMLESS_MIGRATION);
if (strstr(help, "boot=on"))
qemuCapsSet(caps, QEMU_CAPS_DRIVE_BOOT);
if (strstr(help, "serial=s"))
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 4da2a29..2567fd7 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -145,6 +145,7 @@ enum qemuCapsFlags {
QEMU_CAPS_IDE_DRIVE_WWN = 107, /* Is ide-drive.wwn available? */
QEMU_CAPS_SCSI_DISK_WWN = 108, /* Is scsi-disk.wwn available? */
QEMU_CAPS_SECCOMP_SANDBOX = 109, /* -sandbox */
+ QEMU_CAPS_SEAMLESS_MIGRATION = 110, /* seamless-migration for SPICE */
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cbf4aee..33f7e55 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6081,6 +6081,14 @@ qemuBuildCommandLine(virConnectPtr conn,
if (def->graphics[0]->data.spice.copypaste ==
VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_NO)
virBufferAddLit(&opt, ",disable-copy-paste");
+ if (qemuCapsGet(caps, QEMU_CAPS_SEAMLESS_MIGRATION)) {
+ /* If qemu supports seamless migration turn it
+ * unconditionally on. If migration destination
+ * doesn't support it, it fallbacks to previous
+ * migration algorithm silently. */
+ virBufferAddLit(&opt, ",seamless-migration=on");
+ }
+
virCommandAddArg(cmd, "-spice");
virCommandAddArgBuffer(cmd, &opt);
if (def->graphics[0]->data.spice.keymap)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 99fc8ce..114d04a 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) {
@@ -940,6 +949,8 @@ qemuMigrationUpdateJobStatus(struct qemud_driver *driver,
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;
This addition doesn't do anything, since the previous line
has already set the same value.
ret = 0;
break;
@@ -967,6 +978,13 @@ qemuMigrationWaitForCompletion(struct qemud_driver *driver,
virDomainObjPtr vm,
{
qemuDomainObjPrivatePtr priv = vm->privateData;
const char *job;
+ bool wait_for_spice;
+
+ /* If guest uses SPICE and supports seamles_migration 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)
&&
+ qemuCapsGet(priv->caps, QEMU_CAPS_SEAMLESS_MIGRATION);
I don't find this very readable - it is better to do it as a
regular if() IMHO. eg
bool wait_for_spice = false;
if (...)
wait_for_spice = true;
switch (priv->job.asyncJob) {
case QEMU_ASYNC_JOB_MIGRATION_OUT:
@@ -988,7 +1006,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 +1859,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 +1868,12 @@ qemuMigrationRun(struct qemud_driver *driver,
cookieout, cookieoutlen, flags, resource,
spec, spec->destType, spec->fwdType);
+ /* If guest uses SPICE and supports seamless migration 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)
&&
+ qemuCapsGet(priv->caps, QEMU_CAPS_SEAMLESS_MIGRATION);
Same comment as before.
Since both callers of qemuMigrateUpdateJobStatus() have to repeat
this logic, why not push this down into that method itself and
make 'wait_for_spice' be a local variable there instead of a param.
+
if (virLockManagerPluginUsesState(driver->lockManager) &&
!cookieout) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1946,7 +1972,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 f36c8a8..7695a81 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1827,6 +1827,30 @@ int qemuMonitorGetMigrationStatus(qemuMonitorPtr mon,
}
+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"));
This error code (any in fact all other similar uses in that file)
should be VIR_ERR_OPERATION_UNSUPPORTED, since this isn't an
end user configuration issue.
REgards,
Daniel
--
|:
http://berrange.com -o-
http://www.flickr.com/photos/dberrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|:
http://entangle-photo.org -o-
http://live.gnome.org/gtk-vnc :|