qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v4 1/4] migration: Add block-bitmap-mapping parameter


From: Eric Blake
Subject: Re: [PATCH v4 1/4] migration: Add block-bitmap-mapping parameter
Date: Wed, 19 Aug 2020 20:17:43 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0

On 8/18/20 8:32 AM, 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 <vsementsov@virtuozzo.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---

+##
+# @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.2
+##
+{ 'struct': 'BitmapMigrationNodeAlias',
+  'data': {
+      'node-name': 'str',
+      'alias': 'str',
+      'bitmaps': [ 'BitmapMigrationBitmapAlias' ]
+  } }

Possible change: should 'alias' be optional (if absent, it defaults to 'node-name')? But that can be done on top, if we like it.


+static GHashTable *construct_alias_map(const BitmapMigrationNodeAliasList *bbm,
+                                       bool name_to_alias,
+                                       Error **errp)
+{
+    GHashTable *alias_map;
+    size_t max_node_name_len =
+        sizeof(((BlockDriverState *)NULL)->node_name) - 1;

Looks a bit nicer as = sizeof_field(BlockDriverState, node_name) - 1.

+
+    alias_map = g_hash_table_new_full(g_str_hash, g_str_equal,
+                                      g_free, free_alias_map_inner_node);
+
+    for (; bbm; bbm = bbm->next) {
+        const BitmapMigrationNodeAlias *bmna = bbm->value;
+        const BitmapMigrationBitmapAliasList *bmbal;
+        AliasMapInnerNode *amin;
+        GHashTable *bitmaps_map;
+        const char *node_map_from, *node_map_to;
+
+        if (!id_wellformed(bmna->alias)) {
+            error_setg(errp, "The node alias '%s' is not well-formed",
+                       bmna->alias);
+            goto fail;
+        }
+
+        if (strlen(bmna->alias) > 255) {

Magic number. UINT8_MAX seems better (since the limit really is due to our migration format limiting to one byte).

...
+        g_hash_table_insert(alias_map, g_strdup(node_map_from), amin);
+
+        for (bmbal = bmna->bitmaps; bmbal; bmbal = bmbal->next) {
+            const BitmapMigrationBitmapAlias *bmba = bmbal->value;
+            const char *bmap_map_from, *bmap_map_to;
+
+            if (strlen(bmba->alias) > 255) {

and again

+                error_setg(errp,
+                           "The bitmap alias '%s' is longer than 255 bytes",
+                           bmba->alias);
+                goto fail;
+            }

Thanks for adding in the length checking since last revision!


@@ -326,12 +538,29 @@ static int add_bitmaps_to_list(DBMSaveState *s, 
BlockDriverState *bs,
              return -1;
          }
+ if (bitmap_aliases) {
+            bitmap_alias = g_hash_table_lookup(bitmap_aliases, bitmap_name);
+            if (!bitmap_alias) {
+                /* Skip bitmaps with no alias */
+                continue;
+            }
+        } else {
+            if (strlen(bitmap_name) > 255) {
+                error_report("Cannot migrate bitmap '%s' on node '%s': "
+                             "Name is longer than 255 bytes",
+                             bitmap_name, bs_name);
+                return -1;

Another one.


Reviewed-by: Eric Blake <eblake@redhat.com>

I'm happy to make those touchups, and put this on my bitmaps queue for a pull request as soon as Paolo's meson stuff stabilizes.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org




reply via email to

[Prev in Thread] Current Thread [Next in Thread]