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

[Fabiano Rosas <farosas@suse.de>]

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.