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

[Daniel P . Berrangé]

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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|