[Qemu-devel] [PATCH 6/8] migration, block: better select BDS for VM state loading

[Denis V. Lunev]

This patch does 2 things:
- it merges all snapshot validity checks for load_vmstate into one function
- it now selects BDS to load VM state by availability of the state in the
  snapshot

This commit is preparatory to allow to select BDS to save snapshot for QMP.

Signed-off-by: Denis V. Lunev <address@hidden>
CC: Kevin Wolf <address@hidden>
CC: Juan Quintela <address@hidden>
CC: Amit Shah <address@hidden>
CC: Markus Armbruster <address@hidden>
CC: Eric Blake <address@hidden>
---
 block/snapshot.c         | 61 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/block/snapshot.h |  1 +
 migration/savevm.c       | 33 ++------------------------
 3 files changed, 64 insertions(+), 31 deletions(-)

diff --git a/block/snapshot.c b/block/snapshot.c
index 2d86b88..77affbc 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -489,3 +489,64 @@ BlockDriverState *bdrv_all_find_vmstate_bs(void)
     }
     return bs;
 }
+
+
+static bool validate_bs(BlockDriverState *bs, BlockDriverState **vmstate_bs,
+                        const char *name, Error **errp)
+{
+    QEMUSnapshotInfo sn;
+
+    if (!bdrv_can_snapshot(bs)) {
+        if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
+            return true;
+        }
+        error_setg(errp,
+                   "Device '%s' is writable but does not support snapshots",
+                   bdrv_get_device_name(bs));
+        return false;
+    }
+
+    if (bdrv_snapshot_find(bs, &sn, name) < 0) {
+        error_setg(errp,
+                   "Device '%s' does not have the requested snapshot '%s'",
+                   bdrv_get_device_name(bs), name);
+        return false;
+    }
+
+    if (sn.vm_state_size == 0) {
+        return true;
+    }
+    if (*vmstate_bs != NULL) {
+        error_setg(errp, "Devices '%s' and '%s' both has vmstate with the "
+                   "requested snapshot '%s'", bdrv_get_device_name(bs),
+                   bdrv_get_device_name(*vmstate_bs), name);
+        return false;
+    }
+    *vmstate_bs = bs;
+
+    return true;
+}
+
+BlockDriverState *bdrv_all_validate_snapshot(const char *name, Error **errp)
+{
+    BlockDriverState *bs = NULL;
+    BlockDriverState *vmstate_bs = NULL;
+
+    while ((bs = bdrv_next(bs))) {
+        AioContext *ctx = bdrv_get_aio_context(bs);
+        bool ok;
+
+        aio_context_acquire(ctx);
+        ok = validate_bs(bs, &vmstate_bs, name, errp);
+        aio_context_release(ctx);
+
+        if (!ok) {
+            return NULL;
+        }
+    }
+
+    if (vmstate_bs == NULL) {
+        error_setg(errp, "VM state for the snapshot '%s' not found", name);
+    }
+    return vmstate_bs;
+}
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index c6910da6..a8506e9 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -92,5 +92,6 @@ int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
                              BlockDriverState **first_bad_bs);
 
 BlockDriverState *bdrv_all_find_vmstate_bs(void);
+BlockDriverState *bdrv_all_validate_snapshot(const char *name, Error **errp);
 
 #endif
diff --git a/migration/savevm.c b/migration/savevm.c
index 6b34017..fed8664 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2022,45 +2022,16 @@ void qmp_xen_save_devices_state(const char *filename, 
Error **errp)
 int load_vmstate(const char *name, Error **errp)
 {
     BlockDriverState *bs, *bs_vm_state;
-    QEMUSnapshotInfo sn;
     QEMUFile *f;
     int ret;
     AioContext *aio_context;
 
-    if (!bdrv_all_can_snapshot(&bs)) {
-        error_setg(errp,
-                   "Device '%s' is writable but does not support snapshots",
-                   bdrv_get_device_name(bs));
-        return -ENOTSUP;
-    }
-    ret = bdrv_all_find_snapshot(name, &bs);
-    if (ret < 0) {
-        error_setg(errp,
-                   "Device '%s' does not have the requested snapshot '%s'",
-                   bdrv_get_device_name(bs), name);
-        return ret;
-    }
-
-    bs_vm_state = bdrv_all_find_vmstate_bs();
-    if (!bs_vm_state) {
-        error_setg(errp, "No block device supports snapshots");
+    bs_vm_state = bdrv_all_validate_snapshot(name, errp);
+    if (bs == NULL) {
         return -ENOTSUP;
     }
     aio_context = bdrv_get_aio_context(bs_vm_state);
 
-    /* Don't even try to load empty VM states */
-    aio_context_acquire(aio_context);
-    ret = bdrv_snapshot_find(bs_vm_state, &sn, name);
-    aio_context_release(aio_context);
-    if (ret < 0) {
-        error_setg_errno(errp, -ret, "Snapshot '%s' not found", name);
-        return ret;
-    } else if (sn.vm_state_size == 0) {
-        error_setg(errp, "Snapshot '%s' is a disk-only snapshot and must be "
-                   "reverted offline using qemu-img", name);
-        return -EINVAL;
-    }
-
     /* Flush all IO requests so they don't interfere with the new state.  */
     bdrv_drain_all();
 
-- 
2.5.0