qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 00/11] add qmp/hmp interfaces for snapshot info
@ 2012-12-29  8:45 Wenchao Xia
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 01/11] qemu-img: remove unused parameter in collect_image_info() Wenchao Xia
                   ` (10 more replies)
  0 siblings, 11 replies; 20+ messages in thread
From: Wenchao Xia @ 2012-12-29  8:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, phrdina, stefanha, armbru, lcapitulino, pbonzini,
	Wenchao Xia

  This serial of patches does two things: merge some info code
in qemu-img, and add following interfaces:
1) qmp: query-image
2) qmp: query-snapshot
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

Wenchao Xia (11):
  qemu-img: remove unused parameter in collect_image_info()
  block: add bdrv_get_filename() function
  qemu-img: remove parameter filename in collect_image_info()
  qemu-img: move image retrieving function to block layer
  block: rename bdrv_query_info to bdrv_query_block_info
  qmp: add interface query-image
  block: move bdrv_find_snapshot to block.c
  qmp: add interface query-snapshot
  hmp: export function hmp_handle_error()
  hmp: retrieve info from qmp for snapshot info
  hmp: show snapshot on single block device

 block.c               |  198 ++++++++++++++++++++++++++++++++++++++++++++++++-
 hmp.c                 |    2 +-
 hmp.h                 |    2 +
 include/block/block.h |   14 +++-
 monitor.c             |    6 +-
 qapi-schema.json      |   23 ++++++
 qemu-img.c            |   87 +---------------------
 savevm.c              |  135 +++++++++++++++++-----------------
 8 files changed, 308 insertions(+), 159 deletions(-)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 01/11] qemu-img: remove unused parameter in collect_image_info()
  2012-12-29  8:45 [Qemu-devel] [PATCH 00/11] add qmp/hmp interfaces for snapshot info Wenchao Xia
@ 2012-12-29  8:45 ` Wenchao Xia
  2013-01-02 17:58   ` Eric Blake
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 02/11] block: add bdrv_get_filename() function Wenchao Xia
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Wenchao Xia @ 2012-12-29  8:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, phrdina, stefanha, armbru, lcapitulino, pbonzini,
	Wenchao Xia

  Parameter *fmt was not used, so remove it.

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 69cc028..5a4df3a 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1182,8 +1182,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];
@@ -1357,7 +1356,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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 02/11] block: add bdrv_get_filename() function
  2012-12-29  8:45 [Qemu-devel] [PATCH 00/11] add qmp/hmp interfaces for snapshot info Wenchao Xia
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 01/11] qemu-img: remove unused parameter in collect_image_info() Wenchao Xia
@ 2012-12-29  8:45 ` Wenchao Xia
  2013-01-02 23:08   ` Eric Blake
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 03/11] qemu-img: remove parameter filename in collect_image_info() Wenchao Xia
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Wenchao Xia @ 2012-12-29  8:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, phrdina, stefanha, armbru, lcapitulino, pbonzini,
	Wenchao Xia

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

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 4e28c55..5f95da5 100644
--- a/block.c
+++ b/block.c
@@ -2967,6 +2967,11 @@ BlockStatsList *qmp_query_blockstats(Error **errp)
     return head;
 }
 
+const char *bdrv_get_filename(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 b81d200..cf20257 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(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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 03/11] qemu-img: remove parameter filename in collect_image_info()
  2012-12-29  8:45 [Qemu-devel] [PATCH 00/11] add qmp/hmp interfaces for snapshot info Wenchao Xia
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 01/11] qemu-img: remove unused parameter in collect_image_info() Wenchao Xia
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 02/11] block: add bdrv_get_filename() function Wenchao Xia
@ 2012-12-29  8:45 ` Wenchao Xia
  2013-01-03 18:03   ` Eric Blake
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 04/11] qemu-img: move image retrieving function to block layer Wenchao Xia
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Wenchao Xia @ 2012-12-29  8:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, phrdina, stefanha, armbru, lcapitulino, pbonzini,
	Wenchao Xia

  Switch the filename getting from parameter to block function,
now collect_image_info depends only on *bs.

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

diff --git a/qemu-img.c b/qemu-img.c
index 5a4df3a..d70435f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1180,15 +1180,17 @@ static void dump_json_image_info(ImageInfo *info)
     QDECREF(str);
 }
 
+/* Assume bs is already openned. */
 static void collect_image_info(BlockDriverState *bs,
-                   ImageInfo *info,
-                   const char *filename)
+                               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);
@@ -1356,7 +1358,7 @@ static ImageInfoList *collect_image_info_list(const char *filename,
         }
 
         info = g_new0(ImageInfo, 1);
-        collect_image_info(bs, info, filename);
+        collect_image_info(bs, info);
         collect_snapshots(bs, info);
 
         elem = g_new0(ImageInfoList, 1);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 04/11] qemu-img: move image retrieving function to block layer
  2012-12-29  8:45 [Qemu-devel] [PATCH 00/11] add qmp/hmp interfaces for snapshot info Wenchao Xia
                   ` (2 preceding siblings ...)
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 03/11] qemu-img: remove parameter filename in collect_image_info() Wenchao Xia
@ 2012-12-29  8:45 ` Wenchao Xia
  2013-01-04 17:02   ` Eric Blake
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 05/11] block: rename bdrv_query_info to bdrv_query_block_info Wenchao Xia
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Wenchao Xia @ 2012-12-29  8:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, phrdina, stefanha, armbru, lcapitulino, pbonzini,
	Wenchao Xia

  This patch moves collect_image_info() and collect_snapshot()
to general block layer and encapsulate them as bdrv_query_image_info()
and bdrv_query_snapshot_infolist(), as mirror function to brdv_query_info().
The called function in qemu-img.c is switched to bdrv_query_image_info().
To help filter out snapshot info not needed, a call back function is
added in bdrv_query_snapshot_infolist().

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block.c               |  118 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/block/block.h |    9 ++++
 qemu-img.c            |   88 +-----------------------------------
 3 files changed, 129 insertions(+), 86 deletions(-)

diff --git a/block.c b/block.c
index 5f95da5..d39da3d 100644
--- a/block.c
+++ b/block.c
@@ -2846,6 +2846,124 @@ 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) {
+        if (errp != NULL) {
+            error_setg(errp, "bdrv_snapshot_list: error %d\n", sn_count);
+        }
+        return NULL;
+    }
+
+    for (i = 0; i < sn_count; i++) {
+        if ((filter != NULL) && (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;
+}
+
+/* collect all internal snapshot info in a image for ImageInfo */
+static void collect_snapshots_info(BlockDriverState *bs,
+                                   ImageInfo *info,
+                                   Error **errp)
+{
+    SnapshotInfoList *info_list;
+
+    info_list = bdrv_query_snapshot_infolist(bs, NULL, NULL, errp);
+    if (info_list != NULL) {
+        info->has_snapshots = true;
+        info->snapshots = info_list;
+    }
+    return;
+}
+
+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 cf20257..33d7b7c 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -316,8 +316,17 @@ 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 openned, use g_free to free returned value. */
+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_snapshot(BlockDriverState *bs);
 int bdrv_is_snapshot(BlockDriverState *bs);
 BlockDriverState *bdrv_snapshots(void);
diff --git a/qemu-img.c b/qemu-img.c
index d70435f..6a62c68 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1130,39 +1130,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;
@@ -1180,56 +1147,6 @@ static void dump_json_image_info(ImageInfo *info)
     QDECREF(str);
 }
 
-/* Assume bs is already openned. */
-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;
-        }
-    }
-}
-
 static void dump_human_image_info(ImageInfo *info)
 {
     char size_buf[128], dsize_buf[128];
@@ -1336,6 +1253,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);
 
@@ -1357,9 +1275,7 @@ static ImageInfoList *collect_image_info_list(const char *filename,
             goto err;
         }
 
-        info = g_new0(ImageInfo, 1);
-        collect_image_info(bs, info);
-        collect_snapshots(bs, info);
+        info = bdrv_query_image_info(bs, &err);
 
         elem = g_new0(ImageInfoList, 1);
         elem->value = info;
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 05/11] block: rename bdrv_query_info to bdrv_query_block_info
  2012-12-29  8:45 [Qemu-devel] [PATCH 00/11] add qmp/hmp interfaces for snapshot info Wenchao Xia
                   ` (3 preceding siblings ...)
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 04/11] qemu-img: move image retrieving function to block layer Wenchao Xia
@ 2012-12-29  8:45 ` Wenchao Xia
  2013-01-04 22:49   ` Eric Blake
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 06/11] qmp: add interface query-image Wenchao Xia
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Wenchao Xia @ 2012-12-29  8:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, phrdina, stefanha, armbru, lcapitulino, pbonzini,
	Wenchao Xia

  Now this function have a mirror name with bdrv_query_image_info
to tip well what it is doing.

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 d39da3d..9dcecec 100644
--- a/block.c
+++ b/block.c
@@ -2964,7 +2964,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);
@@ -3030,7 +3030,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 33d7b7c..3b7c818 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_snapshot(BlockDriverState *bs);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 06/11] qmp: add interface query-image
  2012-12-29  8:45 [Qemu-devel] [PATCH 00/11] add qmp/hmp interfaces for snapshot info Wenchao Xia
                   ` (4 preceding siblings ...)
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 05/11] block: rename bdrv_query_info to bdrv_query_block_info Wenchao Xia
@ 2012-12-29  8:45 ` Wenchao Xia
  2013-01-04 23:48   ` Eric Blake
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 07/11] block: move bdrv_find_snapshot to block.c Wenchao Xia
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Wenchao Xia @ 2012-12-29  8:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, phrdina, stefanha, armbru, lcapitulino, pbonzini,
	Wenchao Xia

  This mirror function will return all image info including
snapshots. Now Qemu have both query-image and query-block
interfaces, and qemu-img share the code for image info
retrieving with qemu emulator.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block.c          |   16 ++++++++++++++++
 qapi-schema.json |   11 +++++++++++
 2 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 9dcecec..0e9f414 100644
--- a/block.c
+++ b/block.c
@@ -2964,6 +2964,22 @@ ImageInfo *bdrv_query_image_info(BlockDriverState *bs, Error **errp)
     return info;
 }
 
+ImageInfoList *qmp_query_image(Error **errp)
+{
+    ImageInfoList *head = NULL, **p_next = &head;
+    BlockDriverState *bs;
+
+    QTAILQ_FOREACH(bs, &bdrv_states, list) {
+        ImageInfoList *info = g_malloc0(sizeof(*info));
+        info->value = bdrv_query_image_info(bs, NULL);
+
+        *p_next = info;
+        p_next = &info->next;
+    }
+
+    return head;
+}
+
 BlockInfo *bdrv_query_block_info(BlockDriverState *bs)
 {
     BlockInfo *info = g_malloc0(sizeof(*info));
diff --git a/qapi-schema.json b/qapi-schema.json
index 5dfa052..40f96f3 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -720,6 +720,17 @@
 { 'command': 'query-block', 'returns': ['BlockInfo'] }
 
 ##
+# @query-image:
+#
+# Get a list of BlockImage for all virtual block devices.
+#
+# Returns: a list of @BlockImage describing each virtual block device
+#
+# Since: 1.4
+##
+{ 'command': 'query-image', 'returns': ['ImageInfo'] }
+
+##
 # @BlockDeviceStats:
 #
 # Statistics of a virtual block device or a block backing device.
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 07/11] block: move bdrv_find_snapshot to block.c
  2012-12-29  8:45 [Qemu-devel] [PATCH 00/11] add qmp/hmp interfaces for snapshot info Wenchao Xia
                   ` (5 preceding siblings ...)
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 06/11] qmp: add interface query-image Wenchao Xia
@ 2012-12-29  8:45 ` Wenchao Xia
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 08/11] qmp: add interface query-snapshot Wenchao Xia
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Wenchao Xia @ 2012-12-29  8:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, phrdina, stefanha, armbru, lcapitulino, pbonzini,
	Wenchao Xia

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

diff --git a/block.c b/block.c
index 0e9f414..d7eb213 100644
--- a/block.c
+++ b/block.c
@@ -3349,6 +3349,29 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
     return -ENOTSUP;
 }
 
+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;
+}
+
 /* 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 3b7c818..41ed5fb 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 *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 720eea6..6884416 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2074,28 +2074,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.
  */
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 08/11] qmp: add interface query-snapshot
  2012-12-29  8:45 [Qemu-devel] [PATCH 00/11] add qmp/hmp interfaces for snapshot info Wenchao Xia
                   ` (6 preceding siblings ...)
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 07/11] block: move bdrv_find_snapshot to block.c Wenchao Xia
@ 2012-12-29  8:45 ` Wenchao Xia
  2013-01-07 21:21   ` Eric Blake
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 09/11] hmp: export function hmp_handle_error() Wenchao Xia
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Wenchao Xia @ 2012-12-29  8:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, phrdina, stefanha, armbru, lcapitulino, pbonzini,
	Wenchao Xia

  This interface now return valid internal snapshots.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block.c          |   32 ++++++++++++++++++++++++++++++++
 qapi-schema.json |   12 ++++++++++++
 2 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index d7eb213..ad058f9 100644
--- a/block.c
+++ b/block.c
@@ -2892,6 +2892,38 @@ 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_snapshot(bs1) && bs1 != bs) {
+            ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str);
+            if (ret < 0) {
+                ret = -1;
+                break;
+            }
+        }
+    }
+    return ret;
+}
+
+SnapshotInfoList *qmp_query_snapshot(Error **errp)
+{
+    BlockDriverState *bs;
+
+    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 40f96f3..2918817 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -731,6 +731,18 @@
 { 'command': 'query-image', 'returns': ['ImageInfo'] }
 
 ##
+# @query-snapshot:
+#
+# Get a list of valid Snapshots of virtual machine. Note that only valid
+# internal snapshot will be returned now, inconsitent one will not be returned.
+#
+# Returns: a list of @SnapshotInfo describing virtual machine snapshot.
+#
+# Since: 1.4
+##
+{ 'command': 'query-snapshot', 'returns': ['SnapshotInfo'] }
+
+##
 # @BlockDeviceStats:
 #
 # Statistics of a virtual block device or a block backing device.
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 09/11] hmp: export function hmp_handle_error()
  2012-12-29  8:45 [Qemu-devel] [PATCH 00/11] add qmp/hmp interfaces for snapshot info Wenchao Xia
                   ` (7 preceding siblings ...)
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 08/11] qmp: add interface query-snapshot Wenchao Xia
@ 2012-12-29  8:45 ` Wenchao Xia
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 10/11] hmp: retrieve info from qmp for snapshot info Wenchao Xia
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 11/11] hmp: show snapshot on single block device Wenchao Xia
  10 siblings, 0 replies; 20+ messages in thread
From: Wenchao Xia @ 2012-12-29  8:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, phrdina, stefanha, armbru, lcapitulino, pbonzini,
	Wenchao Xia

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

diff --git a/hmp.c b/hmp.c
index 2465d9b..89a1a8c 100644
--- a/hmp.c
+++ b/hmp.c
@@ -23,7 +23,7 @@
 #include "monitor/monitor.h"
 #include "ui/console.h"
 
-static void hmp_handle_error(Monitor *mon, Error **errp)
+void hmp_handle_error(Monitor *mon, Error **errp)
 {
     if (error_is_set(errp)) {
         monitor_printf(mon, "%s\n", error_get_pretty(*errp));
diff --git a/hmp.h b/hmp.h
index 5cab9c0..da1a35c 100644
--- a/hmp.h
+++ b/hmp.h
@@ -17,7 +17,9 @@
 #include "qemu-common.h"
 #include "qapi-types.h"
 #include "qapi/qmp/qdict.h"
+#include "qapi/error.h"
 
+void hmp_handle_error(Monitor *mon, Error **errp);
 void hmp_info_name(Monitor *mon, const QDict *qdict);
 void hmp_info_version(Monitor *mon, const QDict *qdict);
 void hmp_info_kvm(Monitor *mon, const QDict *qdict);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 10/11] hmp: retrieve info from qmp for snapshot info
  2012-12-29  8:45 [Qemu-devel] [PATCH 00/11] add qmp/hmp interfaces for snapshot info Wenchao Xia
                   ` (8 preceding siblings ...)
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 09/11] hmp: export function hmp_handle_error() Wenchao Xia
@ 2012-12-29  8:45 ` Wenchao Xia
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 11/11] hmp: show snapshot on single block device Wenchao Xia
  10 siblings, 0 replies; 20+ messages in thread
From: Wenchao Xia @ 2012-12-29  8:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, phrdina, stefanha, armbru, lcapitulino, pbonzini,
	Wenchao Xia

  With this patch, hmp command info snapshot simply call a block
layer funtion which will return a qmp object, and then translate
it in monitor console. Now snapshot info retrieving code in qemu
and qemu-tool are merged by calling same block layer function,
and then they just translate the qmp to stdout or monitor.

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>
---
 savevm.c |   83 ++++++++++++++++++++++++-------------------------------------
 1 files changed, 33 insertions(+), 50 deletions(-)

diff --git a/savevm.c b/savevm.c
index 6884416..5f29316 100644
--- a/savevm.c
+++ b/savevm.c
@@ -86,6 +86,7 @@
 #include "qmp-commands.h"
 #include "trace.h"
 #include "qemu/bitops.h"
+#include "hmp.h"
 
 #define SELF_ANNOUNCE_ROUNDS 5
 
@@ -2349,68 +2350,50 @@ void do_delvm(Monitor *mon, const QDict *qdict)
     }
 }
 
-void do_info_snapshots(Monitor *mon, const QDict *qdict)
+/* assume list is valid */
+static void monitor_dump_snapshotinfolist(Monitor *mon, SnapshotInfoList *list)
 {
-    BlockDriverState *bs, *bs1;
-    QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s;
-    int nb_sns, i, ret, available;
-    int total;
-    int *available_snapshots;
+    SnapshotInfoList *elem;
     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;
-    }
+    monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
 
-    if (nb_sns == 0) {
-        monitor_printf(mon, "There is no snapshot available.\n");
-        return;
+    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));
     }
+}
 
-    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);
-                if (ret < 0) {
-                    available = 0;
-                    break;
-                }
-            }
-        }
+static void do_info_snapshots_vm(Monitor *mon)
+{
+    Error *err = NULL;
+    SnapshotInfoList *list;
 
-        if (available) {
-            available_snapshots[total] = i;
-            total++;
-        }
+    list = qmp_query_snapshot(&err);
+    if (error_is_set(&err)) {
+        hmp_handle_error(mon, &err);
+        return;
     }
-
-    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 {
+    if (list == NULL) {
         monitor_printf(mon, "There is no suitable snapshot available\n");
+        return;
     }
 
-    g_free(sn_tab);
-    g_free(available_snapshots);
+    monitor_dump_snapshotinfolist(mon, list);
+    return;
+}
 
+void do_info_snapshots(Monitor *mon, const QDict *qdict)
+{
+    do_info_snapshots_vm(mon);
 }
 
 void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 11/11] hmp: show snapshot on single block device
  2012-12-29  8:45 [Qemu-devel] [PATCH 00/11] add qmp/hmp interfaces for snapshot info Wenchao Xia
                   ` (9 preceding siblings ...)
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 10/11] hmp: retrieve info from qmp for snapshot info Wenchao Xia
@ 2012-12-29  8:45 ` Wenchao Xia
  10 siblings, 0 replies; 20+ messages in thread
From: Wenchao Xia @ 2012-12-29  8:45 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, aliguori, phrdina, stefanha, armbru, lcapitulino, pbonzini,
	Wenchao Xia

  This patch use block layer API to qmp snapshot info on a block
device, then use the same code dumping vm snapshot info, to print
in monitor.

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 |    6 +++---
 savevm.c  |   42 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/monitor.c b/monitor.c
index e1bcaa2..3dfdd4d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2580,9 +2580,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.info = do_info_snapshots,
     },
     {
diff --git a/savevm.c b/savevm.c
index 5f29316..413328a 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2391,9 +2391,49 @@ static void do_info_snapshots_vm(Monitor *mon)
     return;
 }
 
+static void do_info_snapshots_blk(Monitor *mon, const char *device)
+{
+    Error *err = NULL;
+    SnapshotInfoList *list;
+    BlockDriverState *bs;
+
+    /* find the target bs */
+    bs = bdrv_find(device);
+    if (!bs) {
+        monitor_printf(mon, "Device '%s' not found.\n", device);
+        return ;
+    }
+
+    if (!bdrv_can_snapshot(bs)) {
+        monitor_printf(mon, "Device '%s' can't have snapshot.\n", device);
+        return ;
+    }
+
+    list = bdrv_query_snapshot_infolist(bs, NULL, NULL, &err);
+    if (error_is_set(&err)) {
+        hmp_handle_error(mon, &err);
+        return;
+    }
+
+    if (list == NULL) {
+        monitor_printf(mon, "There is no snapshot available.\n");
+        return;
+    }
+
+    monitor_printf(mon, "Device '%s':\n", device);
+    monitor_dump_snapshotinfolist(mon, list);
+    return;
+}
+
 void do_info_snapshots(Monitor *mon, const QDict *qdict)
 {
-    do_info_snapshots_vm(mon);
+    const char *device = qdict_get_try_str(qdict, "device");
+    if (!device) {
+        do_info_snapshots_vm(mon);
+    } else {
+        do_info_snapshots_blk(mon, device);
+    }
+    return;
 }
 
 void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 01/11] qemu-img: remove unused parameter in collect_image_info()
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 01/11] qemu-img: remove unused parameter in collect_image_info() Wenchao Xia
@ 2013-01-02 17:58   ` Eric Blake
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Blake @ 2013-01-02 17:58 UTC (permalink / raw)
  To: Wenchao Xia
  Cc: kwolf, aliguori, phrdina, stefanha, qemu-devel, armbru, pbonzini,
	lcapitulino

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

On 12/29/2012 01:45 AM, Wenchao Xia wrote:
>   Parameter *fmt was not used, so remove it.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  qemu-img.c |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)

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

-- 
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: 619 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 02/11] block: add bdrv_get_filename() function
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 02/11] block: add bdrv_get_filename() function Wenchao Xia
@ 2013-01-02 23:08   ` Eric Blake
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Blake @ 2013-01-02 23:08 UTC (permalink / raw)
  To: Wenchao Xia
  Cc: kwolf, aliguori, phrdina, stefanha, qemu-devel, armbru, pbonzini,
	lcapitulino

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

On 12/29/2012 01:45 AM, Wenchao Xia wrote:
>   This function will simply return the uri or filename used
> to open the image.
> 
> 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(-)

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

-- 
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: 619 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 03/11] qemu-img: remove parameter filename in collect_image_info()
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 03/11] qemu-img: remove parameter filename in collect_image_info() Wenchao Xia
@ 2013-01-03 18:03   ` Eric Blake
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Blake @ 2013-01-03 18:03 UTC (permalink / raw)
  To: Wenchao Xia
  Cc: kwolf, aliguori, phrdina, stefanha, qemu-devel, armbru, pbonzini,
	lcapitulino

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

On 12/29/2012 01:45 AM, Wenchao Xia wrote:
>   Switch the filename getting from parameter to block function,
> now collect_image_info depends only on *bs.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  qemu-img.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 5a4df3a..d70435f 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1180,15 +1180,17 @@ static void dump_json_image_info(ImageInfo *info)
>      QDECREF(str);
>  }
>  
> +/* Assume bs is already openned. */

s/openned/opened/

Once that is fixed,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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: 619 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 04/11] qemu-img: move image retrieving function to block layer
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 04/11] qemu-img: move image retrieving function to block layer Wenchao Xia
@ 2013-01-04 17:02   ` Eric Blake
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Blake @ 2013-01-04 17:02 UTC (permalink / raw)
  To: Wenchao Xia
  Cc: kwolf, aliguori, phrdina, stefanha, qemu-devel, armbru, pbonzini,
	lcapitulino

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

On 12/29/2012 01:45 AM, Wenchao Xia wrote:
>   This patch moves collect_image_info() and collect_snapshot()
> to general block layer and encapsulate them as bdrv_query_image_info()
> and bdrv_query_snapshot_infolist(), as mirror function to brdv_query_info().
> The called function in qemu-img.c is switched to bdrv_query_image_info().
> To help filter out snapshot info not needed, a call back function is
> added in bdrv_query_snapshot_infolist().
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  block.c               |  118 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/block/block.h |    9 ++++
>  qemu-img.c            |   88 +-----------------------------------
>  3 files changed, 129 insertions(+), 86 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) {
> +        if (errp != NULL) {
> +            error_setg(errp, "bdrv_snapshot_list: error %d\n", sn_count);
> +        }
> +        return NULL;
> +    }
> +
> +    for (i = 0; i < sn_count; i++) {
> +        if ((filter != NULL) && (filter(&sn_tab[i], opaque) != 0)) {

My brief glance through qemu sources makes me think you
over-parenthesized this, and that:

if (filter && filter(&sn_tab[i], opaque) != 0) {

would be sufficient.


> +    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);

I know it's just code motion, but why the line wrap here?

> +++ b/include/block/block.h
> @@ -316,8 +316,17 @@ 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 openned, use g_free to free returned value. */

s/openned/opened/

-- 
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: 619 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 05/11] block: rename bdrv_query_info to bdrv_query_block_info
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 05/11] block: rename bdrv_query_info to bdrv_query_block_info Wenchao Xia
@ 2013-01-04 22:49   ` Eric Blake
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Blake @ 2013-01-04 22:49 UTC (permalink / raw)
  To: Wenchao Xia
  Cc: kwolf, aliguori, phrdina, stefanha, qemu-devel, armbru, pbonzini,
	lcapitulino

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

On 12/29/2012 01:45 AM, Wenchao Xia wrote:
>   Now this function have a mirror name with bdrv_query_image_info
> to tip well what it is doing.

Awkward to read.  Maybe:

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

> 
> 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(-)

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

-- 
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: 619 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 06/11] qmp: add interface query-image
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 06/11] qmp: add interface query-image Wenchao Xia
@ 2013-01-04 23:48   ` Eric Blake
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Blake @ 2013-01-04 23:48 UTC (permalink / raw)
  To: Wenchao Xia
  Cc: kwolf, aliguori, phrdina, stefanha, qemu-devel, armbru, pbonzini,
	lcapitulino

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

On 12/29/2012 01:45 AM, Wenchao Xia wrote:
>   This mirror function will return all image info including
> snapshots. Now Qemu have both query-image and query-block
> interfaces, and qemu-img share the code for image info
> retrieving with qemu emulator.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  block.c          |   16 ++++++++++++++++
>  qapi-schema.json |   11 +++++++++++
>  2 files changed, 27 insertions(+), 0 deletions(-)

Should there be a counterpart change to qmp-commands.hx, demonstrating
the type of output expected?

>  
>  ##
> +# @query-image:
> +#
> +# Get a list of BlockImage for all virtual block devices.
> +#
> +# Returns: a list of @BlockImage describing each virtual block device

Here, you use BlockImage twice,

> +#
> +# Since: 1.4
> +##
> +{ 'command': 'query-image', 'returns': ['ImageInfo'] }

but here, the type is named ImageInfo.

This interface is weak - it tells me a list of filenames that are in
use, but doesn't tell me which element of the array is tied to which device.

Also, this command is singular, but returns a plural listing; elsewhere,
we have used plurals (think query-commands).

I'd rather see something like:

{ 'type': 'DeviceImageInfo',
  'data': {'device': 'str', 'info': 'ImageInfo' } }
{ 'command': 'query-images', 'returns': ['DeviceImageInfo'] }

so that I can get the pairings between
 [{ 'device':'virtio0', 'info':{ 'filename':'/path1', ...} },
  { 'device':'virtio1', 'info':{ 'filename':'/path2', ...} }]

-- 
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: 619 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 08/11] qmp: add interface query-snapshot
  2012-12-29  8:45 ` [Qemu-devel] [PATCH 08/11] qmp: add interface query-snapshot Wenchao Xia
@ 2013-01-07 21:21   ` Eric Blake
  2013-01-08  2:29     ` Wenchao Xia
  0 siblings, 1 reply; 20+ messages in thread
From: Eric Blake @ 2013-01-07 21:21 UTC (permalink / raw)
  To: Wenchao Xia
  Cc: kwolf, aliguori, phrdina, stefanha, qemu-devel, armbru, pbonzini,
	lcapitulino

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

On 12/29/2012 01:45 AM, Wenchao Xia wrote:
>   This interface now return valid internal snapshots.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  block.c          |   32 ++++++++++++++++++++++++++++++++
>  qapi-schema.json |   12 ++++++++++++
>  2 files changed, 44 insertions(+), 0 deletions(-)
> 

> +++ b/block.c
> @@ -2892,6 +2892,38 @@ SnapshotInfoList *bdrv_query_snapshot_infolist(BlockDriverState *bs,
>      return head;
>  }
>  
> +/* check if sn exist on all block devices, 0 means valid */

s/exist/exists/

> +++ b/qapi-schema.json
> @@ -731,6 +731,18 @@
>  { 'command': 'query-image', 'returns': ['ImageInfo'] }
>  
>  ##
> +# @query-snapshot:
> +#
> +# Get a list of valid Snapshots of virtual machine. Note that only valid
> +# internal snapshot will be returned now, inconsitent one will not be returned.

s/snapshot/snapshots/; s/ now//; s/inconsitent one/inconsistent ones/

> +#
> +# Returns: a list of @SnapshotInfo describing virtual machine snapshot.

s/describing virtual machine snapshot/describing all consistent virtual
machine snapshots/

> +#
> +# Since: 1.4
> +##
> +{ 'command': 'query-snapshot', 'returns': ['SnapshotInfo'] }
> +

-- 
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: 619 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 08/11] qmp: add interface query-snapshot
  2013-01-07 21:21   ` Eric Blake
@ 2013-01-08  2:29     ` Wenchao Xia
  0 siblings, 0 replies; 20+ messages in thread
From: Wenchao Xia @ 2013-01-08  2:29 UTC (permalink / raw)
  To: Eric Blake
  Cc: kwolf, aliguori, phrdina, stefanha, qemu-devel, armbru, pbonzini,
	lcapitulino

Sorry for bad spelling, will fix them in next version.

> On 12/29/2012 01:45 AM, Wenchao Xia wrote:
>>    This interface now return valid internal snapshots.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   block.c          |   32 ++++++++++++++++++++++++++++++++
>>   qapi-schema.json |   12 ++++++++++++
>>   2 files changed, 44 insertions(+), 0 deletions(-)
>>
>
>> +++ b/block.c
>> @@ -2892,6 +2892,38 @@ SnapshotInfoList *bdrv_query_snapshot_infolist(BlockDriverState *bs,
>>       return head;
>>   }
>>
>> +/* check if sn exist on all block devices, 0 means valid */
>
> s/exist/exists/
>
>> +++ b/qapi-schema.json
>> @@ -731,6 +731,18 @@
>>   { 'command': 'query-image', 'returns': ['ImageInfo'] }
>>
>>   ##
>> +# @query-snapshot:
>> +#
>> +# Get a list of valid Snapshots of virtual machine. Note that only valid
>> +# internal snapshot will be returned now, inconsitent one will not be returned.
>
> s/snapshot/snapshots/; s/ now//; s/inconsitent one/inconsistent ones/
>
>> +#
>> +# Returns: a list of @SnapshotInfo describing virtual machine snapshot.
>
> s/describing virtual machine snapshot/describing all consistent virtual
> machine snapshots/
>
>> +#
>> +# Since: 1.4
>> +##
>> +{ 'command': 'query-snapshot', 'returns': ['SnapshotInfo'] }
>> +
>


-- 
Best Regards

Wenchao Xia

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2013-01-08  2:29 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-29  8:45 [Qemu-devel] [PATCH 00/11] add qmp/hmp interfaces for snapshot info Wenchao Xia
2012-12-29  8:45 ` [Qemu-devel] [PATCH 01/11] qemu-img: remove unused parameter in collect_image_info() Wenchao Xia
2013-01-02 17:58   ` Eric Blake
2012-12-29  8:45 ` [Qemu-devel] [PATCH 02/11] block: add bdrv_get_filename() function Wenchao Xia
2013-01-02 23:08   ` Eric Blake
2012-12-29  8:45 ` [Qemu-devel] [PATCH 03/11] qemu-img: remove parameter filename in collect_image_info() Wenchao Xia
2013-01-03 18:03   ` Eric Blake
2012-12-29  8:45 ` [Qemu-devel] [PATCH 04/11] qemu-img: move image retrieving function to block layer Wenchao Xia
2013-01-04 17:02   ` Eric Blake
2012-12-29  8:45 ` [Qemu-devel] [PATCH 05/11] block: rename bdrv_query_info to bdrv_query_block_info Wenchao Xia
2013-01-04 22:49   ` Eric Blake
2012-12-29  8:45 ` [Qemu-devel] [PATCH 06/11] qmp: add interface query-image Wenchao Xia
2013-01-04 23:48   ` Eric Blake
2012-12-29  8:45 ` [Qemu-devel] [PATCH 07/11] block: move bdrv_find_snapshot to block.c Wenchao Xia
2012-12-29  8:45 ` [Qemu-devel] [PATCH 08/11] qmp: add interface query-snapshot Wenchao Xia
2013-01-07 21:21   ` Eric Blake
2013-01-08  2:29     ` Wenchao Xia
2012-12-29  8:45 ` [Qemu-devel] [PATCH 09/11] hmp: export function hmp_handle_error() Wenchao Xia
2012-12-29  8:45 ` [Qemu-devel] [PATCH 10/11] hmp: retrieve info from qmp for snapshot info Wenchao Xia
2012-12-29  8:45 ` [Qemu-devel] [PATCH 11/11] hmp: show snapshot on single block device Wenchao Xia

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).