Re: [Qemu-devel] [PATCH V12 08/18] block: add image info query function bdrv_query_image_info()

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]

On 04/13/2013 02:56 AM, Wenchao Xia wrote:
>   This patch adds function bdrv_query_image_info(), which will
> retrieve image info in qmp object format. The implementation is
> based on the code moved from qemu-img.c, but uses block layer
> function to get snapshot info.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  block/qapi.c         |   37 +++++++++++++++++++++++++++++++------
>  include/block/qapi.h |    6 +++---
>  qemu-img.c           |    7 ++-----
>  3 files changed, 36 insertions(+), 14 deletions(-)
> 

> +/* return 0 on success, and @p_info will be set only on success. */
> +int bdrv_query_image_info(BlockDriverState *bs,
> +                          ImageInfo **p_info,
> +                          Error **errp)

Is it necessary to return a value?  If all callers pass in an errp, then
this function can be void, and you can use the error parameter as the
lone indication of problems.

> +++ b/qemu-img.c
> @@ -1733,11 +1733,8 @@ static ImageInfoList *collect_image_info_list(const char *filename,
>              goto err;
>          }
>  
> -        info = g_new0(ImageInfo, 1);
> -        bdrv_collect_image_info(bs, info, filename);
> -        bdrv_query_snapshot_info_list(bs, &info->snapshots, false, NULL);
> -        if (info->snapshots) {
> -            info->has_snapshots = true;
> +        if (bdrv_query_image_info(bs, &info, NULL)) {
> +            goto err;

But then this caller would need to pass in a local error parameter,
where Pavel's idea of qemu_img_handle_error would make it easier.
https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg03288.html

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]