* Max Reitz (mre...@redhat.com) wrote:
> On 30.06.20 12:51, Dr. David Alan Gilbert wrote:
> > * Max Reitz (mre...@redhat.com) wrote:
> >> This migration parameter allows mapping block node names and bitmap
> >> names to aliases for the purpose of block dirty bitmap migration.
> >>
> >> This way, management tools can use different node and bitmap names on
> >> the source and destination and pass the mapping of how bitmaps are to be
> >> transferred to qemu (on the source, the destination, or even both with
> >> arbitrary aliases in the migration stream).
> >>
> >> Suggested-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> >> Signed-off-by: Max Reitz <mre...@redhat.com>
> >> ---
> >> qapi/migration.json | 83 +++++++-
> >> migration/migration.h | 3 +
> >> migration/block-dirty-bitmap.c | 372 ++++++++++++++++++++++++++++-----
> >> migration/migration.c | 29 +++
> >> 4 files changed, 432 insertions(+), 55 deletions(-)
> >>
> >> diff --git a/qapi/migration.json b/qapi/migration.json
> >> index d5000558c6..5aeae9bea8 100644
> >> --- a/qapi/migration.json
> >> +++ b/qapi/migration.json
> >> @@ -507,6 +507,44 @@
> >> 'data': [ 'none', 'zlib',
> >> { 'name': 'zstd', 'if': 'defined(CONFIG_ZSTD)' } ] }
> >>
> >> +##
> >> +# @BitmapMigrationBitmapAlias:
> >> +#
> >> +# @name: The name of the bitmap.
> >> +#
> >> +# @alias: An alias name for migration (for example the bitmap name on
> >> +# the opposite site).
> >> +#
> >> +# Since: 5.1
> >> +##
> >> +{ 'struct': 'BitmapMigrationBitmapAlias',
> >> + 'data': {
> >> + 'name': 'str',
> >> + 'alias': 'str'
> >> + } }
> >> +
> >> +##
> >> +# @BitmapMigrationNodeAlias:
> >> +#
> >> +# Maps a block node name and the bitmaps it has to aliases for dirty
> >> +# bitmap migration.
> >> +#
> >> +# @node-name: A block node name.
> >> +#
> >> +# @alias: An alias block node name for migration (for example the
> >> +# node name on the opposite site).
> >> +#
> >> +# @bitmaps: Mappings for the bitmaps on this node.
> >> +#
> >> +# Since: 5.1
> >> +##
> >> +{ 'struct': 'BitmapMigrationNodeAlias',
> >> + 'data': {
> >> + 'node-name': 'str',
> >> + 'alias': 'str',
> >> + 'bitmaps': [ 'BitmapMigrationBitmapAlias' ]
> >> + } }
> >> +
> >> ##
> >> # @MigrationParameter:
> >> #
> >> @@ -641,6 +679,18 @@
> >> # will consume more CPU.
> >> # Defaults to 1. (Since 5.0)
> >> #
> >> +# @block-bitmap-mapping: Maps block nodes and bitmaps on them to
> >> +# aliases for the purpose of dirty bitmap migration. Such
> >> +# aliases may for example be the corresponding names on the
> >> +# opposite site.
> >> +# The mapping must be one-to-one and complete: On the source,
> >> +# migrating a bitmap from a node when either is not mapped
> >> +# will result in an error. On the destination, similarly,
> >> +# receiving a bitmap (by alias) from a node (by alias) when
> >> +# either alias is not mapped will result in an error.
> >> +# By default, all node names and bitmap names are mapped to
> >> +# themselves. (Since 5.1)
> >> +#
> >> # Since: 2.4
> >> ##
> >> { 'enum': 'MigrationParameter',
> >> @@ -655,7 +705,8 @@
> >> 'multifd-channels',
> >> 'xbzrle-cache-size', 'max-postcopy-bandwidth',
> >> 'max-cpu-throttle', 'multifd-compression',
> >> - 'multifd-zlib-level' ,'multifd-zstd-level' ] }
> >> + 'multifd-zlib-level' ,'multifd-zstd-level',
> >> + 'block-bitmap-mapping' ] }
> >>
> >> ##
> >> # @MigrateSetParameters:
> >> @@ -781,6 +832,18 @@
> >> # will consume more CPU.
> >> # Defaults to 1. (Since 5.0)
> >> #
> >> +# @block-bitmap-mapping: Maps block nodes and bitmaps on them to
> >> +# aliases for the purpose of dirty bitmap migration. Such
> >> +# aliases may for example be the corresponding names on the
> >> +# opposite site.
> >> +# The mapping must be one-to-one and complete: On the source,
> >> +# migrating a bitmap from a node when either is not mapped
> >> +# will result in an error. On the destination, similarly,
> >> +# receiving a bitmap (by alias) from a node (by alias) when
> >> +# either alias is not mapped will result in an error.
> >> +# By default, all node names and bitmap names are mapped to
> >> +# themselves. (Since 5.1)
> >> +#
> >> # Since: 2.4
> >> ##
> >> # TODO either fuse back into MigrationParameters, or make
> >> @@ -811,7 +874,8 @@
> >> '*max-cpu-throttle': 'int',
> >> '*multifd-compression': 'MultiFDCompression',
> >> '*multifd-zlib-level': 'int',
> >> - '*multifd-zstd-level': 'int' } }
> >> + '*multifd-zstd-level': 'int',
> >> + '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ] } }
> >
> > That's a hairy type for a migration parameter!
> > I'm curious what 'info migrate_parameters' does in hmp or what happens
> > if you try and set it?
>
> Oh. I didn’t know these were accessible via HMP.
>
> As for setting it, that fails an assertion because I thus forgot to
> handle it in hmp_migrate_set_parameter(). I think the best thing to do
> would be to just error out, stating that you cannot set that parameter
> via HMP.
OK, we can clean that up sometime later; it seems an easy enough mapping
to add (I'd appreciate if you did, but won't force it).
> Similarly, info migrate_parameters doesn’t suport it, because I didn’t
> implement it in hmp_info_migrate_parameters(). Since
> hmp_info_migrate_parameters() seems to allow free-form reporting, I
> suppose we could report it as:
>
> block-bitmap-mapping:
> node1 -> alias1
> bitmap1 -> bmap-alias1
> bitmap2 -> bmap-alias2
> node2 -> alias2
> bitmap1 -> bmap-alias1
>
> etc.
>
> Or we just don’t report it there (apart from maybe “(present)”), because
> you can’t set it via migrate_set_parameter.
>
>
> If there’s any problem with exposing this via the migration parameters,
> I have no problem with adding a new QMP command as I did in the RFC. I
> was just pointed towards the migration parameters by reviewers, which
> made sense to me, because, well, this kind of is a migration parameter.
Yep, seems like a migration parameter to me; it's just the first time
we've had such a hairy type in there; make sure the libvirt people are
happy with using it that way please.
Dave
> Max
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
