Patchwork [V11,10/17] qmp: add recursive member in ImageInfo

login
register
mail settings
Submitter Wayne Xia
Date April 2, 2013, 11:47 a.m.
Message ID <1364903250-10429-11-git-send-email-xiawenc@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/232977/
State New
Headers show

Comments

Wayne Xia - April 2, 2013, 11:47 a.m.
New member *backing-image is added to reflect the backing chain
status.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/qapi.c     |    6 +++++-
 qapi-schema.json |    5 ++++-
 2 files changed, 9 insertions(+), 2 deletions(-)
Markus Armbruster - April 10, 2013, 4:06 p.m.
Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:

>   New member *backing-image is added to reflect the backing chain
> status.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  block/qapi.c     |    6 +++++-
>  qapi-schema.json |    5 ++++-
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/block/qapi.c b/block/qapi.c
> index 5e91ab8..fa61c85 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -123,7 +123,11 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>      return 0;
>  }
>  
> -/* return 0 on success, and @p_info will be set only on success. */
> +/*
> + * return 0 on success, and @p_info will be set only on success,
> + * (*pinfo)->has_backing_image will be false and (*pinfo)->backing_image will
> + * be NULL.
> + */

Sounds like this function computes incomplete ImageInfo.  Correct?  If
yes, why?

>  int bdrv_query_image_info(BlockDriverState *bs,
>                            ImageInfo **p_info,
>                            Error **errp)
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 225afef..ad9dd82 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -233,6 +233,8 @@
>  #
>  # @snapshots: #optional list of VM snapshots
>  #
> +# @backing-image: #optional info of the backing image (since 1.5)
> +#
>  # Since: 1.3
>  #
>  ##
> @@ -242,7 +244,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:
Wayne Xia - April 11, 2013, 6:06 a.m.
于 2013-4-11 0:06, Markus Armbruster 写道:
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:
> 
>>    New member *backing-image is added to reflect the backing chain
>> status.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>>   block/qapi.c     |    6 +++++-
>>   qapi-schema.json |    5 ++++-
>>   2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/qapi.c b/block/qapi.c
>> index 5e91ab8..fa61c85 100644
>> --- a/block/qapi.c
>> +++ b/block/qapi.c
>> @@ -123,7 +123,11 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>>       return 0;
>>   }
>>   
>> -/* return 0 on success, and @p_info will be set only on success. */
>> +/*
>> + * return 0 on success, and @p_info will be set only on success,
>> + * (*pinfo)->has_backing_image will be false and (*pinfo)->backing_image will
>> + * be NULL.
>> + */
> 
> Sounds like this function computes incomplete ImageInfo.  Correct?  If
> yes, why?
> 
  yes, qemu-img will use it to get info of an image that may be broken
in backing file chain(can't get backing file's info).

>>   int bdrv_query_image_info(BlockDriverState *bs,
>>                             ImageInfo **p_info,
>>                             Error **errp)
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 225afef..ad9dd82 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -233,6 +233,8 @@
>>   #
>>   # @snapshots: #optional list of VM snapshots
>>   #
>> +# @backing-image: #optional info of the backing image (since 1.5)
>> +#
>>   # Since: 1.3
>>   #
>>   ##
>> @@ -242,7 +244,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:
>
Markus Armbruster - April 11, 2013, 9:28 a.m.
Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:

> 于 2013-4-11 0:06, Markus Armbruster 写道:
>> Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:
>> 
>>>    New member *backing-image is added to reflect the backing chain
>>> status.
>>>
>>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>>> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>> ---
>>>   block/qapi.c     |    6 +++++-
>>>   qapi-schema.json |    5 ++++-
>>>   2 files changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/block/qapi.c b/block/qapi.c
>>> index 5e91ab8..fa61c85 100644
>>> --- a/block/qapi.c
>>> +++ b/block/qapi.c
>>> @@ -123,7 +123,11 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>>>       return 0;
>>>   }
>>>   
>>> -/* return 0 on success, and @p_info will be set only on success. */
>>> +/*
>>> + * return 0 on success, and @p_info will be set only on success,
>>> + * (*pinfo)->has_backing_image will be false and (*pinfo)->backing_image will
>>> + * be NULL.
>>> + */
>> 
>> Sounds like this function computes incomplete ImageInfo.  Correct?  If
>> yes, why?
>> 
>   yes, qemu-img will use it to get info of an image that may be broken
> in backing file chain(can't get backing file's info).

Ah, that makes sense.

Suggest:

/**
 * 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.
 *
 * On error, store error in @errp.
 *
 * Returns: 0 on success, -errno on error.
 */

>>>   int bdrv_query_image_info(BlockDriverState *bs,
>>>                             ImageInfo **p_info,
>>>                             Error **errp)
>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>> index 225afef..ad9dd82 100644
>>> --- a/qapi-schema.json
>>> +++ b/qapi-schema.json
>>> @@ -233,6 +233,8 @@
>>>   #
>>>   # @snapshots: #optional list of VM snapshots
>>>   #
>>> +# @backing-image: #optional info of the backing image (since 1.5)
>>> +#
>>>   # Since: 1.3
>>>   #
>>>   ##
>>> @@ -242,7 +244,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:
>>

Patch

diff --git a/block/qapi.c b/block/qapi.c
index 5e91ab8..fa61c85 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -123,7 +123,11 @@  int bdrv_query_snapshot_info_list(BlockDriverState *bs,
     return 0;
 }
 
-/* return 0 on success, and @p_info will be set only on success. */
+/*
+ * return 0 on success, and @p_info will be set only on success,
+ * (*pinfo)->has_backing_image will be false and (*pinfo)->backing_image will
+ * be NULL.
+ */
 int bdrv_query_image_info(BlockDriverState *bs,
                           ImageInfo **p_info,
                           Error **errp)
diff --git a/qapi-schema.json b/qapi-schema.json
index 225afef..ad9dd82 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -233,6 +233,8 @@ 
 #
 # @snapshots: #optional list of VM snapshots
 #
+# @backing-image: #optional info of the backing image (since 1.5)
+#
 # Since: 1.3
 #
 ##
@@ -242,7 +244,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: