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

[Jiri Denemark]

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 at cs.umu.se>
> >> ---
> >>   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.

Jirka