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

[Nikita]

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