It's useful to know how much space can be occupied by qcow2 persistent
bitmaps, even though such metadata is unrelated to the guest-visible
data. Report this value as an additional field. Update iotest 190 to
cover it and a portion of the just-added qemu-img bitmap command.
The addition of a new field demonstrates why we should always
zero-initialize qapi C structs; while the qcow2 driver still fully
populates all fields, the raw and crypto drivers had to be tweaked.
See also: https://bugzilla.redhat.com/1779904
Reported-by: Nir Soffer <nsoffer@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
qapi/block-core.json | 15 ++++++++++-----
block/crypto.c | 2 +-
block/qcow2.c | 29 ++++++++++++++++++++++++++++-
block/raw-format.c | 2 +-
qemu-img.c | 3 +++
tests/qemu-iotests/190 | 15 +++++++++++++--
tests/qemu-iotests/190.out | 13 ++++++++++++-
7 files changed, 68 insertions(+), 11 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 943df1926a91..b47c6d69ba27 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -633,18 +633,23 @@
# efficiently so file size may be smaller than virtual disk size.
#
# The values are upper bounds that are guaranteed to fit the new image file.
-# Subsequent modification, such as internal snapshot or bitmap creation, may
-# require additional space and is not covered here.
+# Subsequent modification, such as internal snapshot or further bitmap
+# creation, may require additional space and is not covered here.
#
-# @required: Size required for a new image file, in bytes.
+# @required: Size required for a new image file, in bytes, when copying just
+# guest-visible contents.
#
# @fully-allocated: Image file size, in bytes, once data has been written
-# to all sectors.
+# to all sectors, when copying just guest-visible contents.
+#
+# @bitmaps: Additional size required for bitmap metadata not directly used
+# for guest contents, when that metadata can be copied in addition
+# to guest contents. (since 5.1)
#
# Since: 2.10
##
{ 'struct': 'BlockMeasureInfo',
- 'data': {'required': 'int', 'fully-allocated': 'int'} }
+ 'data': {'required': 'int', 'fully-allocated': 'int', '*bitmaps': 'int'} }
##
# @query-block:
diff --git a/block/crypto.c b/block/crypto.c
index d577f89659fa..4e0f3ec97f0e 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -535,7 +535,7 @@ static BlockMeasureInfo *block_crypto_measure(QemuOpts *opts,
* Unallocated blocks are still encrypted so allocation status makes no
* difference to the file size.
*/
- info = g_new(BlockMeasureInfo, 1);
+ info = g_new0(BlockMeasureInfo, 1);
info->fully_allocated = luks_payload_size + size;
info->required = luks_payload_size + size;
return info;
diff --git a/block/qcow2.c b/block/qcow2.c
index b524b0c53f84..9fd650928016 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4657,6 +4657,7 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
PreallocMode prealloc;
bool has_backing_file;
bool has_luks;
+ uint64_t bitmaps_size = 0; /* size occupied by bitmaps in in_bs */
/* Parse image creation options */
cluster_size = qcow2_opt_get_cluster_size_del(opts, &local_err);
@@ -4732,6 +4733,8 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
/* Account for input image */
if (in_bs) {
+ BdrvDirtyBitmap *bm;
+ size_t bitmap_overhead = 0;
int64_t ssize = bdrv_getlength(in_bs);
if (ssize < 0) {
error_setg_errno(&local_err, -ssize,
@@ -4739,6 +4742,28 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
goto err;
}
+ FOR_EACH_DIRTY_BITMAP(in_bs, bm) {
+ if (bdrv_dirty_bitmap_get_persistence(bm)) {
+ const char *name = bdrv_dirty_bitmap_name(bm);
+ uint32_t granularity = bdrv_dirty_bitmap_granularity(bm);
+ uint64_t bmbits = DIV_ROUND_UP(bdrv_dirty_bitmap_size(bm),
+ granularity);
+ uint64_t bmclusters = DIV_ROUND_UP(DIV_ROUND_UP(bmbits,
+ CHAR_BIT),
+ cluster_size);
+
+ /* Assume the entire bitmap is allocated */
+ bitmaps_size += bmclusters * cluster_size;
+ /* Also reserve space for the bitmap table entries */
+ bitmaps_size += ROUND_UP(bmclusters * sizeof(uint64_t),
+ cluster_size);
+ /* Guess at contribution to bitmap directory size */
+ bitmap_overhead += ROUND_UP(strlen(name) + 24,
+ sizeof(uint64_t));
+ }
+ }
+ bitmaps_size += ROUND_UP(bitmap_overhead, cluster_size);
+
virtual_size = ROUND_UP(ssize, cluster_size);
if (has_backing_file) {
@@ -4785,7 +4810,7 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
required = virtual_size;
}
- info = g_new(BlockMeasureInfo, 1);
+ info = g_new0(BlockMeasureInfo, 1);
info->fully_allocated =
qcow2_calc_prealloc_size(virtual_size, cluster_size,
ctz32(refcount_bits)) + luks_payload_size;
@@ -4795,6 +4820,8 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
* still counted.
*/
info->required = info->fully_allocated - virtual_size + required;
+ info->has_bitmaps = !!bitmaps_size;
+ info->bitmaps = bitmaps_size;
return info;
err:
diff --git a/block/raw-format.c b/block/raw-format.c
index 93b25e1b6b0b..4bb54f4ac6c5 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -346,7 +346,7 @@ static BlockMeasureInfo *raw_measure(QemuOpts *opts, BlockDriverState *in_bs,
BDRV_SECTOR_SIZE);
}
- info = g_new(BlockMeasureInfo, 1);
+ info = g_new0(BlockMeasureInfo, 1);
info->required = required;
/* Unallocated sectors count towards the file size in raw images */
diff --git a/qemu-img.c b/qemu-img.c
index 02ebd870faa1..e1127273f21e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -5207,6 +5207,9 @@ static int img_measure(int argc, char **argv)
if (output_format == OFORMAT_HUMAN) {
printf("required size: %" PRIu64 "\n", info->required);
printf("fully allocated size: %" PRIu64 "\n", info->fully_allocated);
+ if (info->has_bitmaps) {
+ printf("bitmaps size: %" PRIu64 "\n", info->bitmaps);
+ }
} else {
dump_json_block_measure_info(info);
}
diff --git a/tests/qemu-iotests/190 b/tests/qemu-iotests/190
index 6d41650438e1..cae643149a01 100755
--- a/tests/qemu-iotests/190
+++ b/tests/qemu-iotests/190
@@ -2,7 +2,7 @@
#
# qemu-img measure sub-command tests on huge qcow2 files
#
-# Copyright (C) 2017 Red Hat, Inc.
+# Copyright (C) 2017-2020 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
@@ -42,7 +42,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_fmt qcow2
_supported_proto file
-echo "== Huge file =="
+echo "== Huge file without bitmaps =="
echo
_make_test_img -o 'cluster_size=2M' 2T
@@ -51,6 +51,17 @@ $QEMU_IMG measure -O raw -f qcow2 "$TEST_IMG"
$QEMU_IMG measure -O qcow2 -o cluster_size=64k -f qcow2 "$TEST_IMG"
$QEMU_IMG measure -O qcow2 -o cluster_size=2M -f qcow2 "$TEST_IMG"
+echo
+echo "== Huge file with bitmaps =="
+echo
+
+$QEMU_IMG bitmap --add --granularity 512 -f qcow2 "$TEST_IMG" b1
+$QEMU_IMG bitmap --add -g 2M -f qcow2 "$TEST_IMG" b2
+
+$QEMU_IMG measure -O raw -f qcow2 "$TEST_IMG"
+$QEMU_IMG measure -O qcow2 -o cluster_size=64k -f qcow2 "$TEST_IMG"
+$QEMU_IMG measure -O qcow2 -o cluster_size=2M -f qcow2 "$TEST_IMG"
+
# success, all done
echo "*** done"
rm -f $seq.full
diff --git a/tests/qemu-iotests/190.out b/tests/qemu-iotests/190.out
index d001942002db..11962f972429 100644
--- a/tests/qemu-iotests/190.out
+++ b/tests/qemu-iotests/190.out
@@ -1,5 +1,5 @@
QA output created by 190
-== Huge file ==
+== Huge file without bitmaps ==
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2199023255552
required size: 2199023255552
@@ -8,4 +8,15 @@ required size: 335806464
fully allocated size: 2199359062016
required size: 18874368
fully allocated size: 2199042129920
+
+== Huge file with bitmaps ==
+
+required size: 2199023255552
+fully allocated size: 2199023255552
+required size: 335806464
+fully allocated size: 2199359062016
+bitmaps size: 537198592
+required size: 18874368
+fully allocated size: 2199042129920
+bitmaps size: 545259520
*** done
--
2.26.2
On 21.04.20 23:20, Eric Blake wrote:
> It's useful to know how much space can be occupied by qcow2 persistent
> bitmaps, even though such metadata is unrelated to the guest-visible
> data. Report this value as an additional field. Update iotest 190 to
> cover it and a portion of the just-added qemu-img bitmap command.
>
> The addition of a new field demonstrates why we should always
> zero-initialize qapi C structs; while the qcow2 driver still fully
> populates all fields, the raw and crypto drivers had to be tweaked.
>
> See also: https://bugzilla.redhat.com/1779904
>
> Reported-by: Nir Soffer <nsoffer@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> qapi/block-core.json | 15 ++++++++++-----
> block/crypto.c | 2 +-
> block/qcow2.c | 29 ++++++++++++++++++++++++++++-
> block/raw-format.c | 2 +-
> qemu-img.c | 3 +++
> tests/qemu-iotests/190 | 15 +++++++++++++--
> tests/qemu-iotests/190.out | 13 ++++++++++++-
> 7 files changed, 68 insertions(+), 11 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 943df1926a91..b47c6d69ba27 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -633,18 +633,23 @@
> # efficiently so file size may be smaller than virtual disk size.
> #
> # The values are upper bounds that are guaranteed to fit the new image file.
> -# Subsequent modification, such as internal snapshot or bitmap creation, may
> -# require additional space and is not covered here.
> +# Subsequent modification, such as internal snapshot or further bitmap
> +# creation, may require additional space and is not covered here.
> #
> -# @required: Size required for a new image file, in bytes.
> +# @required: Size required for a new image file, in bytes, when copying just
> +# guest-visible contents.
> #
> # @fully-allocated: Image file size, in bytes, once data has been written
> -# to all sectors.
> +# to all sectors, when copying just guest-visible contents.
> +#
> +# @bitmaps: Additional size required for bitmap metadata not directly used
> +# for guest contents,
Not sure how I feel about the “not directly used for guest contents”,
because it feels a bit superfluous, and it sounds like there might be
bitmap data that is directly used for guest contents.
> when that metadata can be copied in addition
> +# to guest contents. (since 5.1)
> #
> # Since: 2.10
> ##
> { 'struct': 'BlockMeasureInfo',
> - 'data': {'required': 'int', 'fully-allocated': 'int'} }
> + 'data': {'required': 'int', 'fully-allocated': 'int', '*bitmaps': 'int'} }
Why is @bitmaps optional? I.e., what does absence mean, besides “not
supported yet”?
Right now, absence means anything in “format doesn’t support bitmaps, so
nothing can be copied”, “no input image given, so there’s no data on
bitmaps”, to “0 bytes to copy”.
I think in the latter case we should emit it as 0, maybe even in the
former case, because I think the fact that there won’t be any bitmap
data to copy might be interesting. (And it’s also definitely 0, not
just “don’t know”.)
I suppose absence does make sense in case the user didn’t specify an
input image, because then we just really don’t know.
[...]
> diff --git a/block/qcow2.c b/block/qcow2.c
> index b524b0c53f84..9fd650928016 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
[...]
> @@ -4739,6 +4742,28 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
> goto err;
> }
>
> + FOR_EACH_DIRTY_BITMAP(in_bs, bm) {
> + if (bdrv_dirty_bitmap_get_persistence(bm)) {
> + const char *name = bdrv_dirty_bitmap_name(bm);
> + uint32_t granularity = bdrv_dirty_bitmap_granularity(bm);
> + uint64_t bmbits = DIV_ROUND_UP(bdrv_dirty_bitmap_size(bm),
> + granularity);
> + uint64_t bmclusters = DIV_ROUND_UP(DIV_ROUND_UP(bmbits,
> + CHAR_BIT),
I suppose if we allowed CHAR_BIT to be anything but 8, it would be wrong
to use it here. So maybe just a plain 8 would be more correct; although
I suppose CHAR_BIT kind of describes what constant we want here.
> + cluster_size);
> +
> + /* Assume the entire bitmap is allocated */
> + bitmaps_size += bmclusters * cluster_size;
> + /* Also reserve space for the bitmap table entries */
> + bitmaps_size += ROUND_UP(bmclusters * sizeof(uint64_t),
> + cluster_size);
> + /* Guess at contribution to bitmap directory size */
> + bitmap_overhead += ROUND_UP(strlen(name) + 24,
> + sizeof(uint64_t));
Seems not just a guess to me, but actually correct. Maybe
bitmap_overhead should be called bitmap_directory_size (or
bitmap_dir_size, or bmap_dir_size, or whatever (but probably not bds!
:)), because that’s what it is.
> + }
> + }
> + bitmaps_size += ROUND_UP(bitmap_overhead, cluster_size);
> +
> virtual_size = ROUND_UP(ssize, cluster_size);
>
> if (has_backing_file) {
[...]
> diff --git a/tests/qemu-iotests/190.out b/tests/qemu-iotests/190.out
> index d001942002db..11962f972429 100644
> --- a/tests/qemu-iotests/190.out
> +++ b/tests/qemu-iotests/190.out
> @@ -1,5 +1,5 @@
> QA output created by 190
> -== Huge file ==
> +== Huge file without bitmaps ==
>
> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2199023255552
> required size: 2199023255552
> @@ -8,4 +8,15 @@ required size: 335806464
> fully allocated size: 2199359062016
> required size: 18874368
> fully allocated size: 2199042129920
> +
> +== Huge file with bitmaps ==
> +
> +required size: 2199023255552
> +fully allocated size: 2199023255552
> +required size: 335806464
> +fully allocated size: 2199359062016
> +bitmaps size: 537198592
> +required size: 18874368
> +fully allocated size: 2199042129920
> +bitmaps size: 545259520
Looks correct.
(It might be nicer to calculate the reference value in the script,
because this way I had to verify it by hand, but, well, now I did verify
it, so...)
Max
On 5/4/20 6:36 AM, Max Reitz wrote:
> On 21.04.20 23:20, Eric Blake wrote:
>> It's useful to know how much space can be occupied by qcow2 persistent
>> bitmaps, even though such metadata is unrelated to the guest-visible
>> data. Report this value as an additional field. Update iotest 190 to
>> cover it and a portion of the just-added qemu-img bitmap command.
>>
>> The addition of a new field demonstrates why we should always
>> zero-initialize qapi C structs; while the qcow2 driver still fully
>> populates all fields, the raw and crypto drivers had to be tweaked.
>>
>> See also: https://bugzilla.redhat.com/1779904
>>
>> Reported-by: Nir Soffer <nsoffer@redhat.com>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>> # @fully-allocated: Image file size, in bytes, once data has been written
>> -# to all sectors.
>> +# to all sectors, when copying just guest-visible contents.
>> +#
>> +# @bitmaps: Additional size required for bitmap metadata not directly used
>> +# for guest contents,
>
> Not sure how I feel about the “not directly used for guest contents”,
> because it feels a bit superfluous, and it sounds like there might be
> bitmap data that is directly used for guest contents.
Hmm. I was trying to make it obvious that bitmap size is separate from
fully-allocated (and not double-counted), but you may have a point that
just using "Additional size required for bitmap metadata, when that
metadata can be copied in addition..." would work.
>
>> when that metadata can be copied in addition
>> +# to guest contents. (since 5.1)
>> #
>> # Since: 2.10
>> ##
>> { 'struct': 'BlockMeasureInfo',
>> - 'data': {'required': 'int', 'fully-allocated': 'int'} }
>> + 'data': {'required': 'int', 'fully-allocated': 'int', '*bitmaps': 'int'} }
>
> Why is @bitmaps optional? I.e., what does absence mean, besides “not
> supported yet”?
>
> Right now, absence means anything in “format doesn’t support bitmaps, so
> nothing can be copied”, “no input image given, so there’s no data on
> bitmaps”, to “0 bytes to copy”.
>
> I think in the latter case we should emit it as 0, maybe even in the
> former case, because I think the fact that there won’t be any bitmap
> data to copy might be interesting. (And it’s also definitely 0, not
> just “don’t know”.)
>
> I suppose absence does make sense in case the user didn’t specify an
> input image, because then we just really don’t know.
The patch will require a tweak to report an explicit 0 (when there is
nothing to be copied), but doing that makes sense (explicit 0 for v3
qcow2 images, and maybe even for v2, but omitted for other formats that
have no bitmap support).
>> @@ -4739,6 +4742,28 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
>> goto err;
>> }
>>
>> + FOR_EACH_DIRTY_BITMAP(in_bs, bm) {
>> + if (bdrv_dirty_bitmap_get_persistence(bm)) {
>> + const char *name = bdrv_dirty_bitmap_name(bm);
>> + uint32_t granularity = bdrv_dirty_bitmap_granularity(bm);
>> + uint64_t bmbits = DIV_ROUND_UP(bdrv_dirty_bitmap_size(bm),
>> + granularity);
>> + uint64_t bmclusters = DIV_ROUND_UP(DIV_ROUND_UP(bmbits,
>> + CHAR_BIT),
>
> I suppose if we allowed CHAR_BIT to be anything but 8, it would be wrong
> to use it here. So maybe just a plain 8 would be more correct; although
> I suppose CHAR_BIT kind of describes what constant we want here.
POSIX requires CHAR_BIT to be 8. (C99 allows some odd machines where
CHAR_BIT is 9, 16, or even 32, but we don't ever try to port to such
machines).
>> +== Huge file with bitmaps ==
>> +
>> +required size: 2199023255552
>> +fully allocated size: 2199023255552
>> +required size: 335806464
>> +fully allocated size: 2199359062016
>> +bitmaps size: 537198592
>> +required size: 18874368
>> +fully allocated size: 2199042129920
>> +bitmaps size: 545259520
>
> Looks correct.
>
> (It might be nicer to calculate the reference value in the script,
> because this way I had to verify it by hand, but, well, now I did verify
> it, so...)
Feels like duplicate work, which would require tweaking in two spots if
we ever change our implementation or the qcow2 format is further
enhanced, but it would also make it obvious that we are aware of the
impact of such future changes. Okay, I'll see what I can add.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
© 2016 - 2026 Red Hat, Inc.