Re: [PATCH 1/8] migration: Implemented new parameter stream_content

[Nikita <nikita.lapshin@openvz.org>]

On 6/16/22 4:10 PM, Daniel P. Berrangé wrote:
> On Thu, Jun 16, 2022 at 03:53:29PM +0300, Nikita wrote:
>> On 6/16/22 1:32 PM, Daniel P. Berrangé wrote:
>>> On Thu, Jun 16, 2022 at 01:19:57PM +0300, nikita.lapshin@openvz.org wrote:
>>>> From: Nikita Lapshin <nikita.lapshin@openvz.org>
>>>>
>>>> This new optional parameter contains inormation about migration
>>>> stream parts to be sent (such as RAM, block, bitmap). This looks
>>>> better than using capabilities to solve problem of dividing
>>>> migration stream.
>>>>
>>>> Signed-off-by: Nikita Lapshin <nikita.lapshin@openvz.org>
>>>> ---
>>>>    migration/migration.c | 47 ++++++++++++++++++++++++++++++++++++++++++-
>>>>    migration/migration.h |  2 ++
>>>>    qapi/migration.json   | 21 ++++++++++++++++---
>>>>    3 files changed, 66 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>>> index 18e2610e88..80acf6dbc3 100644
>>>> --- a/qapi/migration.json
>>>> +++ b/qapi/migration.json
>>>> @@ -760,6 +760,12 @@
>>>>    #                        block device name if there is one, and to their node name
>>>>    #                        otherwise. (Since 5.2)
>>>>    #
>>>> +# @stream-content-list: Parameter control content of migration stream such as RAM,
>>>> +#                       vmstate, block and dirty-bitmaps. This is optional parameter
>>>> +#                       so migration will work correctly without it.
>>>> +#                       This parameter takes string list as description of content
>>>> +#                       and include that part of migration stream. (Since 7.0)
>>>> +#
>>>>    # Features:
>>>>    # @unstable: Member @x-checkpoint-delay is experimental.
>>>>    #
>>>> @@ -780,7 +786,8 @@
>>>>               'xbzrle-cache-size', 'max-postcopy-bandwidth',
>>>>               'max-cpu-throttle', 'multifd-compression',
>>>>               'multifd-zlib-level' ,'multifd-zstd-level',
>>>> -           'block-bitmap-mapping' ] }
>>>> +           'block-bitmap-mapping',
>>>> +           'stream-content-list' ] }
>>>>    ##
>>>>    # @MigrateSetParameters:
>>>> @@ -925,6 +932,12 @@
>>>>    #                        block device name if there is one, and to their node name
>>>>    #                        otherwise. (Since 5.2)
>>>>    #
>>>> +# @stream-content-list: Parameter control content of migration stream such as RAM,
>>>> +#                       vmstate, block and dirty-bitmaps. This is optional parameter
>>>> +#                       so migration will work correctly without it.
>>>> +#                       This parameter takes string list as description of content
>>>> +#                       and include that part of migration stream. (Since 7.0)
>>>> +#
>>>>    # Features:
>>>>    # @unstable: Member @x-checkpoint-delay is experimental.
>>>>    #
>>>> @@ -960,7 +973,8 @@
>>>>                '*multifd-compression': 'MultiFDCompression',
>>>>                '*multifd-zlib-level': 'uint8',
>>>>                '*multifd-zstd-level': 'uint8',
>>>> -            '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ] } }
>>>> +            '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ],
>>>> +            '*stream-content-list': [ 'str' ] } }
>>>>    ##
>>>>    # @migrate-set-parameters:
>>>> @@ -1158,7 +1172,8 @@
>>>>                '*multifd-compression': 'MultiFDCompression',
>>>>                '*multifd-zlib-level': 'uint8',
>>>>                '*multifd-zstd-level': 'uint8',
>>>> -            '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ] } }
>>>> +            '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ],
>>>> +            '*stream-content-list': [ 'str' ] } }
>>> These will need to be represented using an enum type rather than
>>> a string, since this value accepts a fixed pre-determined list of
>>> strings.
>>>
>>> With regards,
>>> Daniel
>> First of all thank you for your review!
>>
>> May be I misunderstood you, but is enum convenient for this purpose? List
>> for describing looks pretty good.
>>
>> Or you mean that it is better to use list of enums?
> Yes, sorry, I meant list of enums, so
>
>     '*stream-content-list': [ 'MigrationSteamContent']
>
> With regards,
> Daniel

Okay sounds good. Thank you!


Best regards,
Nikita