[Qemu-devel] [PATCH RFC 1/2] snapshot: create helper to test that block drivers supports snapshots

[Denis V. Lunev]

The patch enforces proper locking for this operation.

Signed-off-by: Denis V. Lunev <address@hidden>
CC: Stefan Hajnoczi <address@hidden>
CC: Juan Quintela <address@hidden>
---
Patches are compile-tested only. Sent to check the approach, naming and
functions placement. Functions are returning bad BlockDriver via
parameter to make clear distinction when I'll have to return BS to write.

 block/snapshot.c         | 35 +++++++++++++++++++++++++++++++++++
 include/block/snapshot.h |  9 +++++++++
 migration/savevm.c       | 17 ++++-------------
 3 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/block/snapshot.c b/block/snapshot.c
index 89500f2..6b5ce4e 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -25,6 +25,7 @@
 #include "block/snapshot.h"
 #include "block/block_int.h"
 #include "qapi/qmp/qerror.h"
+#include "monitor/monitor.h"
 
 QemuOptsList internal_snapshot_opts = {
     .name = "snapshot",
@@ -356,3 +357,37 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState 
*bs,
 
     return ret;
 }
+
+
+/* Group operations. All block drivers are involved.
+ * These functions will properly handle dataplace (take aio_context_acquire
+ * when appropriate for appropriate block drivers
+ *
+ * Returned block driver will be always locked.
+ */
+
+bool bdrv_snapshot_all_can_snapshot(BlockDriverState **first_bad_bs)
+{
+    BlockDriverState *bs;
+
+    while ((bs = bdrv_next(bs))) {
+        bool ok;
+        AioContext *ctx = bdrv_get_aio_context(bs);
+
+        if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
+            continue;
+        }
+
+        aio_context_acquire(ctx);
+        ok = bdrv_can_snapshot(bs);
+        aio_context_release(ctx);
+
+        if (!ok) {
+            *first_bad_bs = bs;
+            return false;
+        }
+    }
+
+    *first_bad_bs = NULL;
+    return true;
+}
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 770d9bb..61b4b5d 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -75,4 +75,13 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
 int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
                                          const char *id_or_name,
                                          Error **errp);
+
+
+/* Group operations. All block drivers are involved.
+ * These functions will properly handle dataplace (take aio_context_acquire
+ * when appropriate for appropriate block drivers
+ *
+ */
+bool bdrv_snapshot_all_can_snapshot(BlockDriverState **first_bad_bs);
+
 #endif
diff --git a/migration/savevm.c b/migration/savevm.c
index dbcc39a..91ba0bf 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1290,19 +1290,10 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
     const char *name = qdict_get_try_str(qdict, "name");
     Error *local_err = NULL;
 
-    /* Verify if there is a device that doesn't support snapshots and is 
writable */
-    bs = NULL;
-    while ((bs = bdrv_next(bs))) {
-
-        if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
-            continue;
-        }
-
-        if (!bdrv_can_snapshot(bs)) {
-            monitor_printf(mon, "Device '%s' is writable but does not support 
snapshots.\n",
-                               bdrv_get_device_name(bs));
-            return;
-        }
+    if (bdrv_snapshot_all_can_snapshot(&bs)) {
+        monitor_printf(mon, "Device '%s' is writable but does not "
+                       "support snapshots.\n", bdrv_get_device_name(bs));
+        return;
     }
 
     bs = find_vmstate_bs();
-- 
2.5.0