[Qemu-devel] [PATCH v2 04/17] block: add error parameter to bdrv_snapshot_delete() and related functions

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina <address@hidden>
---
 block.c                | 27 +++++++++++++++++++--------
 block.h                |  4 +++-
 block/qcow2-snapshot.c | 10 +++++++++-
 block/qcow2.h          |  4 +++-
 block/rbd.c            |  6 +++++-
 block/sheepdog.c       |  4 +++-
 block_int.h            |  4 +++-
 qemu-img.c             |  6 +-----
 savevm.c               |  4 ++--
 9 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/block.c b/block.c
index d012b0e..1ec4d24 100644
--- a/block.c
+++ b/block.c
@@ -3128,16 +3128,27 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
     return ret;
 }
 
-int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
+int bdrv_snapshot_delete(BlockDriverState *bs,
+                         const char *snapshot_id,
+                         Error **errp)
 {
     BlockDriver *drv = bs->drv;
-    if (!drv)
-        return -ENOMEDIUM;
-    if (drv->bdrv_snapshot_delete)
-        return drv->bdrv_snapshot_delete(bs, snapshot_id);
-    if (bs->file)
-        return bdrv_snapshot_delete(bs->file, snapshot_id);
-    return -ENOTSUP;
+    int ret;
+
+    if (!drv) {
+        error_setg(errp, "Device '%s' has no medium.",
+                   bdrv_get_device_name(bs));
+        ret = -ENOMEDIUM;
+    } else if (drv->bdrv_snapshot_delete) {
+        ret = drv->bdrv_snapshot_delete(bs, snapshot_id, errp);
+    } else if (bs->file) {
+        ret = bdrv_snapshot_delete(bs->file, snapshot_id, errp);
+    } else {
+        error_setg(errp, "Not supported.");
+        ret = -ENOTSUP;
+    }
+
+    return ret;
 }
 
 int bdrv_snapshot_list(BlockDriverState *bs,
diff --git a/block.h b/block.h
index 31642c2..b6b25a7 100644
--- a/block.h
+++ b/block.h
@@ -326,7 +326,9 @@ int bdrv_snapshot_create(BlockDriverState *bs,
 int bdrv_snapshot_goto(BlockDriverState *bs,
                        const char *snapshot_id,
                        Error **errp);
-int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id);
+int bdrv_snapshot_delete(BlockDriverState *bs,
+                         const char *snapshot_id,
+                         Error **errp);
 int bdrv_snapshot_list(BlockDriverState *bs,
                        QEMUSnapshotInfo **psn_info);
 int bdrv_snapshot_load_tmp(BlockDriverState *bs,
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index e07f9ac..0bd0d1f 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -550,7 +550,9 @@ fail:
     return ret;
 }
 
-int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
+int qcow2_snapshot_delete(BlockDriverState *bs,
+                          const char *snapshot_id,
+                          Error **errp)
 {
     BDRVQcowState *s = bs->opaque;
     QCowSnapshot sn;
@@ -559,6 +561,8 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const char 
*snapshot_id)
     /* Search the snapshot */
     snapshot_index = find_snapshot_by_id_or_name(bs, snapshot_id);
     if (snapshot_index < 0) {
+        error_setg(errp, "Failed to find snapshot '%s' by id or name.",
+                   snapshot_id);
         return -ENOENT;
     }
     sn = s->snapshots[snapshot_index];
@@ -570,6 +574,8 @@ 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 '%s' from snapshot list",
+                   snapshot_id);
         return ret;
     }
 
@@ -587,6 +593,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 update snapshot refcount.");
         return ret;
     }
     qcow2_free_clusters(bs, sn.l1_table_offset, sn.l1_size * sizeof(uint64_t));
@@ -594,6 +601,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 refcount.");
         return ret;
     }
 
diff --git a/block/qcow2.h b/block/qcow2.h
index 6babb56..8dfdbb7 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -314,7 +314,9 @@ int qcow2_snapshot_create(BlockDriverState *bs,
 int qcow2_snapshot_goto(BlockDriverState *bs,
                         const char *snapshot_id,
                         Error **errp);
-int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id);
+int qcow2_snapshot_delete(BlockDriverState *bs,
+                          const char *snapshot_id,
+                          Error **errp);
 int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab);
 int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name);
 
diff --git a/block/rbd.c b/block/rbd.c
index 3e789c6..f8fe938 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -848,12 +848,16 @@ static int qemu_rbd_snap_create(BlockDriverState *bs,
 }
 
 static int qemu_rbd_snap_remove(BlockDriverState *bs,
-                                const char *snapshot_name)
+                                const char *snapshot_name,
+                                Error **errp)
 {
     BDRVRBDState *s = bs->opaque;
     int r;
 
     r = rbd_snap_remove(s->image, snapshot_name);
+    if (r < 0) {
+        error_setg(errp, "Failed to remove snapshot.");
+    }
     return r;
 }
 
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 34ef75c..c7f8d9e 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1879,7 +1879,9 @@ out:
     return ret;
 }
 
-static int sd_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
+static int sd_snapshot_delete(BlockDriverState *bs,
+                              const char *snapshot_id,
+                              Error **errp)
 {
     /* FIXME: Delete specified snapshot id.  */
     return 0;
diff --git a/block_int.h b/block_int.h
index 016782c..779e3cb 100644
--- a/block_int.h
+++ b/block_int.h
@@ -152,7 +152,9 @@ struct BlockDriver {
     int (*bdrv_snapshot_goto)(BlockDriverState *bs,
                               const char *snapshot_id,
                               Error **errp);
-    int (*bdrv_snapshot_delete)(BlockDriverState *bs, const char *snapshot_id);
+    int (*bdrv_snapshot_delete)(BlockDriverState *bs,
+                                const char *snapshot_id,
+                                Error **errp);
     int (*bdrv_snapshot_list)(BlockDriverState *bs,
                               QEMUSnapshotInfo **psn_info);
     int (*bdrv_snapshot_load_tmp)(BlockDriverState *bs,
diff --git a/qemu-img.c b/qemu-img.c
index 91671a8..fd9b525 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1568,11 +1568,7 @@ static int img_snapshot(int argc, char **argv)
         break;
 
     case SNAPSHOT_DELETE:
-        ret = bdrv_snapshot_delete(bs, snapshot_name);
-        if (ret) {
-            error_report("Could not delete snapshot '%s': %d (%s)",
-                snapshot_name, ret, strerror(-ret));
-        }
+        ret = bdrv_snapshot_delete(bs, snapshot_name, &err);
         break;
     }
 
diff --git a/savevm.c b/savevm.c
index 29e41fc..2aaa08b 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2097,7 +2097,7 @@ static int del_existing_snapshots(Monitor *mon, const 
char *name)
         if (bdrv_can_snapshot(bs) &&
             bdrv_snapshot_find(bs, snapshot, name) >= 0)
         {
-            ret = bdrv_snapshot_delete(bs, name);
+            ret = bdrv_snapshot_delete(bs, name, NULL);
             if (ret < 0) {
                 monitor_printf(mon,
                                "Error while deleting snapshot on '%s'\n",
@@ -2344,7 +2344,7 @@ void do_delvm(Monitor *mon, const QDict *qdict)
     bs1 = NULL;
     while ((bs1 = bdrv_next(bs1))) {
         if (bdrv_can_snapshot(bs1)) {
-            ret = bdrv_snapshot_delete(bs1, name);
+            ret = bdrv_snapshot_delete(bs1, name, NULL);
             if (ret < 0) {
                 if (ret == -ENOTSUP)
                     monitor_printf(mon,
-- 
1.8.0.2