Re: [External] Re: [PATCH v3 2/7] migration/multifd: Implement zero page transmission on the multifd thread.

[Hao Xiang]

On Thu, Feb 29, 2024 at 7:47 AM Fabiano Rosas <farosas@suse.de> wrote:
>
> Markus Armbruster <armbru@redhat.com> writes:
>
> > Hao Xiang <hao.xiang@bytedance.com> writes:
> >
> >> On Wed, Feb 28, 2024 at 1:50 AM Markus Armbruster <armbru@redhat.com> 
> >> wrote:
> >>>
> >>> Hao Xiang <hao.xiang@bytedance.com> writes:
> >>>
> >>> > 1. Add zero_pages field in MultiFDPacket_t.
> >>> > 2. Implements the zero page detection and handling on the multifd
> >>> > threads for non-compression, zlib and zstd compression backends.
> >>> > 3. Added a new value 'multifd' in ZeroPageDetection enumeration.
> >>> > 4. Handle migration QEMU9.0 -> QEMU8.2 compatibility.
> >>> > 5. Adds zero page counters and updates multifd send/receive tracing
> >>> > format to track the newly added counters.
> >>> >
> >>> > Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
> >>>
> >>> [...]
> >>>
> >>> > diff --git a/qapi/migration.json b/qapi/migration.json
> >>> > index 1e66272f8f..5a1bb8ad62 100644
> >>> > --- a/qapi/migration.json
> >>> > +++ b/qapi/migration.json
> >>> > @@ -660,10 +660,13 @@
> >>> >  #
> >>> >  # @legacy: Perform zero page checking from main migration thread.
> >>> >  #
> >>> > +# @multifd: Perform zero page checking from multifd sender thread.
> >>> > +#
> >>> >  # Since: 9.0
> >>> > +#
> >>> >  ##
> >>> >  { 'enum': 'ZeroPageDetection',
> >>> > -  'data': [ 'none', 'legacy' ] }
> >>> > +  'data': [ 'none', 'legacy', 'multifd' ] }
> >>> >
> >>> >  ##
> >>> >  # @BitmapMigrationBitmapAliasTransform:
> >>>
> >>> What happens when you set "zero-page-detection": "multifd" *without*
> >>> enabling multifd migration?
> >>
> >> Very good question! Right now the behavior is that if "multifd
> >> migration" is not enabled, it goes through the legacy code path and
> >> the "multifd zero page" option is ignored. The legacy path has its own
> >> zero page checking and will run the same way as before. This is for
> >> backward compatibility.
> >
> > We need one of two improvements then:
> >
> > 1. Make "zero-page-detection" reject value "multifd" when multifd
> >    migration is not enabled.  Document this: "Requires migration
> >    capability @multifd" or similar.
> >
> > 2. Document that "multifd" means multifd only when multifd is enabled,
> >    else same as "legacy".
> >
> > I prefer 1., because it's easier to document.  But migration maintainers
> > may have their own preference.  Peter, Fabiano?
>
> I think we need to go with 2 for consistency with the other multifd_*
> parameters. I don't see any validation at options.c.

Yeah, it's hard to do 1. Someone can also set multifd and then disable
multifd migration. This is an existing problem. I will update the
document for "multifd".