[libvirt PATCH 7/7] qemu_migration: Implement VIR_MIGRATE_ZEROCOPY flag

[Daniel P. Berrangé]

On Thu, Jun 23, 2022 at 04:20:08PM +0200, Jiri Denemark wrote:
> On Thu, Jun 23, 2022 at 15:08:30 +0100, Daniel P. Berrangé wrote:
> > On Thu, Jun 23, 2022 at 03:58:12PM +0200, Jiri Denemark wrote:
> > > Resolves: https://gitlab.com/libvirt/libvirt/-/issues/306
> > > 
> > > Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> > > ---
> > >  src/qemu/qemu_migration.c        | 21 +++++++++++++++++++++
> > >  src/qemu/qemu_migration.h        |  1 +
> > >  src/qemu/qemu_migration_params.c |  6 ++++++
> > >  src/qemu/qemu_migration_params.h |  1 +
> > >  4 files changed, 29 insertions(+)
> > > 
> > > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > > index 272f1b1b59..02a465f6cb 100644
> > > --- a/src/qemu/qemu_migration.c
> > > +++ b/src/qemu/qemu_migration.c
> > > @@ -2634,6 +2634,12 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver,
> > >          }
> > >      }
> > >  
> > > +    if (flags & VIR_MIGRATE_ZEROCOPY && !(flags & VIR_MIGRATE_PARALLEL)) {
> > > +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> > > +                       _("zero-copy is only available for parallel migration"));
> > > +        return NULL;
> > > +    }
> > 
> > It is also not compatible with compression, or TLS.
> 
> Yeah, no zero-copy when you need to transform the data in any way. I
> think QEMU provides a reasonable error message already. But since it
> mentions "multifd" while our flag is called "parallel", I decided to
> explicitly handle this case:
> 
>     internal error: unable to execute QEMU command
>     'migrate-set-capabilities': Zero copy only available for
>     non-compressed non-TLS multifd migration
> 
> Do you think I should explicitly check all flags instead?

I'm not fussed, I just wanted to mention it in case it was
important.

> 
> > > diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
> > > index 4e83d8d8bd..cc66ed8229 100644
> > > --- a/src/qemu/qemu_migration_params.c
> > > +++ b/src/qemu/qemu_migration_params.c
> > > @@ -94,6 +94,7 @@ VIR_ENUM_IMPL(qemuMigrationCapability,
> > >                "multifd",
> > >                "dirty-bitmaps",
> > >                "return-path",
> > > +              "zero-copy-send",
> > >  );
> > 
> > Note the QEMU name was picked in case we later get zero copy
> > receive, as a separately enabled feature.
> 
> The question is whether we need to do the same or if we can have a
> single zero copy which would enable the right one depending on the side
> of migration...

The main reason for needing a flag in QEMU was because mgmt apps need
to aware of the mem pinning requirements. QEMU wants separate flags
so it doesn't silently chuange behaviour on upgrade and break mgmt
apps. Overall it seems like libvirt has enough knowledge to do the
right thing without needing two flags.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|