[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V11 07/17] block: add image info query function
From: |
Wenchao Xia |
Subject: |
Re: [Qemu-devel] [PATCH V11 07/17] block: add image info query function bdrv_query_image_info() |
Date: |
Thu, 11 Apr 2013 13:59:26 +0800 |
User-agent: |
Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/20130328 Thunderbird/17.0.5 |
δΊ 2013-4-10 23:28, Markus Armbruster ει:
> Wenchao Xia <address@hidden> writes:
>
>> 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 <address@hidden>
>> ---
>> block/qapi.c | 41 ++++++++++++++++++++++++++++++++++-------
>> include/block/qapi.h | 6 +++---
>> qemu-img.c | 8 ++------
>> 3 files changed, 39 insertions(+), 16 deletions(-)
>>
>> diff --git a/block/qapi.c b/block/qapi.c
>> index 19d4d93..176a479 100644
>> --- a/block/qapi.c
>> +++ b/block/qapi.c
>> @@ -122,18 +122,22 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>> return 0;
>> }
>>
>> -void bdrv_collect_image_info(BlockDriverState *bs,
>> - ImageInfo *info,
>> - const char *filename)
>> +/* 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)
>> {
>> uint64_t total_sectors;
>> - char backing_filename[1024];
>> + const char *backing_filename;
>> char backing_filename2[1024];
>> BlockDriverInfo bdi;
>> + int ret;
>> + Error *err = NULL;
>> + ImageInfo *info = g_new0(ImageInfo, 1);
>>
>> bdrv_get_geometry(bs, &total_sectors);
>>
>> - info->filename = g_strdup(filename);
>> + info->filename = g_strdup(bs->filename);
>> info->format = g_strdup(bdrv_get_format_name(bs));
>> info->virtual_size = total_sectors * 512;
>> info->actual_size = bdrv_get_allocated_file_size(bs);
>> @@ -150,8 +154,8 @@ void bdrv_collect_image_info(BlockDriverState *bs,
>> info->dirty_flag = bdi.is_dirty;
>> info->has_dirty_flag = true;
>> }
>> - bdrv_get_backing_filename(bs, backing_filename,
>> sizeof(backing_filename));
>> - if (backing_filename[0] != '\0') {
>> + backing_filename = bs->backing_file;
>> + if (backing_filename && backing_filename[0] != '\0') {
>
> backing_filename can't possibly be null here, because bs->backing_file
> is an array.
>
OK, will remove it.
>> info->backing_filename = g_strdup(backing_filename);
>> info->has_backing_filename = true;
>> bdrv_get_full_backing_filename(bs, backing_filename2,
>> @@ -168,4 +172,27 @@ void bdrv_collect_image_info(BlockDriverState *bs,
>> info->has_backing_filename_format = true;
>> }
>> }
>> +
>> + ret = bdrv_query_snapshot_info_list(bs, &info->snapshots, false, &err);
>> + switch (ret) {
>> + case 0:
>> + if (info->snapshots) {
>> + info->has_snapshots = true;
>> + }
>> + break;
>> + /* recoverable error */
>> + case -ENOMEDIUM:
>> + error_free(err);
>> + break;
>> + case -ENOTSUP:
>> + error_free(err);
>> + break;
>
> Suggest
>
> case -ENOMEDIUM:
> case -ENOTSUP:
> /* no snapshots on this device */
> error_free(err);
> break;
>
> for clarity.
>
OK.
>> + default:
>> + error_propagate(errp, err);
>> + qapi_free_ImageInfo(info);
>> + return ret;
>> + }
>> +
>> + *p_info = info;
>> + return 0;
>> }
>> diff --git a/include/block/qapi.h b/include/block/qapi.h
>> index fe66053..2c62fdf 100644
>> --- a/include/block/qapi.h
>> +++ b/include/block/qapi.h
>> @@ -32,7 +32,7 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>> SnapshotInfoList **p_list,
>> bool vm_snapshot,
>> Error **errp);
>> -void bdrv_collect_image_info(BlockDriverState *bs,
>> - ImageInfo *info,
>> - const char *filename);
>> +int bdrv_query_image_info(BlockDriverState *bs,
>> + ImageInfo **p_info,
>> + Error **errp);
>> #endif
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 261c277..1dd0a60 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -1733,12 +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);
>> - if (!bdrv_query_snapshot_info_list(bs, &info->snapshots,
>> - false, NULL) &&
>> - info->snapshots) {
>> - info->has_snapshots = true;
>> + if (bdrv_query_image_info(bs, &info, NULL)) {
>> + goto err;
>> }
>>
>> elem = g_new0(ImageInfoList, 1);
>
--
Best Regards
Wenchao Xia
[Qemu-devel] [PATCH V11 11/17] qmp: add ImageInfo in BlockDeviceInfo used by query-block, Wenchao Xia, 2013/04/02