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

[Daniel P . Berrangé]

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.

Also 7.0 since this has missed the 6.2 deadline.


Both these notes apply to later in this file too



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


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