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

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

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?