[Qemu-devel] [PATCH V4 4/9] qmp: add internal snapshot support in qmp_transaction

[Wenchao Xia]

Unlike savevm, the qmp_transaction interface will not generate
snapshot name automatically, saving trouble to return information
of the new created snapshot. The snapshot name should not collide
with snapshot ID, there is a check for it.

Although qcow2 support storing multiple snapshots with same name
but different ID, here it will fail when an snapshot with that name
already exist before the operation. Format such as rbd do not support
ID at all, and in most case, it means trouble to user when he faces
multiple snapshots with same name, so ban that case.

Snapshot ID can't be specified in this interface.

Signed-off-by: Wenchao Xia <address@hidden>
---
 blockdev.c       |  120 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json |   18 ++++++++-
 qmp-commands.hx  |   34 ++++++++++++---
 3 files changed, 164 insertions(+), 8 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index b3a57e0..2fda33b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -808,6 +808,121 @@ struct BlkTransactionState {
     QSIMPLEQ_ENTRY(BlkTransactionState) entry;
 };
 
+/* internal snapshot private data */
+typedef struct InternalSnapshotState {
+    BlkTransactionState common;
+    BlockDriverState *bs;
+    QEMUSnapshotInfo sn;
+} InternalSnapshotState;
+
+static void internal_snapshot_prepare(BlkTransactionState *common,
+                                      Error **errp)
+{
+    const char *device;
+    const char *name;
+    BlockDriverState *bs;
+    QEMUSnapshotInfo sn, *sn1;
+    bool ret;
+    qemu_timeval tv;
+    BlockdevSnapshotInternal *internal;
+    InternalSnapshotState *state;
+    int ret1;
+
+    g_assert(common->action->kind ==
+             TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_INTERNAL_SYNC);
+    internal = common->action->blockdev_snapshot_internal_sync;
+    state = DO_UPCAST(InternalSnapshotState, common, common);
+
+    /* 1. parse input */
+    device = internal->device;
+    name = internal->name;
+
+    /* 2. check for validation */
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
+    }
+
+    if (!bdrv_is_inserted(bs)) {
+        error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
+        return;
+    }
+
+    if (bdrv_is_read_only(bs)) {
+        error_set(errp, QERR_DEVICE_IS_READ_ONLY, device);
+        return;
+    }
+
+    if (!bdrv_can_snapshot(bs)) {
+        error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+                  bs->drv->format_name, device, "internal snapshot");
+        return;
+    }
+
+    /* check whether a snapshot with name exist, no need to check id, since
+       name will be checked later to make sure it does not conflict with id. */
+    ret = bdrv_snapshot_find_by_id_and_name(bs, NULL, name, &sn, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+    if (ret) {
+        error_setg(errp,
+                   "Snapshot with name '%s' already exists on device '%s'",
+                   name, device);
+        return;
+    }
+
+    /* Forbid having a name similar to id, empty name is also forbidden. */
+    if (!snapshot_name_wellformed(name)) {
+        error_setg(errp, "Name '%s' on device '%s' is not a valid one",
+                   name, device);
+        return;
+    }
+
+    /* 3. take the snapshot */
+    sn1 = &state->sn;
+    pstrcpy(sn1->name, sizeof(sn1->name), name);
+    qemu_gettimeofday(&tv);
+    sn1->date_sec = tv.tv_sec;
+    sn1->date_nsec = tv.tv_usec * 1000;
+    sn1->vm_clock_nsec = qemu_get_clock_ns(vm_clock);
+
+    ret1 = bdrv_snapshot_create(bs, sn1);
+    if (ret1 < 0) {
+        error_setg_errno(errp, -ret1,
+                         "Failed to create snapshot '%s' on device '%s'",
+                         name, device);
+        return;
+    }
+
+    /* 4. succeed, mark a snapshot is created */
+    state->bs = bs;
+}
+
+static void internal_snapshot_abort(BlkTransactionState *common)
+{
+    InternalSnapshotState *state =
+                             DO_UPCAST(InternalSnapshotState, common, common);
+    BlockDriverState *bs = state->bs;
+    QEMUSnapshotInfo *sn = &state->sn;
+    Error *local_error = NULL;
+
+    if (!bs) {
+        return;
+    }
+
+    if (bdrv_snapshot_delete(bs, sn->id_str, sn->name, &local_error) < 0) {
+        error_report("Failed to delete snapshot with id '%s' and name '%s' on "
+                     "device '%s' in abort, reason is: '%s'",
+                     sn->id_str,
+                     sn->name,
+                     bdrv_get_device_name(bs),
+                     error_get_pretty(local_error));
+        error_free(local_error);
+    }
+}
+
 /* external snapshot private data */
 typedef struct ExternalSnapshotState {
     BlkTransactionState common;
@@ -990,6 +1105,11 @@ static const BdrvActionOps actions[] = {
         .prepare = abort_prepare,
         .commit = abort_commit,
     },
+    [TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_INTERNAL_SYNC] = {
+        .instance_size = sizeof(InternalSnapshotState),
+        .prepare  = internal_snapshot_prepare,
+        .abort = internal_snapshot_abort,
+    },
 };
 
 /*
diff --git a/qapi-schema.json b/qapi-schema.json
index 5c32528..a2d27c8 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1621,6 +1621,21 @@
 { 'type': 'BlockdevSnapshot',
   'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str',
             '*mode': 'NewImageMode' } }
+##
+# @BlockdevSnapshotInternal
+#
+# @device: the name of the device to generate the snapshot from
+#
+# @name: the name of the internal snapshot to be created
+#
+# Notes: In transaction, if any snapshot matching @name exists, the operation
+#        will fail. If the name is a numeric string, it will also fail. Only
+#        some image formats support it, for example, qcow2, rbd, and sheepdog.
+#
+# Since: 1.6
+##
+{ 'type': 'BlockdevSnapshotInternal',
+  'data': { 'device': 'str', 'name': 'str' } }
 
 ##
 # @DriveBackup
@@ -1679,7 +1694,8 @@
   'data': {
        'blockdev-snapshot-sync': 'BlockdevSnapshot',
        'drive-backup': 'DriveBackup',
-       'abort': 'Abort'
+       'abort': 'Abort',
+       'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal'
    } }
 
 ##
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 362f0e1..d154506 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -994,13 +994,14 @@ transaction
 -----------
 
 Atomically operate on one or more block devices.  The only supported
-operation for now is snapshotting.  If there is any failure performing
-any of the operations, all snapshots for the group are abandoned, and
-the original disks pre-snapshot attempt are used.
+operations for now are internal and external snapshotting.  A list of
+dictionaries is accepted, that contains the actions to be performed.
+If there is any failure performing any of the operations, all operations
+for the group are abandoned.
 
-A list of dictionaries is accepted, that contains the actions to be performed.
-For snapshots this is the device, the file to use for the new snapshot,
-and the format.  The default format, if not specified, is qcow2.
+For external snapshots, the dictionary contains the device, the file to use for
+the new snapshot, and the format.  The default format, if not specified, is
+qcow2.
 
 Each new snapshot defaults to being created by QEMU (wiping any
 contents if the file already exists), but it is also possible to reuse
@@ -1009,6 +1010,19 @@ the new image file has the same contents as the current 
one; QEMU cannot
 perform any meaningful check.  Typically this is achieved by using the
 current image file as the backing file for the new image.
 
+On failure, the original disks pre-snapshot attempt will be used.
+
+For internal snapshots, the dictionary contains the device and the snapshot's
+name.  The name must not be a numeric string since this collides with snapshot
+IDs and an error will be returned.  For example, name "99" is not a valid name.
+If an internal snapshot matching name already exists, the request will be also
+rejected.  Only some image formats support it, for example, qcow2, rbd, and
+sheepdog.
+
+On failure, qemu will try delete the newly created internal snapshot in the
+transaction.  When an I/O error occurs during deletion, the user needs to fix
+it later with qemu-img or other command.
+
 Arguments:
 
 actions array:
@@ -1021,6 +1035,9 @@ actions array:
       - "format": format of new image (json-string, optional)
       - "mode": whether and how QEMU should create the snapshot file
         (NewImageMode, optional, default "absolute-paths")
+      When "type" is "blockdev-snapshot-internal-sync":
+      - "device": device name to snapshot (json-string)
+      - "name": name of the new snapshot (json-string)
 
 Example:
 
@@ -1032,7 +1049,10 @@ Example:
          { 'type': 'blockdev-snapshot-sync', 'data' : { "device": "ide-hd1",
                                          "snapshot-file": 
"/some/place/my-image2",
                                          "mode": "existing",
-                                         "format": "qcow2" } } ] } }
+                                         "format": "qcow2" } },
+         { 'type': 'blockdev-snapshot-internal-sync', 'data' : {
+                                         "device": "ide-hd2",
+                                         "name": "snapshot0" } } ] } }
 <- { "return": {} }
 
 EQMP
-- 
1.7.1