[Qemu-devel] [PATCH v2 03/12] savevm: update bdrv_snapshot_find() to find snapshot by id or name and add error parameter

[Pavel Hrdina]

Finding snapshot by a name which could also be an id isn't best way
how to do it. There will be rewrite of savevm, loadvm and delvm to
improve the behavior of these commands. The savevm and loadvm will
have their own patch series.

Now bdrv_snapshot_find takes more parameters. The name parameter will
be matched only against the name of the snapshot and the same applies
to id parameter.

There is one exception. If you set the last parameter, the name parameter
will be matched against the name or the id of a snapshot. This exception
is only for backward compatibility for other commands and it will be
dropped after all commands will be rewritten.

We only need to know if that snapshot exists or not. We don't care
about any error message. If snapshot exists it returns TRUE otherwise
it returns FALSE.

There is also new Error parameter which will containt error messeage if
something goes wrong.

Signed-off-by: Pavel Hrdina <address@hidden>
---
 savevm.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 67 insertions(+), 26 deletions(-)

diff --git a/savevm.c b/savevm.c
index ba97c41..1622c55 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2262,26 +2262,66 @@ out:
     return ret;
 }
 
-static int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
-                              const char *name)
+static bool bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
+                               const char *name, const char *id, Error **errp,
+                               bool old_match)
 {
     QEMUSnapshotInfo *sn_tab, *sn;
-    int nb_sns, i, ret;
+    int nb_sns, i;
+    bool found = false;
+
+    assert(name || id);
 
-    ret = -ENOENT;
     nb_sns = bdrv_snapshot_list(bs, &sn_tab);
-    if (nb_sns < 0)
-        return ret;
-    for(i = 0; i < nb_sns; i++) {
+    if (nb_sns < 0) {
+        error_setg_errno(errp, -nb_sns, "Failed to get a snapshot list");
+        return found;
+    }
+
+    if (nb_sns == 0) {
+        error_setg(errp, "Device has no snapshots");
+        return found;
+    }
+
+    for (i = 0; i < nb_sns; i++) {
         sn = &sn_tab[i];
-        if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) {
-            *sn_info = *sn;
-            ret = 0;
-            break;
+        if (name && id) {
+            if (!strcmp(sn->id_str, id) && !strcmp(sn->name, name)) {
+                *sn_info = *sn;
+                found = true;
+                break;
+            }
+        } else if (name) {
+            /* for compatibility for old bdrv_snapshot_find call
+             * will be removed */
+            if (old_match) {
+                if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) {
+                    *sn_info = *sn;
+                    found = true;
+                    break;
+                }
+            } else {
+                if (!strcmp(sn->name, name)) {
+                    *sn_info = *sn;
+                    found = true;
+                    break;
+                }
+            }
+        } else if (id) {
+            if (!strcmp(sn->id_str, id)) {
+                *sn_info = *sn;
+                found = true;
+                break;
+            }
         }
     }
+
+    if (!found) {
+        error_setg(errp, "Failed to find snapshot '%s'", name ? name : id);
+    }
+
     g_free(sn_tab);
-    return ret;
+    return found;
 }
 
 /*
@@ -2296,7 +2336,7 @@ static int del_existing_snapshots(Monitor *mon, const 
char *name)
     bs = NULL;
     while ((bs = bdrv_next(bs))) {
         if (bdrv_can_snapshot(bs) &&
-            bdrv_snapshot_find(bs, snapshot, name) >= 0)
+            bdrv_snapshot_find(bs, snapshot, name, NULL, NULL, true))
         {
             bdrv_snapshot_delete(bs, name, &local_err);
             if (error_is_set(&local_err)) {
@@ -2358,8 +2398,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
     sn->vm_clock_nsec = qemu_get_clock_ns(vm_clock);
 
     if (name) {
-        ret = bdrv_snapshot_find(bs, old_sn, name);
-        if (ret >= 0) {
+        if (bdrv_snapshot_find(bs, old_sn, name, NULL, NULL, true)) {
             pstrcpy(sn->name, sizeof(sn->name), old_sn->name);
             pstrcpy(sn->id_str, sizeof(sn->id_str), old_sn->id_str);
         } else {
@@ -2440,6 +2479,7 @@ int load_vmstate(const char *name)
     BlockDriverState *bs, *bs_vm_state;
     QEMUSnapshotInfo sn;
     QEMUFile *f;
+    Error *local_err = NULL;
     int ret;
 
     bs_vm_state = bdrv_snapshots();
@@ -2449,9 +2489,10 @@ int load_vmstate(const char *name)
     }
 
     /* Don't even try to load empty VM states */
-    ret = bdrv_snapshot_find(bs_vm_state, &sn, name);
-    if (ret < 0) {
-        return ret;
+    if (!bdrv_snapshot_find(bs_vm_state, &sn, name, NULL, &local_err, true)) {
+        error_report("Snapshot doesn't exist: %s", 
error_get_pretty(local_err));
+        error_free(local_err);
+        return -ENOENT;
     } else if (sn.vm_state_size == 0) {
         error_report("This is a disk-only snapshot. Revert to it offline "
             "using qemu-img.");
@@ -2473,11 +2514,11 @@ int load_vmstate(const char *name)
             return -ENOTSUP;
         }
 
-        ret = bdrv_snapshot_find(bs, &sn, name);
-        if (ret < 0) {
-            error_report("Device '%s' does not have the requested snapshot 
'%s'",
-                           bdrv_get_device_name(bs), name);
-            return ret;
+        if (!bdrv_snapshot_find(bs, &sn, name, NULL, &local_err, true)) {
+            error_report("Snapshot doesn't exist for device '%s': %s",
+                         bdrv_get_device_name(bs), 
error_get_pretty(local_err));
+            error_free(local_err);
+            return -ENOENT;
         }
     }
 
@@ -2546,7 +2587,7 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict)
 {
     BlockDriverState *bs, *bs1;
     QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s;
-    int nb_sns, i, ret, available;
+    int nb_sns, i, available;
     int total;
     int *available_snapshots;
     char buf[256];
@@ -2577,8 +2618,8 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict)
 
         while ((bs1 = bdrv_next(bs1))) {
             if (bdrv_can_snapshot(bs1) && bs1 != bs) {
-                ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str);
-                if (ret < 0) {
+                if (!bdrv_snapshot_find(bs1, sn_info, sn->id_str, NULL, NULL,
+                                        true)) {
                     available = 0;
                     break;
                 }
-- 
1.8.1.4