On 20.08.20 14:58, Vladimir Sementsov-Ogievskiy wrote: > 18.08.2020 16:32, Max Reitz 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). >> >> While touching this code, fix a bug where bitmap names longer than 255 >> bytes would fail an assertion in qemu_put_counted_string(). >> >> Suggested-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> Signed-off-by: Max Reitz <mre...@redhat.com> >> --- >> qapi/migration.json | 101 +++++++- >> migration/migration.h | 3 + >> migration/block-dirty-bitmap.c | 409 ++++++++++++++++++++++++++++----- >> migration/migration.c | 30 +++ >> monitor/hmp-cmds.c | 30 +++ >> 5 files changed, 517 insertions(+), 56 deletions(-) >> >> diff --git a/qapi/migration.json b/qapi/migration.json >> index ea53b23dca..0c4ae102b1 100644 >> --- a/qapi/migration.json >> +++ b/qapi/migration.json > > [..] > >> # >> +# @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, but not necessarily >> +# complete: On the source, unmapped bitmaps and all bitmaps >> +# on unmapped nodes will be ignored. On the destination, >> +# all unmapped aliases in the incoming migration stream will >> +# be reported, but they will not result in failure. > Actually, on unknown alias we cancel incoming bitmap migration, which > means that destination vm continues to run, other (non-bitmap) migration > states continue to migrate but all further chunks of bitmap migration > will be ignored. (I'm not sure it worth be mentioned)
Ah, yeah. [...] >> @@ -303,21 +497,39 @@ static int add_bitmaps_to_list(DBMSaveState *s, >> BlockDriverState *bs, >> return 0; >> } >> + bitmap_name = bdrv_dirty_bitmap_name(bitmap); >> + >> if (!bs_name || strcmp(bs_name, "") == 0) { >> error_report("Bitmap '%s' in unnamed node can't be migrated", >> - bdrv_dirty_bitmap_name(bitmap)); >> + bitmap_name); >> return -1; >> } >> - if (bs_name[0] == '#') { >> + if (alias_map) { >> + const AliasMapInnerNode *amin = >> g_hash_table_lookup(alias_map, bs_name); >> + >> + if (!amin) { >> + /* Skip bitmaps on nodes with no alias */ >> + return 0; >> + } >> + >> + node_alias = amin->string; >> + bitmap_aliases = amin->subtree; >> + } else { >> + node_alias = bs_name; >> + bitmap_aliases = NULL; >> + } >> + >> + if (node_alias[0] == '#') { >> error_report("Bitmap '%s' in a node with auto-generated " >> "name '%s' can't be migrated", >> - bdrv_dirty_bitmap_name(bitmap), bs_name); >> + bitmap_name, node_alias); >> return -1; >> } > > This check is related only to pre-alias_map behavior, so it's probably > better to keep it inside else{} branch above. Still, aliases already > checked to be wellformed, so this check will be always false anyway for > aliases and will not hurt. Hm, it’s a trade off. It does look a bit weird, because how can aliases be auto-generated? But OTOH it makes it clearer that we’ll never allow non-wellformed aliases through. [...] >> if (s->flags & DIRTY_BITMAP_MIG_FLAG_BITMAP_NAME) { >> - if (!qemu_get_counted_string(f, s->bitmap_name)) { >> + const char *bitmap_name; >> + >> + if (!qemu_get_counted_string(f, s->bitmap_alias)) { >> error_report("Unable to read bitmap name string"); > > Probably s/name/alias/ like for node error message. Why not. [...] >> --- a/monitor/hmp-cmds.c >> +++ b/monitor/hmp-cmds.c >> @@ -469,6 +469,32 @@ void hmp_info_migrate_parameters(Monitor *mon, >> const QDict *qdict) >> monitor_printf(mon, "%s: '%s'\n", >> MigrationParameter_str(MIGRATION_PARAMETER_TLS_AUTHZ), >> params->tls_authz); >> + >> + if (params->has_block_bitmap_mapping) { >> + const BitmapMigrationNodeAliasList *bmnal; >> + >> + monitor_printf(mon, "%s:\n", >> + MigrationParameter_str( >> + >> MIGRATION_PARAMETER_BLOCK_BITMAP_MAPPING)); >> + >> + for (bmnal = params->block_bitmap_mapping; >> + bmnal; >> + bmnal = bmnal->next) >> + { >> + const BitmapMigrationNodeAlias *bmna = bmnal->value; >> + const BitmapMigrationBitmapAliasList *bmbal; >> + >> + monitor_printf(mon, " '%s' -> '%s'\n", > > '->' would look strange for incoming. Maybe, change to '--' or '~'. Hmm, I prefer ->. The object’s name is the node/bitmap name, and that object gets an alias. So I find this to make sense even on the incoming side. Max
signature.asc
Description: OpenPGP digital signature