On Wed, Jan 19, 2022 at 3:16 PM Daniel P. Berrangé <berra...@redhat.com> wrote:
>
> On Wed, Jan 19, 2022 at 03:03:29PM -0300, Leonardo Bras Soares Passos wrote:
> > Hello Daniel,
> >
> > On Thu, Jan 13, 2022 at 10:10 AM Daniel P. Berrangé <berra...@redhat.com>
> > wrote:
> > >
> > > On Thu, Jan 06, 2022 at 07:13:40PM -0300, Leonardo Bras wrote:
> > > > Add property that allows zero-copy migration of memory pages,
> > > > and also includes a helper function migrate_use_zero_copy() 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 <leob...@redhat.com>
> > > > ---
> > > > qapi/migration.json | 24 ++++++++++++++++++++++++
> > > > migration/migration.h | 5 +++++
> > > > migration/migration.c | 32 ++++++++++++++++++++++++++++++++
> > > > migration/socket.c | 5 +++++
> > > > monitor/hmp-cmds.c | 6 ++++++
> > > > 5 files changed, 72 insertions(+)
> > >
> > > Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>
> >
> > Thanks!
> >
> > >
> > > >
> > > > diff --git a/qapi/migration.json b/qapi/migration.json
> > > > index bbfd48cf0b..2e62ea6ebd 100644
> > > > --- a/qapi/migration.json
> > > > +++ b/qapi/migration.json
> > > > @@ -730,6 +730,13 @@
> > > > # will consume more CPU.
> > > > # Defaults to 1. (Since 5.0)
> > > > #
> > > > +# @zero-copy: Controls behavior on sending memory pages on migration.
> > > > +# When true, enables a zero-copy mechanism for sending
> > > > memory
> > > > +# pages, if host supports it.
> > > > +# Requires that QEMU be permitted to use locked memory for
> > > > guest
> > > > +# RAM pages.
> > > > +# Defaults to false. (Since 7.0)
> > > > +#
> > > > # @block-bitmap-mapping: Maps block nodes and bitmaps on them to
> > > > # aliases for the purpose of dirty bitmap
> > > > migration. Such
> > > > # aliases may for example be the corresponding
> > > > names on the
> > > > @@ -769,6 +776,7 @@
> > > > 'xbzrle-cache-size', 'max-postcopy-bandwidth',
> > > > 'max-cpu-throttle', 'multifd-compression',
> > > > 'multifd-zlib-level' ,'multifd-zstd-level',
> > > > + { 'name': 'zero-copy', 'if' : 'CONFIG_LINUX'},
> > > > 'block-bitmap-mapping' ] }
> > > >
> > > > ##
> > > > @@ -895,6 +903,13 @@
> > > > # will consume more CPU.
> > > > # Defaults to 1. (Since 5.0)
> > > > #
> > > > +# @zero-copy: Controls behavior on sending memory pages on migration.
> > > > +# When true, enables a zero-copy mechanism for sending
> > > > memory
> > > > +# pages, if host supports it.
> > > > +# Requires that QEMU be permitted to use locked memory for
> > > > guest
> > > > +# RAM pages.
> > > > +# Defaults to false. (Since 7.0)
> > > > +#
> > > > # @block-bitmap-mapping: Maps block nodes and bitmaps on them to
> > > > # aliases for the purpose of dirty bitmap
> > > > migration. Such
> > > > # aliases may for example be the corresponding
> > > > names on the
> > > > @@ -949,6 +964,7 @@
> > > > '*multifd-compression': 'MultiFDCompression',
> > > > '*multifd-zlib-level': 'uint8',
> > > > '*multifd-zstd-level': 'uint8',
> > > > + '*zero-copy': { 'type': 'bool', 'if': 'CONFIG_LINUX' },
> > > > '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ] } }
> > >
> > > The current zerocopy impl is for the send path.
> > >
> > > Do you expect we might get zerocopy in the receive path
> > > later ?
> >
> > It's possible, but I haven't started the implementation yet.
> >
> > >
> > > If so then either call this 'send-zero-copy', or change it
> > > from a bool to an enum taking '["send", "recv", "both"]'.
> > >
> > > I'd probably take the former and just rename it.
> > >
> >
> > Well, my rationale:
> > - I want to set zero copy sending:
> > zero-copy is set in the sending host, start migration.
> >
> > - I want to set zero copy receiving:
> > zero-copy is set in the receiving host, wait for migration.
> > (Of course host support is checked when setting the parameter).
> >
> > The problem with the current approach is trying to enable zero-copy on
> > receive before it's implemented, which will 'fail' silently .
> > A possible solution would be to add a patch to check in the receiving
> > path if zero-copy is enabled, and fail for now.
>
> That's not good because mgmt apps cannot query the QAPI schema
> to find out if this feature is supported or not.
>
> If we wantt o support zero copy recv, then we need an explicit
> flag for it that is distinct from zero copy send, so that apps
> can introspect whether the feature is implemneted in QEMU or
> not.
>
> Distinct named bool flags feels better, and also makes it clear
> to anyone not familiar with the impl that the current code is
> strictly send only.
>
Ok, I see the point.
I will try to refactor the code changing zero-copy to zero-copy-send
or something like that.
Best regards,
Leo
