Merge remote-tracking branch 'kwolf/for-anthony' into staging

# By Max Reitz (16) and others
# Via Kevin Wolf
* kwolf/for-anthony: (33 commits)
  qemu-iotests: Fix test 038
  block: Assert validity of BdrvActionOps
  qemu-iotests: Cleanup test image in test number 007
  qemu-img: fix invalid JSON
  coroutine: add ./configure --disable-coroutine-pool
  qemu-iotests: Adjustments due to error propagation
  qcow2: Use Error parameter
  qemu-img create: Emit filename on error
  block: Error parameter for create functions
  block: Error parameter for open functions
  bdrv: Use "Error" for creating images
  bdrv: Use "Error" for opening images
  qemu-iotests: add 057 internal snapshot for block device test case
  hmp: add interface hmp_snapshot_delete_blkdev_internal
  hmp: add interface hmp_snapshot_blkdev_internal
  qmp: add interface blockdev-snapshot-delete-internal-sync
  qmp: add interface blockdev-snapshot-internal-sync
  qmp: add internal snapshot support in qmp_transaction
  snapshot: distinguish id and name in snapshot delete
  snapshot: new function bdrv_snapshot_find_by_id_and_name()
  ...

Message-id: 1379073063-14963-1-git-send-email-kwolf@redhat.com

1 files changed, 46 insertions, 16 deletions

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index e7e601301a..7d144205c3 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -297,31 +297,47 @@ static void find_new_snapshot_id(BlockDriverState *bs,
     snprintf(id_str, id_str_size, "%d", id_max + 1);
 }
 
-static int find_snapshot_by_id(BlockDriverState *bs, const char *id_str)
+static int find_snapshot_by_id_and_name(BlockDriverState *bs,
+                                        const char *id,
+                                        const char *name)
 {
     BDRVQcowState *s = bs->opaque;
     int i;
 
-    for(i = 0; i < s->nb_snapshots; i++) {
-        if (!strcmp(s->snapshots[i].id_str, id_str))
-            return i;
+    if (id && name) {
+        for (i = 0; i < s->nb_snapshots; i++) {
+            if (!strcmp(s->snapshots[i].id_str, id) &&
+                !strcmp(s->snapshots[i].name, name)) {
+                return i;
+            }
+        }
+    } else if (id) {
+        for (i = 0; i < s->nb_snapshots; i++) {
+            if (!strcmp(s->snapshots[i].id_str, id)) {
+                return i;
+            }
+        }
+    } else if (name) {
+        for (i = 0; i < s->nb_snapshots; i++) {
+            if (!strcmp(s->snapshots[i].name, name)) {
+                return i;
+            }
+        }
     }
+
     return -1;
 }
 
-static int find_snapshot_by_id_or_name(BlockDriverState *bs, const char *name)
+static int find_snapshot_by_id_or_name(BlockDriverState *bs,
+                                       const char *id_or_name)
 {
-    BDRVQcowState *s = bs->opaque;
-    int i, ret;
+    int ret;
 
-    ret = find_snapshot_by_id(bs, name);
-    if (ret >= 0)
+    ret = find_snapshot_by_id_and_name(bs, id_or_name, NULL);
+    if (ret >= 0) {
         return ret;
-    for(i = 0; i < s->nb_snapshots; i++) {
-        if (!strcmp(s->snapshots[i].name, name))
-            return i;
     }
-    return -1;
+    return find_snapshot_by_id_and_name(bs, NULL, id_or_name);
 }
 
 /* if no id is provided, a new one is constructed */
@@ -343,7 +359,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
     }
 
     /* Check that the ID is unique */
-    if (find_snapshot_by_id(bs, sn_info->id_str) >= 0) {
+    if (find_snapshot_by_id_and_name(bs, sn_info->id_str, NULL) >= 0) {
         return -EEXIST;
     }
 
@@ -416,6 +432,13 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
 
     g_free(old_snapshot_list);
 
+    /* The VM state isn't needed any more in the active L1 table; in fact, it
+     * hurts by causing expensive COW for the next snapshot. */
+    qcow2_discard_clusters(bs, qcow2_vm_state_offset(s),
+                           align_offset(sn->vm_state_size, s->cluster_size)
+                                >> BDRV_SECTOR_BITS,
+                           QCOW2_DISCARD_NEVER);
+
 #ifdef DEBUG_ALLOC
     {
       BdrvCheckResult result = {0};
@@ -553,15 +576,19 @@ fail:
     return ret;
 }
 
-int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
+int qcow2_snapshot_delete(BlockDriverState *bs,
+                          const char *snapshot_id,
+                          const char *name,
+                          Error **errp)
 {
     BDRVQcowState *s = bs->opaque;
     QCowSnapshot sn;
     int snapshot_index, ret;
 
     /* Search the snapshot */
-    snapshot_index = find_snapshot_by_id_or_name(bs, snapshot_id);
+    snapshot_index = find_snapshot_by_id_and_name(bs, snapshot_id, name);
     if (snapshot_index < 0) {
+        error_setg(errp, "Can't find the snapshot");
         return -ENOENT;
     }
     sn = s->snapshots[snapshot_index];
@@ -573,6 +600,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
     s->nb_snapshots--;
     ret = qcow2_write_snapshots(bs);
     if (ret < 0) {
+        error_setg(errp, "Failed to remove snapshot from snapshot list");
         return ret;
     }
 
@@ -590,6 +618,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
     ret = qcow2_update_snapshot_refcount(bs, sn.l1_table_offset,
                                          sn.l1_size, -1);
     if (ret < 0) {
+        error_setg(errp, "Failed to free the cluster and L1 table");
         return ret;
     }
     qcow2_free_clusters(bs, sn.l1_table_offset, sn.l1_size * sizeof(uint64_t),
@@ -598,6 +627,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
     /* must update the copied flag on the current cluster offsets */
     ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 0);
     if (ret < 0) {
+        error_setg(errp, "Failed to update snapshot status in disk");
         return ret;
     }