[Qemu-devel] [PATCH 5/6] snapshot: qmp interface

[Wenchao Xia]

  This patch changes the implemtion of external block snapshot
to use internal unified interface, now qmp handler just do
a translation of request and submit.
  Also internal block snapshot qmp interface was added.
  Now add external snapshot, add/delete internal snapshot
can be started in their own qmp interface or a group of
BlockAction in qmp transaction interface.

Signed-off-by: Wenchao Xia <address@hidden>
---
 blockdev.c       |  352 +++++++++++++++++++++++++++++++-----------------------
 qapi-schema.json |  102 ++++++++++++++--
 2 files changed, 292 insertions(+), 162 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 1c38c67..d1bbe23 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1175,6 +1175,195 @@ delete_and_fail:
     return -1;
 }
 
+/* translation from qmp commands */
+static int fill_blk_trsact_create_sync(BlockdevSnapshot *create_sync,
+                                       BlkTransactionStatesSync *st_sync,
+                                       SNTime *time,
+                                       const char *time_str,
+                                       Error **errp)
+{
+    const char *format = "qcow2";
+    enum NewImageMode mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
+    enum SnapshotType type = SNAPSHOT_TYPE_EXTERNAL;
+    BlockDriverState *bs;
+
+    const char *device = create_sync->device;
+    const char *name = create_sync->snapshot_file;
+    if (create_sync->has_mode) {
+        mode = create_sync->mode;
+    }
+    if (create_sync->has_format) {
+        format = create_sync->format;
+    }
+    if (create_sync->has_type) {
+        type = create_sync->type;
+    }
+
+    /* find the target bs */
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return -1;
+    }
+
+    switch (type) {
+    case SNAPSHOT_TYPE_INTERNAL:
+        st_sync->type = BLK_SNAPSHOT_INTERNAL;
+        break;
+    case SNAPSHOT_TYPE_EXTERNAL:
+        st_sync->type = BLK_SNAPSHOT_EXTERNAL;
+        break;
+    default:
+        st_sync->type = BLK_SNAPSHOT_NOSUPPORT;
+        error_setg(errp, "Device %s requested invalid snapshot"
+                         " type %d.", device, type);
+        return -1;
+    }
+
+    switch (mode) {
+    case NEW_IMAGE_MODE_EXISTING:
+        st_sync->use_existing = TRUE;
+        break;
+    case NEW_IMAGE_MODE_ABSOLUTE_PATHS:
+        st_sync->use_existing = FALSE;
+        break;
+    default:
+        error_setg(errp, "Device %s requested invalid snapshot"
+                         " mode %d.", device, mode);
+        return -1;
+    }
+
+    if (st_sync->type == BLK_SNAPSHOT_INTERNAL) {
+        /* internal case, if caller need create new one with default string */
+        if (((name == NULL) || (name[0] == '\0')) &&
+           (!st_sync->use_existing)) {
+            st_sync->internal.sn_name = time_str;
+        } else {
+            st_sync->internal.sn_name = name;
+        }
+        st_sync->internal.bs = bs;
+        st_sync->internal.time = *time;
+    } else if (st_sync->type == BLK_SNAPSHOT_EXTERNAL) {
+        st_sync->external.new_image_file = name;
+        st_sync->external.format = format;
+        st_sync->external.old_bs = bs;
+    }
+
+    return 0;
+}
+static int fill_blk_trsact_delete_sync(BlockdevSnapshotDelete *delete_sync,
+                                       BlkTransactionStatesSync *st_sync,
+                                       Error **errp)
+{
+    enum SnapshotType type = SNAPSHOT_TYPE_EXTERNAL;
+    BlockDriverState *bs;
+
+    const char *device = delete_sync->device;
+    const char *name = delete_sync->snapshot_file;
+    if (delete_sync->has_type) {
+        type = delete_sync->type;
+    }
+
+    /* find the target bs */
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return -1;
+    }
+
+    switch (type) {
+    case SNAPSHOT_TYPE_INTERNAL:
+        st_sync->type = BLK_SNAPSHOT_INTERNAL;
+        break;
+    case SNAPSHOT_TYPE_EXTERNAL:
+        st_sync->type = BLK_SNAPSHOT_EXTERNAL;
+        break;
+    default:
+        st_sync->type = BLK_SNAPSHOT_NOSUPPORT;
+        error_setg(errp, "Device %s requested invalid snapshot"
+                         " type %d.", device, type);
+        return -1;
+    }
+
+    if (st_sync->type == BLK_SNAPSHOT_INTERNAL) {
+        st_sync->internal.sn_name = name;
+        st_sync->internal.bs = bs;
+    } else if (st_sync->type == BLK_SNAPSHOT_EXTERNAL) {
+        st_sync->external.new_image_file = name;
+        st_sync->external.old_bs = bs;
+    }
+
+    return 0;
+}
+
+static int fill_blk_trsact(BlockdevAction *dev_info,
+                           BlkTransactionStates *states,
+                           SNTime *time,
+                           const char *time_str,
+                           Error **errp)
+{
+    switch (dev_info->kind) {
+    case BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC:
+        states->st_sync.op = BLK_SN_SYNC_CREATE;
+        states->async = FALSE;
+        return fill_blk_trsact_create_sync(dev_info->blockdev_snapshot_sync,
+                                   &states->st_sync, time, time_str, errp);
+        break;
+    case BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_DELETE_SYNC:
+        states->st_sync.op = BLK_SN_SYNC_DELETE;
+        states->async = FALSE;
+        return fill_blk_trsact_delete_sync(
+                                    dev_info->blockdev_snapshot_delete_sync,
+                                    &states->st_sync, errp);
+        break;
+    default:
+        abort();
+    }
+    return 0;
+}
+
+/* Here this funtion prepare the request list, submit for atomic snapshot. */
+void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
+{
+    BlockdevActionList *dev_entry = dev_list;
+    BlkTransactionStates *states;
+    int ret;
+
+    BlkTransactionStatesList *snap_bdrv_states = blk_trans_st_list_new();
+
+    /* translate qmp request */
+    /* for group snapshot create we use same time stamp here */
+    SNTime time = get_sn_time();
+    char time_str[256];
+    generate_sn_name_from_time(&time, time_str, sizeof(time_str));
+    while (NULL != dev_entry) {
+        BlockdevAction *dev_info = NULL;
+
+        dev_info = dev_entry->value;
+        dev_entry = dev_entry->next;
+
+        states = blk_trans_st_new();
+        ret = fill_blk_trsact(dev_info, states, &time, time_str, errp);
+        if (ret < 0) {
+            blk_trans_st_delete(&states);
+            goto exit;
+        }
+
+        ret = add_transaction(snap_bdrv_states, states, errp);
+        if (ret < 0) {
+            blk_trans_st_delete(&states);
+            goto exit;
+        }
+    }
+
+    /* submit to internal API, no need to check return for no following
+       action now. */
+    submit_transaction(snap_bdrv_states, errp);
+
+exit:
+    blk_trans_st_list_delete(&snap_bdrv_states);
+}
+
 static void blockdev_do_action(int kind, void *data, Error **errp)
 {
     BlockdevAction action;
@@ -1190,6 +1379,7 @@ static void blockdev_do_action(int kind, void *data, 
Error **errp)
 void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file,
                                 bool has_format, const char *format,
                                 bool has_mode, enum NewImageMode mode,
+                                bool has_type, enum SnapshotType type,
                                 Error **errp)
 {
     BlockdevSnapshot snapshot = {
@@ -1199,162 +1389,28 @@ void qmp_blockdev_snapshot_sync(const char *device, 
const char *snapshot_file,
         .format = (char *) format,
         .has_mode = has_mode,
         .mode = mode,
+        .has_type = has_type,
+        .type = type,
     };
     blockdev_do_action(BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC, &snapshot,
                        errp);
 }
 
-
-/* New and old BlockDriverState structs for group snapshots */
-typedef struct BlkTransactionStates {
-    BlockDriverState *old_bs;
-    BlockDriverState *new_bs;
-    QSIMPLEQ_ENTRY(BlkTransactionStates) entry;
-} BlkTransactionStates;
-
-/*
- * 'Atomic' group snapshots.  The snapshots are taken as a set, and if any fail
- *  then we do not pivot any of the devices in the group, and abandon the
- *  snapshots
- */
-void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
+void qmp_blockdev_snapshot_delete_sync(const char *device,
+                                       const char *snapshot_file,
+                                       bool has_type, enum SnapshotType type,
+                                       Error **errp)
 {
-    int ret = 0;
-    BlockdevActionList *dev_entry = dev_list;
-    BlkTransactionStates *states, *next;
-    Error *local_err = NULL;
-
-    QSIMPLEQ_HEAD(snap_bdrv_states, BlkTransactionStates) snap_bdrv_states;
-    QSIMPLEQ_INIT(&snap_bdrv_states);
-
-    /* drain all i/o before any snapshots */
-    bdrv_drain_all();
-
-    /* We don't do anything in this loop that commits us to the snapshot */
-    while (NULL != dev_entry) {
-        BlockdevAction *dev_info = NULL;
-        BlockDriver *proto_drv;
-        BlockDriver *drv;
-        int flags;
-        enum NewImageMode mode;
-        const char *new_image_file;
-        const char *device;
-        const char *format = "qcow2";
-
-        dev_info = dev_entry->value;
-        dev_entry = dev_entry->next;
-
-        states = g_malloc0(sizeof(BlkTransactionStates));
-        QSIMPLEQ_INSERT_TAIL(&snap_bdrv_states, states, entry);
-
-        switch (dev_info->kind) {
-        case BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC:
-            device = dev_info->blockdev_snapshot_sync->device;
-            if (!dev_info->blockdev_snapshot_sync->has_mode) {
-                dev_info->blockdev_snapshot_sync->mode = 
NEW_IMAGE_MODE_ABSOLUTE_PATHS;
-            }
-            new_image_file = dev_info->blockdev_snapshot_sync->snapshot_file;
-            if (dev_info->blockdev_snapshot_sync->has_format) {
-                format = dev_info->blockdev_snapshot_sync->format;
-            }
-            mode = dev_info->blockdev_snapshot_sync->mode;
-            break;
-        default:
-            abort();
-        }
-
-        drv = bdrv_find_format(format);
-        if (!drv) {
-            error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
-            goto delete_and_fail;
-        }
-
-        states->old_bs = bdrv_find(device);
-        if (!states->old_bs) {
-            error_set(errp, QERR_DEVICE_NOT_FOUND, device);
-            goto delete_and_fail;
-        }
-
-        if (!bdrv_is_inserted(states->old_bs)) {
-            error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
-            goto delete_and_fail;
-        }
-
-        if (bdrv_in_use(states->old_bs)) {
-            error_set(errp, QERR_DEVICE_IN_USE, device);
-            goto delete_and_fail;
-        }
-
-        if (!bdrv_is_read_only(states->old_bs)) {
-            if (bdrv_flush(states->old_bs)) {
-                error_set(errp, QERR_IO_ERROR);
-                goto delete_and_fail;
-            }
-        }
-
-        flags = states->old_bs->open_flags;
-
-        proto_drv = bdrv_find_protocol(new_image_file);
-        if (!proto_drv) {
-            error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
-            goto delete_and_fail;
-        }
-
-        /* create new image w/backing file */
-        if (mode != NEW_IMAGE_MODE_EXISTING) {
-            bdrv_img_create(new_image_file, format,
-                            states->old_bs->filename,
-                            states->old_bs->drv->format_name,
-                            NULL, -1, flags, &local_err);
-            if (error_is_set(&local_err)) {
-                error_propagate(errp, local_err);
-                goto delete_and_fail;
-            }
-        }
-
-        /* We will manually add the backing_hd field to the bs later */
-        states->new_bs = bdrv_new("");
-        ret = bdrv_open(states->new_bs, new_image_file,
-                        flags | BDRV_O_NO_BACKING, drv);
-        if (ret != 0) {
-            error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
-            goto delete_and_fail;
-        }
-    }
-
-
-    /* Now we are going to do the actual pivot.  Everything up to this point
-     * is reversible, but we are committed at this point */
-    QSIMPLEQ_FOREACH(states, &snap_bdrv_states, entry) {
-        /* This removes our old bs from the bdrv_states, and adds the new bs */
-        bdrv_append(states->new_bs, states->old_bs);
-        /* We don't need (or want) to use the transactional
-         * bdrv_reopen_multiple() across all the entries at once, because we
-         * don't want to abort all of them if one of them fails the reopen */
-        bdrv_reopen(states->new_bs, states->new_bs->open_flags & ~BDRV_O_RDWR,
-                    NULL);
-    }
-
-    /* success */
-    goto exit;
-
-delete_and_fail:
-    /*
-    * failure, and it is all-or-none; abandon each new bs, and keep using
-    * the original bs for all images
-    */
-    QSIMPLEQ_FOREACH(states, &snap_bdrv_states, entry) {
-        if (states->new_bs) {
-             bdrv_delete(states->new_bs);
-        }
-    }
-exit:
-    QSIMPLEQ_FOREACH_SAFE(states, &snap_bdrv_states, entry, next) {
-        g_free(states);
-    }
+    BlockdevSnapshotDelete snapshot_delete = {
+        .device = (char *) device,
+        .snapshot_file = (char *) snapshot_file,
+        .has_type = has_type,
+        .type = type,
+    };
+    blockdev_do_action(BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_DELETE_SYNC,
+                       &snapshot_delete, errp);
 }
 
-
 static void eject_device(BlockDriverState *bs, int force, Error **errp)
 {
     if (bdrv_in_use(bs)) {
diff --git a/qapi-schema.json b/qapi-schema.json
index 5dfa052..46f4c6b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1458,17 +1458,36 @@
 { 'command': 'block_resize', 'data': { 'device': 'str', 'size': 'int' }}
 
 ##
+# @SnapshotType
+#
+# An enumeration that tells QEMU what type of snapshot to access.
+#
+# @internal: QEMU should use internal snapshot in format such as qcow2.
+#
+# @external: QEMU should use backing file chain.
+#
+# Since: 1.4.
+##
+{ 'enum': 'SnapshotType'
+  'data': [ 'internal', 'external' ] }
+
+##
 # @NewImageMode
 #
 # An enumeration that tells QEMU how to set the backing file path in
-# a new image file.
+# a new image file, or how to use internal snapshot record.
 #
-# @existing: QEMU should look for an existing image file.
+# @existing: QEMU should look for an existing image file or internal snapshot
+#            record. In external snapshot case, qemu will skip create new image
+#            file, In internal snapshot case qemu will try use the existing
+#            one. if not found operation would fail.
 #
-# @absolute-paths: QEMU should create a new image with absolute paths
-# for the backing file.
+# @absolute-paths: QEMU should create a new image with absolute paths for
+#                  the backing file in external snapshot case, or create a new
+#                  snapshot record in internal snapshot case which will
+#                  overwrite internal snapshot record if it already exist.
 #
-# Since: 1.1
+# Since: 1.1, internal support since 1.4.
 ##
 { 'enum': 'NewImageMode'
   'data': [ 'existing', 'absolute-paths' ] }
@@ -1478,16 +1497,39 @@
 #
 # @device:  the name of the device to generate the snapshot from.
 #
-# @snapshot-file: the target of the new image. A new file will be created.
+# @snapshot-file: the target name of the snapshot. In external case, it is
+#                 the new file's name, A new file will be created. In internal
+#                 case, it is the internal snapshot record's name and if it is
+#                 'blank' name will be generated according to time.
 #
 # @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'.
+# @mode: #optional whether QEMU should create a new snapshot or use existing
+#        one, default is 'absolute-paths'.
+#
+# @type: #optional internal snapshot or external, default is 'external'.
+#
 ##
 { 'type': 'BlockdevSnapshot',
   'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str',
-            '*mode': 'NewImageMode' } }
+            '*mode': 'NewImageMode', '*type': 'SnapshotType'} }
+
+##
+# @BlockdevSnapshotDelete
+#
+# @device:  the name of the device to delete the snapshot from.
+#
+# @snapshot-file: the target name of the snapshot. In external case, it is
+#                 the file's name to be merged, In internal case, it is the
+#                 internal snapshot record's name.
+#
+# @type: #optional internal snapshot or external, default is
+#        'external', note that delete 'external' snapshot is not supported
+#        now for that it is the same to commit it.
+##
+{ 'type': 'BlockdevSnapshotDelete',
+  'data': { 'device': 'str', 'snapshot-file': 'str',
+            '*type': 'SnapshotType'} }
 
 ##
 # @BlockdevAction
@@ -1498,6 +1540,7 @@
 { 'union': 'BlockdevAction',
   'data': {
        'blockdev-snapshot-sync': 'BlockdevSnapshot',
+       'blockdev-snapshot-delete-sync': 'BlockdevSnapshotDelete',
    } }
 
 ##
@@ -1530,23 +1573,54 @@
 #
 # @device:  the name of the device to generate the snapshot from.
 #
-# @snapshot-file: the target of the new image. If the file exists, or if it
-#                 is a device, the snapshot will be created in the existing
-#                 file/device. If does not exist, a new file will be created.
+# @snapshot-file: the target name of the snapshot. In external case, it is
+#                 the new file's name, A new file will be created. If the file
+#                 exists, or if it is a device, the snapshot will be created in
+#                 the existing file/device. If does not exist, a new file will
+#                 be created. In internal case, it is the internal snapshot
+#                 record's name, if it is 'blank' name will be generated
+#                 according to time.
 #
 # @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: #optional internal snapshot or external, default is
+#        'external'.
+#
 # Returns: nothing on success
 #          If @device is not a valid block device, DeviceNotFound
 #
-# Since 0.14.0
+# Since 0.14.0, internal snapshot supprt since 1.4.
 ##
 { 'command': 'blockdev-snapshot-sync',
   'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str',
-            '*mode': 'NewImageMode'} }
+            '*mode': 'NewImageMode', '*type': 'SnapshotType'} }
+
+##
+# @blockdev-snapshot-delete-sync
+#
+# Delete a synchronous snapshot of a block device.
+#
+# @device:  the name of the device to delete the snapshot from.
+#
+# @snapshot-file: the target name of the snapshot. In external case, it is
+#                 the file's name to be merged, In internal case, it is the
+#                 internal snapshot record's name.
+#
+# @type: #optional internal snapshot or external, default is
+#        'external', note that delete 'external' snapshot is not supported
+#        now for that it is the same to commit it.
+#
+# Returns: nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#
+# Since 1.4
+##
+{ 'command': 'blockdev-snapshot-delete-sync',
+  'data': { 'device': 'str', 'snapshot-file': 'str',
+            '*type': 'SnapshotType'} }
 
 ##
 # @human-monitor-command:
-- 
1.7.1