All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Henrique Barboza <danielhb413@gmail.com>
To: qemu-devel@nongnu.org
Cc: dgilbert@redhat.com, kwolf@redhat.com, mreitz@redhat.com,
	armbru@redhat.com,
	Daniel Henrique Barboza <danielhb413@gmail.com>
Subject: [Qemu-devel] [PATCH for-3.2 v3 2/3] block/snapshot: remove bdrv_snapshot_delete_by_id_or_name
Date: Wed,  7 Nov 2018 11:09:59 -0200	[thread overview]
Message-ID: <20181107131000.27744-3-danielhb413@gmail.com> (raw)
In-Reply-To: <20181107131000.27744-1-danielhb413@gmail.com>

After the previous patch, the only instance of this function left
is inside qemu-img.c.

qemu-img is using it inside the 'img_snapshot' function to delete
snapshots in the SNAPSHOT_DELETE case, based on a "snapshot_name"
string that refers to the tag, not ID, of the QEMUSnapshotInfo struct.
This can be verified by checking the SNAPSHOT_CREATE case that
comes shortly before SNAPSHOT_DELETE. In that case, the same
"snapshot_name" variable is being strcpy to the 'name' field
of the QEMUSnapshotInfo struct sn:

pstrcpy(sn.name, sizeof(sn.name), snapshot_name);

Based on that, it is unlikely that "snapshot_name" might contain
an "id" in SNAPSHOT_DELETE.

This patch changes SNAPSHOT_DELETE to use snapshot_find() and
snapshot_delete() instead of bdrv_snapshot_delete_by_id_or_name.
After that, there is no instances left of bdrv_snapshot_delete_by_id_or_name
in the code, so it is safe to remove it entirely.

Suggested-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 block/snapshot.c         | 20 --------------------
 include/block/snapshot.h |  3 ---
 qemu-img.c               | 15 +++++++++++----
 3 files changed, 11 insertions(+), 27 deletions(-)

diff --git a/block/snapshot.c b/block/snapshot.c
index e371d2243d..f2f48f926a 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -301,26 +301,6 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
     return ret;
 }
 
-int 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);
-    }
-    return ret;
-}
-
 int bdrv_snapshot_list(BlockDriverState *bs,
                        QEMUSnapshotInfo **psn_info)
 {
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index f73d1094af..b5d5084a12 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -61,9 +61,6 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
                          const char *snapshot_id,
                          const char *name,
                          Error **errp);
-int 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 4c96db7ba4..7b4910adcf 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3121,11 +3121,18 @@ static int img_snapshot(int argc, char **argv)
         break;
 
     case SNAPSHOT_DELETE:
-        bdrv_snapshot_delete_by_id_or_name(bs, snapshot_name, &err);
-        if (err) {
-            error_reportf_err(err, "Could not delete snapshot '%s': ",
-                              snapshot_name);
+        ret = bdrv_snapshot_find(bs, &sn, snapshot_name);
+        if (ret < 0) {
+            error_report("Could not delete snapshot '%s': snapshot not "
+                         "found", snapshot_name);
             ret = 1;
+        } else {
+            ret = bdrv_snapshot_delete(bs, sn.id_str, sn.name, &err);
+            if (ret < 0) {
+                error_reportf_err(err, "Could not delete snapshot '%s': ",
+                                  snapshot_name);
+                ret = 1;
+            }
         }
         break;
     }
-- 
2.17.2

  parent reply	other threads:[~2018-11-07 13:10 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-07 13:09 [Qemu-devel] [PATCH for-3.2 v3 0/3] HMP/snapshot changes - do not use ID anymore Daniel Henrique Barboza
2018-11-07 13:09 ` [Qemu-devel] [PATCH for-3.2 v3 1/3] block/snapshot.c: eliminate use of ID input in snapshot operations Daniel Henrique Barboza
2018-12-14 12:09   ` Dr. David Alan Gilbert
2019-02-15 16:21   ` Eric Blake
2019-02-15 16:34     ` Kevin Wolf
2018-11-07 13:09 ` Daniel Henrique Barboza [this message]
2018-11-07 13:10 ` [Qemu-devel] [PATCH for-3.2 v3 3/3] qcow2-snapshot: remove redundant find_snapshot_by_id_and_name call Daniel Henrique Barboza
2018-12-02 21:10 ` [Qemu-devel] [PATCH for-3.2 v3 0/3] HMP/snapshot changes - do not use ID anymore Daniel Henrique Barboza
2019-01-21  9:43 ` Daniel Henrique Barboza
2019-02-15 16:09 ` Kevin Wolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181107131000.27744-3-danielhb413@gmail.com \
    --to=danielhb413@gmail.com \
    --cc=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.