* [Qemu-devel] [PATCH v2 1/6] qapi: Add ImageInfoSpecific type
2013-09-06 13:12 [Qemu-devel] [PATCH v2 0/6] Provide additional info through qemu-img info Max Reitz
@ 2013-09-06 13:12 ` Max Reitz
2013-09-06 13:12 ` [Qemu-devel] [PATCH v2 2/6] block: Add ImageInfoSpecific to BlockDriverInfo Max Reitz
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Max Reitz @ 2013-09-06 13:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add a new type ImageInfoSpecific as a union for image format specific
information in ImageInfo.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qapi-schema.json | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index a51f7d2..eebf851 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -210,6 +210,18 @@
'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }
##
+# @ImageInfoSpecific:
+#
+# A discriminated record of image format specific information structures.
+#
+# Since: 1.7
+##
+
+{ 'union': 'ImageInfoSpecific',
+ 'data': {
+ } }
+
+##
# @ImageInfo:
#
# Information about a QEMU image file
@@ -238,6 +250,9 @@
#
# @backing-image: #optional info of the backing image (since 1.6)
#
+# @info-string: #optional string supplying additional format-specific
+# information (since 1.7)
+#
# Since: 1.3
#
##
@@ -248,7 +263,8 @@
'*cluster-size': 'int', '*encrypted': 'bool',
'*backing-filename': 'str', '*full-backing-filename': 'str',
'*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'],
- '*backing-image': 'ImageInfo' } }
+ '*backing-image': 'ImageInfo',
+ '*format-specific': 'ImageInfoSpecific' } }
##
# @ImageCheck:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH v2 2/6] block: Add ImageInfoSpecific to BlockDriverInfo
2013-09-06 13:12 [Qemu-devel] [PATCH v2 0/6] Provide additional info through qemu-img info Max Reitz
2013-09-06 13:12 ` [Qemu-devel] [PATCH v2 1/6] qapi: Add ImageInfoSpecific type Max Reitz
@ 2013-09-06 13:12 ` Max Reitz
2013-09-10 3:26 ` Fam Zheng
2013-09-06 13:12 ` [Qemu-devel] [PATCH v2 3/6] block/qapi: Human-readable ImageInfoSpecific dump Max Reitz
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Max Reitz @ 2013-09-06 13:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add the new ImageInfoSpecific type also to BlockDriverInfo.
To prevent memory leaks, this field has to be initialized to NULL every
time before calling bdrv_get_info and qapi_free_ImageInfoSpecific has to
be called on it when the BlockDriverInfo object is no longer required.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 3 ++-
block/mirror.c | 6 ++++--
block/qapi.c | 6 +++++-
include/block/block.h | 2 ++
qemu-img.c | 3 ++-
qemu-io-cmds.c | 6 +++++-
6 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/block.c b/block.c
index 26639e8..1a5d2a4 100644
--- a/block.c
+++ b/block.c
@@ -1921,7 +1921,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
int64_t *cluster_sector_num,
int *cluster_nb_sectors)
{
- BlockDriverInfo bdi;
+ BlockDriverInfo bdi = { .format_specific = NULL };
if (bdrv_get_info(bs, &bdi) < 0 || bdi.cluster_size == 0) {
*cluster_sector_num = sector_num;
@@ -1932,6 +1932,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
*cluster_nb_sectors = QEMU_ALIGN_UP(sector_num - *cluster_sector_num +
nb_sectors, c);
}
+ qapi_free_ImageInfoSpecific(bdi.format_specific);
}
static bool tracked_request_overlaps(BdrvTrackedRequest *req,
diff --git a/block/mirror.c b/block/mirror.c
index 86de458..cfef7e9 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -295,7 +295,7 @@ static void coroutine_fn mirror_run(void *opaque)
BlockDriverState *bs = s->common.bs;
int64_t sector_num, end, sectors_per_chunk, length;
uint64_t last_pause_ns;
- BlockDriverInfo bdi;
+ BlockDriverInfo bdi = { .format_specific = NULL };
char backing_filename[1024];
int ret = 0;
int n;
@@ -325,6 +325,7 @@ static void coroutine_fn mirror_run(void *opaque)
s->buf_size = MAX(s->buf_size, bdi.cluster_size);
s->cow_bitmap = bitmap_new(length);
}
+ qapi_free_ImageInfoSpecific(bdi.format_specific);
}
end = s->common.len >> BDRV_SECTOR_BITS;
@@ -544,13 +545,14 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
if (granularity == 0) {
/* Choose the default granularity based on the target file's cluster
* size, clamped between 4k and 64k. */
- BlockDriverInfo bdi;
+ BlockDriverInfo bdi = { .format_specific = NULL };
if (bdrv_get_info(target, &bdi) >= 0 && bdi.cluster_size != 0) {
granularity = MAX(4096, bdi.cluster_size);
granularity = MIN(65536, granularity);
} else {
granularity = 65536;
}
+ qapi_free_ImageInfoSpecific(bdi.format_specific);
}
assert ((granularity & (granularity - 1)) == 0);
diff --git a/block/qapi.c b/block/qapi.c
index a4bc411..f13fbd5 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -110,7 +110,7 @@ void bdrv_query_image_info(BlockDriverState *bs,
uint64_t total_sectors;
const char *backing_filename;
char backing_filename2[1024];
- BlockDriverInfo bdi;
+ BlockDriverInfo bdi = { .format_specific = NULL };
int ret;
Error *err = NULL;
ImageInfo *info = g_new0(ImageInfo, 1);
@@ -133,6 +133,10 @@ void bdrv_query_image_info(BlockDriverState *bs,
}
info->dirty_flag = bdi.is_dirty;
info->has_dirty_flag = true;
+ if (bdi.format_specific) {
+ info->format_specific = bdi.format_specific;
+ info->has_format_specific = true;
+ }
}
backing_filename = bs->backing_file;
if (backing_filename[0] != '\0') {
diff --git a/include/block/block.h b/include/block/block.h
index e6b391c..85e9703 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -18,6 +18,8 @@ typedef struct BlockDriverInfo {
/* offset at which the VM state can be saved (0 if not possible) */
int64_t vm_state_offset;
bool is_dirty;
+ /* additional information; NULL if none */
+ ImageInfoSpecific *format_specific;
} BlockDriverInfo;
typedef struct BlockFragInfo {
diff --git a/qemu-img.c b/qemu-img.c
index b9a848d..ec1ecca 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1125,7 +1125,7 @@ static int img_convert(int argc, char **argv)
uint64_t bs_sectors;
uint8_t * buf = NULL;
const uint8_t *buf1;
- BlockDriverInfo bdi;
+ BlockDriverInfo bdi = { .format_specific = NULL };
QEMUOptionParameter *param = NULL, *create_options = NULL;
QEMUOptionParameter *out_baseimg_param;
char *options = NULL;
@@ -1369,6 +1369,7 @@ static int img_convert(int argc, char **argv)
error_report("could not get block driver info");
goto out;
}
+ qapi_free_ImageInfoSpecific(bdi.format_specific);
cluster_size = bdi.cluster_size;
if (cluster_size <= 0 || cluster_size > IO_BUF_SIZE) {
error_report("invalid cluster size");
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index f91b6c4..563dd40 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1677,7 +1677,7 @@ static const cmdinfo_t length_cmd = {
static int info_f(BlockDriverState *bs, int argc, char **argv)
{
- BlockDriverInfo bdi;
+ BlockDriverInfo bdi = { .format_specific = NULL };
char s1[64], s2[64];
int ret;
@@ -1699,6 +1699,10 @@ static int info_f(BlockDriverState *bs, int argc, char **argv)
printf("cluster size: %s\n", s1);
printf("vm state offset: %s\n", s2);
+ if (bdi.format_specific) {
+ qapi_free_ImageInfoSpecific(bdi.format_specific);
+ }
+
return 0;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/6] block: Add ImageInfoSpecific to BlockDriverInfo
2013-09-06 13:12 ` [Qemu-devel] [PATCH v2 2/6] block: Add ImageInfoSpecific to BlockDriverInfo Max Reitz
@ 2013-09-10 3:26 ` Fam Zheng
2013-09-10 7:22 ` Max Reitz
0 siblings, 1 reply; 15+ messages in thread
From: Fam Zheng @ 2013-09-10 3:26 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
On Fri, 09/06 15:12, Max Reitz wrote:
> Add the new ImageInfoSpecific type also to BlockDriverInfo.
>
> To prevent memory leaks, this field has to be initialized to NULL every
> time before calling bdrv_get_info and qapi_free_ImageInfoSpecific has to
> be called on it when the BlockDriverInfo object is no longer required.
>
I don't understand here. I think bdi is always passed into bdrv_get_info()
uninitialized and in bdrv_get_info() there is:
memset(bdi, 0, sizeof(*bdi));
before passing it on to driver, so it's always set to NULL.
And why pointer, not a member to save a free() call?
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block.c | 3 ++-
> block/mirror.c | 6 ++++--
> block/qapi.c | 6 +++++-
> include/block/block.h | 2 ++
> qemu-img.c | 3 ++-
> qemu-io-cmds.c | 6 +++++-
> 6 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/block.c b/block.c
> index 26639e8..1a5d2a4 100644
> --- a/block.c
> +++ b/block.c
> @@ -1921,7 +1921,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
> int64_t *cluster_sector_num,
> int *cluster_nb_sectors)
> {
> - BlockDriverInfo bdi;
> + BlockDriverInfo bdi = { .format_specific = NULL };
>
> if (bdrv_get_info(bs, &bdi) < 0 || bdi.cluster_size == 0) {
> *cluster_sector_num = sector_num;
> @@ -1932,6 +1932,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
> *cluster_nb_sectors = QEMU_ALIGN_UP(sector_num - *cluster_sector_num +
> nb_sectors, c);
> }
> + qapi_free_ImageInfoSpecific(bdi.format_specific);
> }
>
> static bool tracked_request_overlaps(BdrvTrackedRequest *req,
> diff --git a/block/mirror.c b/block/mirror.c
> index 86de458..cfef7e9 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -295,7 +295,7 @@ static void coroutine_fn mirror_run(void *opaque)
> BlockDriverState *bs = s->common.bs;
> int64_t sector_num, end, sectors_per_chunk, length;
> uint64_t last_pause_ns;
> - BlockDriverInfo bdi;
> + BlockDriverInfo bdi = { .format_specific = NULL };
> char backing_filename[1024];
> int ret = 0;
> int n;
> @@ -325,6 +325,7 @@ static void coroutine_fn mirror_run(void *opaque)
> s->buf_size = MAX(s->buf_size, bdi.cluster_size);
> s->cow_bitmap = bitmap_new(length);
> }
> + qapi_free_ImageInfoSpecific(bdi.format_specific);
> }
>
> end = s->common.len >> BDRV_SECTOR_BITS;
> @@ -544,13 +545,14 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
> if (granularity == 0) {
> /* Choose the default granularity based on the target file's cluster
> * size, clamped between 4k and 64k. */
> - BlockDriverInfo bdi;
> + BlockDriverInfo bdi = { .format_specific = NULL };
> if (bdrv_get_info(target, &bdi) >= 0 && bdi.cluster_size != 0) {
> granularity = MAX(4096, bdi.cluster_size);
> granularity = MIN(65536, granularity);
> } else {
> granularity = 65536;
> }
> + qapi_free_ImageInfoSpecific(bdi.format_specific);
> }
>
> assert ((granularity & (granularity - 1)) == 0);
> diff --git a/block/qapi.c b/block/qapi.c
> index a4bc411..f13fbd5 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -110,7 +110,7 @@ void bdrv_query_image_info(BlockDriverState *bs,
> uint64_t total_sectors;
> const char *backing_filename;
> char backing_filename2[1024];
> - BlockDriverInfo bdi;
> + BlockDriverInfo bdi = { .format_specific = NULL };
> int ret;
> Error *err = NULL;
> ImageInfo *info = g_new0(ImageInfo, 1);
> @@ -133,6 +133,10 @@ void bdrv_query_image_info(BlockDriverState *bs,
> }
> info->dirty_flag = bdi.is_dirty;
> info->has_dirty_flag = true;
> + if (bdi.format_specific) {
> + info->format_specific = bdi.format_specific;
> + info->has_format_specific = true;
> + }
> }
> backing_filename = bs->backing_file;
> if (backing_filename[0] != '\0') {
> diff --git a/include/block/block.h b/include/block/block.h
> index e6b391c..85e9703 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -18,6 +18,8 @@ typedef struct BlockDriverInfo {
> /* offset at which the VM state can be saved (0 if not possible) */
> int64_t vm_state_offset;
> bool is_dirty;
> + /* additional information; NULL if none */
> + ImageInfoSpecific *format_specific;
> } BlockDriverInfo;
>
> typedef struct BlockFragInfo {
> diff --git a/qemu-img.c b/qemu-img.c
> index b9a848d..ec1ecca 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1125,7 +1125,7 @@ static int img_convert(int argc, char **argv)
> uint64_t bs_sectors;
> uint8_t * buf = NULL;
> const uint8_t *buf1;
> - BlockDriverInfo bdi;
> + BlockDriverInfo bdi = { .format_specific = NULL };
> QEMUOptionParameter *param = NULL, *create_options = NULL;
> QEMUOptionParameter *out_baseimg_param;
> char *options = NULL;
> @@ -1369,6 +1369,7 @@ static int img_convert(int argc, char **argv)
> error_report("could not get block driver info");
> goto out;
> }
> + qapi_free_ImageInfoSpecific(bdi.format_specific);
> cluster_size = bdi.cluster_size;
> if (cluster_size <= 0 || cluster_size > IO_BUF_SIZE) {
> error_report("invalid cluster size");
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index f91b6c4..563dd40 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -1677,7 +1677,7 @@ static const cmdinfo_t length_cmd = {
>
> static int info_f(BlockDriverState *bs, int argc, char **argv)
> {
> - BlockDriverInfo bdi;
> + BlockDriverInfo bdi = { .format_specific = NULL };
> char s1[64], s2[64];
> int ret;
>
> @@ -1699,6 +1699,10 @@ static int info_f(BlockDriverState *bs, int argc, char **argv)
> printf("cluster size: %s\n", s1);
> printf("vm state offset: %s\n", s2);
>
> + if (bdi.format_specific) {
> + qapi_free_ImageInfoSpecific(bdi.format_specific);
> + }
> +
> return 0;
> }
>
> --
> 1.8.3.1
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/6] block: Add ImageInfoSpecific to BlockDriverInfo
2013-09-10 3:26 ` Fam Zheng
@ 2013-09-10 7:22 ` Max Reitz
2013-09-10 7:37 ` Fam Zheng
0 siblings, 1 reply; 15+ messages in thread
From: Max Reitz @ 2013-09-10 7:22 UTC (permalink / raw)
To: famz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
On 2013-09-10 05:26, Fam Zheng wrote:
> On Fri, 09/06 15:12, Max Reitz wrote:
>> Add the new ImageInfoSpecific type also to BlockDriverInfo.
>>
>> To prevent memory leaks, this field has to be initialized to NULL every
>> time before calling bdrv_get_info and qapi_free_ImageInfoSpecific has to
>> be called on it when the BlockDriverInfo object is no longer required.
>>
> I don't understand here. I think bdi is always passed into bdrv_get_info()
> uninitialized and in bdrv_get_info() there is:
>
> memset(bdi, 0, sizeof(*bdi));
>
> before passing it on to driver, so it's always set to NULL.
Oh, you're right, I missed that. Thanks!
> And why pointer, not a member to save a free() call?
As far as I understand it (and if I don't miss anything again),
ImageInfoSpecific is a auto-generated QAPI type, so it may contain
pointers to other types anyway (as it does in the case of QCow2, which
is the only driver where I have implemented this new field at all; in
that case, the compatiblity level is a string), therefore we always need
some function to clean up the data referenced by ImageInfoSpecific;
qapi_free_ImageInfoSpecific is the perfect one for this, but it takes a
heap pointer.
Hence, I think using a pointer (to a heap-allocated object) is easier in
this case, since the QAPI clean-up function assumes this case.
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> block.c | 3 ++-
>> block/mirror.c | 6 ++++--
>> block/qapi.c | 6 +++++-
>> include/block/block.h | 2 ++
>> qemu-img.c | 3 ++-
>> qemu-io-cmds.c | 6 +++++-
>> 6 files changed, 20 insertions(+), 6 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 26639e8..1a5d2a4 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -1921,7 +1921,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
>> int64_t *cluster_sector_num,
>> int *cluster_nb_sectors)
>> {
>> - BlockDriverInfo bdi;
>> + BlockDriverInfo bdi = { .format_specific = NULL };
>>
>> if (bdrv_get_info(bs, &bdi) < 0 || bdi.cluster_size == 0) {
>> *cluster_sector_num = sector_num;
>> @@ -1932,6 +1932,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
>> *cluster_nb_sectors = QEMU_ALIGN_UP(sector_num - *cluster_sector_num +
>> nb_sectors, c);
>> }
>> + qapi_free_ImageInfoSpecific(bdi.format_specific);
>> }
>>
>> static bool tracked_request_overlaps(BdrvTrackedRequest *req,
>> diff --git a/block/mirror.c b/block/mirror.c
>> index 86de458..cfef7e9 100644
>> --- a/block/mirror.c
>> +++ b/block/mirror.c
>> @@ -295,7 +295,7 @@ static void coroutine_fn mirror_run(void *opaque)
>> BlockDriverState *bs = s->common.bs;
>> int64_t sector_num, end, sectors_per_chunk, length;
>> uint64_t last_pause_ns;
>> - BlockDriverInfo bdi;
>> + BlockDriverInfo bdi = { .format_specific = NULL };
>> char backing_filename[1024];
>> int ret = 0;
>> int n;
>> @@ -325,6 +325,7 @@ static void coroutine_fn mirror_run(void *opaque)
>> s->buf_size = MAX(s->buf_size, bdi.cluster_size);
>> s->cow_bitmap = bitmap_new(length);
>> }
>> + qapi_free_ImageInfoSpecific(bdi.format_specific);
>> }
>>
>> end = s->common.len >> BDRV_SECTOR_BITS;
>> @@ -544,13 +545,14 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
>> if (granularity == 0) {
>> /* Choose the default granularity based on the target file's cluster
>> * size, clamped between 4k and 64k. */
>> - BlockDriverInfo bdi;
>> + BlockDriverInfo bdi = { .format_specific = NULL };
>> if (bdrv_get_info(target, &bdi) >= 0 && bdi.cluster_size != 0) {
>> granularity = MAX(4096, bdi.cluster_size);
>> granularity = MIN(65536, granularity);
>> } else {
>> granularity = 65536;
>> }
>> + qapi_free_ImageInfoSpecific(bdi.format_specific);
>> }
>>
>> assert ((granularity & (granularity - 1)) == 0);
>> diff --git a/block/qapi.c b/block/qapi.c
>> index a4bc411..f13fbd5 100644
>> --- a/block/qapi.c
>> +++ b/block/qapi.c
>> @@ -110,7 +110,7 @@ void bdrv_query_image_info(BlockDriverState *bs,
>> uint64_t total_sectors;
>> const char *backing_filename;
>> char backing_filename2[1024];
>> - BlockDriverInfo bdi;
>> + BlockDriverInfo bdi = { .format_specific = NULL };
>> int ret;
>> Error *err = NULL;
>> ImageInfo *info = g_new0(ImageInfo, 1);
>> @@ -133,6 +133,10 @@ void bdrv_query_image_info(BlockDriverState *bs,
>> }
>> info->dirty_flag = bdi.is_dirty;
>> info->has_dirty_flag = true;
>> + if (bdi.format_specific) {
>> + info->format_specific = bdi.format_specific;
>> + info->has_format_specific = true;
>> + }
>> }
>> backing_filename = bs->backing_file;
>> if (backing_filename[0] != '\0') {
>> diff --git a/include/block/block.h b/include/block/block.h
>> index e6b391c..85e9703 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -18,6 +18,8 @@ typedef struct BlockDriverInfo {
>> /* offset at which the VM state can be saved (0 if not possible) */
>> int64_t vm_state_offset;
>> bool is_dirty;
>> + /* additional information; NULL if none */
>> + ImageInfoSpecific *format_specific;
>> } BlockDriverInfo;
>>
>> typedef struct BlockFragInfo {
>> diff --git a/qemu-img.c b/qemu-img.c
>> index b9a848d..ec1ecca 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -1125,7 +1125,7 @@ static int img_convert(int argc, char **argv)
>> uint64_t bs_sectors;
>> uint8_t * buf = NULL;
>> const uint8_t *buf1;
>> - BlockDriverInfo bdi;
>> + BlockDriverInfo bdi = { .format_specific = NULL };
>> QEMUOptionParameter *param = NULL, *create_options = NULL;
>> QEMUOptionParameter *out_baseimg_param;
>> char *options = NULL;
>> @@ -1369,6 +1369,7 @@ static int img_convert(int argc, char **argv)
>> error_report("could not get block driver info");
>> goto out;
>> }
>> + qapi_free_ImageInfoSpecific(bdi.format_specific);
>> cluster_size = bdi.cluster_size;
>> if (cluster_size <= 0 || cluster_size > IO_BUF_SIZE) {
>> error_report("invalid cluster size");
>> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
>> index f91b6c4..563dd40 100644
>> --- a/qemu-io-cmds.c
>> +++ b/qemu-io-cmds.c
>> @@ -1677,7 +1677,7 @@ static const cmdinfo_t length_cmd = {
>>
>> static int info_f(BlockDriverState *bs, int argc, char **argv)
>> {
>> - BlockDriverInfo bdi;
>> + BlockDriverInfo bdi = { .format_specific = NULL };
>> char s1[64], s2[64];
>> int ret;
>>
>> @@ -1699,6 +1699,10 @@ static int info_f(BlockDriverState *bs, int argc, char **argv)
>> printf("cluster size: %s\n", s1);
>> printf("vm state offset: %s\n", s2);
>>
>> + if (bdi.format_specific) {
>> + qapi_free_ImageInfoSpecific(bdi.format_specific);
>> + }
>> +
>> return 0;
>> }
>>
>> --
>> 1.8.3.1
>>
>>
Max
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/6] block: Add ImageInfoSpecific to BlockDriverInfo
2013-09-10 7:22 ` Max Reitz
@ 2013-09-10 7:37 ` Fam Zheng
2013-09-10 7:45 ` Max Reitz
0 siblings, 1 reply; 15+ messages in thread
From: Fam Zheng @ 2013-09-10 7:37 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
On Tue, 09/10 09:22, Max Reitz wrote:
> On 2013-09-10 05:26, Fam Zheng wrote:
> >On Fri, 09/06 15:12, Max Reitz wrote:
> >>Add the new ImageInfoSpecific type also to BlockDriverInfo.
> >>
> >>To prevent memory leaks, this field has to be initialized to NULL every
> >>time before calling bdrv_get_info and qapi_free_ImageInfoSpecific has to
> >>be called on it when the BlockDriverInfo object is no longer required.
> >>
> >I don't understand here. I think bdi is always passed into bdrv_get_info()
> >uninitialized and in bdrv_get_info() there is:
> >
> > memset(bdi, 0, sizeof(*bdi));
> >
> >before passing it on to driver, so it's always set to NULL.
> Oh, you're right, I missed that. Thanks!
>
> >And why pointer, not a member to save a free() call?
> As far as I understand it (and if I don't miss anything again),
> ImageInfoSpecific is a auto-generated QAPI type, so it may contain
> pointers to other types anyway (as it does in the case of QCow2,
> which is the only driver where I have implemented this new field at
> all; in that case, the compatiblity level is a string), therefore we
> always need some function to clean up the data referenced by
> ImageInfoSpecific; qapi_free_ImageInfoSpecific is the perfect one
> for this, but it takes a heap pointer.
>
> Hence, I think using a pointer (to a heap-allocated object) is
> easier in this case, since the QAPI clean-up function assumes this
> case.
>
OK, learning this from you. Since this is allocated in bdrv_get_info() (from
the caller PoV), and, the info requires releasing after use, a bdrv_put_info()
may be a good function to do it, instead of directly operate on the field
everywhere.
> >>Signed-off-by: Max Reitz <mreitz@redhat.com>
> >>---
> >> block.c | 3 ++-
> >> block/mirror.c | 6 ++++--
> >> block/qapi.c | 6 +++++-
> >> include/block/block.h | 2 ++
> >> qemu-img.c | 3 ++-
> >> qemu-io-cmds.c | 6 +++++-
> >> 6 files changed, 20 insertions(+), 6 deletions(-)
> >>
> >>diff --git a/block.c b/block.c
> >>index 26639e8..1a5d2a4 100644
> >>--- a/block.c
> >>+++ b/block.c
> >>@@ -1921,7 +1921,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
> >> int64_t *cluster_sector_num,
> >> int *cluster_nb_sectors)
> >> {
> >>- BlockDriverInfo bdi;
> >>+ BlockDriverInfo bdi = { .format_specific = NULL };
> >> if (bdrv_get_info(bs, &bdi) < 0 || bdi.cluster_size == 0) {
> >> *cluster_sector_num = sector_num;
> >>@@ -1932,6 +1932,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
> >> *cluster_nb_sectors = QEMU_ALIGN_UP(sector_num - *cluster_sector_num +
> >> nb_sectors, c);
> >> }
> >>+ qapi_free_ImageInfoSpecific(bdi.format_specific);
> >> }
> >> static bool tracked_request_overlaps(BdrvTrackedRequest *req,
> >>diff --git a/block/mirror.c b/block/mirror.c
> >>index 86de458..cfef7e9 100644
> >>--- a/block/mirror.c
> >>+++ b/block/mirror.c
> >>@@ -295,7 +295,7 @@ static void coroutine_fn mirror_run(void *opaque)
> >> BlockDriverState *bs = s->common.bs;
> >> int64_t sector_num, end, sectors_per_chunk, length;
> >> uint64_t last_pause_ns;
> >>- BlockDriverInfo bdi;
> >>+ BlockDriverInfo bdi = { .format_specific = NULL };
> >> char backing_filename[1024];
> >> int ret = 0;
> >> int n;
> >>@@ -325,6 +325,7 @@ static void coroutine_fn mirror_run(void *opaque)
> >> s->buf_size = MAX(s->buf_size, bdi.cluster_size);
> >> s->cow_bitmap = bitmap_new(length);
> >> }
> >>+ qapi_free_ImageInfoSpecific(bdi.format_specific);
> >> }
> >> end = s->common.len >> BDRV_SECTOR_BITS;
> >>@@ -544,13 +545,14 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
> >> if (granularity == 0) {
> >> /* Choose the default granularity based on the target file's cluster
> >> * size, clamped between 4k and 64k. */
> >>- BlockDriverInfo bdi;
> >>+ BlockDriverInfo bdi = { .format_specific = NULL };
> >> if (bdrv_get_info(target, &bdi) >= 0 && bdi.cluster_size != 0) {
> >> granularity = MAX(4096, bdi.cluster_size);
> >> granularity = MIN(65536, granularity);
> >> } else {
> >> granularity = 65536;
> >> }
> >>+ qapi_free_ImageInfoSpecific(bdi.format_specific);
> >> }
> >> assert ((granularity & (granularity - 1)) == 0);
> >>diff --git a/block/qapi.c b/block/qapi.c
> >>index a4bc411..f13fbd5 100644
> >>--- a/block/qapi.c
> >>+++ b/block/qapi.c
> >>@@ -110,7 +110,7 @@ void bdrv_query_image_info(BlockDriverState *bs,
> >> uint64_t total_sectors;
> >> const char *backing_filename;
> >> char backing_filename2[1024];
> >>- BlockDriverInfo bdi;
> >>+ BlockDriverInfo bdi = { .format_specific = NULL };
> >> int ret;
> >> Error *err = NULL;
> >> ImageInfo *info = g_new0(ImageInfo, 1);
> >>@@ -133,6 +133,10 @@ void bdrv_query_image_info(BlockDriverState *bs,
> >> }
> >> info->dirty_flag = bdi.is_dirty;
> >> info->has_dirty_flag = true;
> >>+ if (bdi.format_specific) {
> >>+ info->format_specific = bdi.format_specific;
> >>+ info->has_format_specific = true;
> >>+ }
> >> }
> >> backing_filename = bs->backing_file;
> >> if (backing_filename[0] != '\0') {
> >>diff --git a/include/block/block.h b/include/block/block.h
> >>index e6b391c..85e9703 100644
> >>--- a/include/block/block.h
> >>+++ b/include/block/block.h
> >>@@ -18,6 +18,8 @@ typedef struct BlockDriverInfo {
> >> /* offset at which the VM state can be saved (0 if not possible) */
> >> int64_t vm_state_offset;
> >> bool is_dirty;
> >>+ /* additional information; NULL if none */
> >>+ ImageInfoSpecific *format_specific;
> >> } BlockDriverInfo;
> >> typedef struct BlockFragInfo {
> >>diff --git a/qemu-img.c b/qemu-img.c
> >>index b9a848d..ec1ecca 100644
> >>--- a/qemu-img.c
> >>+++ b/qemu-img.c
> >>@@ -1125,7 +1125,7 @@ static int img_convert(int argc, char **argv)
> >> uint64_t bs_sectors;
> >> uint8_t * buf = NULL;
> >> const uint8_t *buf1;
> >>- BlockDriverInfo bdi;
> >>+ BlockDriverInfo bdi = { .format_specific = NULL };
> >> QEMUOptionParameter *param = NULL, *create_options = NULL;
> >> QEMUOptionParameter *out_baseimg_param;
> >> char *options = NULL;
> >>@@ -1369,6 +1369,7 @@ static int img_convert(int argc, char **argv)
> >> error_report("could not get block driver info");
> >> goto out;
> >> }
> >>+ qapi_free_ImageInfoSpecific(bdi.format_specific);
> >> cluster_size = bdi.cluster_size;
> >> if (cluster_size <= 0 || cluster_size > IO_BUF_SIZE) {
> >> error_report("invalid cluster size");
> >>diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> >>index f91b6c4..563dd40 100644
> >>--- a/qemu-io-cmds.c
> >>+++ b/qemu-io-cmds.c
> >>@@ -1677,7 +1677,7 @@ static const cmdinfo_t length_cmd = {
> >> static int info_f(BlockDriverState *bs, int argc, char **argv)
> >> {
> >>- BlockDriverInfo bdi;
> >>+ BlockDriverInfo bdi = { .format_specific = NULL };
> >> char s1[64], s2[64];
> >> int ret;
> >>@@ -1699,6 +1699,10 @@ static int info_f(BlockDriverState *bs, int argc, char **argv)
> >> printf("cluster size: %s\n", s1);
> >> printf("vm state offset: %s\n", s2);
> >>+ if (bdi.format_specific) {
> >>+ qapi_free_ImageInfoSpecific(bdi.format_specific);
> >>+ }
> >>+
> >> return 0;
> >> }
> >>--
> >>1.8.3.1
> >>
> >>
>
> Max
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/6] block: Add ImageInfoSpecific to BlockDriverInfo
2013-09-10 7:37 ` Fam Zheng
@ 2013-09-10 7:45 ` Max Reitz
2013-09-10 7:50 ` Fam Zheng
0 siblings, 1 reply; 15+ messages in thread
From: Max Reitz @ 2013-09-10 7:45 UTC (permalink / raw)
To: famz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
On 2013-09-10 09:37, Fam Zheng wrote:
> On Tue, 09/10 09:22, Max Reitz wrote:
>> On 2013-09-10 05:26, Fam Zheng wrote:
>>> On Fri, 09/06 15:12, Max Reitz wrote:
>>>> Add the new ImageInfoSpecific type also to BlockDriverInfo.
>>>>
>>>> To prevent memory leaks, this field has to be initialized to NULL every
>>>> time before calling bdrv_get_info and qapi_free_ImageInfoSpecific has to
>>>> be called on it when the BlockDriverInfo object is no longer required.
>>>>
>>> I don't understand here. I think bdi is always passed into bdrv_get_info()
>>> uninitialized and in bdrv_get_info() there is:
>>>
>>> memset(bdi, 0, sizeof(*bdi));
>>>
>>> before passing it on to driver, so it's always set to NULL.
>> Oh, you're right, I missed that. Thanks!
>>
>>> And why pointer, not a member to save a free() call?
>> As far as I understand it (and if I don't miss anything again),
>> ImageInfoSpecific is a auto-generated QAPI type, so it may contain
>> pointers to other types anyway (as it does in the case of QCow2,
>> which is the only driver where I have implemented this new field at
>> all; in that case, the compatiblity level is a string), therefore we
>> always need some function to clean up the data referenced by
>> ImageInfoSpecific; qapi_free_ImageInfoSpecific is the perfect one
>> for this, but it takes a heap pointer.
>>
>> Hence, I think using a pointer (to a heap-allocated object) is
>> easier in this case, since the QAPI clean-up function assumes this
>> case.
>>
> OK, learning this from you. Since this is allocated in bdrv_get_info() (from
> the caller PoV), and, the info requires releasing after use, a bdrv_put_info()
> may be a good function to do it, instead of directly operate on the field
> everywhere.
You mean a function for filling the ImageInfoSpecific field? Hm, we
can't really use parameters, since every driver would then require its
own function (which, imo, would defeat the purpose); the only thing I
can imagine right now is a function which converts a JSON description to
the object; however, this would again require proper escaping for
strings and conversion to strings for non-string types, so I doubt
whether this would really help...
>>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>>> ---
>>>> block.c | 3 ++-
>>>> block/mirror.c | 6 ++++--
>>>> block/qapi.c | 6 +++++-
>>>> include/block/block.h | 2 ++
>>>> qemu-img.c | 3 ++-
>>>> qemu-io-cmds.c | 6 +++++-
>>>> 6 files changed, 20 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/block.c b/block.c
>>>> index 26639e8..1a5d2a4 100644
>>>> --- a/block.c
>>>> +++ b/block.c
>>>> @@ -1921,7 +1921,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
>>>> int64_t *cluster_sector_num,
>>>> int *cluster_nb_sectors)
>>>> {
>>>> - BlockDriverInfo bdi;
>>>> + BlockDriverInfo bdi = { .format_specific = NULL };
>>>> if (bdrv_get_info(bs, &bdi) < 0 || bdi.cluster_size == 0) {
>>>> *cluster_sector_num = sector_num;
>>>> @@ -1932,6 +1932,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
>>>> *cluster_nb_sectors = QEMU_ALIGN_UP(sector_num - *cluster_sector_num +
>>>> nb_sectors, c);
>>>> }
>>>> + qapi_free_ImageInfoSpecific(bdi.format_specific);
>>>> }
>>>> static bool tracked_request_overlaps(BdrvTrackedRequest *req,
>>>> diff --git a/block/mirror.c b/block/mirror.c
>>>> index 86de458..cfef7e9 100644
>>>> --- a/block/mirror.c
>>>> +++ b/block/mirror.c
>>>> @@ -295,7 +295,7 @@ static void coroutine_fn mirror_run(void *opaque)
>>>> BlockDriverState *bs = s->common.bs;
>>>> int64_t sector_num, end, sectors_per_chunk, length;
>>>> uint64_t last_pause_ns;
>>>> - BlockDriverInfo bdi;
>>>> + BlockDriverInfo bdi = { .format_specific = NULL };
>>>> char backing_filename[1024];
>>>> int ret = 0;
>>>> int n;
>>>> @@ -325,6 +325,7 @@ static void coroutine_fn mirror_run(void *opaque)
>>>> s->buf_size = MAX(s->buf_size, bdi.cluster_size);
>>>> s->cow_bitmap = bitmap_new(length);
>>>> }
>>>> + qapi_free_ImageInfoSpecific(bdi.format_specific);
>>>> }
>>>> end = s->common.len >> BDRV_SECTOR_BITS;
>>>> @@ -544,13 +545,14 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
>>>> if (granularity == 0) {
>>>> /* Choose the default granularity based on the target file's cluster
>>>> * size, clamped between 4k and 64k. */
>>>> - BlockDriverInfo bdi;
>>>> + BlockDriverInfo bdi = { .format_specific = NULL };
>>>> if (bdrv_get_info(target, &bdi) >= 0 && bdi.cluster_size != 0) {
>>>> granularity = MAX(4096, bdi.cluster_size);
>>>> granularity = MIN(65536, granularity);
>>>> } else {
>>>> granularity = 65536;
>>>> }
>>>> + qapi_free_ImageInfoSpecific(bdi.format_specific);
>>>> }
>>>> assert ((granularity & (granularity - 1)) == 0);
>>>> diff --git a/block/qapi.c b/block/qapi.c
>>>> index a4bc411..f13fbd5 100644
>>>> --- a/block/qapi.c
>>>> +++ b/block/qapi.c
>>>> @@ -110,7 +110,7 @@ void bdrv_query_image_info(BlockDriverState *bs,
>>>> uint64_t total_sectors;
>>>> const char *backing_filename;
>>>> char backing_filename2[1024];
>>>> - BlockDriverInfo bdi;
>>>> + BlockDriverInfo bdi = { .format_specific = NULL };
>>>> int ret;
>>>> Error *err = NULL;
>>>> ImageInfo *info = g_new0(ImageInfo, 1);
>>>> @@ -133,6 +133,10 @@ void bdrv_query_image_info(BlockDriverState *bs,
>>>> }
>>>> info->dirty_flag = bdi.is_dirty;
>>>> info->has_dirty_flag = true;
>>>> + if (bdi.format_specific) {
>>>> + info->format_specific = bdi.format_specific;
>>>> + info->has_format_specific = true;
>>>> + }
>>>> }
>>>> backing_filename = bs->backing_file;
>>>> if (backing_filename[0] != '\0') {
>>>> diff --git a/include/block/block.h b/include/block/block.h
>>>> index e6b391c..85e9703 100644
>>>> --- a/include/block/block.h
>>>> +++ b/include/block/block.h
>>>> @@ -18,6 +18,8 @@ typedef struct BlockDriverInfo {
>>>> /* offset at which the VM state can be saved (0 if not possible) */
>>>> int64_t vm_state_offset;
>>>> bool is_dirty;
>>>> + /* additional information; NULL if none */
>>>> + ImageInfoSpecific *format_specific;
>>>> } BlockDriverInfo;
>>>> typedef struct BlockFragInfo {
>>>> diff --git a/qemu-img.c b/qemu-img.c
>>>> index b9a848d..ec1ecca 100644
>>>> --- a/qemu-img.c
>>>> +++ b/qemu-img.c
>>>> @@ -1125,7 +1125,7 @@ static int img_convert(int argc, char **argv)
>>>> uint64_t bs_sectors;
>>>> uint8_t * buf = NULL;
>>>> const uint8_t *buf1;
>>>> - BlockDriverInfo bdi;
>>>> + BlockDriverInfo bdi = { .format_specific = NULL };
>>>> QEMUOptionParameter *param = NULL, *create_options = NULL;
>>>> QEMUOptionParameter *out_baseimg_param;
>>>> char *options = NULL;
>>>> @@ -1369,6 +1369,7 @@ static int img_convert(int argc, char **argv)
>>>> error_report("could not get block driver info");
>>>> goto out;
>>>> }
>>>> + qapi_free_ImageInfoSpecific(bdi.format_specific);
>>>> cluster_size = bdi.cluster_size;
>>>> if (cluster_size <= 0 || cluster_size > IO_BUF_SIZE) {
>>>> error_report("invalid cluster size");
>>>> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
>>>> index f91b6c4..563dd40 100644
>>>> --- a/qemu-io-cmds.c
>>>> +++ b/qemu-io-cmds.c
>>>> @@ -1677,7 +1677,7 @@ static const cmdinfo_t length_cmd = {
>>>> static int info_f(BlockDriverState *bs, int argc, char **argv)
>>>> {
>>>> - BlockDriverInfo bdi;
>>>> + BlockDriverInfo bdi = { .format_specific = NULL };
>>>> char s1[64], s2[64];
>>>> int ret;
>>>> @@ -1699,6 +1699,10 @@ static int info_f(BlockDriverState *bs, int argc, char **argv)
>>>> printf("cluster size: %s\n", s1);
>>>> printf("vm state offset: %s\n", s2);
>>>> + if (bdi.format_specific) {
>>>> + qapi_free_ImageInfoSpecific(bdi.format_specific);
>>>> + }
>>>> +
>>>> return 0;
>>>> }
>>>> --
>>>> 1.8.3.1
>>>>
>>>>
>> Max
Max
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/6] block: Add ImageInfoSpecific to BlockDriverInfo
2013-09-10 7:45 ` Max Reitz
@ 2013-09-10 7:50 ` Fam Zheng
2013-09-10 7:54 ` Max Reitz
0 siblings, 1 reply; 15+ messages in thread
From: Fam Zheng @ 2013-09-10 7:50 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
On Tue, 09/10 09:45, Max Reitz wrote:
> On 2013-09-10 09:37, Fam Zheng wrote:
> >On Tue, 09/10 09:22, Max Reitz wrote:
> >>On 2013-09-10 05:26, Fam Zheng wrote:
> >>>On Fri, 09/06 15:12, Max Reitz wrote:
> >>>>Add the new ImageInfoSpecific type also to BlockDriverInfo.
> >>>>
> >>>>To prevent memory leaks, this field has to be initialized to NULL every
> >>>>time before calling bdrv_get_info and qapi_free_ImageInfoSpecific has to
> >>>>be called on it when the BlockDriverInfo object is no longer required.
> >>>>
> >>>I don't understand here. I think bdi is always passed into bdrv_get_info()
> >>>uninitialized and in bdrv_get_info() there is:
> >>>
> >>> memset(bdi, 0, sizeof(*bdi));
> >>>
> >>>before passing it on to driver, so it's always set to NULL.
> >>Oh, you're right, I missed that. Thanks!
> >>
> >>>And why pointer, not a member to save a free() call?
> >>As far as I understand it (and if I don't miss anything again),
> >>ImageInfoSpecific is a auto-generated QAPI type, so it may contain
> >>pointers to other types anyway (as it does in the case of QCow2,
> >>which is the only driver where I have implemented this new field at
> >>all; in that case, the compatiblity level is a string), therefore we
> >>always need some function to clean up the data referenced by
> >>ImageInfoSpecific; qapi_free_ImageInfoSpecific is the perfect one
> >>for this, but it takes a heap pointer.
> >>
> >>Hence, I think using a pointer (to a heap-allocated object) is
> >>easier in this case, since the QAPI clean-up function assumes this
> >>case.
> >>
> >OK, learning this from you. Since this is allocated in bdrv_get_info() (from
> >the caller PoV), and, the info requires releasing after use, a bdrv_put_info()
> >may be a good function to do it, instead of directly operate on the field
> >everywhere.
> You mean a function for filling the ImageInfoSpecific field? Hm, we
> can't really use parameters, since every driver would then require
> its own function (which, imo, would defeat the purpose); the only
> thing I can imagine right now is a function which converts a JSON
> description to the object; however, this would again require proper
> escaping for strings and conversion to strings for non-string types,
> so I doubt whether this would really help...
>
No, I mean freeing it. Maybe I should suggest bdrv_free_info() or something
else. It's just that too many
qapi_free_ImageInfoSpecific(bdi.format_specific);
lines after all the bdrv_get_info() calls don't look very clean.
> >>>>Signed-off-by: Max Reitz <mreitz@redhat.com>
> >>>>---
> >>>> block.c | 3 ++-
> >>>> block/mirror.c | 6 ++++--
> >>>> block/qapi.c | 6 +++++-
> >>>> include/block/block.h | 2 ++
> >>>> qemu-img.c | 3 ++-
> >>>> qemu-io-cmds.c | 6 +++++-
> >>>> 6 files changed, 20 insertions(+), 6 deletions(-)
> >>>>
> >>>>diff --git a/block.c b/block.c
> >>>>index 26639e8..1a5d2a4 100644
> >>>>--- a/block.c
> >>>>+++ b/block.c
> >>>>@@ -1921,7 +1921,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
> >>>> int64_t *cluster_sector_num,
> >>>> int *cluster_nb_sectors)
> >>>> {
> >>>>- BlockDriverInfo bdi;
> >>>>+ BlockDriverInfo bdi = { .format_specific = NULL };
> >>>> if (bdrv_get_info(bs, &bdi) < 0 || bdi.cluster_size == 0) {
> >>>> *cluster_sector_num = sector_num;
> >>>>@@ -1932,6 +1932,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
> >>>> *cluster_nb_sectors = QEMU_ALIGN_UP(sector_num - *cluster_sector_num +
> >>>> nb_sectors, c);
> >>>> }
> >>>>+ qapi_free_ImageInfoSpecific(bdi.format_specific);
> >>>> }
> >>>> static bool tracked_request_overlaps(BdrvTrackedRequest *req,
> >>>>diff --git a/block/mirror.c b/block/mirror.c
> >>>>index 86de458..cfef7e9 100644
> >>>>--- a/block/mirror.c
> >>>>+++ b/block/mirror.c
> >>>>@@ -295,7 +295,7 @@ static void coroutine_fn mirror_run(void *opaque)
> >>>> BlockDriverState *bs = s->common.bs;
> >>>> int64_t sector_num, end, sectors_per_chunk, length;
> >>>> uint64_t last_pause_ns;
> >>>>- BlockDriverInfo bdi;
> >>>>+ BlockDriverInfo bdi = { .format_specific = NULL };
> >>>> char backing_filename[1024];
> >>>> int ret = 0;
> >>>> int n;
> >>>>@@ -325,6 +325,7 @@ static void coroutine_fn mirror_run(void *opaque)
> >>>> s->buf_size = MAX(s->buf_size, bdi.cluster_size);
> >>>> s->cow_bitmap = bitmap_new(length);
> >>>> }
> >>>>+ qapi_free_ImageInfoSpecific(bdi.format_specific);
> >>>> }
> >>>> end = s->common.len >> BDRV_SECTOR_BITS;
> >>>>@@ -544,13 +545,14 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
> >>>> if (granularity == 0) {
> >>>> /* Choose the default granularity based on the target file's cluster
> >>>> * size, clamped between 4k and 64k. */
> >>>>- BlockDriverInfo bdi;
> >>>>+ BlockDriverInfo bdi = { .format_specific = NULL };
> >>>> if (bdrv_get_info(target, &bdi) >= 0 && bdi.cluster_size != 0) {
> >>>> granularity = MAX(4096, bdi.cluster_size);
> >>>> granularity = MIN(65536, granularity);
> >>>> } else {
> >>>> granularity = 65536;
> >>>> }
> >>>>+ qapi_free_ImageInfoSpecific(bdi.format_specific);
> >>>> }
> >>>> assert ((granularity & (granularity - 1)) == 0);
> >>>>diff --git a/block/qapi.c b/block/qapi.c
> >>>>index a4bc411..f13fbd5 100644
> >>>>--- a/block/qapi.c
> >>>>+++ b/block/qapi.c
> >>>>@@ -110,7 +110,7 @@ void bdrv_query_image_info(BlockDriverState *bs,
> >>>> uint64_t total_sectors;
> >>>> const char *backing_filename;
> >>>> char backing_filename2[1024];
> >>>>- BlockDriverInfo bdi;
> >>>>+ BlockDriverInfo bdi = { .format_specific = NULL };
> >>>> int ret;
> >>>> Error *err = NULL;
> >>>> ImageInfo *info = g_new0(ImageInfo, 1);
> >>>>@@ -133,6 +133,10 @@ void bdrv_query_image_info(BlockDriverState *bs,
> >>>> }
> >>>> info->dirty_flag = bdi.is_dirty;
> >>>> info->has_dirty_flag = true;
> >>>>+ if (bdi.format_specific) {
> >>>>+ info->format_specific = bdi.format_specific;
> >>>>+ info->has_format_specific = true;
> >>>>+ }
> >>>> }
> >>>> backing_filename = bs->backing_file;
> >>>> if (backing_filename[0] != '\0') {
> >>>>diff --git a/include/block/block.h b/include/block/block.h
> >>>>index e6b391c..85e9703 100644
> >>>>--- a/include/block/block.h
> >>>>+++ b/include/block/block.h
> >>>>@@ -18,6 +18,8 @@ typedef struct BlockDriverInfo {
> >>>> /* offset at which the VM state can be saved (0 if not possible) */
> >>>> int64_t vm_state_offset;
> >>>> bool is_dirty;
> >>>>+ /* additional information; NULL if none */
> >>>>+ ImageInfoSpecific *format_specific;
> >>>> } BlockDriverInfo;
> >>>> typedef struct BlockFragInfo {
> >>>>diff --git a/qemu-img.c b/qemu-img.c
> >>>>index b9a848d..ec1ecca 100644
> >>>>--- a/qemu-img.c
> >>>>+++ b/qemu-img.c
> >>>>@@ -1125,7 +1125,7 @@ static int img_convert(int argc, char **argv)
> >>>> uint64_t bs_sectors;
> >>>> uint8_t * buf = NULL;
> >>>> const uint8_t *buf1;
> >>>>- BlockDriverInfo bdi;
> >>>>+ BlockDriverInfo bdi = { .format_specific = NULL };
> >>>> QEMUOptionParameter *param = NULL, *create_options = NULL;
> >>>> QEMUOptionParameter *out_baseimg_param;
> >>>> char *options = NULL;
> >>>>@@ -1369,6 +1369,7 @@ static int img_convert(int argc, char **argv)
> >>>> error_report("could not get block driver info");
> >>>> goto out;
> >>>> }
> >>>>+ qapi_free_ImageInfoSpecific(bdi.format_specific);
> >>>> cluster_size = bdi.cluster_size;
> >>>> if (cluster_size <= 0 || cluster_size > IO_BUF_SIZE) {
> >>>> error_report("invalid cluster size");
> >>>>diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> >>>>index f91b6c4..563dd40 100644
> >>>>--- a/qemu-io-cmds.c
> >>>>+++ b/qemu-io-cmds.c
> >>>>@@ -1677,7 +1677,7 @@ static const cmdinfo_t length_cmd = {
> >>>> static int info_f(BlockDriverState *bs, int argc, char **argv)
> >>>> {
> >>>>- BlockDriverInfo bdi;
> >>>>+ BlockDriverInfo bdi = { .format_specific = NULL };
> >>>> char s1[64], s2[64];
> >>>> int ret;
> >>>>@@ -1699,6 +1699,10 @@ static int info_f(BlockDriverState *bs, int argc, char **argv)
> >>>> printf("cluster size: %s\n", s1);
> >>>> printf("vm state offset: %s\n", s2);
> >>>>+ if (bdi.format_specific) {
> >>>>+ qapi_free_ImageInfoSpecific(bdi.format_specific);
> >>>>+ }
> >>>>+
> >>>> return 0;
> >>>> }
> >>>>--
> >>>>1.8.3.1
> >>>>
> >>>>
> >>Max
>
> Max
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/6] block: Add ImageInfoSpecific to BlockDriverInfo
2013-09-10 7:50 ` Fam Zheng
@ 2013-09-10 7:54 ` Max Reitz
0 siblings, 0 replies; 15+ messages in thread
From: Max Reitz @ 2013-09-10 7:54 UTC (permalink / raw)
To: famz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
On 2013-09-10 09:50, Fam Zheng wrote:
> On Tue, 09/10 09:45, Max Reitz wrote:
>> On 2013-09-10 09:37, Fam Zheng wrote:
>>> On Tue, 09/10 09:22, Max Reitz wrote:
>>>> On 2013-09-10 05:26, Fam Zheng wrote:
>>>>> On Fri, 09/06 15:12, Max Reitz wrote:
>>>>>> Add the new ImageInfoSpecific type also to BlockDriverInfo.
>>>>>>
>>>>>> To prevent memory leaks, this field has to be initialized to NULL every
>>>>>> time before calling bdrv_get_info and qapi_free_ImageInfoSpecific has to
>>>>>> be called on it when the BlockDriverInfo object is no longer required.
>>>>>>
>>>>> I don't understand here. I think bdi is always passed into bdrv_get_info()
>>>>> uninitialized and in bdrv_get_info() there is:
>>>>>
>>>>> memset(bdi, 0, sizeof(*bdi));
>>>>>
>>>>> before passing it on to driver, so it's always set to NULL.
>>>> Oh, you're right, I missed that. Thanks!
>>>>
>>>>> And why pointer, not a member to save a free() call?
>>>> As far as I understand it (and if I don't miss anything again),
>>>> ImageInfoSpecific is a auto-generated QAPI type, so it may contain
>>>> pointers to other types anyway (as it does in the case of QCow2,
>>>> which is the only driver where I have implemented this new field at
>>>> all; in that case, the compatiblity level is a string), therefore we
>>>> always need some function to clean up the data referenced by
>>>> ImageInfoSpecific; qapi_free_ImageInfoSpecific is the perfect one
>>>> for this, but it takes a heap pointer.
>>>>
>>>> Hence, I think using a pointer (to a heap-allocated object) is
>>>> easier in this case, since the QAPI clean-up function assumes this
>>>> case.
>>>>
>>> OK, learning this from you. Since this is allocated in bdrv_get_info() (from
>>> the caller PoV), and, the info requires releasing after use, a bdrv_put_info()
>>> may be a good function to do it, instead of directly operate on the field
>>> everywhere.
>> You mean a function for filling the ImageInfoSpecific field? Hm, we
>> can't really use parameters, since every driver would then require
>> its own function (which, imo, would defeat the purpose); the only
>> thing I can imagine right now is a function which converts a JSON
>> description to the object; however, this would again require proper
>> escaping for strings and conversion to strings for non-string types,
>> so I doubt whether this would really help...
>>
> No, I mean freeing it. Maybe I should suggest bdrv_free_info() or something
> else. It's just that too many
>
> qapi_free_ImageInfoSpecific(bdi.format_specific);
>
> lines after all the bdrv_get_info() calls don't look very clean.
Sounds nice. I'll do it; thanks for the suggestion.
>>>>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>>>>> ---
>>>>>> block.c | 3 ++-
>>>>>> block/mirror.c | 6 ++++--
>>>>>> block/qapi.c | 6 +++++-
>>>>>> include/block/block.h | 2 ++
>>>>>> qemu-img.c | 3 ++-
>>>>>> qemu-io-cmds.c | 6 +++++-
>>>>>> 6 files changed, 20 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/block.c b/block.c
>>>>>> index 26639e8..1a5d2a4 100644
>>>>>> --- a/block.c
>>>>>> +++ b/block.c
>>>>>> @@ -1921,7 +1921,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
>>>>>> int64_t *cluster_sector_num,
>>>>>> int *cluster_nb_sectors)
>>>>>> {
>>>>>> - BlockDriverInfo bdi;
>>>>>> + BlockDriverInfo bdi = { .format_specific = NULL };
>>>>>> if (bdrv_get_info(bs, &bdi) < 0 || bdi.cluster_size == 0) {
>>>>>> *cluster_sector_num = sector_num;
>>>>>> @@ -1932,6 +1932,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
>>>>>> *cluster_nb_sectors = QEMU_ALIGN_UP(sector_num - *cluster_sector_num +
>>>>>> nb_sectors, c);
>>>>>> }
>>>>>> + qapi_free_ImageInfoSpecific(bdi.format_specific);
>>>>>> }
>>>>>> static bool tracked_request_overlaps(BdrvTrackedRequest *req,
>>>>>> diff --git a/block/mirror.c b/block/mirror.c
>>>>>> index 86de458..cfef7e9 100644
>>>>>> --- a/block/mirror.c
>>>>>> +++ b/block/mirror.c
>>>>>> @@ -295,7 +295,7 @@ static void coroutine_fn mirror_run(void *opaque)
>>>>>> BlockDriverState *bs = s->common.bs;
>>>>>> int64_t sector_num, end, sectors_per_chunk, length;
>>>>>> uint64_t last_pause_ns;
>>>>>> - BlockDriverInfo bdi;
>>>>>> + BlockDriverInfo bdi = { .format_specific = NULL };
>>>>>> char backing_filename[1024];
>>>>>> int ret = 0;
>>>>>> int n;
>>>>>> @@ -325,6 +325,7 @@ static void coroutine_fn mirror_run(void *opaque)
>>>>>> s->buf_size = MAX(s->buf_size, bdi.cluster_size);
>>>>>> s->cow_bitmap = bitmap_new(length);
>>>>>> }
>>>>>> + qapi_free_ImageInfoSpecific(bdi.format_specific);
>>>>>> }
>>>>>> end = s->common.len >> BDRV_SECTOR_BITS;
>>>>>> @@ -544,13 +545,14 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
>>>>>> if (granularity == 0) {
>>>>>> /* Choose the default granularity based on the target file's cluster
>>>>>> * size, clamped between 4k and 64k. */
>>>>>> - BlockDriverInfo bdi;
>>>>>> + BlockDriverInfo bdi = { .format_specific = NULL };
>>>>>> if (bdrv_get_info(target, &bdi) >= 0 && bdi.cluster_size != 0) {
>>>>>> granularity = MAX(4096, bdi.cluster_size);
>>>>>> granularity = MIN(65536, granularity);
>>>>>> } else {
>>>>>> granularity = 65536;
>>>>>> }
>>>>>> + qapi_free_ImageInfoSpecific(bdi.format_specific);
>>>>>> }
>>>>>> assert ((granularity & (granularity - 1)) == 0);
>>>>>> diff --git a/block/qapi.c b/block/qapi.c
>>>>>> index a4bc411..f13fbd5 100644
>>>>>> --- a/block/qapi.c
>>>>>> +++ b/block/qapi.c
>>>>>> @@ -110,7 +110,7 @@ void bdrv_query_image_info(BlockDriverState *bs,
>>>>>> uint64_t total_sectors;
>>>>>> const char *backing_filename;
>>>>>> char backing_filename2[1024];
>>>>>> - BlockDriverInfo bdi;
>>>>>> + BlockDriverInfo bdi = { .format_specific = NULL };
>>>>>> int ret;
>>>>>> Error *err = NULL;
>>>>>> ImageInfo *info = g_new0(ImageInfo, 1);
>>>>>> @@ -133,6 +133,10 @@ void bdrv_query_image_info(BlockDriverState *bs,
>>>>>> }
>>>>>> info->dirty_flag = bdi.is_dirty;
>>>>>> info->has_dirty_flag = true;
>>>>>> + if (bdi.format_specific) {
>>>>>> + info->format_specific = bdi.format_specific;
>>>>>> + info->has_format_specific = true;
>>>>>> + }
>>>>>> }
>>>>>> backing_filename = bs->backing_file;
>>>>>> if (backing_filename[0] != '\0') {
>>>>>> diff --git a/include/block/block.h b/include/block/block.h
>>>>>> index e6b391c..85e9703 100644
>>>>>> --- a/include/block/block.h
>>>>>> +++ b/include/block/block.h
>>>>>> @@ -18,6 +18,8 @@ typedef struct BlockDriverInfo {
>>>>>> /* offset at which the VM state can be saved (0 if not possible) */
>>>>>> int64_t vm_state_offset;
>>>>>> bool is_dirty;
>>>>>> + /* additional information; NULL if none */
>>>>>> + ImageInfoSpecific *format_specific;
>>>>>> } BlockDriverInfo;
>>>>>> typedef struct BlockFragInfo {
>>>>>> diff --git a/qemu-img.c b/qemu-img.c
>>>>>> index b9a848d..ec1ecca 100644
>>>>>> --- a/qemu-img.c
>>>>>> +++ b/qemu-img.c
>>>>>> @@ -1125,7 +1125,7 @@ static int img_convert(int argc, char **argv)
>>>>>> uint64_t bs_sectors;
>>>>>> uint8_t * buf = NULL;
>>>>>> const uint8_t *buf1;
>>>>>> - BlockDriverInfo bdi;
>>>>>> + BlockDriverInfo bdi = { .format_specific = NULL };
>>>>>> QEMUOptionParameter *param = NULL, *create_options = NULL;
>>>>>> QEMUOptionParameter *out_baseimg_param;
>>>>>> char *options = NULL;
>>>>>> @@ -1369,6 +1369,7 @@ static int img_convert(int argc, char **argv)
>>>>>> error_report("could not get block driver info");
>>>>>> goto out;
>>>>>> }
>>>>>> + qapi_free_ImageInfoSpecific(bdi.format_specific);
>>>>>> cluster_size = bdi.cluster_size;
>>>>>> if (cluster_size <= 0 || cluster_size > IO_BUF_SIZE) {
>>>>>> error_report("invalid cluster size");
>>>>>> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
>>>>>> index f91b6c4..563dd40 100644
>>>>>> --- a/qemu-io-cmds.c
>>>>>> +++ b/qemu-io-cmds.c
>>>>>> @@ -1677,7 +1677,7 @@ static const cmdinfo_t length_cmd = {
>>>>>> static int info_f(BlockDriverState *bs, int argc, char **argv)
>>>>>> {
>>>>>> - BlockDriverInfo bdi;
>>>>>> + BlockDriverInfo bdi = { .format_specific = NULL };
>>>>>> char s1[64], s2[64];
>>>>>> int ret;
>>>>>> @@ -1699,6 +1699,10 @@ static int info_f(BlockDriverState *bs, int argc, char **argv)
>>>>>> printf("cluster size: %s\n", s1);
>>>>>> printf("vm state offset: %s\n", s2);
>>>>>> + if (bdi.format_specific) {
>>>>>> + qapi_free_ImageInfoSpecific(bdi.format_specific);
>>>>>> + }
>>>>>> +
>>>>>> return 0;
>>>>>> }
>>>>>> --
>>>>>> 1.8.3.1
>>>>>>
>>>>>>
>>>> Max
>> Max
Max
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v2 3/6] block/qapi: Human-readable ImageInfoSpecific dump
2013-09-06 13:12 [Qemu-devel] [PATCH v2 0/6] Provide additional info through qemu-img info Max Reitz
2013-09-06 13:12 ` [Qemu-devel] [PATCH v2 1/6] qapi: Add ImageInfoSpecific type Max Reitz
2013-09-06 13:12 ` [Qemu-devel] [PATCH v2 2/6] block: Add ImageInfoSpecific to BlockDriverInfo Max Reitz
@ 2013-09-06 13:12 ` Max Reitz
2013-09-10 4:04 ` Fam Zheng
2013-09-06 13:12 ` [Qemu-devel] [PATCH v2 4/6] qcow2: Add support for ImageInfoSpecific Max Reitz
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Max Reitz @ 2013-09-06 13:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add a function for generically dumping the ImageInfoSpecific information
in a human-readable format to block/qapi.c.
Use this function in bdrv_image_info_dump and qemu-io-cmds.c:info_f to
allow qemu-img info resp. qemu-io -c info to print that format specific
information.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qapi.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++++++
include/block/qapi.h | 2 +
qemu-io-cmds.c | 3 ++
3 files changed, 126 insertions(+)
diff --git a/block/qapi.c b/block/qapi.c
index f13fbd5..4fe45d5 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -25,6 +25,9 @@
#include "block/qapi.h"
#include "block/block_int.h"
#include "qmp-commands.h"
+#include "qapi-visit.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/qmp/types.h"
/*
* Returns 0 on success, with *p_list either set to describe snapshot
@@ -401,6 +404,119 @@ void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f,
}
}
+static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
+ QDict *dict);
+static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
+ QList *list);
+
+static void dump_qobject(fprintf_function func_fprintf, void *f,
+ int comp_indent, QObject *obj)
+{
+ switch (qobject_type(obj)) {
+ case QTYPE_QINT: {
+ QInt *value = qobject_to_qint(obj);
+ func_fprintf(f, "%" PRId64, qint_get_int(value));
+ break;
+ }
+ case QTYPE_QSTRING: {
+ QString *value = qobject_to_qstring(obj);
+ func_fprintf(f, "%s", qstring_get_str(value));
+ break;
+ }
+ case QTYPE_QDICT: {
+ QDict *value = qobject_to_qdict(obj);
+ dump_qdict(func_fprintf, f, comp_indent, value);
+ break;
+ }
+ case QTYPE_QLIST: {
+ QList *value = qobject_to_qlist(obj);
+ dump_qlist(func_fprintf, f, comp_indent, value);
+ break;
+ }
+ case QTYPE_QFLOAT: {
+ QFloat *value = qobject_to_qfloat(obj);
+ func_fprintf(f, "%g", qfloat_get_double(value));
+ break;
+ }
+ case QTYPE_QBOOL: {
+ QBool *value = qobject_to_qbool(obj);
+ func_fprintf(f, "%s", qbool_get_int(value) ? "true" : "false");
+ break;
+ }
+ case QTYPE_QERROR: {
+ QString *value = qerror_human((QError *)obj);
+ func_fprintf(f, "%s", qstring_get_str(value));
+ break;
+ }
+ case QTYPE_NONE:
+ break;
+ case QTYPE_MAX:
+ default:
+ abort();
+ }
+}
+
+static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
+ QList *list)
+{
+ const QListEntry *entry;
+ int i = 0;
+
+ for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
+ qtype_code type = qobject_type(entry->value);
+ bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
+ const char *format = composite ? "%*s[%i]:\n" : "%*s[%i]: ";
+
+ func_fprintf(f, format, indentation * 4, "", i);
+ dump_qobject(func_fprintf, f, indentation + 1, entry->value);
+ if (!composite) {
+ func_fprintf(f, "\n");
+ }
+ }
+}
+
+static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
+ QDict *dict)
+{
+ const QDictEntry *entry;
+
+ for (entry = qdict_first(dict); entry; entry = qdict_next(dict, entry)) {
+ qtype_code type = qobject_type(entry->value);
+ bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
+ const char *format = composite ? "%*s%s:\n" : "%*s%s: ";
+ char key[strlen(entry->key) + 1];
+ int i;
+
+ /* replace dashes with spaces in key (variable) names */
+ for (i = 0; entry->key[i]; i++) {
+ key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
+ }
+ key[i] = 0;
+
+ func_fprintf(f, format, indentation * 4, "", key);
+ dump_qobject(func_fprintf, f, indentation + 1, entry->value);
+ if (!composite) {
+ func_fprintf(f, "\n");
+ }
+ }
+}
+
+void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f,
+ ImageInfoSpecific *info_spec)
+{
+ Error *local_err = NULL;
+ QmpOutputVisitor *ov = qmp_output_visitor_new();
+ QObject *obj, *data;
+
+ visit_type_ImageInfoSpecific(qmp_output_get_visitor(ov), &info_spec, NULL,
+ &local_err);
+ obj = qmp_output_get_qobject(ov);
+ assert(qobject_type(obj) == QTYPE_QDICT);
+ data = qdict_get(qobject_to_qdict(obj), "data");
+ dump_qobject(func_fprintf, f, 0, data);
+ qmp_output_visitor_cleanup(ov);
+}
+
void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
ImageInfo *info)
{
@@ -471,4 +587,9 @@ void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
func_fprintf(f, "\n");
}
}
+
+ if (info->has_format_specific) {
+ func_fprintf(f, "Format specific information:\n");
+ bdrv_image_info_specific_dump(func_fprintf, f, info->format_specific);
+ }
}
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 0496cc9..9518ee4 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -42,6 +42,8 @@ BlockStats *bdrv_query_stats(const BlockDriverState *bs);
void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f,
QEMUSnapshotInfo *sn);
+void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f,
+ ImageInfoSpecific *info_spec);
void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
ImageInfo *info);
#endif
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 563dd40..d90ae4a 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -10,6 +10,7 @@
#include "qemu-io.h"
#include "block/block_int.h"
+#include "block/qapi.h"
#include "qemu/main-loop.h"
#define CMD_NOFILE_OK 0x01
@@ -1700,6 +1701,8 @@ static int info_f(BlockDriverState *bs, int argc, char **argv)
printf("vm state offset: %s\n", s2);
if (bdi.format_specific) {
+ puts("Format specific information:");
+ bdrv_image_info_specific_dump(fprintf, stdout, bdi.format_specific);
qapi_free_ImageInfoSpecific(bdi.format_specific);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v2 3/6] block/qapi: Human-readable ImageInfoSpecific dump
2013-09-06 13:12 ` [Qemu-devel] [PATCH v2 3/6] block/qapi: Human-readable ImageInfoSpecific dump Max Reitz
@ 2013-09-10 4:04 ` Fam Zheng
2013-09-10 7:24 ` Max Reitz
0 siblings, 1 reply; 15+ messages in thread
From: Fam Zheng @ 2013-09-10 4:04 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
On Fri, 09/06 15:12, Max Reitz wrote:
> Add a function for generically dumping the ImageInfoSpecific information
> in a human-readable format to block/qapi.c.
>
> Use this function in bdrv_image_info_dump and qemu-io-cmds.c:info_f to
> allow qemu-img info resp. qemu-io -c info to print that format specific
> information.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/qapi.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++++++
> include/block/qapi.h | 2 +
> qemu-io-cmds.c | 3 ++
> 3 files changed, 126 insertions(+)
>
> diff --git a/block/qapi.c b/block/qapi.c
> index f13fbd5..4fe45d5 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -25,6 +25,9 @@
> #include "block/qapi.h"
> #include "block/block_int.h"
> #include "qmp-commands.h"
> +#include "qapi-visit.h"
> +#include "qapi/qmp-output-visitor.h"
> +#include "qapi/qmp/types.h"
>
> /*
> * Returns 0 on success, with *p_list either set to describe snapshot
> @@ -401,6 +404,119 @@ void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f,
> }
> }
>
> +static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
> + QDict *dict);
> +static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
> + QList *list);
> +
> +static void dump_qobject(fprintf_function func_fprintf, void *f,
> + int comp_indent, QObject *obj)
> +{
> + switch (qobject_type(obj)) {
> + case QTYPE_QINT: {
> + QInt *value = qobject_to_qint(obj);
> + func_fprintf(f, "%" PRId64, qint_get_int(value));
> + break;
> + }
> + case QTYPE_QSTRING: {
> + QString *value = qobject_to_qstring(obj);
> + func_fprintf(f, "%s", qstring_get_str(value));
> + break;
> + }
> + case QTYPE_QDICT: {
> + QDict *value = qobject_to_qdict(obj);
> + dump_qdict(func_fprintf, f, comp_indent, value);
> + break;
> + }
> + case QTYPE_QLIST: {
> + QList *value = qobject_to_qlist(obj);
> + dump_qlist(func_fprintf, f, comp_indent, value);
> + break;
> + }
> + case QTYPE_QFLOAT: {
> + QFloat *value = qobject_to_qfloat(obj);
> + func_fprintf(f, "%g", qfloat_get_double(value));
> + break;
> + }
> + case QTYPE_QBOOL: {
> + QBool *value = qobject_to_qbool(obj);
> + func_fprintf(f, "%s", qbool_get_int(value) ? "true" : "false");
> + break;
> + }
> + case QTYPE_QERROR: {
> + QString *value = qerror_human((QError *)obj);
> + func_fprintf(f, "%s", qstring_get_str(value));
> + break;
> + }
> + case QTYPE_NONE:
> + break;
> + case QTYPE_MAX:
> + default:
> + abort();
> + }
> +}
> +
> +static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
> + QList *list)
> +{
> + const QListEntry *entry;
> + int i = 0;
> +
> + for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
> + qtype_code type = qobject_type(entry->value);
> + bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
> + const char *format = composite ? "%*s[%i]:\n" : "%*s[%i]: ";
> +
> + func_fprintf(f, format, indentation * 4, "", i);
> + dump_qobject(func_fprintf, f, indentation + 1, entry->value);
> + if (!composite) {
> + func_fprintf(f, "\n");
> + }
> + }
> +}
> +
> +static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
> + QDict *dict)
> +{
> + const QDictEntry *entry;
> +
> + for (entry = qdict_first(dict); entry; entry = qdict_next(dict, entry)) {
> + qtype_code type = qobject_type(entry->value);
> + bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
> + const char *format = composite ? "%*s%s:\n" : "%*s%s: ";
> + char key[strlen(entry->key) + 1];
> + int i;
> +
> + /* replace dashes with spaces in key (variable) names */
> + for (i = 0; entry->key[i]; i++) {
> + key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
> + }
> + key[i] = 0;
> +
> + func_fprintf(f, format, indentation * 4, "", key);
> + dump_qobject(func_fprintf, f, indentation + 1, entry->value);
> + if (!composite) {
> + func_fprintf(f, "\n");
> + }
> + }
> +}
> +
> +void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f,
> + ImageInfoSpecific *info_spec)
> +{
> + Error *local_err = NULL;
> + QmpOutputVisitor *ov = qmp_output_visitor_new();
> + QObject *obj, *data;
> +
> + visit_type_ImageInfoSpecific(qmp_output_get_visitor(ov), &info_spec, NULL,
> + &local_err);
> + obj = qmp_output_get_qobject(ov);
> + assert(qobject_type(obj) == QTYPE_QDICT);
> + data = qdict_get(qobject_to_qdict(obj), "data");
> + dump_qobject(func_fprintf, f, 0, data);
> + qmp_output_visitor_cleanup(ov);
> +}
> +
> void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
> ImageInfo *info)
> {
> @@ -471,4 +587,9 @@ void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
> func_fprintf(f, "\n");
> }
> }
> +
> + if (info->has_format_specific) {
> + func_fprintf(f, "Format specific information:\n");
> + bdrv_image_info_specific_dump(func_fprintf, f, info->format_specific);
> + }
> }
> diff --git a/include/block/qapi.h b/include/block/qapi.h
> index 0496cc9..9518ee4 100644
> --- a/include/block/qapi.h
> +++ b/include/block/qapi.h
> @@ -42,6 +42,8 @@ BlockStats *bdrv_query_stats(const BlockDriverState *bs);
>
> void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f,
> QEMUSnapshotInfo *sn);
> +void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f,
> + ImageInfoSpecific *info_spec);
> void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
> ImageInfo *info);
> #endif
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 563dd40..d90ae4a 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -10,6 +10,7 @@
>
> #include "qemu-io.h"
> #include "block/block_int.h"
> +#include "block/qapi.h"
> #include "qemu/main-loop.h"
>
> #define CMD_NOFILE_OK 0x01
> @@ -1700,6 +1701,8 @@ static int info_f(BlockDriverState *bs, int argc, char **argv)
> printf("vm state offset: %s\n", s2);
>
> if (bdi.format_specific) {
> + puts("Format specific information:");
Could use printf for more consistence.
Fam
> + bdrv_image_info_specific_dump(fprintf, stdout, bdi.format_specific);
> qapi_free_ImageInfoSpecific(bdi.format_specific);
> }
>
> --
> 1.8.3.1
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v2 3/6] block/qapi: Human-readable ImageInfoSpecific dump
2013-09-10 4:04 ` Fam Zheng
@ 2013-09-10 7:24 ` Max Reitz
0 siblings, 0 replies; 15+ messages in thread
From: Max Reitz @ 2013-09-10 7:24 UTC (permalink / raw)
To: famz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
On 2013-09-10 06:04, Fam Zheng wrote:
> On Fri, 09/06 15:12, Max Reitz wrote:
>> Add a function for generically dumping the ImageInfoSpecific information
>> in a human-readable format to block/qapi.c.
>>
>> Use this function in bdrv_image_info_dump and qemu-io-cmds.c:info_f to
>> allow qemu-img info resp. qemu-io -c info to print that format specific
>> information.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> block/qapi.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> include/block/qapi.h | 2 +
>> qemu-io-cmds.c | 3 ++
>> 3 files changed, 126 insertions(+)
>>
>> diff --git a/block/qapi.c b/block/qapi.c
>> index f13fbd5..4fe45d5 100644
>> --- a/block/qapi.c
>> +++ b/block/qapi.c
>> @@ -25,6 +25,9 @@
>> #include "block/qapi.h"
>> #include "block/block_int.h"
>> #include "qmp-commands.h"
>> +#include "qapi-visit.h"
>> +#include "qapi/qmp-output-visitor.h"
>> +#include "qapi/qmp/types.h"
>>
>> /*
>> * Returns 0 on success, with *p_list either set to describe snapshot
>> @@ -401,6 +404,119 @@ void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f,
>> }
>> }
>>
>> +static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
>> + QDict *dict);
>> +static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
>> + QList *list);
>> +
>> +static void dump_qobject(fprintf_function func_fprintf, void *f,
>> + int comp_indent, QObject *obj)
>> +{
>> + switch (qobject_type(obj)) {
>> + case QTYPE_QINT: {
>> + QInt *value = qobject_to_qint(obj);
>> + func_fprintf(f, "%" PRId64, qint_get_int(value));
>> + break;
>> + }
>> + case QTYPE_QSTRING: {
>> + QString *value = qobject_to_qstring(obj);
>> + func_fprintf(f, "%s", qstring_get_str(value));
>> + break;
>> + }
>> + case QTYPE_QDICT: {
>> + QDict *value = qobject_to_qdict(obj);
>> + dump_qdict(func_fprintf, f, comp_indent, value);
>> + break;
>> + }
>> + case QTYPE_QLIST: {
>> + QList *value = qobject_to_qlist(obj);
>> + dump_qlist(func_fprintf, f, comp_indent, value);
>> + break;
>> + }
>> + case QTYPE_QFLOAT: {
>> + QFloat *value = qobject_to_qfloat(obj);
>> + func_fprintf(f, "%g", qfloat_get_double(value));
>> + break;
>> + }
>> + case QTYPE_QBOOL: {
>> + QBool *value = qobject_to_qbool(obj);
>> + func_fprintf(f, "%s", qbool_get_int(value) ? "true" : "false");
>> + break;
>> + }
>> + case QTYPE_QERROR: {
>> + QString *value = qerror_human((QError *)obj);
>> + func_fprintf(f, "%s", qstring_get_str(value));
>> + break;
>> + }
>> + case QTYPE_NONE:
>> + break;
>> + case QTYPE_MAX:
>> + default:
>> + abort();
>> + }
>> +}
>> +
>> +static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
>> + QList *list)
>> +{
>> + const QListEntry *entry;
>> + int i = 0;
>> +
>> + for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
>> + qtype_code type = qobject_type(entry->value);
>> + bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
>> + const char *format = composite ? "%*s[%i]:\n" : "%*s[%i]: ";
>> +
>> + func_fprintf(f, format, indentation * 4, "", i);
>> + dump_qobject(func_fprintf, f, indentation + 1, entry->value);
>> + if (!composite) {
>> + func_fprintf(f, "\n");
>> + }
>> + }
>> +}
>> +
>> +static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
>> + QDict *dict)
>> +{
>> + const QDictEntry *entry;
>> +
>> + for (entry = qdict_first(dict); entry; entry = qdict_next(dict, entry)) {
>> + qtype_code type = qobject_type(entry->value);
>> + bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
>> + const char *format = composite ? "%*s%s:\n" : "%*s%s: ";
>> + char key[strlen(entry->key) + 1];
>> + int i;
>> +
>> + /* replace dashes with spaces in key (variable) names */
>> + for (i = 0; entry->key[i]; i++) {
>> + key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
>> + }
>> + key[i] = 0;
>> +
>> + func_fprintf(f, format, indentation * 4, "", key);
>> + dump_qobject(func_fprintf, f, indentation + 1, entry->value);
>> + if (!composite) {
>> + func_fprintf(f, "\n");
>> + }
>> + }
>> +}
>> +
>> +void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f,
>> + ImageInfoSpecific *info_spec)
>> +{
>> + Error *local_err = NULL;
>> + QmpOutputVisitor *ov = qmp_output_visitor_new();
>> + QObject *obj, *data;
>> +
>> + visit_type_ImageInfoSpecific(qmp_output_get_visitor(ov), &info_spec, NULL,
>> + &local_err);
>> + obj = qmp_output_get_qobject(ov);
>> + assert(qobject_type(obj) == QTYPE_QDICT);
>> + data = qdict_get(qobject_to_qdict(obj), "data");
>> + dump_qobject(func_fprintf, f, 0, data);
>> + qmp_output_visitor_cleanup(ov);
>> +}
>> +
>> void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
>> ImageInfo *info)
>> {
>> @@ -471,4 +587,9 @@ void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
>> func_fprintf(f, "\n");
>> }
>> }
>> +
>> + if (info->has_format_specific) {
>> + func_fprintf(f, "Format specific information:\n");
>> + bdrv_image_info_specific_dump(func_fprintf, f, info->format_specific);
>> + }
>> }
>> diff --git a/include/block/qapi.h b/include/block/qapi.h
>> index 0496cc9..9518ee4 100644
>> --- a/include/block/qapi.h
>> +++ b/include/block/qapi.h
>> @@ -42,6 +42,8 @@ BlockStats *bdrv_query_stats(const BlockDriverState *bs);
>>
>> void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f,
>> QEMUSnapshotInfo *sn);
>> +void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f,
>> + ImageInfoSpecific *info_spec);
>> void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
>> ImageInfo *info);
>> #endif
>> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
>> index 563dd40..d90ae4a 100644
>> --- a/qemu-io-cmds.c
>> +++ b/qemu-io-cmds.c
>> @@ -10,6 +10,7 @@
>>
>> #include "qemu-io.h"
>> #include "block/block_int.h"
>> +#include "block/qapi.h"
>> #include "qemu/main-loop.h"
>>
>> #define CMD_NOFILE_OK 0x01
>> @@ -1700,6 +1701,8 @@ static int info_f(BlockDriverState *bs, int argc, char **argv)
>> printf("vm state offset: %s\n", s2);
>>
>> if (bdi.format_specific) {
>> + puts("Format specific information:");
> Could use printf for more consistence.
Since I'll be resubmitting this series because of patch 2 anyway - I'll
change it.
> Fam
>
>> + bdrv_image_info_specific_dump(fprintf, stdout, bdi.format_specific);
>> qapi_free_ImageInfoSpecific(bdi.format_specific);
>> }
>>
>> --
>> 1.8.3.1
>>
>>
Max
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v2 4/6] qcow2: Add support for ImageInfoSpecific
2013-09-06 13:12 [Qemu-devel] [PATCH v2 0/6] Provide additional info through qemu-img info Max Reitz
` (2 preceding siblings ...)
2013-09-06 13:12 ` [Qemu-devel] [PATCH v2 3/6] block/qapi: Human-readable ImageInfoSpecific dump Max Reitz
@ 2013-09-06 13:12 ` Max Reitz
2013-09-06 13:12 ` [Qemu-devel] [PATCH v2 5/6] qemu-iotests: Discard specific info in _img_info Max Reitz
2013-09-06 13:12 ` [Qemu-devel] [PATCH v2 6/6] qemu-iotests: Additional info from qemu-img info Max Reitz
5 siblings, 0 replies; 15+ messages in thread
From: Max Reitz @ 2013-09-06 13:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add a new ImageInfoSpecificQCow2 type as a subtype of ImageInfoSpecific.
This contains the compatibility level as a string and an optional
lazy_refcounts boolean (optional means mandatory for compat >= 1.1 and
not available for compat == 0.10).
In qcow2_get_info, fill the BlockDriverInfo.format_specific field with
that information.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2.c | 12 ++++++++++++
qapi-schema.json | 16 ++++++++++++++++
2 files changed, 28 insertions(+)
diff --git a/block/qcow2.c b/block/qcow2.c
index 4bc679a..e088c0a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1755,8 +1755,20 @@ static int64_t qcow2_vm_state_offset(BDRVQcowState *s)
static int qcow2_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
{
BDRVQcowState *s = bs->opaque;
+
bdi->cluster_size = s->cluster_size;
bdi->vm_state_offset = qcow2_vm_state_offset(s);
+
+ bdi->format_specific = g_new0(ImageInfoSpecific, 1);
+ bdi->format_specific->kind = IMAGE_INFO_SPECIFIC_KIND_QCOW2;
+ bdi->format_specific->qcow2 = g_new0(ImageInfoSpecificQCow2, 1);
+ if (s->qcow_version == 2) {
+ bdi->format_specific->qcow2->compat = g_strdup("0.10");
+ } else if (s->qcow_version == 3) {
+ bdi->format_specific->qcow2->compat = g_strdup("1.1");
+ bdi->format_specific->qcow2->lazy_refcounts = s->use_lazy_refcounts;
+ bdi->format_specific->qcow2->has_lazy_refcounts = true;
+ }
return 0;
}
diff --git a/qapi-schema.json b/qapi-schema.json
index eebf851..cadf40b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -210,6 +210,21 @@
'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }
##
+# @ImageInfoSpecificQCow2:
+#
+# @compat: compatibility level
+#
+# @lazy-refcounts: #optional on or off; only valid for compat >= 1.1
+#
+# Since: 1.7
+##
+{ 'type': 'ImageInfoSpecificQCow2',
+ 'data': {
+ 'compat': 'str',
+ '*lazy-refcounts': 'bool'
+ } }
+
+##
# @ImageInfoSpecific:
#
# A discriminated record of image format specific information structures.
@@ -219,6 +234,7 @@
{ 'union': 'ImageInfoSpecific',
'data': {
+ 'qcow2': 'ImageInfoSpecificQCow2'
} }
##
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH v2 5/6] qemu-iotests: Discard specific info in _img_info
2013-09-06 13:12 [Qemu-devel] [PATCH v2 0/6] Provide additional info through qemu-img info Max Reitz
` (3 preceding siblings ...)
2013-09-06 13:12 ` [Qemu-devel] [PATCH v2 4/6] qcow2: Add support for ImageInfoSpecific Max Reitz
@ 2013-09-06 13:12 ` Max Reitz
2013-09-06 13:12 ` [Qemu-devel] [PATCH v2 6/6] qemu-iotests: Additional info from qemu-img info Max Reitz
5 siblings, 0 replies; 15+ messages in thread
From: Max Reitz @ 2013-09-06 13:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
In _img_info, filter out additional information specific to the image
format provided by qemu-img info, since tests designed for multiple
image formats would produce different outputs for every image format
else.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/common.rc | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 5e077c3..13f62d8 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -181,12 +181,29 @@ _check_test_img()
_img_info()
{
+ discard=0
$QEMU_IMG info "$@" $TEST_IMG 2>&1 | \
sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
-e "s#$TEST_DIR#TEST_DIR#g" \
-e "s#$IMGFMT#IMGFMT#g" \
-e "/^disk size:/ D" \
- -e "/actual-size/ D"
+ -e "/actual-size/ D" | \
+ while IFS='' read line; do
+ if [ "$line" == "Format specific information:" ]; then
+ discard=1
+ elif [ "`echo "$line" | sed -e 's/^ *//'`" == '"format-specific": {' ]; then
+ discard=2
+ json_indent="`echo "$line" | sed -e 's/^\( *\).*$/\1/'`"
+ fi
+ if [ $discard == 0 ]; then
+ echo "$line"
+ elif [ $discard == 1 -a -z "$line" ]; then
+ echo
+ discard=0
+ elif [ $discard == 2 -a "`echo "$line" | sed -e 's/ *$//'`" == "${json_indent}}," ]; then
+ discard=0
+ fi
+ done
}
_get_pids_by_name()
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH v2 6/6] qemu-iotests: Additional info from qemu-img info
2013-09-06 13:12 [Qemu-devel] [PATCH v2 0/6] Provide additional info through qemu-img info Max Reitz
` (4 preceding siblings ...)
2013-09-06 13:12 ` [Qemu-devel] [PATCH v2 5/6] qemu-iotests: Discard specific info in _img_info Max Reitz
@ 2013-09-06 13:12 ` Max Reitz
5 siblings, 0 replies; 15+ messages in thread
From: Max Reitz @ 2013-09-06 13:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add a test for the additional information now provided by qemu-img info
when used on qcow2 images.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/064 | 72 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/064.out | 22 ++++++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 95 insertions(+)
create mode 100755 tests/qemu-iotests/064
create mode 100644 tests/qemu-iotests/064.out
diff --git a/tests/qemu-iotests/064 b/tests/qemu-iotests/064
new file mode 100755
index 0000000..4979db5
--- /dev/null
+++ b/tests/qemu-iotests/064
@@ -0,0 +1,72 @@
+#!/bin/bash
+#
+# Test for additional information emitted by qemu-img info on qcow2
+# images
+#
+# Copyright (C) 2013 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# This tests qocw2-specific low-level functionality
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+IMG_SIZE=64M
+
+echo
+echo "=== Testing qcow2 image with -o compat=0.10 ==="
+echo
+IMGOPTS="compat=0.10" _make_test_img $IMG_SIZE
+# don't use _img_info, since that function will filter out the
+# additional information we're about to test for
+$QEMU_IMG info "$TEST_IMG" | grep "Format specific information:" -A 42
+
+echo
+echo "=== Testing qcow2 image with -o compat=1.1,lazy_refcounts=off ==="
+echo
+IMGOPTS="compat=1.1,lazy_refcounts=off" _make_test_img $IMG_SIZE
+$QEMU_IMG info "$TEST_IMG" | grep "Format specific information:" -A 42
+
+echo
+echo "=== Testing qcow2 image with -o compat=1.1,lazy_refcounts=on ==="
+echo
+IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img $IMG_SIZE
+$QEMU_IMG info "$TEST_IMG" | grep "Format specific information:" -A 42
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/064.out b/tests/qemu-iotests/064.out
new file mode 100644
index 0000000..6ce5b43
--- /dev/null
+++ b/tests/qemu-iotests/064.out
@@ -0,0 +1,22 @@
+QA output created by 064
+
+=== Testing qcow2 image with -o compat=0.10 ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+Format specific information:
+compat: 0.10
+
+=== Testing qcow2 image with -o compat=1.1,lazy_refcounts=off ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+Format specific information:
+compat: 1.1
+lazy refcounts: false
+
+=== Testing qcow2 image with -o compat=1.1,lazy_refcounts=on ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+Format specific information:
+compat: 1.1
+lazy refcounts: true
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index b696242..740cd84 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -66,3 +66,4 @@
059 rw auto
060 rw auto
062 rw auto
+064 rw auto
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread