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 <xiaw...@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