[Qemu-devel] [PATCH v3 5/8] qmp: convert blockdev-snapshot-sync to a wrapper around transactions

[Paolo Bonzini <pbonzini@redhat.com>]

Simplify the blockdev-snapshot-sync code and gain failsafe operation
by turning it into a wrapper around the new transaction command.  A new
option is also added matching "mode".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 blockdev.c       |   81 ++++++++++++-----------------------------------------
 hmp-commands.hx  |    9 ++++--
 hmp.c            |    6 +++-
 qapi-schema.json |   15 +++++----
 qmp-commands.hx  |    2 +
 5 files changed, 40 insertions(+), 73 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index c9bd25b..06c3017 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -648,70 +648,27 @@ void do_commit(Monitor *mon, const QDict *qdict)
 
 void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file,
                                 bool has_format, const char *format,
+                                bool has_mode, enum NewImageMode mode,
                                 Error **errp)
 {
-    BlockDriverState *bs;
-    BlockDriver *drv, *old_drv, *proto_drv;
-    int ret = 0;
-    int flags;
-    char old_filename[1024];
-
-    bs = bdrv_find(device);
-    if (!bs) {
-        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
-        return;
-    }
-    if (bdrv_in_use(bs)) {
-        error_set(errp, QERR_DEVICE_IN_USE, device);
-        return;
-    }
-
-    pstrcpy(old_filename, sizeof(old_filename), bs->filename);
-
-    old_drv = bs->drv;
-    flags = bs->open_flags;
-
-    if (!has_format) {
-        format = "qcow2";
-    }
-
-    drv = bdrv_find_format(format);
-    if (!drv) {
-        error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
-        return;
-    }
-
-    proto_drv = bdrv_find_protocol(snapshot_file);
-    if (!proto_drv) {
-        error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
-        return;
-    }
-
-    ret = bdrv_img_create(snapshot_file, format, bs->filename,
-                          bs->drv->format_name, NULL, -1, flags);
-    if (ret) {
-        error_set(errp, QERR_UNDEFINED_ERROR);
-        return;
-    }
-
-    bdrv_drain_all();
-    bdrv_flush(bs);
-
-    bdrv_close(bs);
-    ret = bdrv_open(bs, snapshot_file, flags, drv);
-    /*
-     * If reopening the image file we just created fails, fall back
-     * and try to re-open the original image. If that fails too, we
-     * are in serious trouble.
-     */
-    if (ret != 0) {
-        ret = bdrv_open(bs, old_filename, flags, old_drv);
-        if (ret != 0) {
-            error_set(errp, QERR_OPEN_FILE_FAILED, old_filename);
-        } else {
-            error_set(errp, QERR_OPEN_FILE_FAILED, snapshot_file);
-        }
-    }
+    BlockdevSnapshot snapshot = {
+        .device = (char *) device,
+        .snapshot_file = (char *) snapshot_file,
+        .has_format = has_format,
+        .format = (char *) format,
+        .has_mode = has_mode,
+        .mode = mode,
+    };
+    BlockdevAction action = {
+        .kind = BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC,
+        .blockdev_snapshot_sync = &snapshot,
+    };
+    BlockdevActionList list = {
+        .value = &action,
+        .next = NULL
+    };
+
+    qmp_transaction(&list, errp);
 }
 
 
diff --git a/hmp-commands.hx b/hmp-commands.hx
index ed88877..6980214 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -882,14 +882,17 @@ ETEXI
 
     {
         .name       = "snapshot_blkdev",
-        .args_type  = "device:B,snapshot-file:s?,format:s?",
-        .params     = "device [new-image-file] [format]",
+        .args_type  = "reuse:-n,device:B,snapshot-file:s?,format:s?",
+        .params     = "[-n] device [new-image-file] [format]",
         .help       = "initiates a live snapshot\n\t\t\t"
                       "of device. If a new image file is specified, the\n\t\t\t"
                       "new image file will become the new root image.\n\t\t\t"
                       "If format is specified, the snapshot file will\n\t\t\t"
                       "be created in that format. Otherwise the\n\t\t\t"
-                      "snapshot will be internal! (currently unsupported)",
+                      "snapshot will be internal! (currently unsupported).\n\t\t\t"
+                      "The default format is qcow2.  The -n flag requests QEMU\n\t\t\t"
+                      "to reuse the image found in new-image-file, instead of\n\t\t\t"
+                      "recreating it from scratch.",
         .mhandler.cmd = hmp_snapshot_blkdev,
     },
 
diff --git a/hmp.c b/hmp.c
index 3a54455..290c43d 100644
--- a/hmp.c
+++ b/hmp.c
@@ -692,6 +692,8 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict)
     const char *device = qdict_get_str(qdict, "device");
     const char *filename = qdict_get_try_str(qdict, "snapshot-file");
     const char *format = qdict_get_try_str(qdict, "format");
+    int reuse = qdict_get_try_bool(qdict, "reuse", 0);
+    enum NewImageMode mode;
     Error *errp = NULL;
 
     if (!filename) {
@@ -702,7 +704,9 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict)
         return;
     }
 
-    qmp_blockdev_snapshot_sync(device, filename, !!format, format, &errp);
+    mode = reuse ? NEW_IMAGE_MODE_EXISTING : NEW_IMAGE_MODE_ABSOLUTE_PATHS;
+    qmp_blockdev_snapshot_sync(device, filename, !!format, format,
+                               true, mode, &errp);
     hmp_handle_error(mon, &errp);
 }
 
diff --git a/qapi-schema.json b/qapi-schema.json
index b062d01..0d24240 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1143,6 +1143,9 @@
 # @snapshot-file: the target of the new image. A new file will be created.
 #
 # @format: #optional the format of the snapshot image, default is 'qcow2'.
+#
+# @mode: #optional whether and how QEMU should create a new image, default is
+# 'absolute-paths'.
 ##
 { 'type': 'BlockdevSnapshot',
   'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str',
@@ -1199,21 +1202,19 @@
 #
 # @format: #optional the format of the snapshot image, default is 'qcow2'.
 #
+# @mode: #optional whether and how QEMU should create a new image, default is
+# 'absolute-paths'.
+#
 # Returns: nothing on success
 #          If @device is not a valid block device, DeviceNotFound
 #          If @snapshot-file can't be opened, OpenFileFailed
 #          If @format is invalid, InvalidBlockFormat
 #
-# Notes: One of the last steps taken by this command is to close the current
-#        image being used by @device and open the @snapshot-file one. If that
-#        fails, the command will try to reopen the original image file. If
-#        that also fails OpenFileFailed will be returned and the guest may get
-#        unexpected errors.
-#
 # Since 0.14.0
 ##
 { 'command': 'blockdev-snapshot-sync',
-  'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str' } }
+  'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str',
+            '*mode': 'NewImageMode'} }
 
 ##
 # @human-monitor-command:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 7c03b62..dfe8a5b 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -760,6 +760,8 @@ Arguments:
 
 - "device": device name to snapshot (json-string)
 - "snapshot-file": name of new image file (json-string)
+- "mode": whether and how QEMU should create the snapshot file
+  (NewImageMode, optional, default "absolute-paths")
 - "format": format of new image (json-string, optional)
 
 Example:
-- 
1.7.7.6