Signed-off-by: Cristian Klein <cristian.klein(a)cs.umu.se&gt; --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5217ab3..82f3aeb 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1225,6 +1225,7 @@ typedef enum { VIR_MIGRATE_ABORT_ON_ERROR = (1 << 12), /* abort migration on I/O errors happened during migration */ VIR_MIGRATE_AUTO_CONVERGE = (1 << 13), /* force convergence */ VIR_MIGRATE_RDMA_PIN_ALL = (1 << 14), /* RDMA memory pinning */ + VIR_MIGRATE_POSTCOPY = (1 << 15), /* enable (but don't start) post-copy */ } virDomainMigrateFlags; diff --git a/src/libvirt.c b/src/libvirt.c index 6dc7c73..2c8ffbe 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -5265,6 +5265,7 @@ virDomainMigrateDirect(virDomainPtr domain, * automatically when supported). * VIR_MIGRATE_UNSAFE Force migration even if it is considered unsafe. * VIR_MIGRATE_OFFLINE Migrate offline + * VIR_MIGRATE_POSTCOPY Enable (but don't start) post-copy * * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set. * Applications using the VIR_MIGRATE_PEER2PEER flag will probably @@ -5291,6 +5292,12 @@ virDomainMigrateDirect(virDomainPtr domain, * can use either VIR_MIGRATE_NON_SHARED_DISK or * VIR_MIGRATE_NON_SHARED_INC as they are mutually exclusive. * + * If you want to enable post-copy migration you must set the + * VIR_MIGRATE_POSTCOPY flag. Once migration is active, you may + * start post-copy by calling virDomainMigrateStartPostCopy. + * When to start post-copy is entirely left to the user, libvirt + * only implements the necessary mechanism. + * * In either case it is typically only necessary to specify a * URI if the destination host has multiple interfaces and a * specific interface is required to transmit migration data. -- 1.9.1

On Tue, Sep 30, 2014 at 05:11:03PM +0200, Jiri Denemark wrote: Your point about spawning another thread makes me wonder if we should actually look at adding a 'VIR_MIGRATE_ASYNC' method (that would require P2P migration of course). If this flag were set, virDomainMigrateXXX would only block for long enough to start the migration and then return. Callers can use the job info API to monitor progress & success/failure. Then we wouldn't have to keep adding flags like you suggest - apps can just easily call the appropriate API right away with no threads needed 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 :|

On Wed, Oct 01, 2014 at 10:45:33AM +0200, Cristian KLEIN wrote: I think it is still acceptably simple - the root complaint that Jiri had was that he doesn't want apps to have to spawn threads, which this proposal of mine achieves. 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 :|

On 01 Oct 2014, at 12:07 , Jiri Denemark <jdenemar(a)redhat.com&gt; wrote: Hi Jirka, I talked to the qemu post-copy guys (Andrea and Dave in CC). Starting post-copy immediately is a bad performance choice: The VM will start on the destination hypervisor before the read-only or kernel memory is there. This means that those pages need to be pulled on-demand, hence a lot of overhead and interruptions in the VM’s execution. Instead, it is better to first do one pass of pre-copy and only then trigger post-copy. In fact, I did an experiment with a video streaming VM and starting post-copy after the first pass of pre-copy (instead of starting post-copy immediately) reduces downtime from 3.5 seconds to under 1 second. Given all above, I propose the following post-copy API in libvirt: virDomainMigrateXXX(..., VIR_MIGRATE_ENABLE_POSTCOPY) virDomainMigrateStartPostCopy(...) // from a different thread This is for those who just need the post-copy mechanism and want to implement a policy themselves. virDomainMigrateXXX(..., VIR_MIGRATE_POSTCOPY_AFTER_PRECOPY) This is for those who want to use post-copy without caring about any low-level details, offering a good enough policy for most cases. What do you think? Would you accept patches that implement this API? Cristian

Signed-off-by: Cristian Klein <cristian.klein(a)cs.umu.se&gt; --- include/libvirt/libvirt.h.in | 3 +++ src/driver.h | 5 +++++ src/libvirt.c | 39 +++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 ++++++++++++- src/remote_protocol-structs | 5 +++++ 7 files changed, 70 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 82f3aeb..84cd5a4 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1346,6 +1346,9 @@ int virDomainMigrateToURI3(virDomainPtr domain, unsigned int nparams, unsigned int flags); +int virDomainMigrateStartPostCopy (virDomainPtr domain, + unsigned int flags); + int virDomainMigrateSetMaxDowntime (virDomainPtr domain, unsigned long long downtime, unsigned int flags); diff --git a/src/driver.h b/src/driver.h index dc62a8e..9eb29e2 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1221,6 +1221,10 @@ typedef int unsigned int cellCount, unsigned int flags); +typedef int +(*virDrvDomainMigrateStartPostCopy)(virDomainPtr domain, + unsigned int flags); + typedef struct _virDriver virDriver; typedef virDriver *virDriverPtr; @@ -1445,6 +1449,7 @@ struct _virDriver { virDrvConnectGetDomainCapabilities connectGetDomainCapabilities; virDrvConnectGetAllDomainStats connectGetAllDomainStats; virDrvNodeAllocPages nodeAllocPages; + virDrvDomainMigrateStartPostCopy domainMigrateStartPostCopy; }; diff --git a/src/libvirt.c b/src/libvirt.c index 2c8ffbe..5bb1897 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17803,6 +17803,45 @@ virDomainMigrateSetCompressionCache(virDomainPtr domain, /** + * virDomainMigrateStartPostCopy: + * @domain: a domain object + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Starts post-copy migration. This function has to be called while + * migration (initially pre-copy) is in progress. The migration operation + * must be called with the VIR_MIGRATE_POSTCOPY flag. + * + * Returns 0 in case of success, -1 otherwise. + */ +int +virDomainMigrateStartPostCopy(virDomainPtr domain, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + + if (conn->driver->domainMigrateStartPostCopy) { + if (conn->driver->domainMigrateStartPostCopy(domain, flags) < 0) + goto error; + return 0; + } + + virReportUnsupportedError(); + error: + virDispatchError(conn); + return -1; +} + + +/** * virDomainMigrateSetMaxSpeed: * @domain: a domain object * @bandwidth: migration bandwidth limit in MiB/s diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 5f95802..190eb11 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -684,4 +684,9 @@ LIBVIRT_1.2.9 { virNodeAllocPages; } LIBVIRT_1.2.8; +LIBVIRT_1.2.10 { + global: + virDomainMigrateStartPostCopy; +} LIBVIRT_1.2.9; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 6c49e49..03257ec 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8239,6 +8239,7 @@ static virDriver remote_driver = { .connectGetDomainCapabilities = remoteConnectGetDomainCapabilities, /* 1.2.7 */ .connectGetAllDomainStats = remoteConnectGetAllDomainStats, /* 1.2.8 */ .nodeAllocPages = remoteNodeAllocPages, /* 1.2.9 */ + .domainMigrateStartPostCopy = remoteDomainMigrateStartPostCopy, /* 1.2.10 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index db12cda..dfbc119 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3111,6 +3111,11 @@ struct remote_connect_get_all_domain_stats_args { struct remote_connect_get_all_domain_stats_ret { remote_domain_stats_record retStats<REMOTE_DOMAIN_LIST_MAX>; }; + +struct remote_domain_migrate_start_post_copy_args { + remote_nonnull_domain dom; + unsigned int flags; +}; /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -5505,5 +5510,11 @@ enum remote_procedure { * @generate: none * @acl: connect:write */ - REMOTE_PROC_NODE_ALLOC_PAGES = 347 + REMOTE_PROC_NODE_ALLOC_PAGES = 347, + + /** + * @generate: both + * @acl: domain:migrate + */ + REMOTE_PROC_DOMAIN_MIGRATE_START_POST_COPY = 348 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 362baf9..fd98c80 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2579,6 +2579,10 @@ struct remote_connect_get_all_domain_stats_ret { remote_domain_stats_record * retStats_val; } retStats; }; +struct remote_domain_migrate_start_post_copy_args { + remote_nonnull_domain dom; + u_int flags; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -2927,4 +2931,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_BLOCK_COPY = 345, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE = 346, REMOTE_PROC_NODE_ALLOC_PAGES = 347, + REMOTE_PROC_DOMAIN_MIGRATE_START_POST_COPY = 348, }; -- 1.9.1

Signed-off-by: Cristian Klein <cristian.klein(a)cs.umu.se&gt; --- src/qemu/qemu_monitor.c | 24 ++++++++++++++++++++++-- src/qemu/qemu_monitor.h | 4 ++++ src/qemu/qemu_monitor_json.c | 23 ++++++++++++++++++++++- src/qemu/qemu_monitor_json.h | 1 + 4 files changed, 49 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 543384d..dbd854c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -117,11 +117,11 @@ VIR_ONCE_GLOBAL_INIT(qemuMonitor) VIR_ENUM_IMPL(qemuMonitorMigrationStatus, QEMU_MONITOR_MIGRATION_STATUS_LAST, - "inactive", "active", "completed", "failed", "cancelled", "setup") + "inactive", "active", "completed", "failed", "cancelled", "setup", "postcopy-active") VIR_ENUM_IMPL(qemuMonitorMigrationCaps, QEMU_MONITOR_MIGRATION_CAPS_LAST, - "xbzrle", "auto-converge", "rdma-pin-all") + "xbzrle", "auto-converge", "rdma-pin-all", "x-postcopy-ram") VIR_ENUM_IMPL(qemuMonitorVMStatus, QEMU_MONITOR_VM_STATUS_LAST, @@ -2378,6 +2378,26 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon, return ret; } +int qemuMonitorMigrateStartPostCopy(qemuMonitorPtr mon) +{ + VIR_DEBUG("mon=%p", mon); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONMigrateStartPostCopy(mon); +} + + int qemuMonitorMigrateCancel(qemuMonitorPtr mon) { int ret; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index adf18ab..e42c6af 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -427,6 +427,7 @@ enum { QEMU_MONITOR_MIGRATION_STATUS_ERROR, QEMU_MONITOR_MIGRATION_STATUS_CANCELLED, QEMU_MONITOR_MIGRATION_STATUS_SETUP, + QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_ACTIVE, QEMU_MONITOR_MIGRATION_STATUS_LAST }; @@ -480,6 +481,7 @@ typedef enum { QEMU_MONITOR_MIGRATION_CAPS_XBZRLE, QEMU_MONITOR_MIGRATION_CAPS_AUTO_CONVERGE, QEMU_MONITOR_MIGRATION_CAPS_RDMA_PIN_ALL, + QEMU_MONITOR_MIGRATION_CAPS_POSTCOPY, QEMU_MONITOR_MIGRATION_CAPS_LAST } qemuMonitorMigrationCaps; @@ -535,6 +537,8 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon, unsigned int flags, const char *unixfile); +int qemuMonitorMigrateStartPostCopy(qemuMonitorPtr mon); + int qemuMonitorMigrateCancel(qemuMonitorPtr mon); int qemuMonitorGetDumpGuestMemoryCapability(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index b3b6451..1a9e6e1 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2550,7 +2550,8 @@ qemuMonitorJSONGetMigrationStatusReply(virJSONValuePtr reply, status->setup_time_set = true; if (status->status == QEMU_MONITOR_MIGRATION_STATUS_ACTIVE || - status->status == QEMU_MONITOR_MIGRATION_STATUS_COMPLETED) { + status->status == QEMU_MONITOR_MIGRATION_STATUS_COMPLETED || + status->status == QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_ACTIVE) { virJSONValuePtr ram = virJSONValueObjectGet(ret, "ram"); if (!ram) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -2790,6 +2791,26 @@ int qemuMonitorJSONMigrate(qemuMonitorPtr mon, return ret; } +int qemuMonitorJSONMigrateStartPostCopy(qemuMonitorPtr mon) +{ + int ret; + virJSONValuePtr cmd; + cmd = qemuMonitorJSONMakeCommand("migrate-start-postcopy", NULL); + + virJSONValuePtr reply = NULL; + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon) { int ret; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index ef9b552..3fba203 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -149,6 +149,7 @@ int qemuMonitorJSONGetSpiceMigrationStatus(qemuMonitorPtr mon, bool *spice_migrated); +int qemuMonitorJSONMigrateStartPostCopy(qemuMonitorPtr mon); int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon); int qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitorPtr mon, -- 1.9.1

Signed-off-by: Cristian Klein <cristian.klein(a)cs.umu.se&gt; --- src/qemu/qemu_migration.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_migration.h | 3 ++- 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 284cd5a..4a36946 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1801,6 +1801,49 @@ qemuMigrationSetOffline(virQEMUDriverPtr driver, static int +qemuMigrationSetPostCopy(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob job) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, job) < 0) + return -1; + + ret = qemuMonitorGetMigrationCapability( + priv->mon, + QEMU_MONITOR_MIGRATION_CAPS_POSTCOPY); + + if (ret < 0) { + goto cleanup; + } else if (ret == 0) { + if (job == QEMU_ASYNC_JOB_MIGRATION_IN) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Post-copy migration is not supported by " + "target QEMU binary")); + } else { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Post-copy migration is not supported by " + "source QEMU binary")); + } + ret = -1; + goto cleanup; + } + + /* Post-copy only needs to be enabled on source qemu, + * for target, this function only acts as a capability check */ + if (job == QEMU_ASYNC_JOB_MIGRATION_OUT) { + ret = qemuMonitorSetMigrationCapability( + priv->mon, + QEMU_MONITOR_MIGRATION_CAPS_POSTCOPY); + } + + cleanup: + qemuDomainObjExitMonitor(driver, vm); + return ret; +} +static int qemuMigrationSetCompression(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob job) @@ -2741,6 +2784,11 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, dataFD[1] = -1; /* 'st' owns the FD now & will close it */ } + if (flags & VIR_MIGRATE_POSTCOPY && + qemuMigrationSetPostCopy(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_IN) < 0) + goto stop; + if (flags & VIR_MIGRATE_COMPRESSED && qemuMigrationSetCompression(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN) < 0) @@ -3583,6 +3631,18 @@ qemuMigrationRun(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) goto cleanup; + if (flags & VIR_MIGRATE_POSTCOPY) { + if (!(flags & VIR_MIGRATE_LIVE)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Enabling post-copy only makes sense with " + "live migration")); + goto cleanup; + } + if (qemuMigrationSetPostCopy(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) + goto cleanup; + } + if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) goto cleanup; diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index e7a90c3..349c9c4 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -41,7 +41,8 @@ VIR_MIGRATE_COMPRESSED | \ VIR_MIGRATE_ABORT_ON_ERROR | \ VIR_MIGRATE_AUTO_CONVERGE | \ - VIR_MIGRATE_RDMA_PIN_ALL) + VIR_MIGRATE_RDMA_PIN_ALL | \ + VIR_MIGRATE_POSTCOPY) /* All supported migration parameters and their types. */ # define QEMU_MIGRATION_PARAMETERS \ -- 1.9.1

Some jobs feature several phases. For example, post-copy migration is composed of a first phase, from migration start to switching to post-copy, and a second phase, to migration completion. This job type allows to flag that the job has completed the first phase, but is not yet fully completed. Signed-off-by: Cristian Klein <cristian.klein(a)cs.umu.se&gt; --- include/libvirt/libvirt.h.in | 1 + tools/virsh-domain.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 84cd5a4..81044f0 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4307,6 +4307,7 @@ typedef enum { VIR_DOMAIN_JOB_COMPLETED = 3, /* Job has finished, but isn't cleaned up */ VIR_DOMAIN_JOB_FAILED = 4, /* Job hit error, but isn't cleaned up */ VIR_DOMAIN_JOB_CANCELLED = 5, /* Job was aborted, but isn't cleaned up */ + VIR_DOMAIN_JOB_PHASE1_COMPLETED = 6, /* Job completed first phase, e.g., post-copy activation */ #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_JOB_LAST diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ce59406..36a6d52 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5407,7 +5407,8 @@ VIR_ENUM_IMPL(vshDomainJob, N_("Unbounded"), N_("Completed"), N_("Failed"), - N_("Cancelled")) + N_("Cancelled"), + N_("InPhase2")) static const char * vshDomainJobToString(int type) -- 1.9.1

On 07 Oct 2014, at 22:08 , Jiri Denemark <jdenemar(a)redhat.com&gt; wrote: My rationale for introducing this flag was to prevent a future breakage of an internal API. I felt like none of the previous flags where accurately representing the status of this job after post-copy has started. Sure, I could use `job->status.status` in `qemuMigrationWaitForCompletion`, but I was concerned that a future `libvirt` addition might not take this subtly into account and might break post-copy. If you think I should go ahead, suppress this job type and use `job->status.status` instead, what should I use instead, `VIR_DOMAIN_JOB_UNBOUNDED` or `VIR_DOMAIN_JOB_BOUNDED`? To be honest, I’m not sure to have completely understood the difference between them, except that `VIR_DOMAIN_JOB_BOUNDED` seems to be used to signal that a job is almost complete.

On Tue, Sep 30, 2014 at 16:39:27 +0200, Cristian Klein wrote: This code should be unreachable. If both source and destination daemons support VIR_MIGRATE_POSTCOPY, they support v3 protocol as well. And a client new enough to specify VIR_MIGRATE_POSTCOPY will also support v3 migration protocol. However, it doesn't hurt to check this to be safe. This will need to be dropped after 5/8 is removed. However, it reminds me... enum { QEMU_MONITOR_MIGRATION_STATUS_INACTIVE, QEMU_MONITOR_MIGRATION_STATUS_ACTIVE, QEMU_MONITOR_MIGRATION_STATUS_COMPLETED, QEMU_MONITOR_MIGRATION_STATUS_ERROR, QEMU_MONITOR_MIGRATION_STATUS_CANCELLED, QEMU_MONITOR_MIGRATION_STATUS_SETUP, QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_ACTIVE, QEMU_MONITOR_MIGRATION_STATUS_LAST }; in qemu_monitor.h needs to be turned into a proper typedef so that switch (status.status) { line in qemuMigrationUpdateJobStatus may be changed to explicitly mention the enum so that the compiler may report a warning whenever we add new status but forgot to handle it in this switch. Which means the new state will need to be handled in the same patch it was introduced, i.e, in 3/8. I think it would be cleaner to pass this info in a new parameter for qemuMigrationWaitForCompletion instead of doing a hidden black magic here. Just use jobInfo->status.status directly. This shouldn't be needed when 5/8 is dropped. This has to be done after setting the phase. We may need to add a new so that we can recover from post-copy migrations when libvirtd is restarted. Jirka

Signed-off-by: Cristian Klein <cristian.klein(a)cs.umu.se&gt; --- src/qemu/qemu_driver.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3fe2216..1242b77 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11950,6 +11950,55 @@ qemuDomainGetJobStats(virDomainPtr dom, } +static int qemuDomainMigrateStartPostCopy(virDomainPtr dom, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + qemuDomainObjPrivatePtr priv; + + virCheckFlags(0, -1); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainMigrateStartPostCopyEnsureACL(dom->conn, vm->def) < 0) + return -1; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MIGRATION_OP) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + + priv = vm->privateData; + + if (priv->job.asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("post-copy can only be started " + "while migration is in progress")); + goto cleanup; + } + + VIR_DEBUG("Starting post-copy"); + qemuDomainObjEnterMonitor(driver, vm); + ret = qemuMonitorMigrateStartPostCopy(priv->mon); + qemuDomainObjExitMonitor(driver, vm); + + endjob: + if (!qemuDomainObjEndJob(driver, vm)) + vm = NULL; + + cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + static int qemuDomainAbortJob(virDomainPtr dom) { virQEMUDriverPtr driver = dom->conn->privateData; @@ -18419,6 +18468,7 @@ static virDriver qemuDriver = { .connectGetDomainCapabilities = qemuConnectGetDomainCapabilities, /* 1.2.7 */ .connectGetAllDomainStats = qemuConnectGetAllDomainStats, /* 1.2.8 */ .nodeAllocPages = qemuNodeAllocPages, /* 1.2.9 */ + .domainMigrateStartPostCopy = qemuDomainMigrateStartPostCopy, /* 1.2.10 */ }; -- 1.9.1

Signed-off-by: Cristian Klein <cristian.klein(a)cs.umu.se&gt; --- tools/virsh-domain.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++-- tools/virsh.pod | 5 ++++ 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 36a6d52..395c73c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9251,6 +9251,10 @@ static const vshCmdOptDef opts_migrate[] = { .type = VSH_OT_INT, .help = N_("force guest to suspend if live migration exceeds timeout (in seconds)") }, + {.name = "postcopy-after", + .type = VSH_OT_INT, + .help = N_("switch to post-copy migration if live migration exceeds timeout (in seconds)") + }, {.name = "xml", .type = VSH_OT_STRING, .help = N_("filename containing updated XML for the target") @@ -9332,6 +9336,8 @@ doMigrate(void *opaque) VIR_FREE(xml); } + if (vshCommandOptBool(cmd, "postcopy-after")) /* actually an int */ + flags |= VIR_MIGRATE_POSTCOPY; if (vshCommandOptBool(cmd, "live")) flags |= VIR_MIGRATE_LIVE; if (vshCommandOptBool(cmd, "p2p")) @@ -9423,6 +9429,50 @@ vshMigrationTimeout(vshControl *ctl, virDomainSuspend(dom); } +static void +vshMigrationPostCopyAfter(vshControl *ctl, + virDomainPtr dom, + void *opaque ATTRIBUTE_UNUSED) +{ + vshDebug(ctl, VSH_ERR_DEBUG, "starting post-copy\n"); + int rv = virDomainMigrateStartPostCopy(dom, 0); + if (rv < 0) { + vshError(ctl, "%s", _("start post-copy command failed")); + } else { + vshDebug(ctl, VSH_ERR_INFO, "switched to post-copy\n"); + } +} + +/* Parse the --postcopy-after parameter in seconds, but store its + * value in milliseconds. Return -1 on error, 0 if + * no timeout was requested, and 1 if timeout was set. + * Copy-paste-adapted from vshCommandOptTimeoutToMs. + */ +static int +vshCommandOptPostCopyAfterToMs(vshControl *ctl, const vshCmd *cmd, int *postCopyAfter) +{ + int rv = vshCommandOptInt(cmd, "postcopy-after", postCopyAfter); + + if (rv < 0 || (rv > 0 && *postCopyAfter < 0)) { + vshError(ctl, "%s", _("invalid postcopy-after parameter")); + return -1; + } + if (rv > 0) { + /* Ensure that we can multiply by 1000 without overflowing. */ + if (*postCopyAfter > INT_MAX / 1000) { + vshError(ctl, "%s", _("post-copy after parameter is too big")); + return -1; + } + *postCopyAfter *= 1000; + /* 0 is a special value inside virsh, which means no timeout, so + * use 1ms instead for "start post-copy immediately" + */ + if (*postCopyAfter == 0) + *postCopyAfter = 1; + } + return rv; +} + static bool cmdMigrate(vshControl *ctl, const vshCmd *cmd) { @@ -9432,6 +9482,7 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) bool verbose = false; bool functionReturn = false; int timeout = 0; + int postCopyAfter = 0; bool live_flag = false; vshCtrlData data = { .dconn = NULL }; @@ -9451,6 +9502,18 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) goto cleanup; } + if (vshCommandOptPostCopyAfterToMs(ctl, cmd, &postCopyAfter) < 0) { + goto cleanup; + } else if (postCopyAfter > 0 && !live_flag) { + vshError(ctl, "%s", + _("migrate: Unexpected postcopy-after for offline migration")); + goto cleanup; + } else if (postCopyAfter > 0 && timeout > 0) { + vshError(ctl, "%s", + _("migrate: --postcopy-after is incompatible with --timeout")); + goto cleanup; + } + if (pipe(p) < 0) goto cleanup; @@ -9480,8 +9543,13 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) doMigrate, &data) < 0) goto cleanup; - functionReturn = vshWatchJob(ctl, dom, verbose, p[0], timeout, - vshMigrationTimeout, NULL, _("Migration")); + if (postCopyAfter != 0) { + functionReturn = vshWatchJob(ctl, dom, verbose, p[0], postCopyAfter, + vshMigrationPostCopyAfter, NULL, _("Migration")); + } else { + functionReturn = vshWatchJob(ctl, dom, verbose, p[0], timeout, + vshMigrationTimeout, NULL, _("Migration")); + } virThreadJoin(&workerThread); diff --git a/tools/virsh.pod b/tools/virsh.pod index eae9195..2ae7afa 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1404,6 +1404,7 @@ to the I<uri> namespace is displayed instead of being modified. [I<--compressed>] [I<--abort-on-error>] [I<--auto-converge>] I<domain> I<desturi> [I<migrateuri>] [I<graphicsuri>] [I<listen-address>] [I<dname>] [I<--timeout> B<seconds>] [I<--xml> B<file>] +[I<--postcopy-after> B<seconds>] Migrate domain to another host. Add I<--live> for live migration; <--p2p> for peer-2-peer migration; I<--direct> for direct migration; or I<--tunnelled> @@ -1451,6 +1452,10 @@ I<--timeout> B<seconds> forces guest to suspend when live migration exceeds that many seconds, and then the migration will complete offline. It can only be used with I<--live>. +I<--postcopy-after> switches to post-copy migration when pre-copy migration +exceeds that many seconds. Zero means start post-copy as soon as possible. +It can only be used with I<--live>. + Running migration can be canceled by interrupting virsh (usually using C<Ctrl-C>) or by B<domjobabort> command sent from another virsh instance. -- 1.9.1

On 07 Oct 2014, at 23:48 , Jiri Denemark <jdenemar(a)redhat.com&gt; wrote: I’m still not convinced this is a good idea. To use —postcopy and migrate-startpostcopy, the user would have to launch a second instance of virsh, since the first one is busy monitoring the migration. I feel that this is a bit too low level for a tool like virsh: It would be as if we offered the user a dedicated migrate-cancel command, instead of pressing CTRL+C. I’ll fix the other issues you have raised. Cristian

On 10 Oct 2014, at 5:59 , Jiri Denemark <jdenemar(a)redhat.com&gt; wrote: I didn’t know about domjobabort. That’s a pretty compelling argument to add migrate-startpostcopy. Okey, I’ll add it in the next version of the patch series.