| Message ID | 1369455886-30677-4-git-send-email-xiawenc@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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?
于 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.
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
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:
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(-)