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

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 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