Patchwork [V9,06/14] block: add check for VM snapshot in bdrv_query_snapshot_info_list()

login
register
mail settings
Submitter Wayne Xia
Date March 11, 2013, 11:23 a.m.
Message ID <1363000996-13221-7-git-send-email-xiawenc@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/226528/
State New
Headers show

Comments

Wayne Xia - March 11, 2013, 11:23 a.m.
This patch adds a parameter to tell whether return valid snapshots
for whole VM only.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block/qapi.c         |   42 ++++++++++++++++++++++++++++++++++++++++--
 include/block/qapi.h |    1 +
 qemu-img.c           |    3 ++-
 3 files changed, 43 insertions(+), 3 deletions(-)
Eric Blake - March 12, 2013, 7:45 p.m.
On 03/11/2013 05:23 AM, Wenchao Xia wrote:
>   This patch adds a parameter to tell whether return valid snapshots
> for whole VM only.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  block/qapi.c         |   42 ++++++++++++++++++++++++++++++++++++++++--
>  include/block/qapi.h |    1 +
>  qemu-img.c           |    3 ++-
>  3 files changed, 43 insertions(+), 3 deletions(-)

> +    /* Check logic is connected with load_vmstate():
> +       Only check the devices that can snapshot, other devices that can't
> +       take snapshot, for example, readonly ones, will be ignored in
> +       load_vmstate(). */
> +    while ((bs1 = bdrv_next(bs1))) {
> +        if (bdrv_can_snapshot(bs1) && bs1 != bs) {

Minor optimization - you could rearrange the conjunct and check 'bs1 !=
bs' first, to avoid the overhead of bdrv_can_snapshot on bs.  But that
doesn't stop me from giving:

Reviewed-by: Eric Blake <eblake@redhat.com>

Patch

diff --git a/block/qapi.c b/block/qapi.c
index 2da28db..1d99f5a 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -12,11 +12,47 @@ 
  */
 
 #include "block/qapi.h"
+#include "block/snapshot.h"
 #include "block/block_int.h"
 
-/* return 0 on success, and @p_list will be set only on success. */
+/*
+ * check whether the snapshot is valid for whole vm.
+ *
+ * @sn: snapshot info to be checked.
+ * @bs: where @sn was found.
+ *
+ * return true if the snapshot is consistent for the VM.
+ */
+static bool snapshot_valid_for_vm(const QEMUSnapshotInfo *sn,
+                                  BlockDriverState *bs)
+{
+    BlockDriverState *bs1 = NULL;
+    QEMUSnapshotInfo s, *sn_info = &s;
+    int ret;
+
+    /* Check logic is connected with load_vmstate():
+       Only check the devices that can snapshot, other devices that can't
+       take snapshot, for example, readonly ones, will be ignored in
+       load_vmstate(). */
+    while ((bs1 = bdrv_next(bs1))) {
+        if (bdrv_can_snapshot(bs1) && bs1 != bs) {
+            ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str, NULL);
+            if (ret < 0) {
+                return false;
+            }
+        }
+    }
+    return true;
+}
+
+/*
+ * return 0 on success, and @p_list will be set only on success. If
+ * @vm_snapshot is true, limit the results to snapshots valid for the
+ * whole VM.
+ */
 int bdrv_query_snapshot_info_list(BlockDriverState *bs,
                                   SnapshotInfoList **p_list,
+                                  bool vm_snapshot,
                                   Error **errp)
 {
     int i, sn_count;
@@ -45,7 +81,9 @@  int bdrv_query_snapshot_info_list(BlockDriverState *bs,
     }
 
     for (i = 0; i < sn_count; i++) {
-
+        if (vm_snapshot && !snapshot_valid_for_vm(&sn_tab[i], bs)) {
+            continue;
+        }
         info = g_new0(SnapshotInfo, 1);
         info->id            = g_strdup(sn_tab[i].id_str);
         info->name          = g_strdup(sn_tab[i].name);
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 4b46165..092ad8c 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -19,6 +19,7 @@ 
 
 int bdrv_query_snapshot_info_list(BlockDriverState *bs,
                                   SnapshotInfoList **p_list,
+                                  bool vm_snapshot,
                                   Error **errp);
 void bdrv_collect_image_info(BlockDriverState *bs,
                              ImageInfo *info,
diff --git a/qemu-img.c b/qemu-img.c
index cb534c9..83532b1 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1735,7 +1735,8 @@  static ImageInfoList *collect_image_info_list(const char *filename,
 
         info = g_new0(ImageInfo, 1);
         bdrv_collect_image_info(bs, info, filename);
-        if (!bdrv_query_snapshot_info_list(bs, &info->snapshots, NULL)) {
+        if (!bdrv_query_snapshot_info_list(bs, &info->snapshots,
+                                          false, NULL)) {
             info->has_snapshots = true;
         }