Re: [PATCH v5 4/6] migration: Add zerocopy parameter for QMP/HMP for Linux

[Leonardo Bras Soares Passos]

Hello Daniel,

On Fri, Nov 12, 2021 at 8:05 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Fri, Nov 12, 2021 at 02:10:39AM -0300, Leonardo Bras wrote:
> > Add property that allows zerocopy migration of memory pages,
> > and also includes a helper function migrate_use_zerocopy() to check
> > if it's enabled.
> >
> > No code is introduced to actually do the migration, but it allow
> > future implementations to enable/disable this feature.
> >
> > On non-Linux builds this parameter is compiled-out.
> >
> > Signed-off-by: Leonardo Bras <leobras@redhat.com>
> > ---
> >  qapi/migration.json   | 18 ++++++++++++++++++
> >  migration/migration.h |  5 +++++
> >  migration/migration.c | 32 ++++++++++++++++++++++++++++++++
> >  migration/multifd.c   | 17 +++++++++--------
> >  migration/socket.c    |  5 +++++
> >  monitor/hmp-cmds.c    |  6 ++++++
> >  6 files changed, 75 insertions(+), 8 deletions(-)
> >
> > diff --git a/qapi/migration.json b/qapi/migration.json
> > index bbfd48cf0b..9534c299d7 100644
> > --- a/qapi/migration.json
> > +++ b/qapi/migration.json
> > @@ -730,6 +730,11 @@
> >  #                      will consume more CPU.
> >  #                      Defaults to 1. (Since 5.0)
> >  #
> > +# @zerocopy: Controls behavior on sending memory pages on migration.
> > +#            When true, enables a zerocopy mechanism for sending memory
> > +#            pages, if host supports it.
> > +#            Defaults to false. (Since 6.2)
>
> Add
>
>    Requires that QEMU be permitted to use locked memory for guest
>    RAM pages.
>

Done

>
> Also 7.0 since this has missed the 6.2 deadline.
>

Done

>
>
> Both these notes apply to later in this file too
>

Replaced thrice in this file.

>
>
>
> > diff --git a/migration/multifd.c b/migration/multifd.c
> > index 7c9deb1921..ab8f0f97be 100644
> > --- a/migration/multifd.c
> > +++ b/migration/multifd.c
> > @@ -854,16 +854,17 @@ static void multifd_new_send_channel_async(QIOTask 
> > *task, gpointer opaque)
> >      trace_multifd_new_send_channel_async(p->id);
> >      if (qio_task_propagate_error(task, &local_err)) {
> >          goto cleanup;
> > -    } else {
> > -        p->c = QIO_CHANNEL(sioc);
> > -        qio_channel_set_delay(p->c, false);
> > -        p->running = true;
> > -        if (!multifd_channel_connect(p, sioc, local_err)) {
> > -            goto cleanup;
> > -        }
> > -        return;
> >      }
> >
> > +    p->c = QIO_CHANNEL(sioc);
> > +    qio_channel_set_delay(p->c, false);
> > +    p->running = true;
> > +    if (!multifd_channel_connect(p, sioc, local_err)) {
> > +        goto cleanup;
> > +    }
> > +
> > +    return;
> > +
> >  cleanup:
> >      multifd_new_send_channel_cleanup(p, sioc, local_err);
> >  }
>
> This change is just a code style alteration with no relation to
> zerocopy. Either remove it, or do this change in its own patch
> seprate from zerocopy.
>

Removed.

>
>
> 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 :|
>

Thanks for reviewing.

Best regards,
Leo