[Qemu-devel] [PATCH V13 0/6] enhancement for qmp/hmp interfaces of block info

[Qemu-devel] [PATCH V13 0/6] enhancement for qmp/hmp interfaces of block info

[Wenchao Xia]

  This serial let qmp interface show delaied info, including internal snapshot
/backing chain on all block device at runtime, which helps management stack and
human user, by retrieving exactly the same info of what qemu saws.

Example:
-> { "execute": "query-block" }
<- {
      "return":[
         {
            "io-status": "ok",
            "device":"ide0-hd0",
            "locked":false,
            "removable":false,
            "inserted":{
               "ro":false,
               "drv":"qcow2",
               "encrypted":false,
               "file":"disks/test.qcow2",
               "backing_file_depth":1,
               "bps":1000000,
               "bps_rd":0,
               "bps_wr":0,
               "iops":1000000,
               "iops_rd":0,
               "iops_wr":0,
               "image":{
                  "filename":"disks/test.qcow2",
                  "format":"qcow2",
                  "virtual-size":2048000,
                  "backing_file":"base.qcow2",
                  "full-backing-filename":"disks/base.qcow2",
                  "backing-filename-format:"qcow2",
                  "snapshots":[
                     {
                        "id": "1",
                        "name": "snapshot1",
                        "vm-state-size": 0,
                        "date-sec": 10000200,
                        "date-nsec": 12,
                        "vm-clock-sec": 206,
                        "vm-clock-nsec": 30
                     }
                  ],
                  "backing-image":{
                      "filename":"disks/base.qcow2",
                      "format":"qcow2",
                      "virtual-size":2048000
                  }
               }
            },
            "type":"unknown"
         },
         {
            "io-status": "ok",
            "device":"ide1-cd0",
            "locked":false,
            "removable":true,
            "type":"unknown"
         },
         {
            "device":"floppy0",
            "locked":false,
            "removable":true,
            "type":"unknown"
         },
         {
            "device":"sd0",
            "locked":false,
            "removable":true,
            "type":"unknown"
         }
      ]
   }

  These patches follows the rule that use qmp to retieve information,
hmp layer just does a translation from qmp object it got. To make code
graceful, snapshot and image info retrieving code in qemu and qemu-img are
merged into block layer, and some function name was adjusted to make it tips
better. For the part touch by the serial, it works as:

   qemu          qemu-img

dump_monitor    dump_stdout
     |--------------| 
            |
       block/qapi.c

  Special thanks for Markus, Stefan, Kevin, Eric reviewing many times.

v13:
  Renamed the serial as "enhancement for qmp/hmp interfaces of block info".
  Seperated the common part of code moving and hmp printf as a standalone
serial, which can be used by both mine and Pavel's work. This serial depend
on it: "[PATCH V3 0/4] qapi and snapshot code clean up in block layer",
https://lists.gnu.org/archive/html/qemu-devel/2013-05/msg03539.html
  Removed the VM snapshot info part, since it relate to VM snapshot creating
logic, which should be changed together with Pavel's serial.
  Address Eric's comments:
  2/6: bdrv_query_image_info() returns void now, only use *errp to tip error.

Wenchao Xia (6):
  1 block: add snapshot info query function bdrv_query_snapshot_info_list()
  2 block: add image info query function bdrv_query_image_info()
  3 qmp: add recursive member in ImageInfo
  4 qmp: add ImageInfo in BlockDeviceInfo used by query-block
  5 hmp: show ImageInfo in 'info block'
  6 hmp: add parameters device and -v for info block

 block/qapi.c         |  148 ++++++++++++++++++++++++++++++++++++++++++--------
 hmp.c                |   21 +++++++
 include/block/qapi.h |   14 +++--
 monitor.c            |    7 ++-
 qapi-schema.json     |   10 +++-
 qemu-img.c           |   10 +++-
 qmp-commands.hx      |   69 +++++++++++++++++++++++-
 7 files changed, 242 insertions(+), 37 deletions(-)

[Qemu-devel] [PATCH V13 1/6] block: add snapshot info query function bdrv_query_snapshot_info_list()

[Wenchao Xia]

This patch adds function bdrv_query_snapshot_info_list(), which will
retrieve snapshot info of an image in qmp object format. The implementation
is based on the code moved from qemu-img.c with modification to fit more
for qmp based block layer API.

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

diff --git a/block/qapi.c b/block/qapi.c
index 794dbf8..1ed56da 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -26,29 +26,56 @@
 #include "block/block_int.h"
 #include "qmp-commands.h"
 
-void bdrv_collect_snapshots(BlockDriverState *bs , ImageInfo *info)
+/*
+ * Returns 0 on success, with *p_list either set to describe snapshot
+ * information, or NULL because there are no snapshots.  Returns -errno on
+ * error, with *p_list untouched.
+ */
+int bdrv_query_snapshot_info_list(BlockDriverState *bs,
+                                  SnapshotInfoList **p_list,
+                                  Error **errp)
 {
     int i, sn_count;
     QEMUSnapshotInfo *sn_tab = NULL;
-    SnapshotInfoList *info_list, *cur_item = NULL;
+    SnapshotInfoList *info_list, *cur_item = NULL, *head = NULL;
+    SnapshotInfo *info;
+
     sn_count = bdrv_snapshot_list(bs, &sn_tab);
+    if (sn_count < 0) {
+        const char *dev = bdrv_get_device_name(bs);
+        switch (sn_count) {
+        case -ENOMEDIUM:
+            error_setg(errp, "Device '%s' is not inserted", dev);
+            break;
+        case -ENOTSUP:
+            error_setg(errp,
+                       "Device '%s' does not support internal snapshots",
+                       dev);
+            break;
+        default:
+            error_setg_errno(errp, -sn_count,
+                             "Can't list snapshots of device '%s'", dev);
+            break;
+        }
+        return sn_count;
+    }
 
     for (i = 0; i < sn_count; i++) {
-        info->has_snapshots = true;
-        info_list = g_new0(SnapshotInfoList, 1);
+        info = g_new0(SnapshotInfo, 1);
+        info->id            = g_strdup(sn_tab[i].id_str);
+        info->name          = g_strdup(sn_tab[i].name);
+        info->vm_state_size = sn_tab[i].vm_state_size;
+        info->date_sec      = sn_tab[i].date_sec;
+        info->date_nsec     = sn_tab[i].date_nsec;
+        info->vm_clock_sec  = sn_tab[i].vm_clock_nsec / 1000000000;
+        info->vm_clock_nsec = sn_tab[i].vm_clock_nsec % 1000000000;
 
-        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;
+        info_list = g_new0(SnapshotInfoList, 1);
+        info_list->value = info;
 
         /* XXX: waiting for the qapi to support qemu-queue.h types */
         if (!cur_item) {
-            info->snapshots = cur_item = info_list;
+            head = cur_item = info_list;
         } else {
             cur_item->next = info_list;
             cur_item = info_list;
@@ -57,6 +84,8 @@ void bdrv_collect_snapshots(BlockDriverState *bs , ImageInfo *info)
     }
 
     g_free(sn_tab);
+    *p_list = head;
+    return 0;
 }
 
 void bdrv_collect_image_info(BlockDriverState *bs,
diff --git a/include/block/qapi.h b/include/block/qapi.h
index e6e568d..4f223d1 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -29,7 +29,9 @@
 #include "block/block.h"
 #include "block/snapshot.h"
 
-void bdrv_collect_snapshots(BlockDriverState *bs , ImageInfo *info);
+int bdrv_query_snapshot_info_list(BlockDriverState *bs,
+                                  SnapshotInfoList **p_list,
+                                  Error **errp);
 void bdrv_collect_image_info(BlockDriverState *bs,
                              ImageInfo *info,
                              const char *filename);
diff --git a/qemu-img.c b/qemu-img.c
index 82c7977..29929c5 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1665,7 +1665,10 @@ static ImageInfoList *collect_image_info_list(const char *filename,
 
         info = g_new0(ImageInfo, 1);
         bdrv_collect_image_info(bs, info, filename);
-        bdrv_collect_snapshots(bs, info);
+        bdrv_query_snapshot_info_list(bs, &info->snapshots, NULL);
+        if (info->snapshots) {
+            info->has_snapshots = true;
+        }
 
         elem = g_new0(ImageInfoList, 1);
         elem->value = info;
-- 
1.7.1

[Qemu-devel] [PATCH V13 2/6] block: add image info query function bdrv_query_image_info()

[Wenchao Xia]

This patch adds function bdrv_query_image_info(), which will
retrieve image info in qmp object format. The implementation is
based on the code moved from qemu-img.c, but uses block layer
function to get snapshot info.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block/qapi.c         |   36 ++++++++++++++++++++++++++++++------
 include/block/qapi.h |    6 +++---
 qemu-img.c           |   11 ++++++-----
 3 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 1ed56da..680ec23 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -88,18 +88,22 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
     return 0;
 }
 
-void bdrv_collect_image_info(BlockDriverState *bs,
-                             ImageInfo *info,
-                             const char *filename)
+/* @p_info will be set only on success. */
+void bdrv_query_image_info(BlockDriverState *bs,
+                           ImageInfo **p_info,
+                           Error **errp)
 {
     uint64_t total_sectors;
-    char backing_filename[1024];
+    const char *backing_filename;
     char backing_filename2[1024];
     BlockDriverInfo bdi;
+    int ret;
+    Error *err = NULL;
+    ImageInfo *info = g_new0(ImageInfo, 1);
 
     bdrv_get_geometry(bs, &total_sectors);
 
-    info->filename        = g_strdup(filename);
+    info->filename        = g_strdup(bs->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);
@@ -116,7 +120,7 @@ void bdrv_collect_image_info(BlockDriverState *bs,
         info->dirty_flag = bdi.is_dirty;
         info->has_dirty_flag = true;
     }
-    bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
+    backing_filename = bs->backing_file;
     if (backing_filename[0] != '\0') {
         info->backing_filename = g_strdup(backing_filename);
         info->has_backing_filename = true;
@@ -134,6 +138,26 @@ void bdrv_collect_image_info(BlockDriverState *bs,
             info->has_backing_filename_format = true;
         }
     }
+
+    ret = bdrv_query_snapshot_info_list(bs, &info->snapshots, &err);
+    switch (ret) {
+    case 0:
+        if (info->snapshots) {
+            info->has_snapshots = true;
+        }
+        break;
+    /* recoverable error */
+    case -ENOMEDIUM:
+    case -ENOTSUP:
+        error_free(err);
+        break;
+    default:
+        error_propagate(errp, err);
+        qapi_free_ImageInfo(info);
+        return;
+    }
+
+    *p_info = info;
 }
 
 BlockInfo *bdrv_query_info(BlockDriverState *bs)
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 4f223d1..ab1f48f 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -32,9 +32,9 @@
 int bdrv_query_snapshot_info_list(BlockDriverState *bs,
                                   SnapshotInfoList **p_list,
                                   Error **errp);
-void bdrv_collect_image_info(BlockDriverState *bs,
-                             ImageInfo *info,
-                             const char *filename);
+void bdrv_query_image_info(BlockDriverState *bs,
+                           ImageInfo **p_info,
+                           Error **errp);
 BlockInfo *bdrv_query_info(BlockDriverState *s);
 BlockStats *bdrv_query_stats(const BlockDriverState *bs);
 
diff --git a/qemu-img.c b/qemu-img.c
index 29929c5..04a3f7c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1642,6 +1642,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);
 
@@ -1663,11 +1664,11 @@ static ImageInfoList *collect_image_info_list(const char *filename,
             goto err;
         }
 
-        info = g_new0(ImageInfo, 1);
-        bdrv_collect_image_info(bs, info, filename);
-        bdrv_query_snapshot_info_list(bs, &info->snapshots, NULL);
-        if (info->snapshots) {
-            info->has_snapshots = true;
+        bdrv_query_image_info(bs, &info, &err);
+        if (error_is_set(&err)) {
+            error_report("%s", error_get_pretty(err));
+            error_free(err);
+            goto err;
         }
 
         elem = g_new0(ImageInfoList, 1);
-- 
1.7.1

[Qemu-devel] [PATCH V13 3/6] qmp: add recursive member in ImageInfo

[Wenchao Xia]

New member *backing-image is added to reflect the backing chain
status.

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

diff --git a/block/qapi.c b/block/qapi.c
index 680ec23..cbef584 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -88,7 +88,21 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
     return 0;
 }
 
-/* @p_info will be set only on success. */
+/**
+ * bdrv_query_image_info:
+ * @bs: block device to examine
+ * @p_info: location to store image information
+ * @errp: location to store error information
+ *
+ * Store "flat" image inforation in @p_info.
+ *
+ * "Flat" means it does *not* query backing image information,
+ * i.e. (*pinfo)->has_backing_image will be set to false and
+ * (*pinfo)->backing_image to NULL even when the image does in fact have
+ * a backing image.
+ *
+ * @p_info will be set only on success. On error, store error in @errp.
+ */
 void bdrv_query_image_info(BlockDriverState *bs,
                            ImageInfo **p_info,
                            Error **errp)
diff --git a/qapi-schema.json b/qapi-schema.json
index ef1f657..a02999d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -236,6 +236,8 @@
 #
 # @snapshots: #optional list of VM snapshots
 #
+# @backing-image: #optional info of the backing image (since 1.6)
+#
 # Since: 1.3
 #
 ##
@@ -245,7 +247,8 @@
            '*actual-size': 'int', 'virtual-size': 'int',
            '*cluster-size': 'int', '*encrypted': 'bool',
            '*backing-filename': 'str', '*full-backing-filename': 'str',
-           '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'] } }
+           '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'],
+           '*backing-image': 'ImageInfo' } }
 
 ##
 # @ImageCheck:
-- 
1.7.1

[Qemu-devel] [PATCH V13 4/6] qmp: add ImageInfo in BlockDeviceInfo used by query-block

[Wenchao Xia]

Now image info will be retrieved as an embbed json object inside
BlockDeviceInfo, backing chain info and all related internal snapshot
info can be got in the enhanced recursive structure of ImageInfo.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block/qapi.c         |   43 +++++++++++++++++++++++++++++--
 include/block/qapi.h |    4 ++-
 qapi-schema.json     |    5 +++-
 qmp-commands.hx      |   69 ++++++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 114 insertions(+), 7 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index cbef584..b62365a 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -174,9 +174,15 @@ void bdrv_query_image_info(BlockDriverState *bs,
     *p_info = info;
 }
 
-BlockInfo *bdrv_query_info(BlockDriverState *bs)
+/* @p_info will be set only on success. */
+void bdrv_query_info(BlockDriverState *bs,
+                     BlockInfo **p_info,
+                     Error **errp)
 {
     BlockInfo *info = g_malloc0(sizeof(*info));
+    BlockDriverState *bs0;
+    ImageInfo **p_image_info;
+    Error *local_err = NULL;
     info->device = g_strdup(bs->device_name);
     info->type = g_strdup("unknown");
     info->locked = bdrv_dev_is_medium_locked(bs);
@@ -230,8 +236,30 @@ BlockInfo *bdrv_query_info(BlockDriverState *bs)
             info->inserted->iops_wr =
                            bs->io_limits.iops[BLOCK_IO_LIMIT_WRITE];
         }
+
+        bs0 = bs;
+        p_image_info = &info->inserted->image;
+        while (1) {
+            bdrv_query_image_info(bs0, p_image_info, &local_err);
+            if (error_is_set(&local_err)) {
+                error_propagate(errp, local_err);
+                goto err;
+            }
+            if (bs0->drv && bs0->backing_hd) {
+                bs0 = bs0->backing_hd;
+                (*p_image_info)->has_backing_image = true;
+                p_image_info = &((*p_image_info)->backing_image);
+            } else {
+                break;
+            }
+        }
     }
-    return info;
+
+    *p_info = info;
+    return;
+
+ err:
+    qapi_free_BlockInfo(info);
 }
 
 BlockStats *bdrv_query_stats(const BlockDriverState *bs)
@@ -268,16 +296,25 @@ BlockInfoList *qmp_query_block(Error **errp)
 {
     BlockInfoList *head = NULL, **p_next = &head;
     BlockDriverState *bs = NULL;
+    Error *local_err = NULL;
 
      while ((bs = bdrv_next(bs))) {
         BlockInfoList *info = g_malloc0(sizeof(*info));
-        info->value = bdrv_query_info(bs);
+        bdrv_query_info(bs, &info->value, &local_err);
+        if (error_is_set(&local_err)) {
+            error_propagate(errp, local_err);
+            goto err;
+        }
 
         *p_next = info;
         p_next = &info->next;
     }
 
     return head;
+
+ err:
+    qapi_free_BlockInfoList(head);
+    return NULL;
 }
 
 BlockStatsList *qmp_query_blockstats(Error **errp)
diff --git a/include/block/qapi.h b/include/block/qapi.h
index ab1f48f..0496cc9 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -35,7 +35,9 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
 void bdrv_query_image_info(BlockDriverState *bs,
                            ImageInfo **p_info,
                            Error **errp);
-BlockInfo *bdrv_query_info(BlockDriverState *s);
+void bdrv_query_info(BlockDriverState *bs,
+                     BlockInfo **p_info,
+                     Error **errp);
 BlockStats *bdrv_query_stats(const BlockDriverState *bs);
 
 void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f,
diff --git a/qapi-schema.json b/qapi-schema.json
index a02999d..5ad6894 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -759,6 +759,8 @@
 #
 # @iops_wr: write I/O operations per second is specified
 #
+# @image: the info of image used (since: 1.6)
+#
 # Since: 0.14.0
 #
 # Notes: This interface is only found in @BlockInfo.
@@ -768,7 +770,8 @@
             '*backing_file': 'str', 'backing_file_depth': 'int',
             'encrypted': 'bool', 'encryption_key_missing': 'bool',
             'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
-            'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} }
+            'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
+            'image': 'ImageInfo' } }
 
 ##
 # @BlockDeviceIoStatus:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ffd130e..8cea5e5 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1704,6 +1704,47 @@ Each json-object contain the following:
          - "iops": limit total I/O operations per second (json-int)
          - "iops_rd": limit read operations per second (json-int)
          - "iops_wr": limit write operations per second (json-int)
+         - "image": the detail of the image, it is a json-object containing
+            the following:
+             - "filename": image file name (json-string)
+             - "format": image format (json-string)
+             - "virtual-size": image capacity in bytes (json-int)
+             - "dirty-flag": true if image is not cleanly closed, not present
+                             means clean (json-bool, optional)
+             - "actual-size": actual size on disk in bytes of the image, not
+                              present when image does not support thin
+                              provision (json-int, optional)
+             - "cluster-size": size of a cluster in bytes, not present if image
+                               format does not support it (json-int, optional)
+             - "encrypted": true if the image is encrypted, not present means
+                            false or the image format does not support
+                            encryption (json-bool, optional)
+             - "backing_file": backing file name, not present means no backing
+                               file is used or the image format does not
+                               support backing file chain
+                               (json-string, optional)
+             - "full-backing-filename": full path of the backing file, not
+                                        present if it equals backing_file or no
+                                        backing file is used
+                                        (json-string, optional)
+             - "backing-filename-format": the format of the backing file, not
+                                          present means unknown or no 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": 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 nanoseconds 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 nanoseconds to be used
+                                    with vm-clock-sec (json-int)
+             - "backing-image": the detail of the backing image, it is an
+                                optional json-object only present when a
+                                backing image present for this image
 
 - "io-status": I/O operation status, only present if the device supports it
                and the VM is configured to stop on errors. It's always reset
@@ -1724,14 +1765,38 @@ Example:
                "ro":false,
                "drv":"qcow2",
                "encrypted":false,
-               "file":"disks/test.img",
-               "backing_file_depth":0,
+               "file":"disks/test.qcow2",
+               "backing_file_depth":1,
                "bps":1000000,
                "bps_rd":0,
                "bps_wr":0,
                "iops":1000000,
                "iops_rd":0,
                "iops_wr":0,
+               "image":{
+                  "filename":"disks/test.qcow2",
+                  "format":"qcow2",
+                  "virtual-size":2048000,
+                  "backing_file":"base.qcow2",
+                  "full-backing-filename":"disks/base.qcow2",
+                  "backing-filename-format:"qcow2",
+                  "snapshots":[
+                     {
+                        "id": "1",
+                        "name": "snapshot1",
+                        "vm-state-size": 0,
+                        "date-sec": 10000200,
+                        "date-nsec": 12,
+                        "vm-clock-sec": 206,
+                        "vm-clock-nsec": 30
+                     }
+                  ],
+                  "backing-image":{
+                      "filename":"disks/base.qcow2",
+                      "format":"qcow2",
+                      "virtual-size":2048000
+                  }
+               }
             },
             "type":"unknown"
          },
-- 
1.7.1

[Qemu-devel] [PATCH V13 5/6] hmp: show ImageInfo in 'info block'

[Wenchao Xia]

Now human monitor can show image details, include internal
snapshot and backing chain info for every block device.

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

diff --git a/hmp.c b/hmp.c
index 4fb76ec..2aa832c 100644
--- a/hmp.c
+++ b/hmp.c
@@ -22,6 +22,7 @@
 #include "qemu/sockets.h"
 #include "monitor/monitor.h"
 #include "ui/console.h"
+#include "block/qapi.h"
 
 static void hmp_handle_error(Monitor *mon, Error **errp)
 {
@@ -277,6 +278,7 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
 void hmp_info_block(Monitor *mon, const QDict *qdict)
 {
     BlockInfoList *block_list, *info;
+    ImageInfo *image_info;
 
     block_list = qmp_query_block(NULL);
 
@@ -318,6 +320,18 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
                             info->value->inserted->iops,
                             info->value->inserted->iops_rd,
                             info->value->inserted->iops_wr);
+
+            monitor_printf(mon, " images:\n");
+            image_info = info->value->inserted->image;
+            while (1) {
+                bdrv_image_info_dump((fprintf_function)monitor_printf, mon,
+                                     image_info);
+                if (image_info->has_backing_image) {
+                    image_info = image_info->backing_image;
+                } else {
+                    break;
+                }
+            }
         } else {
             monitor_printf(mon, " [not inserted]");
         }
-- 
1.7.1

[Qemu-devel] [PATCH V13 6/6] hmp: add parameters device and -v for info block

[Wenchao Xia]

With these parameters, user can choose the information to be showed,
to avoid message flood in the monitor.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 hmp.c     |   25 ++++++++++++++++---------
 monitor.c |    7 ++++---
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/hmp.c b/hmp.c
index 2aa832c..a590ace 100644
--- a/hmp.c
+++ b/hmp.c
@@ -279,10 +279,15 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
 {
     BlockInfoList *block_list, *info;
     ImageInfo *image_info;
+    const char *device = qdict_get_try_str(qdict, "device");
+    int verbose = qdict_get_try_bool(qdict, "verbose", 0);
 
     block_list = qmp_query_block(NULL);
 
     for (info = block_list; info; info = info->next) {
+        if (device && strcmp(device, info->value->device)) {
+            continue;
+        }
         monitor_printf(mon, "%s: removable=%d",
                        info->value->device, info->value->removable);
 
@@ -321,15 +326,17 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
                             info->value->inserted->iops_rd,
                             info->value->inserted->iops_wr);
 
-            monitor_printf(mon, " images:\n");
-            image_info = info->value->inserted->image;
-            while (1) {
-                bdrv_image_info_dump((fprintf_function)monitor_printf, mon,
-                                     image_info);
-                if (image_info->has_backing_image) {
-                    image_info = image_info->backing_image;
-                } else {
-                    break;
+            if (verbose) {
+                monitor_printf(mon, " images:\n");
+                image_info = info->value->inserted->image;
+                while (1) {
+                        bdrv_image_info_dump((fprintf_function)monitor_printf,
+                                             mon, image_info);
+                    if (image_info->has_backing_image) {
+                        image_info = image_info->backing_image;
+                    } else {
+                        break;
+                    }
                 }
             }
         } else {
diff --git a/monitor.c b/monitor.c
index 6ce2a4e..243f5ae 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2472,9 +2472,10 @@ static mon_cmd_t info_cmds[] = {
     },
     {
         .name       = "block",
-        .args_type  = "",
-        .params     = "",
-        .help       = "show the block devices",
+        .args_type  = "verbose:-v,device:B?",
+        .params     = "[-v] [device]",
+        .help       = "show info of one block device or all block devices "
+                      "(and details of images with -v option)",
         .mhandler.cmd = hmp_info_block,
     },
     {
-- 
1.7.1

Re: [Qemu-devel] [PATCH V13 0/6] enhancement for qmp/hmp interfaces of block info

[Eric Blake]

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

On 05/24/2013 10:24 PM, Wenchao Xia wrote:

The cover letter doesn't get committed into git; but if you do respin,
it might be worth cleaning up some of the grammar.  I know English is
not your native language, and you're doing a fine job of coping as it
is, but this might help you improve your skills.

>   This serial let qmp interface show delaied info, including internal snapshot

s/serial let/series lets/

not sure what word you meant for 'delaied'

> /backing chain on all block device at runtime, which helps management stack and
> human user, by retrieving exactly the same info of what qemu saws.

s/saws/sees/

> 
> Example:
> -> { "execute": "query-block" }
> <- {
>       "return":[
>          {
>             "io-status": "ok",
>             "device":"ide0-hd0",
>             "locked":false,
>             "removable":false,
>             "inserted":{
>                "ro":false,
>                "drv":"qcow2",
>                "encrypted":false,
>                "file":"disks/test.qcow2",

Here...

>                "backing_file_depth":1,
>                "bps":1000000,
>                "bps_rd":0,
>                "bps_wr":0,
>                "iops":1000000,
>                "iops_rd":0,
>                "iops_wr":0,
>                "image":{
>                   "filename":"disks/test.qcow2",

...and here, it looks like we have redundant information.  But that's
okay; I think we're stuck with preserving backwards compatibility while
still providing the new information in a saner structure.

>                   "format":"qcow2",
>                   "virtual-size":2048000,
>                   "backing_file":"base.qcow2",
>                   "full-backing-filename":"disks/base.qcow2",
>                   "backing-filename-format:"qcow2",

And here's another case of information...

>                   "snapshots":[
>                      {
>                         "id": "1",
>                         "name": "snapshot1",
>                         "vm-state-size": 0,
>                         "date-sec": 10000200,
>                         "date-nsec": 12,
>                         "vm-clock-sec": 206,
>                         "vm-clock-nsec": 30
>                      }
>                   ],
>                   "backing-image":{
>                       "filename":"disks/base.qcow2",
>                       "format":"qcow2",

...that gets repeated later.  Oh well.

> 
>   These patches follows the rule that use qmp to retieve information,

s/retieve/retrieve/

> hmp layer just does a translation from qmp object it got. To make code
> graceful, snapshot and image info retrieving code in qemu and qemu-img are
> merged into block layer, and some function name was adjusted to make it tips

s/name was/names were/

> better. For the part touch by the serial, it works as:

s/make it tips better/describe them better/

s/touch/touched/

s/serial/series/

> 
>    qemu          qemu-img
> 
> dump_monitor    dump_stdout
>      |--------------| 
>             |
>        block/qapi.c
> 
>   Special thanks for Markus, Stefan, Kevin, Eric reviewing many times.
> 
> v13:
>   Renamed the serial as "enhancement for qmp/hmp interfaces of block info".
>   Seperated the common part of code moving and hmp printf as a standalone

s/Seperated/Separated/

> serial, which can be used by both mine and Pavel's work. This serial depend

s/serial/series/

> on it: "[PATCH V3 0/4] qapi and snapshot code clean up in block layer",
> https://lists.gnu.org/archive/html/qemu-devel/2013-05/msg03539.html
>   Removed the VM snapshot info part, since it relate to VM snapshot creating
> logic, which should be changed together with Pavel's serial.

s/serial/series/

>   Address Eric's comments:
>   2/6: bdrv_query_image_info() returns void now, only use *errp to tip error.

s/tip/record/

> 
> Wenchao Xia (6):
>   1 block: add snapshot info query function bdrv_query_snapshot_info_list()
>   2 block: add image info query function bdrv_query_image_info()
>   3 qmp: add recursive member in ImageInfo
>   4 qmp: add ImageInfo in BlockDeviceInfo used by query-block
>   5 hmp: show ImageInfo in 'info block'
>   6 hmp: add parameters device and -v for info block
> 
>  block/qapi.c         |  148 ++++++++++++++++++++++++++++++++++++++++++--------
>  hmp.c                |   21 +++++++
>  include/block/qapi.h |   14 +++--
>  monitor.c            |    7 ++-
>  qapi-schema.json     |   10 +++-
>  qemu-img.c           |   10 +++-
>  qmp-commands.hx      |   69 +++++++++++++++++++++++-
>  7 files changed, 242 insertions(+), 37 deletions(-)
> 
> 
> 
> 

-- 
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 V13 2/6] block: add image info query function bdrv_query_image_info()

[Eric Blake]

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

On 05/24/2013 10:24 PM, Wenchao Xia wrote:
> This patch adds function bdrv_query_image_info(), which will
> retrieve image info in qmp object format. The implementation is
> based on the code moved from qemu-img.c, but uses block layer
> function to get snapshot info.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  block/qapi.c         |   36 ++++++++++++++++++++++++++++++------
>  include/block/qapi.h |    6 +++---
>  qemu-img.c           |   11 ++++++-----
>  3 files changed, 39 insertions(+), 14 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: 621 bytes --]

Re: [Qemu-devel] [PATCH V13 3/6] qmp: add recursive member in ImageInfo

[Eric Blake]

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

On 05/24/2013 10:24 PM, Wenchao Xia wrote:
> New member *backing-image is added to reflect the backing chain
> status.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  block/qapi.c     |   16 +++++++++++++++-
>  qapi-schema.json |    5 ++++-
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/block/qapi.c b/block/qapi.c
> index 680ec23..cbef584 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -88,7 +88,21 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>      return 0;
>  }
>  
> -/* @p_info will be set only on success. */
> +/**
> + * bdrv_query_image_info:
> + * @bs: block device to examine
> + * @p_info: location to store image information
> + * @errp: location to store error information
> + *
> + * Store "flat" image inforation in @p_info.

s/inforation/information/

> + *
> + * "Flat" means it does *not* query backing image information,
> + * i.e. (*pinfo)->has_backing_image will be set to false and
> + * (*pinfo)->backing_image to NULL even when the image does in fact have
> + * a backing image.
> + *
> + * @p_info will be set only on success. On error, store error in @errp.
> + */

Does this comment hunk belong in the previous patch?

>  void bdrv_query_image_info(BlockDriverState *bs,
>                             ImageInfo **p_info,
>                             Error **errp)
> diff --git a/qapi-schema.json b/qapi-schema.json
> index ef1f657..a02999d 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -236,6 +236,8 @@
>  #
>  # @snapshots: #optional list of VM snapshots
>  #
> +# @backing-image: #optional info of the backing image (since 1.6)
> +#
>  # Since: 1.3
>  #
>  ##
> @@ -245,7 +247,8 @@
>             '*actual-size': 'int', 'virtual-size': 'int',
>             '*cluster-size': 'int', '*encrypted': 'bool',
>             '*backing-filename': 'str', '*full-backing-filename': 'str',
> -           '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'] } }
> +           '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'],
> +           '*backing-image': 'ImageInfo' } }

The API change looks fine, except there is no code change to actually
populate the new field.  This hunk should probably be squashed with the
patch that implements the field.  Also, are you missing any changes to
qmp-commands.hx?

-- 
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 V13 3/6] qmp: add recursive member in ImageInfo

[Wenchao Xia]

于 2013-5-26 0:10, Eric Blake 写道:
> On 05/24/2013 10:24 PM, Wenchao Xia wrote:
>> New member *backing-image is added to reflect the backing chain
>> status.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   block/qapi.c     |   16 +++++++++++++++-
>>   qapi-schema.json |    5 ++++-
>>   2 files changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/qapi.c b/block/qapi.c
>> index 680ec23..cbef584 100644
>> --- a/block/qapi.c
>> +++ b/block/qapi.c
>> @@ -88,7 +88,21 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>>       return 0;
>>   }
>>
>> -/* @p_info will be set only on success. */
>> +/**
>> + * bdrv_query_image_info:
>> + * @bs: block device to examine
>> + * @p_info: location to store image information
>> + * @errp: location to store error information
>> + *
>> + * Store "flat" image inforation in @p_info.
>
> s/inforation/information/
>
>> + *
>> + * "Flat" means it does *not* query backing image information,
>> + * i.e. (*pinfo)->has_backing_image will be set to false and
>> + * (*pinfo)->backing_image to NULL even when the image does in fact have
>> + * a backing image.
>> + *
>> + * @p_info will be set only on success. On error, store error in @errp.
>> + */
>
> Does this comment hunk belong in the previous patch?
>
   Yes, it got modified again in this one, will move it.

>>   void bdrv_query_image_info(BlockDriverState *bs,
>>                              ImageInfo **p_info,
>>                              Error **errp)
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index ef1f657..a02999d 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -236,6 +236,8 @@
>>   #
>>   # @snapshots: #optional list of VM snapshots
>>   #
>> +# @backing-image: #optional info of the backing image (since 1.6)
>> +#
>>   # Since: 1.3
>>   #
>>   ##
>> @@ -245,7 +247,8 @@
>>              '*actual-size': 'int', 'virtual-size': 'int',
>>              '*cluster-size': 'int', '*encrypted': 'bool',
>>              '*backing-filename': 'str', '*full-backing-filename': 'str',
>> -           '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'] } }
>> +           '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'],
>> +           '*backing-image': 'ImageInfo' } }
>
> The API change looks fine, except there is no code change to actually
> populate the new field.  This hunk should probably be squashed with the
> patch that implements the field.  Also, are you missing any changes to
> qmp-commands.hx?
>
   nop, in next patch qmp-commands.hx parts is added. Just to make
review easier, after that I am fine to squash them.



-- 
Best Regards

Wenchao Xia

Re: [Qemu-devel] [PATCH V13 3/6] qmp: add recursive member in ImageInfo

[Stefan Hajnoczi]

On Mon, May 27, 2013 at 09:28:59AM +0800, Wenchao Xia wrote:
> 于 2013-5-26 0:10, Eric Blake 写道:
> >On 05/24/2013 10:24 PM, Wenchao Xia wrote:
> >>  void bdrv_query_image_info(BlockDriverState *bs,
> >>                             ImageInfo **p_info,
> >>                             Error **errp)
> >>diff --git a/qapi-schema.json b/qapi-schema.json
> >>index ef1f657..a02999d 100644
> >>--- a/qapi-schema.json
> >>+++ b/qapi-schema.json
> >>@@ -236,6 +236,8 @@
> >>  #
> >>  # @snapshots: #optional list of VM snapshots
> >>  #
> >>+# @backing-image: #optional info of the backing image (since 1.6)
> >>+#
> >>  # Since: 1.3
> >>  #
> >>  ##
> >>@@ -245,7 +247,8 @@
> >>             '*actual-size': 'int', 'virtual-size': 'int',
> >>             '*cluster-size': 'int', '*encrypted': 'bool',
> >>             '*backing-filename': 'str', '*full-backing-filename': 'str',
> >>-           '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'] } }
> >>+           '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'],
> >>+           '*backing-image': 'ImageInfo' } }
> >
> >The API change looks fine, except there is no code change to actually
> >populate the new field.  This hunk should probably be squashed with the
> >patch that implements the field.  Also, are you missing any changes to
> >qmp-commands.hx?
> >
>   nop, in next patch qmp-commands.hx parts is added. Just to make
> review easier, after that I am fine to squash them.

The qapi change should be together with the code that implements it.  I
need to see the code in order to review the documentation change.

Stefan

Re: [Qemu-devel] [PATCH V13 6/6] hmp: add parameters device and -v for info block

[Stefan Hajnoczi]

On Sat, May 25, 2013 at 12:24:46PM +0800, Wenchao Xia wrote:
> diff --git a/hmp.c b/hmp.c
> index 2aa832c..a590ace 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -279,10 +279,15 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
>  {
>      BlockInfoList *block_list, *info;
>      ImageInfo *image_info;
> +    const char *device = qdict_get_try_str(qdict, "device");
> +    int verbose = qdict_get_try_bool(qdict, "verbose", 0);

bool verbose

Re: [Qemu-devel] [PATCH V13 0/6] enhancement for qmp/hmp interfaces of block info

[Stefan Hajnoczi]

On Sat, May 25, 2013 at 12:24:40PM +0800, Wenchao Xia wrote:
>   This serial let qmp interface show delaied info, including internal snapshot
> /backing chain on all block device at runtime, which helps management stack and
> human user, by retrieving exactly the same info of what qemu saws.
> 
> Example:
> -> { "execute": "query-block" }
> <- {
>       "return":[
>          {
>             "io-status": "ok",
>             "device":"ide0-hd0",
>             "locked":false,
>             "removable":false,
>             "inserted":{
>                "ro":false,
>                "drv":"qcow2",
>                "encrypted":false,
>                "file":"disks/test.qcow2",
>                "backing_file_depth":1,
>                "bps":1000000,
>                "bps_rd":0,
>                "bps_wr":0,
>                "iops":1000000,
>                "iops_rd":0,
>                "iops_wr":0,
>                "image":{
>                   "filename":"disks/test.qcow2",
>                   "format":"qcow2",
>                   "virtual-size":2048000,
>                   "backing_file":"base.qcow2",
>                   "full-backing-filename":"disks/base.qcow2",
>                   "backing-filename-format:"qcow2",
>                   "snapshots":[
>                      {
>                         "id": "1",
>                         "name": "snapshot1",
>                         "vm-state-size": 0,
>                         "date-sec": 10000200,
>                         "date-nsec": 12,
>                         "vm-clock-sec": 206,
>                         "vm-clock-nsec": 30
>                      }
>                   ],
>                   "backing-image":{
>                       "filename":"disks/base.qcow2",
>                       "format":"qcow2",
>                       "virtual-size":2048000
>                   }
>                }
>             },
>             "type":"unknown"
>          },
>          {
>             "io-status": "ok",
>             "device":"ide1-cd0",
>             "locked":false,
>             "removable":true,
>             "type":"unknown"
>          },
>          {
>             "device":"floppy0",
>             "locked":false,
>             "removable":true,
>             "type":"unknown"
>          },
>          {
>             "device":"sd0",
>             "locked":false,
>             "removable":true,
>             "type":"unknown"
>          }
>       ]
>    }
> 
>   These patches follows the rule that use qmp to retieve information,
> hmp layer just does a translation from qmp object it got. To make code
> graceful, snapshot and image info retrieving code in qemu and qemu-img are
> merged into block layer, and some function name was adjusted to make it tips
> better. For the part touch by the serial, it works as:
> 
>    qemu          qemu-img
> 
> dump_monitor    dump_stdout
>      |--------------| 
>             |
>        block/qapi.c
> 
>   Special thanks for Markus, Stefan, Kevin, Eric reviewing many times.
> 
> v13:
>   Renamed the serial as "enhancement for qmp/hmp interfaces of block info".
>   Seperated the common part of code moving and hmp printf as a standalone
> serial, which can be used by both mine and Pavel's work. This serial depend
> on it: "[PATCH V3 0/4] qapi and snapshot code clean up in block layer",
> https://lists.gnu.org/archive/html/qemu-devel/2013-05/msg03539.html
>   Removed the VM snapshot info part, since it relate to VM snapshot creating
> logic, which should be changed together with Pavel's serial.
>   Address Eric's comments:
>   2/6: bdrv_query_image_info() returns void now, only use *errp to tip error.
> 
> Wenchao Xia (6):
>   1 block: add snapshot info query function bdrv_query_snapshot_info_list()
>   2 block: add image info query function bdrv_query_image_info()
>   3 qmp: add recursive member in ImageInfo
>   4 qmp: add ImageInfo in BlockDeviceInfo used by query-block
>   5 hmp: show ImageInfo in 'info block'
>   6 hmp: add parameters device and -v for info block
> 
>  block/qapi.c         |  148 ++++++++++++++++++++++++++++++++++++++++++--------
>  hmp.c                |   21 +++++++
>  include/block/qapi.h |   14 +++--
>  monitor.c            |    7 ++-
>  qapi-schema.json     |   10 +++-
>  qemu-img.c           |   10 +++-
>  qmp-commands.hx      |   69 +++++++++++++++++++++++-
>  7 files changed, 242 insertions(+), 37 deletions(-)

I left comments but overall this looks very close to merge now.