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 <address@hidden>
> > ---
> >   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