diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index ffead08ca2cdd9a292cb482efe70a54119757a50..7d144205c3fdd65bae8a728e2f9d7ad54bb6bdc7 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;
}
@@ -560,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];
@@ -580,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;
}
@@ -597,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),
@@ -605,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;
}
diff --git a/block/qcow2.h b/block/qcow2.h
index bea6ddb43a442bc1a18b91808e3b3e5ce2b59637..c90e5d6c6e6cca0f12afb523b98577d958923373 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -467,7 +467,10 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs);
/* qcow2-snapshot.c functions */
int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info);
int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id);
-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);
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 e798e19f814180998ffb0a2490d8b733d256f326..b1ab80ce191269a7f1c70e7ab808ecd84066ac93 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -891,12 +891,31 @@ static int qemu_rbd_snap_create(BlockDriverState *bs,
}
static int qemu_rbd_snap_remove(BlockDriverState *bs,
- const char *snapshot_name)
+ const char *snapshot_id,
+ const char *snapshot_name,
+ Error **errp)
{
BDRVRBDState *s = bs->opaque;
int r;
+ if (!snapshot_name) {
+ error_setg(errp, "rbd need a valid snapshot name");
+ return -EINVAL;
+ }
+
+ /* If snapshot_id is specified, it must be equal to name, see
+ qemu_rbd_snap_list() */
+ if (snapshot_id && strcmp(snapshot_id, snapshot_name)) {
+ error_setg(errp,
+ "rbd do not support snapshot id, it should be NULL or "
+ "equal to snapshot name");
+ return -EINVAL;
+ }
+
r = rbd_snap_remove(s->image, snapshot_name);
+ if (r < 0) {
+ error_setg_errno(errp, -r, "Failed to remove the snapshot");
+ }
return r;
}
diff --git a/block/sheepdog.c b/block/sheepdog.c
index f9988d35ba3fd6a954182acc4098adb6c9a38008..fe438e0fa2d129ceb78ffa0beca5e13b4a710aff 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2072,7 +2072,10 @@ 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,
+ const char *name,
+ Error **errp)
{
/* FIXME: Delete specified snapshot id. */
return 0;
diff --git a/block/snapshot.c b/block/snapshot.c
index a923b386b61016d177278c9f7677f40190b42c5c..82e602fccfde97e416f95818118f1d4e9530c448 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -182,21 +182,73 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
return -ENOTSUP;
}
-int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
+/**
+ * Delete an internal snapshot by @snapshot_id and @name.
+ * @bs: block device used in the operation
+ * @snapshot_id: unique snapshot ID, or NULL
+ * @name: snapshot name, or NULL
+ * @errp: location to store error
+ *
+ * If both @snapshot_id and @name are specified, delete the first one with
+ * id @snapshot_id and name @name.
+ * If only @snapshot_id is specified, delete the first one with id
+ * @snapshot_id.
+ * If only @name is specified, delete the first one with name @name.
+ * if none is specified, return -ENINVAL.
+ *
+ * Returns: 0 on success, -errno on failure. If @bs is not inserted, return
+ * -ENOMEDIUM. If @snapshot_id and @name are both NULL, return -EINVAL. If @bs
+ * does not support internal snapshot deletion, return -ENOTSUP. If @bs does
+ * not support parameter @snapshot_id or @name, or one of them is not correctly
+ * specified, return -EINVAL. If @bs can't find one matching @id and @name,
+ * return -ENOENT. If @errp != NULL, it will always be filled with error
+ * message on failure.
+ */
+int bdrv_snapshot_delete(BlockDriverState *bs,
+ const char *snapshot_id,
+ const char *name,
+ Error **errp)
{
BlockDriver *drv = bs->drv;
if (!drv) {
+ error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs));
return -ENOMEDIUM;
}
+ if (!snapshot_id && !name) {
+ error_setg(errp, "snapshot_id and name are both NULL");
+ return -EINVAL;
+ }
if (drv->bdrv_snapshot_delete) {
- return drv->bdrv_snapshot_delete(bs, snapshot_id);
+ return drv->bdrv_snapshot_delete(bs, snapshot_id, name, errp);
}
if (bs->file) {
- return bdrv_snapshot_delete(bs->file, snapshot_id);
+ return bdrv_snapshot_delete(bs->file, snapshot_id, name, errp);
}
+ error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+ drv->format_name, bdrv_get_device_name(bs),
+ "internal snapshot deletion");
return -ENOTSUP;
}
+void bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
+ const char *id_or_name,
+ Error **errp)
+{
+ int ret;
+ Error *local_err = NULL;
+
+ ret = bdrv_snapshot_delete(bs, id_or_name, NULL, &local_err);
+ if (ret == -ENOENT || ret == -EINVAL) {
+ error_free(local_err);
+ local_err = NULL;
+ ret = bdrv_snapshot_delete(bs, NULL, id_or_name, &local_err);
+ }
+
+ if (ret < 0) {
+ error_propagate(errp, local_err);
+ }
+}
+
int bdrv_snapshot_list(BlockDriverState *bs,
QEMUSnapshotInfo **psn_info)
{
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1541777d427426204f66b9f6d34cca2b8009515f..e7d1766ae0803949e25178fe2b3e2c2c0c34b2a3 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -150,7 +150,10 @@ struct BlockDriver {
QEMUSnapshotInfo *sn_info);
int (*bdrv_snapshot_goto)(BlockDriverState *bs,
const char *snapshot_id);
- int (*bdrv_snapshot_delete)(BlockDriverState *bs, const char *snapshot_id);
+ int (*bdrv_snapshot_delete)(BlockDriverState *bs,
+ const char *snapshot_id,
+ const char *name,
+ Error **errp);
int (*bdrv_snapshot_list)(BlockDriverState *bs,
QEMUSnapshotInfo **psn_info);
int (*bdrv_snapshot_load_tmp)(BlockDriverState *bs,
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 9d06dc17a67e26916975331c49444cab0082df87..012bf226d3689e218b3461b35c8a8841ef8473f6 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -51,7 +51,13 @@ int bdrv_snapshot_create(BlockDriverState *bs,
QEMUSnapshotInfo *sn_info);
int bdrv_snapshot_goto(BlockDriverState *bs,
const char *snapshot_id);
-int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id);
+int bdrv_snapshot_delete(BlockDriverState *bs,
+ const char *snapshot_id,
+ const char *name,
+ Error **errp);
+void bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
+ const char *id_or_name,
+ 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 0cf6be228033119d36d1f549aacc69432ece043e..139526f348510636a30e5fe039cdaeee93f98498 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2006,6 +2006,7 @@ static int img_snapshot(int argc, char **argv)
int action = 0;
qemu_timeval tv;
bool quiet = false;
+ Error *err = NULL;
bdrv_oflags = BDRV_O_FLAGS | BDRV_O_RDWR;
/* Parse commandline parameters */
@@ -2098,10 +2099,12 @@ 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));
+ bdrv_snapshot_delete_by_id_or_name(bs, snapshot_name, &err);
+ if (error_is_set(&err)) {
+ error_report("Could not delete snapshot '%s': (%s)",
+ snapshot_name, error_get_pretty(err));
+ error_free(err);
+ ret = 1;
}
break;
}
diff --git a/savevm.c b/savevm.c
index c536aa4986bd5381d53f363577c4cf4a5e3465b4..4a3c819fcdda5a4ded652c510f6178a03f9324d9 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2325,18 +2325,21 @@ static int del_existing_snapshots(Monitor *mon, const char *name)
{
BlockDriverState *bs;
QEMUSnapshotInfo sn1, *snapshot = &sn1;
- int ret;
+ Error *err = NULL;
bs = NULL;
while ((bs = bdrv_next(bs))) {
if (bdrv_can_snapshot(bs) &&
bdrv_snapshot_find(bs, snapshot, name) >= 0)
{
- ret = bdrv_snapshot_delete(bs, name);
- if (ret < 0) {
+ bdrv_snapshot_delete_by_id_or_name(bs, name, &err);
+ if (error_is_set(&err)) {
monitor_printf(mon,
- "Error while deleting snapshot on '%s'\n",
- bdrv_get_device_name(bs));
+ "Error while deleting snapshot on device '%s':"
+ " %s\n",
+ bdrv_get_device_name(bs),
+ error_get_pretty(err));
+ error_free(err);
return -1;
}
}
@@ -2550,7 +2553,7 @@ int load_vmstate(const char *name)
void do_delvm(Monitor *mon, const QDict *qdict)
{
BlockDriverState *bs, *bs1;
- int ret;
+ Error *err = NULL;
const char *name = qdict_get_str(qdict, "name");
bs = find_vmstate_bs();
@@ -2562,15 +2565,14 @@ 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);
- if (ret < 0) {
- if (ret == -ENOTSUP)
- monitor_printf(mon,
- "Snapshots not supported on device '%s'\n",
- bdrv_get_device_name(bs1));
- else
- monitor_printf(mon, "Error %d while deleting snapshot on "
- "'%s'\n", ret, bdrv_get_device_name(bs1));
+ bdrv_snapshot_delete_by_id_or_name(bs, name, &err);
+ if (error_is_set(&err)) {
+ monitor_printf(mon,
+ "Error while deleting snapshot on device '%s':"
+ " %s\n",
+ bdrv_get_device_name(bs),
+ error_get_pretty(err));
+ error_free(err);
}
}
}