[Qemu-devel] [PATCH V4 00/13] add qmp/hmp interfaces for snapshot info

[Qemu-devel] [PATCH V4 00/13] add qmp/hmp interfaces for snapshot info

[Wenchao Xia]

  This serial of patches does two things: merge some info code
in qemu-img, and add following interfaces:
1) qmp: query-images
2) qmp: query-snapshots
3) hmp: show snapshot info on a single block device
  These patches follows the rule that use qmp to retieve information,
hmp layer just do a translation from qmp object it got, so almost
every hmp interface may have a correlated qmp interface.
  To make code graceful, snapshot retrieving code in qemu and qemu-img
are merged into block.c, and some function name was adjusted to make it
tips better. Now it works as:

   qemu          qemu-img

dump_monitor    dump_stdout
     |--------------| 
            |
           qmp
            |
          block

Note:
  Last two patches need previous sent patches which extend hmp sub command, at:
http://lists.nongnu.org/archive/html/qemu-devel/2012-12/msg03487.html

v2:
  Rename and adjusted qmp interface according to comments from Eric.
  Spelling fix.
  Information retrieving function in block layer goes to seperated patch.
  Free qmp object after usage in hmp.
  Added counterpart in qmp-commands.hx.
  Better tips in qmp-schema.json.

v3:
  Spelling fix in commit message, patch 03/11.
  Spelling fix in code, patch 06/11.
  Add comments that vm-state-size is in bytes, and change size of it in
example to a reasonable number, patch 08/11.

v4:
  02/13: in bdrv_get_filename(), add const to parameter *bs.
  03/13: new added, in which the function correct the behavior in info
retrieving.
  04/13: in bdrv_query_snapshot_infolist(), remove NULL check before call
err_setg(), added TODO comments that let block layer function set error instead
of this layer to tip better for errors, Split out patch about image info to
patch 05/13.
  05/13: new splitted, and it checks *bs by calling bdrv_can_read_snapshot()
before collect internal snasphot info to avoid *err is set unexpectly now.
  06/13: check if error happens after calling bdrv_query_image_info().
  08/13: rename info to image in DeviceImageInfo and make it optional,
when device is not inserted it will be empty, added error handling code
when met error in calling block layer API.
  09/13: distinguish *id and *name in bdrv_find_snapshots(), caller
can choose what to search with. id_wellformed() should be called in
new snapshot creation interface above this function in the future.
  10/13: now this interface have addtional parameter *device, which
enable showing internal snapshots on a single device. Also use
bdrv_can_read_snapshot() instead of bdrv_can_snapshot() now.
  11/13: this function goes to hmp.c so hmp_handler_error is not exported
any more, split out patch that switch snapshot info function to patch 12/13.
  12/13: new splitted.
  13/13: use qmp API instead of directly calling block layer API, now
all hmp function have correspond qmp funtion in this serial.

Wenchao Xia (13):
  1 qemu-img: remove unused parameter in collect_image_info()
  2 block: add bdrv_get_filename() function
  3 block: add bdrv_can_read_snapshot() function
  4 block: add snapshot info query function bdrv_query_snapshot_infolist()
  5 block: add image info query function bdrv_query_image_info()
  6 qemu-img: switch image retrieving function
  7 block: rename bdrv_query_info to bdrv_query_block_info
  8 qmp: add interface query-images.
  9 block: export function bdrv_find_snapshot()
  10 qmp: add interface query-snapshots
  11 hmp: add function hmp_info_snapshots()
  12 hmp: switch snapshot info function to qmp based one
  13 hmp: show snapshots on single block device

 block.c               |  274 ++++++++++++++++++++++++++++++++++++++++++++++++-
 hmp.c                 |   47 +++++++++
 hmp.h                 |    1 +
 include/block/block.h |   14 +++-
 monitor.c             |    8 +-
 qapi-schema.json      |   46 ++++++++
 qemu-img.c            |   91 +---------------
 qmp-commands.hx       |  129 +++++++++++++++++++++++
 savevm.c              |   94 +----------------
 9 files changed, 522 insertions(+), 182 deletions(-)

[Qemu-devel] [PATCH V4 01/13] qemu-img: remove unused parameter in collect_image_info()

[Wenchao Xia]

  Parameter *fmt was not used, so remove it.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 qemu-img.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 85d3740..9dab48f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1186,8 +1186,7 @@ static void dump_json_image_info(ImageInfo *info)
 
 static void collect_image_info(BlockDriverState *bs,
                    ImageInfo *info,
-                   const char *filename,
-                   const char *fmt)
+                   const char *filename)
 {
     uint64_t total_sectors;
     char backing_filename[1024];
@@ -1361,7 +1360,7 @@ static ImageInfoList *collect_image_info_list(const char *filename,
         }
 
         info = g_new0(ImageInfo, 1);
-        collect_image_info(bs, info, filename, fmt);
+        collect_image_info(bs, info, filename);
         collect_snapshots(bs, info);
 
         elem = g_new0(ImageInfoList, 1);
-- 
1.7.1

[Qemu-devel] [PATCH V4 02/13] block: add bdrv_get_filename() function

[Wenchao Xia]

  This function will simply return the uri or filename used
to open the image.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block.c               |    5 +++++
 include/block/block.h |    1 +
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 6fa7c90..a86257d 100644
--- a/block.c
+++ b/block.c
@@ -2963,6 +2963,11 @@ BlockStatsList *qmp_query_blockstats(Error **errp)
     return head;
 }
 
+const char *bdrv_get_filename(const BlockDriverState *bs)
+{
+    return bs->filename;
+}
+
 const char *bdrv_get_encrypted_filename(BlockDriverState *bs)
 {
     if (bs->backing_hd && bs->backing_hd->encrypted)
diff --git a/include/block/block.h b/include/block/block.h
index ffd1936..0b84e9b 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -310,6 +310,7 @@ int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
                           const uint8_t *buf, int nb_sectors);
 int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
 
+const char *bdrv_get_filename(const BlockDriverState *bs);
 const char *bdrv_get_encrypted_filename(BlockDriverState *bs);
 void bdrv_get_backing_filename(BlockDriverState *bs,
                                char *filename, int filename_size);
-- 
1.7.1

[Qemu-devel] [PATCH V4 03/13] block: add bdrv_can_read_snapshot() function

[Wenchao Xia]

  Compared to bdrv_can_snapshot(), this function return whether
bs* is ready to read snapshot info from instead of write. If yes,
caller can then query snapshot information, but taking snapshot
is not always possible for that *bs may be read only.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block.c               |   19 +++++++++++++++++++
 include/block/block.h |    1 +
 2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index a86257d..934bb3f 100644
--- a/block.c
+++ b/block.c
@@ -3091,6 +3091,25 @@ bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag)
 /**************************************************************/
 /* handling of snapshots */
 
+/* return whether internal snapshot can be read on @bs */
+int bdrv_can_read_snapshot(BlockDriverState *bs)
+{
+    BlockDriver *drv = bs->drv;
+    if (!drv || !bdrv_is_inserted(bs)) {
+        return 0;
+    }
+
+    if (!drv->bdrv_snapshot_create) {
+        if (bs->file != NULL) {
+            return bdrv_can_read_snapshot(bs->file);
+        }
+        return 0;
+    }
+
+    return 1;
+}
+
+/* return whether internal snapshot can be write on @bs */
 int bdrv_can_snapshot(BlockDriverState *bs)
 {
     BlockDriver *drv = bs->drv;
diff --git a/include/block/block.h b/include/block/block.h
index 0b84e9b..b4c1612 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -318,6 +318,7 @@ void bdrv_get_full_backing_filename(BlockDriverState *bs,
                                     char *dest, size_t sz);
 BlockInfo *bdrv_query_info(BlockDriverState *s);
 BlockStats *bdrv_query_stats(const BlockDriverState *bs);
+int bdrv_can_read_snapshot(BlockDriverState *bs);
 int bdrv_can_snapshot(BlockDriverState *bs);
 int bdrv_is_snapshot(BlockDriverState *bs);
 BlockDriverState *bdrv_snapshots(void);
-- 
1.7.1

[Qemu-devel] [PATCH V4 04/13] block: add snapshot info query function bdrv_query_snapshot_infolist()

[Wenchao Xia]

  This patch add function bdrv_query_snapshot_infolist(), which will
return snapshot info of an image in qmp object format. The implementation
code are mostly copied from qemu-img.c with modification to fit more
for qmp based block layer API.
  To help filter out snapshot info not needed, a call back function is
added in bdrv_query_snapshot_infolist().
  bdrv_can_read_snapshot() should be called before call this function,
to avoid got *errp set unexpectly.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block.c               |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 include/block/block.h |    7 +++++++
 2 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 934bb3f..fba1be4 100644
--- a/block.c
+++ b/block.c
@@ -2842,6 +2842,52 @@ int coroutine_fn bdrv_co_is_allocated_above(BlockDriverState *top,
     return 0;
 }
 
+SnapshotInfoList *bdrv_query_snapshot_infolist(BlockDriverState *bs,
+                                               SnapshotFilterFunc filter,
+                                               void *opaque,
+                                               Error **errp)
+{
+    int i, sn_count;
+    QEMUSnapshotInfo *sn_tab = NULL;
+    SnapshotInfoList *info_list, *cur_item = NULL, *head = NULL;
+
+    sn_count = bdrv_snapshot_list(bs, &sn_tab);
+    if (sn_count < 0) {
+        /* Fix me:  set errp in bdrv_snapshot_list() instead of here */
+        error_setg(errp, "bdrv_snapshot_list: error %d\n", sn_count);
+        return NULL;
+    }
+
+    for (i = 0; i < sn_count; i++) {
+        if (filter && filter(&sn_tab[i], opaque) != 0) {
+            continue;
+        }
+
+        info_list = g_new0(SnapshotInfoList, 1);
+
+        info_list->value                = g_new0(SnapshotInfo, 1);
+        info_list->value->id            = g_strdup(sn_tab[i].id_str);
+        info_list->value->name          = g_strdup(sn_tab[i].name);
+        info_list->value->vm_state_size = sn_tab[i].vm_state_size;
+        info_list->value->date_sec      = sn_tab[i].date_sec;
+        info_list->value->date_nsec     = sn_tab[i].date_nsec;
+        info_list->value->vm_clock_sec  = sn_tab[i].vm_clock_nsec / 1000000000;
+        info_list->value->vm_clock_nsec = sn_tab[i].vm_clock_nsec % 1000000000;
+
+        /* XXX: waiting for the qapi to support qemu-queue.h types */
+        if (!cur_item) {
+            head = cur_item = info_list;
+        } else {
+            cur_item->next = info_list;
+            cur_item = info_list;
+        }
+
+    }
+
+    g_free(sn_tab);
+    return head;
+}
+
 BlockInfo *bdrv_query_info(BlockDriverState *bs)
 {
     BlockInfo *info = g_malloc0(sizeof(*info));
diff --git a/include/block/block.h b/include/block/block.h
index b4c1612..ce18d4b 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -316,6 +316,13 @@ void bdrv_get_backing_filename(BlockDriverState *bs,
                                char *filename, int filename_size);
 void bdrv_get_full_backing_filename(BlockDriverState *bs,
                                     char *dest, size_t sz);
+
+typedef int (*SnapshotFilterFunc)(const QEMUSnapshotInfo *sn, void *opaque);
+/* assume bs is already opened, use qapi_free_* to free returned value. */
+SnapshotInfoList *bdrv_query_snapshot_infolist(BlockDriverState *bs,
+                                               SnapshotFilterFunc filter,
+                                               void *opaque,
+                                               Error **errp);
 BlockInfo *bdrv_query_info(BlockDriverState *s);
 BlockStats *bdrv_query_stats(const BlockDriverState *bs);
 int bdrv_can_read_snapshot(BlockDriverState *bs);
-- 
1.7.1

[Qemu-devel] [PATCH V4 05/13] block: add image info query function bdrv_query_image_info()

[Wenchao Xia]

  This patch add function bdrv_query_image_info(), which will return
image info in qmp object format. The implementation code are mostly
copied from qemu-img.c, but use block layer function to get snapshot
info.
  A check with bdrv_can_read_snapshot(), was done before collecting
snapshot info.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block.c               |   73 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/block/block.h |    1 +
 2 files changed, 74 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index fba1be4..245a082 100644
--- a/block.c
+++ b/block.c
@@ -2888,6 +2888,79 @@ SnapshotInfoList *bdrv_query_snapshot_infolist(BlockDriverState *bs,
     return head;
 }
 
+/* collect all internal snapshot info in a image for ImageInfo */
+static void collect_snapshots_info(BlockDriverState *bs,
+                                   ImageInfo *info,
+                                   Error **errp)
+{
+    SnapshotInfoList *info_list;
+
+    if (!bdrv_can_read_snapshot(bs)) {
+        return;
+    }
+    info_list = bdrv_query_snapshot_infolist(bs, NULL, NULL, errp);
+    if (info_list != NULL) {
+        info->has_snapshots = true;
+        info->snapshots = info_list;
+    }
+}
+
+static void collect_image_info(BlockDriverState *bs,
+                               ImageInfo *info)
+{
+    uint64_t total_sectors;
+    char backing_filename[1024];
+    char backing_filename2[1024];
+    BlockDriverInfo bdi;
+    const char *filename;
+
+    filename = bdrv_get_filename(bs);
+    bdrv_get_geometry(bs, &total_sectors);
+
+    info->filename        = g_strdup(filename);
+    info->format          = g_strdup(bdrv_get_format_name(bs));
+    info->virtual_size    = total_sectors * 512;
+    info->actual_size     = bdrv_get_allocated_file_size(bs);
+    info->has_actual_size = info->actual_size >= 0;
+    if (bdrv_is_encrypted(bs)) {
+        info->encrypted = true;
+        info->has_encrypted = true;
+    }
+    if (bdrv_get_info(bs, &bdi) >= 0) {
+        if (bdi.cluster_size != 0) {
+            info->cluster_size = bdi.cluster_size;
+            info->has_cluster_size = true;
+        }
+        info->dirty_flag = bdi.is_dirty;
+        info->has_dirty_flag = true;
+    }
+    bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
+    if (backing_filename[0] != '\0') {
+        info->backing_filename = g_strdup(backing_filename);
+        info->has_backing_filename = true;
+        bdrv_get_full_backing_filename(bs, backing_filename2,
+                                       sizeof(backing_filename2));
+
+        if (strcmp(backing_filename, backing_filename2) != 0) {
+            info->full_backing_filename = g_strdup(backing_filename2);
+            info->has_full_backing_filename = true;
+        }
+
+        if (bs->backing_format[0]) {
+            info->backing_filename_format = g_strdup(bs->backing_format);
+            info->has_backing_filename_format = true;
+        }
+    }
+}
+
+ImageInfo *bdrv_query_image_info(BlockDriverState *bs, Error **errp)
+{
+    ImageInfo *info = g_new0(ImageInfo, 1);
+    collect_image_info(bs, info);
+    collect_snapshots_info(bs, info, errp);
+    return info;
+}
+
 BlockInfo *bdrv_query_info(BlockDriverState *bs)
 {
     BlockInfo *info = g_malloc0(sizeof(*info));
diff --git a/include/block/block.h b/include/block/block.h
index ce18d4b..b6f80c3 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -323,6 +323,7 @@ SnapshotInfoList *bdrv_query_snapshot_infolist(BlockDriverState *bs,
                                                SnapshotFilterFunc filter,
                                                void *opaque,
                                                Error **errp);
+ImageInfo *bdrv_query_image_info(BlockDriverState *bs, Error **errp);
 BlockInfo *bdrv_query_info(BlockDriverState *s);
 BlockStats *bdrv_query_stats(const BlockDriverState *bs);
 int bdrv_can_read_snapshot(BlockDriverState *bs);
-- 
1.7.1

[Qemu-devel] [PATCH V4 06/13] qemu-img: switch image retrieving function

[Wenchao Xia]

  Now qemu-img call block layer function to get image info and check
if error happens.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 qemu-img.c |   90 ++++--------------------------------------------------------
 1 files changed, 6 insertions(+), 84 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 9dab48f..90f4bf4 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1134,39 +1134,6 @@ static void dump_json_image_info_list(ImageInfoList *list)
     QDECREF(str);
 }
 
-static void collect_snapshots(BlockDriverState *bs , ImageInfo *info)
-{
-    int i, sn_count;
-    QEMUSnapshotInfo *sn_tab = NULL;
-    SnapshotInfoList *info_list, *cur_item = NULL;
-    sn_count = bdrv_snapshot_list(bs, &sn_tab);
-
-    for (i = 0; i < sn_count; i++) {
-        info->has_snapshots = true;
-        info_list = g_new0(SnapshotInfoList, 1);
-
-        info_list->value                = g_new0(SnapshotInfo, 1);
-        info_list->value->id            = g_strdup(sn_tab[i].id_str);
-        info_list->value->name          = g_strdup(sn_tab[i].name);
-        info_list->value->vm_state_size = sn_tab[i].vm_state_size;
-        info_list->value->date_sec      = sn_tab[i].date_sec;
-        info_list->value->date_nsec     = sn_tab[i].date_nsec;
-        info_list->value->vm_clock_sec  = sn_tab[i].vm_clock_nsec / 1000000000;
-        info_list->value->vm_clock_nsec = sn_tab[i].vm_clock_nsec % 1000000000;
-
-        /* XXX: waiting for the qapi to support qemu-queue.h types */
-        if (!cur_item) {
-            info->snapshots = cur_item = info_list;
-        } else {
-            cur_item->next = info_list;
-            cur_item = info_list;
-        }
-
-    }
-
-    g_free(sn_tab);
-}
-
 static void dump_json_image_info(ImageInfo *info)
 {
     Error *errp = NULL;
@@ -1184,54 +1151,6 @@ static void dump_json_image_info(ImageInfo *info)
     QDECREF(str);
 }
 
-static void collect_image_info(BlockDriverState *bs,
-                   ImageInfo *info,
-                   const char *filename)
-{
-    uint64_t total_sectors;
-    char backing_filename[1024];
-    char backing_filename2[1024];
-    BlockDriverInfo bdi;
-
-    bdrv_get_geometry(bs, &total_sectors);
-
-    info->filename        = g_strdup(filename);
-    info->format          = g_strdup(bdrv_get_format_name(bs));
-    info->virtual_size    = total_sectors * 512;
-    info->actual_size     = bdrv_get_allocated_file_size(bs);
-    info->has_actual_size = info->actual_size >= 0;
-    if (bdrv_is_encrypted(bs)) {
-        info->encrypted = true;
-        info->has_encrypted = true;
-    }
-    if (bdrv_get_info(bs, &bdi) >= 0) {
-        if (bdi.cluster_size != 0) {
-            info->cluster_size = bdi.cluster_size;
-            info->has_cluster_size = true;
-        }
-        info->dirty_flag = bdi.is_dirty;
-        info->has_dirty_flag = true;
-    }
-    bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
-    if (backing_filename[0] != '\0') {
-        info->backing_filename = g_strdup(backing_filename);
-        info->has_backing_filename = true;
-        bdrv_get_full_backing_filename(bs, backing_filename2,
-                                       sizeof(backing_filename2));
-
-        if (strcmp(backing_filename, backing_filename2) != 0) {
-            info->full_backing_filename =
-                        g_strdup(backing_filename2);
-            info->has_full_backing_filename = true;
-        }
-
-        if (bs->backing_format[0]) {
-            info->backing_filename_format = g_strdup(bs->backing_format);
-            info->has_backing_filename_format = true;
-        }
-    }
-}
-
 static void dump_human_image_info(ImageInfo *info)
 {
     char size_buf[128], dsize_buf[128];
@@ -1338,6 +1257,7 @@ static ImageInfoList *collect_image_info_list(const char *filename,
     ImageInfoList *head = NULL;
     ImageInfoList **last = &head;
     GHashTable *filenames;
+    Error *err = NULL;
 
     filenames = g_hash_table_new_full(g_str_hash, str_equal_func, NULL, NULL);
 
@@ -1359,9 +1279,11 @@ static ImageInfoList *collect_image_info_list(const char *filename,
             goto err;
         }
 
-        info = g_new0(ImageInfo, 1);
-        collect_image_info(bs, info, filename);
-        collect_snapshots(bs, info);
+        info = bdrv_query_image_info(bs, &err);
+        if (error_is_set(&err)) {
+            bdrv_delete(bs);
+            goto err;
+        }
 
         elem = g_new0(ImageInfoList, 1);
         elem->value = info;
-- 
1.7.1

[Qemu-devel] [PATCH V4 07/13] block: rename bdrv_query_info to bdrv_query_block_info

[Wenchao Xia]

  Now that we have bdrv_query_image_info, rename this function to make it
more obvious what it is doing.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block.c               |    4 ++--
 include/block/block.h |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 245a082..1bcd165 100644
--- a/block.c
+++ b/block.c
@@ -2961,7 +2961,7 @@ ImageInfo *bdrv_query_image_info(BlockDriverState *bs, Error **errp)
     return info;
 }
 
-BlockInfo *bdrv_query_info(BlockDriverState *bs)
+BlockInfo *bdrv_query_block_info(BlockDriverState *bs)
 {
     BlockInfo *info = g_malloc0(sizeof(*info));
     info->device = g_strdup(bs->device_name);
@@ -3027,7 +3027,7 @@ BlockInfoList *qmp_query_block(Error **errp)
 
     QTAILQ_FOREACH(bs, &bdrv_states, list) {
         BlockInfoList *info = g_malloc0(sizeof(*info));
-        info->value = bdrv_query_info(bs);
+        info->value = bdrv_query_block_info(bs);
 
         *p_next = info;
         p_next = &info->next;
diff --git a/include/block/block.h b/include/block/block.h
index b6f80c3..9838c48 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -324,7 +324,7 @@ SnapshotInfoList *bdrv_query_snapshot_infolist(BlockDriverState *bs,
                                                void *opaque,
                                                Error **errp);
 ImageInfo *bdrv_query_image_info(BlockDriverState *bs, Error **errp);
-BlockInfo *bdrv_query_info(BlockDriverState *s);
+BlockInfo *bdrv_query_block_info(BlockDriverState *bs);
 BlockStats *bdrv_query_stats(const BlockDriverState *bs);
 int bdrv_can_read_snapshot(BlockDriverState *bs);
 int bdrv_can_snapshot(BlockDriverState *bs);
-- 
1.7.1

[Qemu-devel] [PATCH V4 08/13] qmp: add interface query-images.

[Wenchao Xia]

  This mirror function will return all image info including
snapshots. Now Qemu have both query-images and query-block
interfaces.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block.c          |   32 ++++++++++++++++++++++
 qapi-schema.json |   27 +++++++++++++++++++
 qmp-commands.hx  |   76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 135 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 1bcd165..7c9727b 100644
--- a/block.c
+++ b/block.c
@@ -2961,6 +2961,38 @@ ImageInfo *bdrv_query_image_info(BlockDriverState *bs, Error **errp)
     return info;
 }
 
+DeviceImageInfoList *qmp_query_images(Error **errp)
+{
+    DeviceImageInfoList *head = NULL, **p_next = &head;
+    BlockDriverState *bs;
+    Error *err = NULL;
+
+    QTAILQ_FOREACH(bs, &bdrv_states, list) {
+        DeviceImageInfo *dii = g_malloc0(sizeof(*dii));
+        DeviceImageInfoList *diil = g_malloc0(sizeof(*diil));
+        diil->value = dii;
+        *p_next = diil;
+        p_next = &diil->next;
+
+        dii->device = g_strdup(bs->device_name);
+        if (!bdrv_is_inserted(bs)) {
+            dii->has_image = false;
+            continue;
+        }
+        dii->has_image = true;
+        dii->image = bdrv_query_image_info(bs, &err);
+        if (error_is_set(&err)) {
+            goto err;
+        }
+    }
+
+    return head;
+
+ err:
+    qapi_free_DeviceImageInfoList(head);
+    return NULL;
+}
+
 BlockInfo *bdrv_query_block_info(BlockDriverState *bs)
 {
     BlockInfo *info = g_malloc0(sizeof(*info));
diff --git a/qapi-schema.json b/qapi-schema.json
index 6d7252b..7881a9a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -245,6 +245,22 @@
            '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'] } }
 
 ##
+# @DeviceImageInfo:
+#
+# Information about an image used by QEMU block device
+#
+# @device: name of the block device
+#
+# @image: #optional info of the image used.
+#
+# Since: 1.4
+#
+##
+
+{ 'type': 'DeviceImageInfo',
+  'data': {'device': 'str', '*image': 'ImageInfo' } }
+
+##
 # @StatusInfo:
 #
 # Information about VCPU run state
@@ -720,6 +736,17 @@
 { 'command': 'query-block', 'returns': ['BlockInfo'] }
 
 ##
+# @query-images:
+#
+# Get a list of DeviceImageInfo for all virtual block devices.
+#
+# Returns: a list of @DeviceImageInfo describing each virtual block device
+#
+# Since: 1.4
+##
+{ 'command': 'query-images', 'returns': ['DeviceImageInfo'] }
+
+##
 # @BlockDeviceStats:
 #
 # Statistics of a virtual block device or a block backing device.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index cbf1280..9bd6288 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1668,6 +1668,82 @@ EQMP
     },
 
 SQMP
+query-images
+-----------
+
+Show the block devices' images.
+
+Each block image information is stored in a json-object and the returned value
+is a json-array of all devices' images.
+
+Each json-object contain the following:
+
+- "device": device name (json-string)
+- "image": related image information, it is a optional json-object containing the following:
+         - "filename": image file name (json-string)
+         - "format": image format (json-string)
+         - "virtual-size": maximum capacity in bytes of the image (json-int)
+         - "dirty-flag": true if image is not cleanly closed (json-bool, optional)
+         - "actual-size": actual size on disk in bytes of the image (json-int, optional)
+         - "cluster-size": size of a cluster in bytes (json-int, optional)
+         - "encrypted": true if the image is encrypted (json-bool, optional)
+         - "backing_file": backing file name (json-string, optional)
+         - "full-backing-filename": full path of the backing file (json-string, optional)
+         - "backing-filename-format": the format of the backing file (json-string, optional)
+         - "snapshots": the internal snapshot info, it is an optional list of json-object
+            containing the following:
+             - "id": unique snapshot id (json-string)
+             - "name": internal snapshot name (json-string)
+             - "vm-state-size": size of the VM state in bytes (json-int)
+             - "date-sec": UTC date of the snapshot in seconds (json-int)
+             - "date-nsec": fractional part in nano seconds to be used with date-sec(json-int)
+             - "vm-clock-sec": VM clock relative to boot in seconds (json-int)
+             - "vm-clock-nsec": fractional part in nano seconds to be used with vm-clock-sec (json-int)
+
+Example:
+
+-> { "execute": "query-images" }
+<- {
+      "return":[
+         {
+            "device":"ide0-hd0",
+            "image":{
+               "filename":"disks/test0.img",
+               "format":"qcow2",
+               "virtual-size":1024000
+            }
+         },
+         {
+            "device":"ide0-hd1",
+            "image":{
+               "filename":"disks/test1.img",
+               "format":"qcow2",
+               "virtual-size":2048000,
+               "snapshots":[
+                  {
+                     "id": "1",
+                     "name": "snapshot1",
+                     "vm-state-size": 0,
+                     "date-sec": 10000200,
+                     "date-nsec": 12,
+                     "vm-clock-sec": 206,
+                     "vm-clock-nsec": 30
+                  }
+               ]
+            }
+         }
+      ]
+   }
+
+EQMP
+
+    {
+        .name       = "query-images",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_images,
+    },
+
+SQMP
 query-blockstats
 ----------------
 
-- 
1.7.1

[Qemu-devel] [PATCH V4 09/13] block: export function bdrv_find_snapshot()

[Wenchao Xia]

  This patch move it from savevm.c to block.c and export it. To make
it clear about id and name in searching, the API was changed a bit
to distinguish them. Caller can choose to search by id or name now.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block.c               |   51 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/block/block.h |    2 +
 savevm.c              |   32 ++++-------------------------
 3 files changed, 58 insertions(+), 27 deletions(-)

diff --git a/block.c b/block.c
index 7c9727b..4313d1b 100644
--- a/block.c
+++ b/block.c
@@ -3381,6 +3381,57 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
     return -ENOTSUP;
 }
 
+/*
+ * Try find an internal snapshot with @id or @name, @id have higher priority
+ * in searching.
+ * @bs block device to search on, must not be NULL.
+ * @sn_info snapshot information to be filled in, must not be NULL.
+ * @id snapshot id to search with, can be NULL.
+ * @name snapshot name to search with, can be NULL.
+ * returns 0 and @sn_info is filled with related information if found,
+ * otherwise it returns negative value, -ENOENT.
+ */
+int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
+                       const char *id, const char *name)
+{
+    QEMUSnapshotInfo *sn_tab, *sn;
+    int nb_sns, i, ret;
+
+    ret = -ENOENT;
+    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
+    if (nb_sns < 0) {
+        return ret;
+    }
+
+    /* search by id */
+    if (id) {
+        for (i = 0; i < nb_sns; i++) {
+            sn = &sn_tab[i];
+            if (!strcmp(sn->id_str, id)) {
+                *sn_info = *sn;
+                ret = 0;
+                goto out;
+            }
+        }
+    }
+
+    /* search by name */
+    if (name) {
+        for (i = 0; i < nb_sns; i++) {
+            sn = &sn_tab[i];
+            if (!strcmp(sn->name, name)) {
+                *sn_info = *sn;
+                ret = 0;
+                goto out;
+            }
+        }
+    }
+
+ out:
+    g_free(sn_tab);
+    return ret;
+}
+
 /* backing_file can either be relative, or absolute, or a protocol.  If it is
  * relative, it must be relative to the chain.  So, passing in bs->filename
  * from a BDS as backing_file should not be done, as that may be relative to
diff --git a/include/block/block.h b/include/block/block.h
index 9838c48..1e7f4a1 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -340,6 +340,8 @@ int bdrv_snapshot_list(BlockDriverState *bs,
 int bdrv_snapshot_load_tmp(BlockDriverState *bs,
                            const char *snapshot_name);
 char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn);
+int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
+                       const char *id, const char *name);
 
 char *get_human_readable_size(char *buf, int buf_size, int64_t size);
 int path_is_absolute(const char *path);
diff --git a/savevm.c b/savevm.c
index d9ff1d9..5607105 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2027,28 +2027,6 @@ out:
     return ret;
 }
 
-static int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
-                              const char *name)
-{
-    QEMUSnapshotInfo *sn_tab, *sn;
-    int nb_sns, i, ret;
-
-    ret = -ENOENT;
-    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
-    if (nb_sns < 0)
-        return ret;
-    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;
-        }
-    }
-    g_free(sn_tab);
-    return ret;
-}
-
 /*
  * Deletes snapshots of a given name in all opened images.
  */
@@ -2061,7 +2039,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, name) >= 0)
         {
             ret = bdrv_snapshot_delete(bs, name);
             if (ret < 0) {
@@ -2121,7 +2099,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);
+        ret = bdrv_snapshot_find(bs, old_sn, name, name);
         if (ret >= 0) {
             pstrcpy(sn->name, sizeof(sn->name), old_sn->name);
             pstrcpy(sn->id_str, sizeof(sn->id_str), old_sn->id_str);
@@ -2212,7 +2190,7 @@ int load_vmstate(const char *name)
     }
 
     /* Don't even try to load empty VM states */
-    ret = bdrv_snapshot_find(bs_vm_state, &sn, name);
+    ret = bdrv_snapshot_find(bs_vm_state, &sn, name, name);
     if (ret < 0) {
         return ret;
     } else if (sn.vm_state_size == 0) {
@@ -2236,7 +2214,7 @@ int load_vmstate(const char *name)
             return -ENOTSUP;
         }
 
-        ret = bdrv_snapshot_find(bs, &sn, name);
+        ret = bdrv_snapshot_find(bs, &sn, name, name);
         if (ret < 0) {
             error_report("Device '%s' does not have the requested snapshot '%s'",
                            bdrv_get_device_name(bs), name);
@@ -2342,7 +2320,7 @@ 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);
+                ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str, NULL);
                 if (ret < 0) {
                     available = 0;
                     break;
-- 
1.7.1

[Qemu-devel] [PATCH V4 10/13] qmp: add interface query-snapshots

[Wenchao Xia]

  This interface now return valid internal snapshots for whole vm
or a single block device.
  Note that filter use bdrv_can_read_snapshot() instead of
bdrv_can_snapshot(), which should be the correct behavior in information
retrieving funtion.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block.c          |   44 ++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json |   19 +++++++++++++++++++
 qmp-commands.hx  |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 116 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 4313d1b..763b1f9 100644
--- a/block.c
+++ b/block.c
@@ -2888,6 +2888,50 @@ SnapshotInfoList *bdrv_query_snapshot_infolist(BlockDriverState *bs,
     return head;
 }
 
+/* check if sn exist on all block devices, 0 means valid */
+static int snapshot_filter_vm(const QEMUSnapshotInfo *sn, void *opaque)
+{
+    BlockDriverState *bs = (BlockDriverState *)opaque, *bs1 = NULL;
+    QEMUSnapshotInfo s, *sn_info = &s;
+    int ret = 0;
+
+    while ((bs1 = bdrv_next(bs1))) {
+        if (bdrv_can_read_snapshot(bs1) && bs1 != bs) {
+            ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str, NULL);
+            if (ret < 0) {
+                ret = -1;
+                break;
+            }
+        }
+    }
+    return ret;
+}
+
+SnapshotInfoList *qmp_query_snapshots(bool has_device,
+                                      const char *device,
+                                      Error **errp)
+{
+    BlockDriverState *bs;
+
+    if (has_device) {
+        /* internal snapshots for single device */
+        bs = bdrv_find(device);
+        if (!bs) {
+            error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+            return NULL;
+        }
+        return bdrv_query_snapshot_infolist(bs, NULL, bs, errp);
+    }
+
+    /* internal snapshot for whole vm */
+    bs = bdrv_snapshots();
+    if (!bs) {
+        error_setg(errp, "No available block device supports snapshots\n");
+        return NULL;
+    }
+    return bdrv_query_snapshot_infolist(bs, snapshot_filter_vm, bs, errp);
+}
+
 /* collect all internal snapshot info in a image for ImageInfo */
 static void collect_snapshots_info(BlockDriverState *bs,
                                    ImageInfo *info,
diff --git a/qapi-schema.json b/qapi-schema.json
index 7881a9a..2a2a8c0 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -747,6 +747,25 @@
 { 'command': 'query-images', 'returns': ['DeviceImageInfo'] }
 
 ##
+# @query-snapshots:
+#
+# Get a list of internal snapshots for whole virtual machine or a single
+# block device. Note that in first case, only valid internal snapshot will be
+# returned, inconsistent ones will be ignored.
+#
+# @device: #optional the name of the device to get snapshot info from. If not
+#          specified, only valid snapshots for whole vm would be returned.
+#
+# Returns: a list of @SnapshotInfo describing all consistent virtual machine
+#          snapshots.
+#
+# Since: 1.4
+##
+{ 'command': 'query-snapshots',
+  'data': { '*device': 'str' },
+  'returns': ['SnapshotInfo'] }
+
+##
 # @BlockDeviceStats:
 #
 # Statistics of a virtual block device or a block backing device.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 9bd6288..79c6578 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1744,6 +1744,59 @@ EQMP
     },
 
 SQMP
+query-snapshots
+-----------
+
+Show the internal consistent snapshot information.
+
+Each snapshot information is stored in a json-object and the returned value
+is a json-array of all snapshots.
+
+Each json-object contain the following:
+
+- "id": unique snapshot id (json-string)
+- "name": internal snapshot name (json-string)
+- "vm-state-size": size of the VM state in bytes (json-int)
+- "date-sec": UTC date of the snapshot in seconds (json-int)
+- "date-nsec": fractional part in nano seconds to be used with date-sec(json-int)
+- "vm-clock-sec": VM clock relative to boot in seconds (json-int)
+- "vm-clock-nsec": fractional part in nano seconds to be used with vm-clock-sec (json-int)
+
+Example:
+
+-> { "execute": "query-snapshots" }
+<- {
+      "return":[
+         {
+            "id": "1",
+            "name": "snapshot1",
+            "vm-state-size": 0,
+            "date-sec": 10000200,
+            "date-nsec": 12,
+            "vm-clock-sec": 206,
+            "vm-clock-nsec": 30
+         },
+         {
+            "id": "2",
+            "name": "snapshot2",
+            "vm-state-size": 34000000,
+            "date-sec": 13000200,
+            "date-nsec": 32,
+            "vm-clock-sec": 406,
+            "vm-clock-nsec": 31
+         }
+      ]
+   }
+
+EQMP
+
+    {
+        .name       = "query-snapshots",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_snapshots,
+    },
+
+SQMP
 query-blockstats
 ----------------
 
-- 
1.7.1

[Qemu-devel] [PATCH V4 11/13] hmp: add function hmp_info_snapshots()

[Wenchao Xia]

  This function will simply call qmp interface qmp_query_snapshots()
added in last commit and then dump information in monitor console.
  Now snapshot info retrieving code in qemu and qemu-img are merged
by calling same block layer function, and then they just translate
the qmp object to strings in stdout or monitor console.

Note:
  This patch need previous hmp extention patch which enable
info sub command take qdict * as paramter.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 hmp.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 hmp.h |    1 +
 2 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/hmp.c b/hmp.c
index c7b6ba0..1e8f9eb 100644
--- a/hmp.c
+++ b/hmp.c
@@ -629,6 +629,48 @@ void hmp_info_block_jobs(Monitor *mon, const QDict *qdict)
     }
 }
 
+/* assume list is valid */
+static void monitor_dump_snapshotinfolist(Monitor *mon, SnapshotInfoList *list)
+{
+    SnapshotInfoList *elem;
+    char buf[256];
+
+    monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+
+    for (elem = list; elem; elem = elem->next) {
+        QEMUSnapshotInfo sn = {
+            .vm_state_size = elem->value->vm_state_size,
+            .date_sec = elem->value->date_sec,
+            .date_nsec = elem->value->date_nsec,
+            .vm_clock_nsec = elem->value->vm_clock_sec * 1000000000ULL +
+                             elem->value->vm_clock_nsec,
+        };
+        pstrcpy(sn.id_str, sizeof(sn.id_str), elem->value->id);
+        pstrcpy(sn.name, sizeof(sn.name), elem->value->name);
+        monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), &sn));
+    }
+}
+
+void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    SnapshotInfoList *list;
+
+    list = qmp_query_snapshots(false, NULL, &err);
+    if (error_is_set(&err)) {
+        hmp_handle_error(mon, &err);
+        return;
+    }
+
+    if (list == NULL) {
+        monitor_printf(mon, "There is no suitable snapshot available\n");
+        return;
+    }
+
+    monitor_dump_snapshotinfolist(mon, list);
+    qapi_free_SnapshotInfoList(list);
+}
+
 void hmp_quit(Monitor *mon, const QDict *qdict)
 {
     monitor_suspend(mon);
diff --git a/hmp.h b/hmp.h
index 44be683..b020a7b 100644
--- a/hmp.h
+++ b/hmp.h
@@ -36,6 +36,7 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict);
 void hmp_info_balloon(Monitor *mon, const QDict *qdict);
 void hmp_info_pci(Monitor *mon, const QDict *qdict);
 void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
+void hmp_info_snapshots(Monitor *mon, const QDict *qdict);
 void hmp_quit(Monitor *mon, const QDict *qdict);
 void hmp_stop(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
-- 
1.7.1

[Qemu-devel] [PATCH V4 12/13] hmp: switch snapshot info function to qmp based one

[Wenchao Xia]

  This patch using new added function in last commit which retrieve
info from qmp for snapshot info.

Note:
  This patch need previous hmp extention patch which enable
info sub command take qdict * as paramter.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 monitor.c |    2 +-
 savevm.c  |   64 -------------------------------------------------------------
 2 files changed, 1 insertions(+), 65 deletions(-)

diff --git a/monitor.c b/monitor.c
index 20bd19b..4bb80c0 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2594,7 +2594,7 @@ static mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show the currently saved VM snapshots",
-        .mhandler.cmd = do_info_snapshots,
+        .mhandler.cmd = hmp_info_snapshots,
     },
     {
         .name       = "status",
diff --git a/savevm.c b/savevm.c
index 5607105..549d799 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2285,70 +2285,6 @@ void do_delvm(Monitor *mon, const QDict *qdict)
     }
 }
 
-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 total;
-    int *available_snapshots;
-    char buf[256];
-
-    bs = bdrv_snapshots();
-    if (!bs) {
-        monitor_printf(mon, "No available block device supports snapshots\n");
-        return;
-    }
-
-    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
-    if (nb_sns < 0) {
-        monitor_printf(mon, "bdrv_snapshot_list: error %d\n", nb_sns);
-        return;
-    }
-
-    if (nb_sns == 0) {
-        monitor_printf(mon, "There is no snapshot available.\n");
-        return;
-    }
-
-    available_snapshots = g_malloc0(sizeof(int) * nb_sns);
-    total = 0;
-    for (i = 0; i < nb_sns; i++) {
-        sn = &sn_tab[i];
-        available = 1;
-        bs1 = NULL;
-
-        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) {
-                    available = 0;
-                    break;
-                }
-            }
-        }
-
-        if (available) {
-            available_snapshots[total] = i;
-            total++;
-        }
-    }
-
-    if (total > 0) {
-        monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
-        for (i = 0; i < total; i++) {
-            sn = &sn_tab[available_snapshots[i]];
-            monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
-        }
-    } else {
-        monitor_printf(mon, "There is no suitable snapshot available\n");
-    }
-
-    g_free(sn_tab);
-    g_free(available_snapshots);
-
-}
-
 void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
 {
     qemu_ram_set_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK,
-- 
1.7.1

[Qemu-devel] [PATCH V4 13/13] hmp: show snapshots on single block device

[Wenchao Xia]

  This patch added the support of showing internal snapshots on a
single block device in hmp layer, by simply change parameter in
calling of qmp funtion.

Note:
  This patch need previous hmp extention patch which enable
info sub command take qdict * as paramter.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 hmp.c     |    7 ++++++-
 monitor.c |    6 +++---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/hmp.c b/hmp.c
index 1e8f9eb..4a6c3ee 100644
--- a/hmp.c
+++ b/hmp.c
@@ -655,8 +655,10 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
     SnapshotInfoList *list;
+    const char *device = qdict_get_try_str(qdict, "device");
+    bool has_device = device ? true : false;
 
-    list = qmp_query_snapshots(false, NULL, &err);
+    list = qmp_query_snapshots(has_device, device, &err);
     if (error_is_set(&err)) {
         hmp_handle_error(mon, &err);
         return;
@@ -667,6 +669,9 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
         return;
     }
 
+    if (has_device) {
+        monitor_printf(mon, "Device '%s':\n", device);
+    }
     monitor_dump_snapshotinfolist(mon, list);
     qapi_free_SnapshotInfoList(list);
 }
diff --git a/monitor.c b/monitor.c
index 4bb80c0..a2aa10c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2591,9 +2591,9 @@ static mon_cmd_t info_cmds[] = {
     },
     {
         .name       = "snapshots",
-        .args_type  = "",
-        .params     = "",
-        .help       = "show the currently saved VM snapshots",
+        .args_type  = "device:B?",
+        .params     = "[device]",
+        .help       = "show snapshots of whole vm or a single device",
         .mhandler.cmd = hmp_info_snapshots,
     },
     {
-- 
1.7.1

Re: [Qemu-devel] [PATCH V4 04/13] block: add snapshot info query function bdrv_query_snapshot_infolist()

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1788 bytes --]

On 01/17/2013 12:40 AM, Wenchao Xia wrote:
>   This patch add function bdrv_query_snapshot_infolist(), which will
> return snapshot info of an image in qmp object format. The implementation
> code are mostly copied from qemu-img.c with modification to fit more
> for qmp based block layer API.
>   To help filter out snapshot info not needed, a call back function is
> added in bdrv_query_snapshot_infolist().
>   bdrv_can_read_snapshot() should be called before call this function,
> to avoid got *errp set unexpectly.

s/unexpectly/unexpectedly/

> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  block.c               |   46 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/block/block.h |    7 +++++++
>  2 files changed, 53 insertions(+), 0 deletions(-)
> 

> +SnapshotInfoList *bdrv_query_snapshot_infolist(BlockDriverState *bs,
> +                                               SnapshotFilterFunc filter,
> +                                               void *opaque,
> +                                               Error **errp)
> +{
> +    int i, sn_count;
> +    QEMUSnapshotInfo *sn_tab = NULL;
> +    SnapshotInfoList *info_list, *cur_item = NULL, *head = NULL;
> +
> +    sn_count = bdrv_snapshot_list(bs, &sn_tab);
> +    if (sn_count < 0) {
> +        /* Fix me:  set errp in bdrv_snapshot_list() instead of here */

You didn't remove this fixme anywhere in the series; it would be better
to respin things to avoid a net gain of fixmes.

> +        error_setg(errp, "bdrv_snapshot_list: error %d\n", sn_count);
> +        return NULL;
> +    }

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH V4 04/13] block: add snapshot info query function bdrv_query_snapshot_infolist()

[Wenchao Xia]

于 2013-1-18 5:53, Eric Blake 写道:
> On 01/17/2013 12:40 AM, Wenchao Xia wrote:
>>    This patch add function bdrv_query_snapshot_infolist(), which will
>> return snapshot info of an image in qmp object format. The implementation
>> code are mostly copied from qemu-img.c with modification to fit more
>> for qmp based block layer API.
>>    To help filter out snapshot info not needed, a call back function is
>> added in bdrv_query_snapshot_infolist().
>>    bdrv_can_read_snapshot() should be called before call this function,
>> to avoid got *errp set unexpectly.
>
> s/unexpectly/unexpectedly/
>
  sorry for it.

>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>>   block.c               |   46 ++++++++++++++++++++++++++++++++++++++++++++++
>>   include/block/block.h |    7 +++++++
>>   2 files changed, 53 insertions(+), 0 deletions(-)
>>
>
>> +SnapshotInfoList *bdrv_query_snapshot_infolist(BlockDriverState *bs,
>> +                                               SnapshotFilterFunc filter,
>> +                                               void *opaque,
>> +                                               Error **errp)
>> +{
>> +    int i, sn_count;
>> +    QEMUSnapshotInfo *sn_tab = NULL;
>> +    SnapshotInfoList *info_list, *cur_item = NULL, *head = NULL;
>> +
>> +    sn_count = bdrv_snapshot_list(bs, &sn_tab);
>> +    if (sn_count < 0) {
>> +        /* Fix me:  set errp in bdrv_snapshot_list() instead of here */
>
> You didn't remove this fixme anywhere in the series; it would be better
> to respin things to avoid a net gain of fixmes.
>
   yep, but solving that may need more work focusing on the function
enhancement and caller behavior which already exist, not quite related
to this serial. This serial is a bit long already, I'd like a
standalone patch for it later.

>> +        error_setg(errp, "bdrv_snapshot_list: error %d\n", sn_count);
>> +        return NULL;
>> +    }
>


-- 
Best Regards

Wenchao Xia

Re: [Qemu-devel] [PATCH V4 00/13] add qmp/hmp interfaces for snapshot info

[Wenchao Xia]

于 2013-1-17 15:39, Wenchao Xia 写道:
>    This serial of patches does two things: merge some info code
> in qemu-img, and add following interfaces:
> 1) qmp: query-images
> 2) qmp: query-snapshots
> 3) hmp: show snapshot info on a single block device
>    These patches follows the rule that use qmp to retieve information,
> hmp layer just do a translation from qmp object it got, so almost
> every hmp interface may have a correlated qmp interface.
>    To make code graceful, snapshot retrieving code in qemu and qemu-img
> are merged into block.c, and some function name was adjusted to make it
> tips better. Now it works as:
> 
>     qemu          qemu-img
> 
> dump_monitor    dump_stdout
>       |--------------|
>              |
>             qmp
>              |
>            block
> 
> Note:
>    Last two patches need previous sent patches which extend hmp sub command, at:
> http://lists.nongnu.org/archive/html/qemu-devel/2012-12/msg03487.html
> 
> v2:
>    Rename and adjusted qmp interface according to comments from Eric.
>    Spelling fix.
>    Information retrieving function in block layer goes to seperated patch.
>    Free qmp object after usage in hmp.
>    Added counterpart in qmp-commands.hx.
>    Better tips in qmp-schema.json.
> 
> v3:
>    Spelling fix in commit message, patch 03/11.
>    Spelling fix in code, patch 06/11.
>    Add comments that vm-state-size is in bytes, and change size of it in
> example to a reasonable number, patch 08/11.
> 
> v4:
>    02/13: in bdrv_get_filename(), add const to parameter *bs.
>    03/13: new added, in which the function correct the behavior in info
> retrieving.
>    04/13: in bdrv_query_snapshot_infolist(), remove NULL check before call
> err_setg(), added TODO comments that let block layer function set error instead
> of this layer to tip better for errors, Split out patch about image info to
> patch 05/13.
>    05/13: new splitted, and it checks *bs by calling bdrv_can_read_snapshot()
> before collect internal snasphot info to avoid *err is set unexpectly now.
>    06/13: check if error happens after calling bdrv_query_image_info().
>    08/13: rename info to image in DeviceImageInfo and make it optional,
> when device is not inserted it will be empty, added error handling code
> when met error in calling block layer API.
>    09/13: distinguish *id and *name in bdrv_find_snapshots(), caller
> can choose what to search with. id_wellformed() should be called in
> new snapshot creation interface above this function in the future.
>    10/13: now this interface have addtional parameter *device, which
> enable showing internal snapshots on a single device. Also use
> bdrv_can_read_snapshot() instead of bdrv_can_snapshot() now.
>    11/13: this function goes to hmp.c so hmp_handler_error is not exported
> any more, split out patch that switch snapshot info function to patch 12/13.
>    12/13: new splitted.
>    13/13: use qmp API instead of directly calling block layer API, now
> all hmp function have correspond qmp funtion in this serial.
> 
> Wenchao Xia (13):
>    1 qemu-img: remove unused parameter in collect_image_info()
>    2 block: add bdrv_get_filename() function
>    3 block: add bdrv_can_read_snapshot() function
>    4 block: add snapshot info query function bdrv_query_snapshot_infolist()
>    5 block: add image info query function bdrv_query_image_info()
>    6 qemu-img: switch image retrieving function
>    7 block: rename bdrv_query_info to bdrv_query_block_info
>    8 qmp: add interface query-images.
>    9 block: export function bdrv_find_snapshot()
>    10 qmp: add interface query-snapshots
>    11 hmp: add function hmp_info_snapshots()
>    12 hmp: switch snapshot info function to qmp based one
>    13 hmp: show snapshots on single block device
> 
>   block.c               |  274 ++++++++++++++++++++++++++++++++++++++++++++++++-
>   hmp.c                 |   47 +++++++++
>   hmp.h                 |    1 +
>   include/block/block.h |   14 +++-
>   monitor.c             |    8 +-
>   qapi-schema.json      |   46 ++++++++
>   qemu-img.c            |   91 +---------------
>   qmp-commands.hx       |  129 +++++++++++++++++++++++
>   savevm.c              |   94 +----------------
>   9 files changed, 522 insertions(+), 182 deletions(-)
> 
> 
Hi all,
  any more comments for this serial?
-- 
Best Regards

Wenchao Xia