[PATCH] migration: block-dirty-bitmap: rewrite dirty_bitmap_load_header

[Paolo Bonzini]

Alex reported an uninitialized variable warning in dirty_bitmap_load_header,
where the compiler cannot understand that the !s->cancelled check must be
true for the following one to pass.

Even though the issue happened only because of -Og, the function is very
convoluted.  Just rewrite it to first look up s->bs and then s->bitmap,
and to avoid long sequences of "else if"s.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 migration/block-dirty-bitmap.c | 138 ++++++++++++++++-----------------
 1 file changed, 68 insertions(+), 70 deletions(-)

diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 5bef793ac0..24d749e35e 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -1010,102 +1010,100 @@ static int dirty_bitmap_load_bits(QEMUFile *f, 
DBMLoadState *s)
 static int dirty_bitmap_load_header(QEMUFile *f, DBMLoadState *s,
                                     GHashTable *alias_map)
 {
-    GHashTable *bitmap_alias_map = NULL;
-    Error *local_err = NULL;
-    bool nothing;
     s->flags = qemu_get_bitmap_flags(f);
     trace_dirty_bitmap_load_header(s->flags);
 
-    nothing = s->flags == (s->flags & DIRTY_BITMAP_MIG_FLAG_EOS);
-
+    /* Read the whole header early so we can easily cancel in case of errors.  
*/
     if (s->flags & DIRTY_BITMAP_MIG_FLAG_DEVICE_NAME) {
         if (!qemu_get_counted_string(f, s->node_alias)) {
             error_report("Unable to read node alias string");
             return -EINVAL;
         }
-
-        if (!s->cancelled) {
-            if (alias_map) {
-                const AliasMapInnerNode *amin;
-
-                amin = g_hash_table_lookup(alias_map, s->node_alias);
-                if (!amin) {
-                    error_setg(&local_err, "Error: Unknown node alias '%s'",
-                               s->node_alias);
-                    s->bs = NULL;
-                } else {
-                    bitmap_alias_map = amin->subtree;
-                    s->bs = bdrv_lookup_bs(NULL, amin->string, &local_err);
-                }
-            } else {
-                s->bs = bdrv_lookup_bs(s->node_alias, s->node_alias,
-                                       &local_err);
-            }
-            if (!s->bs) {
-                error_report_err(local_err);
-                cancel_incoming_locked(s);
-            }
+    }
+    if (s->flags & DIRTY_BITMAP_MIG_FLAG_BITMAP_NAME) {
+        if (!qemu_get_counted_string(f, s->bitmap_alias)) {
+            error_report("Unable to read bitmap alias string");
+            return -EINVAL;
         }
-    } else if (s->bs) {
+    }
+
+    if ((s->flags & ~DIRTY_BITMAP_MIG_FLAG_EOS) == 0 || s->cancelled) {
+        return 0;
+    }
+
+    if (s->flags & DIRTY_BITMAP_MIG_FLAG_DEVICE_NAME) {
+        Error *local_err = NULL;
         if (alias_map) {
             const AliasMapInnerNode *amin;
 
-            /* Must be present in the map, or s->bs would not be set */
             amin = g_hash_table_lookup(alias_map, s->node_alias);
-            assert(amin != NULL);
-
-            bitmap_alias_map = amin->subtree;
+            if (!amin) {
+                error_report("Error: Unknown node alias '%s'", s->node_alias);
+                s->bs = NULL;
+                goto cancel;
+            }
+            s->bs = bdrv_lookup_bs(NULL, amin->string, &local_err);
+        } else {
+            s->bs = bdrv_lookup_bs(s->node_alias, s->node_alias, &local_err);
         }
-    } else if (!nothing && !s->cancelled) {
+        if (!s->bs) {
+            error_report_err(local_err);
+            goto cancel;
+        }
+        s->bitmap_name[0] = 0;
+    }
+    if (!s->bs) {
         error_report("Error: block device name is not set");
-        cancel_incoming_locked(s);
+        goto cancel;
     }
 
-    assert(nothing || s->cancelled || !!alias_map == !!bitmap_alias_map);
-
     if (s->flags & DIRTY_BITMAP_MIG_FLAG_BITMAP_NAME) {
         const char *bitmap_name;
 
-        if (!qemu_get_counted_string(f, s->bitmap_alias)) {
-            error_report("Unable to read bitmap alias string");
-            return -EINVAL;
-        }
+        if (alias_map) {
+            const AliasMapInnerNode *amin;
+            GHashTable *bitmap_alias_map;
 
-        if (!s->cancelled) {
-            if (bitmap_alias_map) {
-                bitmap_name = g_hash_table_lookup(bitmap_alias_map,
-                                                  s->bitmap_alias);
-                if (!bitmap_name) {
-                    error_report("Error: Unknown bitmap alias '%s' on node "
-                                 "'%s' (alias '%s')", s->bitmap_alias,
-                                 s->bs->node_name, s->node_alias);
-                    cancel_incoming_locked(s);
-                }
-            } else {
-                bitmap_name = s->bitmap_alias;
-            }
-        }
+            amin = g_hash_table_lookup(alias_map, s->node_alias);
+            bitmap_alias_map = amin->subtree;
 
-        if (!s->cancelled) {
-            g_strlcpy(s->bitmap_name, bitmap_name, sizeof(s->bitmap_name));
-            s->bitmap = bdrv_find_dirty_bitmap(s->bs, s->bitmap_name);
-
-            /*
-             * bitmap may be NULL here, it wouldn't be an error if it is the
-             * first occurrence of the bitmap
-             */
-            if (!s->bitmap && !(s->flags & DIRTY_BITMAP_MIG_FLAG_START)) {
-                error_report("Error: unknown dirty bitmap "
-                             "'%s' for block device '%s'",
-                             s->bitmap_name, s->bs->node_name);
-                cancel_incoming_locked(s);
+            /* Must be present in the map, or s->bs would not be set */
+            assert(bitmap_alias_map);
+            bitmap_name = g_hash_table_lookup(bitmap_alias_map,
+                                              s->bitmap_alias);
+            if (!bitmap_name) {
+                error_report("Error: Unknown bitmap alias '%s' on node "
+                             "'%s' (alias '%s')", s->bitmap_alias,
+                             s->bs->node_name, s->node_alias);
+                goto cancel;
             }
+        } else {
+            bitmap_name = s->bitmap_alias;
         }
-    } else if (!s->bitmap && !nothing && !s->cancelled) {
-        error_report("Error: block device name is not set");
-        cancel_incoming_locked(s);
+
+        g_strlcpy(s->bitmap_name, bitmap_name, sizeof(s->bitmap_name));
+    }
+
+    if (!s->bitmap_name[0]) {
+        error_report("Error: dirty bitmap name is not set");
+        goto cancel;
     }
 
+    /*
+     * bitmap may be NULL here, it wouldn't be an error if it is the
+     * first occurrence of the bitmap
+     */
+    s->bitmap = bdrv_find_dirty_bitmap(s->bs, s->bitmap_name);
+    if (!s->bitmap && !(s->flags & DIRTY_BITMAP_MIG_FLAG_START)) {
+        error_report("Error: unknown dirty bitmap "
+                     "'%s' for block device '%s'",
+                     s->bitmap_name, s->bs->node_name);
+        goto cancel;
+    }
+    return 0;
+
+cancel:
+    cancel_incoming_locked(s);
     return 0;
 }
 
-- 
2.26.2