diff mbox

[07/10] snapshot: create bdrv_all_create_snapshot helper

Message ID 1446911700-16803-8-git-send-email-den@openvz.org
State New
Headers show

Commit Message

Denis V. Lunev Nov. 7, 2015, 3:54 p.m. UTC 
to create snapshot for all loaded block drivers.

The patch also ensures proper locking.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 block/snapshot.c         | 22 ++++++++++++++++++++++
 include/block/snapshot.h |  1 +
 migration/savevm.c       | 20 +++++---------------
 3 files changed, 28 insertions(+), 15 deletions(-)

Comments

Stefan Hajnoczi Nov. 9, 2015, 5:33 p.m. UTC | #1 
On Sat, Nov 07, 2015 at 06:54:57PM +0300, Denis V. Lunev wrote:
> +int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn, BlockDriverState **bad)
> +{
> +    int err = 0;
> +    BlockDriverState *bs = NULL;
> +
> +    while (err == 0 && (bs = bdrv_next(bs))) {
> +        AioContext *ctx = bdrv_get_aio_context(bs);
> +
> +        aio_context_acquire(ctx);
> +        if (bdrv_can_snapshot(bs)) {
> +            err = bdrv_snapshot_create(bs, sn);
> +            /* Tricky part here. First image contains VM state. The behavior
> +             * is matched one in bdrv_all_find_vmstate_bs */
> +            sn->vm_state_size = 0;

Please avoid the tricky part by passing in vm_state_bs and
vm_state_size.  Then this function can do:

  /* Write VM state size only to the image that contains the state */
  sn->vm_state_size = (bs == bs1 ? vm_state_size : 0);

without making assumptions about the algorithm for choosing the device
to store vmstate data on.
diff mbox

Patch

diff --git a/block/snapshot.c b/block/snapshot.c
index a7f1f8d..4daf9d4 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -443,3 +443,25 @@  int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
     *first_bad_bs = bs;
     return err;
 }
+
+int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn, BlockDriverState **bad)
+{
+    int err = 0;
+    BlockDriverState *bs = NULL;
+
+    while (err == 0 && (bs = bdrv_next(bs))) {
+        AioContext *ctx = bdrv_get_aio_context(bs);
+
+        aio_context_acquire(ctx);
+        if (bdrv_can_snapshot(bs)) {
+            err = bdrv_snapshot_create(bs, sn);
+            /* Tricky part here. First image contains VM state. The behavior
+             * is matched one in bdrv_all_find_vmstate_bs */
+            sn->vm_state_size = 0;
+        }
+        aio_context_release(ctx);
+    }
+
+    *bad = bs;
+    return err;
+}
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 10ee582..df443b2 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -86,5 +86,6 @@  int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bsd_bs,
                              Error **err);
 int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bsd_bs);
 int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs);
+int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn, BlockDriverState **bad);
 
 #endif
diff --git a/migration/savevm.c b/migration/savevm.c
index 0c2890d..f72349d 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1255,7 +1255,6 @@  void hmp_savevm(Monitor *mon, const QDict *qdict)
     int ret;
     QEMUFile *f;
     int saved_vm_running;
-    uint64_t vm_state_size;
     qemu_timeval tv;
     struct tm tm;
     const char *name = qdict_get_try_str(qdict, "name");
@@ -1320,7 +1319,7 @@  void hmp_savevm(Monitor *mon, const QDict *qdict)
         goto the_end;
     }
     ret = qemu_savevm_state(f, &local_err);
-    vm_state_size = qemu_ftell(f);
+    sn->vm_state_size = qemu_ftell(f);
     qemu_fclose(f);
     if (ret < 0) {
         monitor_printf(mon, "%s\n", error_get_pretty(local_err));
@@ -1328,19 +1327,10 @@  void hmp_savevm(Monitor *mon, const QDict *qdict)
         goto the_end;
     }
 
-    /* create the snapshots */
-
-    bs1 = NULL;
-    while ((bs1 = bdrv_next(bs1))) {
-        if (bdrv_can_snapshot(bs1)) {
-            /* Write VM state size only to the image that contains the state */
-            sn->vm_state_size = (bs == bs1 ? vm_state_size : 0);
-            ret = bdrv_snapshot_create(bs1, sn);
-            if (ret < 0) {
-                monitor_printf(mon, "Error while creating snapshot on '%s'\n",
-                               bdrv_get_device_name(bs1));
-            }
-        }
+    ret = bdrv_all_create_snapshot(sn, &bs);
+    if (ret < 0) {
+        monitor_printf(mon, "Error while creating snapshot on '%s'\n",
+                       bdrv_get_device_name(bs));
     }
 
  the_end: