Re: [libvirt] [PATCH 2/6] Added public API to enable post-copy migration

On Thu, Sep 25, 2014 at 11:42:16 +0200, Cristian KLEIN wrote: > On 2014-09-24 14:32, Jiri Denemark wrote: >> On Tue, Sep 23, 2014 at 16:09:57 +0200, Cristian Klein wrote: >>> Added a new `libvirt` migration flag `VIR_MIGRATE_POSTCOPY` and the >>> necessary code to pass this flag to qemu as migration capability. >>> >>> Signed-off-by: Cristian Klein <cristian.klein(a)cs.umu.se&gt; >>> --- >>> include/libvirt/libvirt.h.in | 1 + >>> src/libvirt.c | 7 +++++++ >>> src/qemu/qemu_migration.c | 43 +++++++++++++++++++++++++++++++++++++++++++ >>> src/qemu/qemu_migration.h | 3 ++- >>> 4 files changed, 53 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in >>> index 6371b7b..bdc33c6 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; In my first review, I forgot to mention that I think another flag requesting post-copy to be started right away (instead of waiting for virDomainMigrateStartPostCopy to be called) could be useful too.

... >>> @@ -3580,6 +3619,10 @@ qemuMigrationRun(virQEMUDriverPtr driver, >>> if (flags & VIR_MIGRATE_RDMA_PIN_ALL && >>> qemuMigrationSetPinAll(driver, vm, >>> QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) >>> + >>> + if (flags & VIR_MIGRATE_POSTCOPY && >>> + qemuMigrationSetPostCopy(driver, vm, >>> + QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) >>> goto cleanup; >>> >>> if (qemuDomainObjEnterMonitorAsync(driver, vm, >> >> I'd expect similar thing would need to be done in the Prepare phase on >> destination... However, if destination does not need to set the >> capability, we at least need to check if destination QEMU supports it >> and report failure from Prepare if it doesn't. And the >> QEMU_ASYNC_JOB_MIGRATION_IN branch in qemuMigrationSetPostCopy would be >> unreachable in that case. > > It's up to the source qemu (through the migration protocol) to activate > post-copy on the destination qemu. So there are no other steps to be > done on the source. > > I have mixed feelings about having libvirt check the necessary > capability on the destination. Personally, I would prefer qemu to return > an error like "post-copy unavailable" when the "migrate" command is > issued, as there might be more factors that influence the availability > of post-copy. For example, it's not sufficient for the destination qemu > to support post-copy, but the destination kernel also needs userfault > support. Yeah, it won't catch all cases, but will at least fail early when someone tries to enable post-copy migration while migrating to a host with older QEMU that does not support it. Moreover, if QEMU is smart enough to check if post-copy migration can actually be used when the migration capability is set, we could catch even cases when userfault is not supported on the destination.