* [Qemu-devel] [PATCH v3 01/22] qcow2: Add two new fields to BDRVQcowState
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-27 13:49 ` Stefan Hajnoczi
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 02/22] qcow2: Add refcount_width to format-specific info Max Reitz
` (20 subsequent siblings)
21 siblings, 1 reply; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add two new fields regarding refcount information (the bit width of
every entry and the maximum refcount value) to the BDRVQcowState.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qcow2-refcount.c | 2 +-
block/qcow2.c | 9 +++++++++
block/qcow2.h | 2 ++
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 9afdb40..6016211 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -584,7 +584,7 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
refcount = be16_to_cpu(refcount_block[block_index]);
refcount += addend;
- if (refcount < 0 || refcount > 0xffff) {
+ if (refcount < 0 || refcount > s->refcount_max) {
ret = -EINVAL;
goto fail;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index d120494..f57aff9 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -684,6 +684,15 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
goto fail;
}
s->refcount_order = header.refcount_order;
+ s->refcount_bits = 1 << s->refcount_order;
+ if (s->refcount_order < 6) {
+ s->refcount_max = (UINT64_C(1) << s->refcount_bits) - 1;
+ } else {
+ /* The above shift would overflow with s->refcount_bits == 64;
+ * furthermore, we do not want to use UINT64_MAX because refcounts will
+ * be passed around in int64_ts (negative values for -errno) */
+ s->refcount_max = INT64_MAX;
+ }
if (header.crypt_method > QCOW_CRYPT_AES) {
error_setg(errp, "Unsupported encryption method: %" PRIu32,
diff --git a/block/qcow2.h b/block/qcow2.h
index 6e39a1b..4d8c902 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -258,6 +258,8 @@ typedef struct BDRVQcowState {
int qcow_version;
bool use_lazy_refcounts;
int refcount_order;
+ int refcount_bits;
+ uint64_t refcount_max;
bool discard_passthrough[QCOW2_DISCARD_MAX];
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 01/22] qcow2: Add two new fields to BDRVQcowState
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 01/22] qcow2: Add two new fields to BDRVQcowState Max Reitz
@ 2014-11-27 13:49 ` Stefan Hajnoczi
0 siblings, 0 replies; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-27 13:49 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 522 bytes --]
On Thu, Nov 20, 2014 at 06:06:17PM +0100, Max Reitz wrote:
> Add two new fields regarding refcount information (the bit width of
> every entry and the maximum refcount value) to the BDRVQcowState.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> block/qcow2-refcount.c | 2 +-
> block/qcow2.c | 9 +++++++++
> block/qcow2.h | 2 ++
> 3 files changed, 12 insertions(+), 1 deletion(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH v3 02/22] qcow2: Add refcount_width to format-specific info
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 01/22] qcow2: Add two new fields to BDRVQcowState Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-27 13:47 ` Stefan Hajnoczi
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 03/22] qcow2: Use 64 bits for refcount values Max Reitz
` (19 subsequent siblings)
21 siblings, 1 reply; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add the bit width of every refcount entry to the format-specific
information.
In contrast to lazy_refcounts and the corrupt flag, this should be
always emitted, even for compat=0.10 although it does not support any
refcount width other than 16 bits. This is because if a boolean is
optional, one normally assumes it to be false when omitted; but if an
integer is not specified, it is rather difficult to guess its value.
This new field breaks some test outputs, fix them.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qcow2.c | 4 +++-
qapi/block-core.json | 5 ++++-
tests/qemu-iotests/060.out | 1 +
tests/qemu-iotests/065 | 23 +++++++++++++++--------
tests/qemu-iotests/067.out | 5 +++++
tests/qemu-iotests/082.out | 7 +++++++
tests/qemu-iotests/089.out | 2 ++
7 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index f57aff9..d70e927 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2475,7 +2475,8 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs)
};
if (s->qcow_version == 2) {
*spec_info->qcow2 = (ImageInfoSpecificQCow2){
- .compat = g_strdup("0.10"),
+ .compat = g_strdup("0.10"),
+ .refcount_width = s->refcount_bits,
};
} else if (s->qcow_version == 3) {
*spec_info->qcow2 = (ImageInfoSpecificQCow2){
@@ -2486,6 +2487,7 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs)
.corrupt = s->incompatible_features &
QCOW2_INCOMPAT_CORRUPT,
.has_corrupt = true,
+ .refcount_width = s->refcount_bits,
};
}
diff --git a/qapi/block-core.json b/qapi/block-core.json
index b7083fb..e3a3cb7 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -41,13 +41,16 @@
# @corrupt: #optional true if the image has been marked corrupt; only valid for
# compat >= 1.1 (since 2.2)
#
+# @refcount-width: width of a refcount entry in bits (since 2.3)
+#
# Since: 1.7
##
{ 'type': 'ImageInfoSpecificQCow2',
'data': {
'compat': 'str',
'*lazy-refcounts': 'bool',
- '*corrupt': 'bool'
+ '*corrupt': 'bool',
+ 'refcount-width': 'int'
} }
##
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index 9419da1..17b3eaf 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -19,6 +19,7 @@ cluster_size: 65536
Format specific information:
compat: 1.1
lazy refcounts: false
+ refcount width: 16
corrupt: true
qemu-io: can't open device TEST_DIR/t.IMGFMT: IMGFMT: Image is corrupt; cannot be opened read/write
read 512/512 bytes at offset 0
diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065
index 8d3a9c9..8539aeb 100755
--- a/tests/qemu-iotests/065
+++ b/tests/qemu-iotests/065
@@ -88,34 +88,41 @@ class TestQMP(TestImageInfoSpecific):
class TestQCow2(TestQemuImgInfo):
'''Testing a qcow2 version 2 image'''
img_options = 'compat=0.10'
- json_compare = { 'compat': '0.10' }
- human_compare = [ 'compat: 0.10' ]
+ json_compare = { 'compat': '0.10', 'refcount-width': 16 }
+ human_compare = [ 'compat: 0.10', 'refcount width: 16' ]
class TestQCow3NotLazy(TestQemuImgInfo):
'''Testing a qcow2 version 3 image with lazy refcounts disabled'''
img_options = 'compat=1.1,lazy_refcounts=off'
- json_compare = { 'compat': '1.1', 'lazy-refcounts': False, 'corrupt': False }
- human_compare = [ 'compat: 1.1', 'lazy refcounts: false', 'corrupt: false' ]
+ json_compare = { 'compat': '1.1', 'lazy-refcounts': False,
+ 'refcount-width': 16, 'corrupt': False }
+ human_compare = [ 'compat: 1.1', 'lazy refcounts: false',
+ 'refcount width: 16', 'corrupt: false' ]
class TestQCow3Lazy(TestQemuImgInfo):
'''Testing a qcow2 version 3 image with lazy refcounts enabled'''
img_options = 'compat=1.1,lazy_refcounts=on'
- json_compare = { 'compat': '1.1', 'lazy-refcounts': True, 'corrupt': False }
- human_compare = [ 'compat: 1.1', 'lazy refcounts: true', 'corrupt: false' ]
+ json_compare = { 'compat': '1.1', 'lazy-refcounts': True,
+ 'refcount-width': 16, 'corrupt': False }
+ human_compare = [ 'compat: 1.1', 'lazy refcounts: true',
+ 'refcount width: 16', 'corrupt: false' ]
class TestQCow3NotLazyQMP(TestQMP):
'''Testing a qcow2 version 3 image with lazy refcounts disabled, opening
with lazy refcounts enabled'''
img_options = 'compat=1.1,lazy_refcounts=off'
qemu_options = 'lazy-refcounts=on'
- compare = { 'compat': '1.1', 'lazy-refcounts': False, 'corrupt': False }
+ compare = { 'compat': '1.1', 'lazy-refcounts': False,
+ 'refcount-width': 16, 'corrupt': False }
+
class TestQCow3LazyQMP(TestQMP):
'''Testing a qcow2 version 3 image with lazy refcounts enabled, opening
with lazy refcounts disabled'''
img_options = 'compat=1.1,lazy_refcounts=on'
qemu_options = 'lazy-refcounts=off'
- compare = { 'compat': '1.1', 'lazy-refcounts': True, 'corrupt': False }
+ compare = { 'compat': '1.1', 'lazy-refcounts': True,
+ 'refcount-width': 16, 'corrupt': False }
TestImageInfoSpecific = None
TestQemuImgInfo = None
diff --git a/tests/qemu-iotests/067.out b/tests/qemu-iotests/067.out
index 929dc74..0deb97c 100644
--- a/tests/qemu-iotests/067.out
+++ b/tests/qemu-iotests/067.out
@@ -32,6 +32,7 @@ Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk -device virti
"data": {
"compat": "1.1",
"lazy-refcounts": false,
+ "refcount-width": 16,
"corrupt": false
}
},
@@ -202,6 +203,7 @@ Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk
"data": {
"compat": "1.1",
"lazy-refcounts": false,
+ "refcount-width": 16,
"corrupt": false
}
},
@@ -402,6 +404,7 @@ Testing:
"data": {
"compat": "1.1",
"lazy-refcounts": false,
+ "refcount-width": 16,
"corrupt": false
}
},
@@ -581,6 +584,7 @@ Testing:
"data": {
"compat": "1.1",
"lazy-refcounts": false,
+ "refcount-width": 16,
"corrupt": false
}
},
@@ -686,6 +690,7 @@ Testing:
"data": {
"compat": "1.1",
"lazy-refcounts": false,
+ "refcount-width": 16,
"corrupt": false
}
},
diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
index 0a3ab5a..4b14b4f 100644
--- a/tests/qemu-iotests/082.out
+++ b/tests/qemu-iotests/082.out
@@ -21,6 +21,7 @@ cluster_size: 4096
Format specific information:
compat: 1.1
lazy refcounts: true
+ refcount width: 16
corrupt: false
Testing: create -f qcow2 -o cluster_size=4k -o lazy_refcounts=on -o cluster_size=8k TEST_DIR/t.qcow2 128M
@@ -35,6 +36,7 @@ cluster_size: 8192
Format specific information:
compat: 1.1
lazy refcounts: true
+ refcount width: 16
corrupt: false
Testing: create -f qcow2 -o cluster_size=4k,cluster_size=8k TEST_DIR/t.qcow2 128M
@@ -199,6 +201,7 @@ cluster_size: 4096
Format specific information:
compat: 1.1
lazy refcounts: true
+ refcount width: 16
corrupt: false
Testing: convert -O qcow2 -o cluster_size=4k -o lazy_refcounts=on -o cluster_size=8k TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -212,6 +215,7 @@ cluster_size: 8192
Format specific information:
compat: 1.1
lazy refcounts: true
+ refcount width: 16
corrupt: false
Testing: convert -O qcow2 -o cluster_size=4k,cluster_size=8k TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -361,6 +365,7 @@ cluster_size: 65536
Format specific information:
compat: 1.1
lazy refcounts: true
+ refcount width: 16
corrupt: false
Testing: amend -f qcow2 -o size=130M -o lazy_refcounts=off TEST_DIR/t.qcow2
@@ -374,6 +379,7 @@ cluster_size: 65536
Format specific information:
compat: 1.1
lazy refcounts: false
+ refcount width: 16
corrupt: false
Testing: amend -f qcow2 -o size=8M -o lazy_refcounts=on -o size=132M TEST_DIR/t.qcow2
@@ -387,6 +393,7 @@ cluster_size: 65536
Format specific information:
compat: 1.1
lazy refcounts: true
+ refcount width: 16
corrupt: false
Testing: amend -f qcow2 -o size=4M,size=148M TEST_DIR/t.qcow2
diff --git a/tests/qemu-iotests/089.out b/tests/qemu-iotests/089.out
index b2b0390..d788b46 100644
--- a/tests/qemu-iotests/089.out
+++ b/tests/qemu-iotests/089.out
@@ -41,6 +41,7 @@ vm state offset: 512 MiB
Format specific information:
compat: 1.1
lazy refcounts: false
+ refcount width: 16
corrupt: false
format name: IMGFMT
cluster size: 64 KiB
@@ -48,5 +49,6 @@ vm state offset: 512 MiB
Format specific information:
compat: 1.1
lazy refcounts: false
+ refcount width: 16
corrupt: false
*** done
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 02/22] qcow2: Add refcount_width to format-specific info
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 02/22] qcow2: Add refcount_width to format-specific info Max Reitz
@ 2014-11-27 13:47 ` Stefan Hajnoczi
2014-11-27 14:19 ` Max Reitz
0 siblings, 1 reply; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-27 13:47 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 1687 bytes --]
On Thu, Nov 20, 2014 at 06:06:18PM +0100, Max Reitz wrote:
> Add the bit width of every refcount entry to the format-specific
> information.
>
> In contrast to lazy_refcounts and the corrupt flag, this should be
> always emitted, even for compat=0.10 although it does not support any
> refcount width other than 16 bits. This is because if a boolean is
> optional, one normally assumes it to be false when omitted; but if an
> integer is not specified, it is rather difficult to guess its value.
>
> This new field breaks some test outputs, fix them.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> block/qcow2.c | 4 +++-
> qapi/block-core.json | 5 ++++-
> tests/qemu-iotests/060.out | 1 +
> tests/qemu-iotests/065 | 23 +++++++++++++++--------
> tests/qemu-iotests/067.out | 5 +++++
> tests/qemu-iotests/082.out | 7 +++++++
> tests/qemu-iotests/089.out | 2 ++
> 7 files changed, 37 insertions(+), 10 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index f57aff9..d70e927 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2475,7 +2475,8 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs)
> };
> if (s->qcow_version == 2) {
> *spec_info->qcow2 = (ImageInfoSpecificQCow2){
> - .compat = g_strdup("0.10"),
> + .compat = g_strdup("0.10"),
> + .refcount_width = s->refcount_bits,
Why call it "width" in ImageInfoSpecificQCow2 when the qcow2 code calls
it "bits"? IMO "bits" is clearer because it tells you the units, and
it's more consistent.
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 02/22] qcow2: Add refcount_width to format-specific info
2014-11-27 13:47 ` Stefan Hajnoczi
@ 2014-11-27 14:19 ` Max Reitz
0 siblings, 0 replies; 58+ messages in thread
From: Max Reitz @ 2014-11-27 14:19 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
On 2014-11-27 at 14:47, Stefan Hajnoczi wrote:
> On Thu, Nov 20, 2014 at 06:06:18PM +0100, Max Reitz wrote:
>> Add the bit width of every refcount entry to the format-specific
>> information.
>>
>> In contrast to lazy_refcounts and the corrupt flag, this should be
>> always emitted, even for compat=0.10 although it does not support any
>> refcount width other than 16 bits. This is because if a boolean is
>> optional, one normally assumes it to be false when omitted; but if an
>> integer is not specified, it is rather difficult to guess its value.
>>
>> This new field breaks some test outputs, fix them.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>> block/qcow2.c | 4 +++-
>> qapi/block-core.json | 5 ++++-
>> tests/qemu-iotests/060.out | 1 +
>> tests/qemu-iotests/065 | 23 +++++++++++++++--------
>> tests/qemu-iotests/067.out | 5 +++++
>> tests/qemu-iotests/082.out | 7 +++++++
>> tests/qemu-iotests/089.out | 2 ++
>> 7 files changed, 37 insertions(+), 10 deletions(-)
>>
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index f57aff9..d70e927 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -2475,7 +2475,8 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs)
>> };
>> if (s->qcow_version == 2) {
>> *spec_info->qcow2 = (ImageInfoSpecificQCow2){
>> - .compat = g_strdup("0.10"),
>> + .compat = g_strdup("0.10"),
>> + .refcount_width = s->refcount_bits,
> Why call it "width" in ImageInfoSpecificQCow2 when the qcow2 code calls
> it "bits"? IMO "bits" is clearer because it tells you the units, and
> it's more consistent.
Well, I'm fine with either. The spec calls it refcount_bits, too, but
also says it's the "width". My main argument against "bits" would be
that I'd have to look through the whole series and make sure I change it
everywhere, and also drop Eric's R-b. *g*
So, yes, I'm fine with changing it to refcount_bits.
Max
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH v3 03/22] qcow2: Use 64 bits for refcount values
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 01/22] qcow2: Add two new fields to BDRVQcowState Max Reitz
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 02/22] qcow2: Add refcount_width to format-specific info Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-27 13:49 ` Stefan Hajnoczi
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 04/22] qcow2: Respect error in qcow2_alloc_bytes() Max Reitz
` (18 subsequent siblings)
21 siblings, 1 reply; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Refcounts may have a width of up to 64 bits, so qemu should use the same
width to represent refcount values internally.
Since for instance qcow2_get_refcount() signals an error by returning a
negative value, refcount values are generally signed to be able to
represent those error values correctly. This limits the maximum refcount
value supported by qemu to INT64_MAX (= 63 bits), as established in
"qcow2: Add two new fields to BDRVQcowState".
This limitation should have no implications in practice for normal valid
images. If the MSb in a 64 bit refcount value is set, we can safely
assume the value to be invalid (because reaching such high refcounts is
impossible due to other limitations of the qcow2 format).
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qcow2-cluster.c | 9 ++++++---
block/qcow2-refcount.c | 37 ++++++++++++++++++++-----------------
block/qcow2.h | 7 ++++---
3 files changed, 30 insertions(+), 23 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index df0b2c9..ab43902 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1640,7 +1640,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
for (i = 0; i < l1_size; i++) {
uint64_t l2_offset = l1_table[i] & L1E_OFFSET_MASK;
bool l2_dirty = false;
- int l2_refcount;
+ int64_t l2_refcount;
if (!l2_offset) {
/* unallocated */
@@ -1696,14 +1696,17 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
}
if (l2_refcount > 1) {
+ int64_t ret64;
+
/* For shared L2 tables, set the refcount accordingly (it is
* already 1 and needs to be l2_refcount) */
- ret = qcow2_update_cluster_refcount(bs,
+ ret64 = qcow2_update_cluster_refcount(bs,
offset >> s->cluster_bits, l2_refcount - 1,
QCOW2_DISCARD_OTHER);
- if (ret < 0) {
+ if (ret64 < 0) {
qcow2_free_clusters(bs, offset, s->cluster_size,
QCOW2_DISCARD_OTHER);
+ ret = ret64;
goto fail;
}
}
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 6016211..6e06531 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -91,14 +91,14 @@ static int load_refcount_block(BlockDriverState *bs,
* return value is the refcount of the cluster, negative values are -errno
* and indicate an error.
*/
-int qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index)
+int64_t qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index)
{
BDRVQcowState *s = bs->opaque;
uint64_t refcount_table_index, block_index;
int64_t refcount_block_offset;
int ret;
uint16_t *refcount_block;
- uint16_t refcount;
+ int64_t refcount;
refcount_table_index = cluster_index >> s->refcount_block_bits;
if (refcount_table_index >= s->refcount_table_size)
@@ -556,9 +556,10 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
for(cluster_offset = start; cluster_offset <= last;
cluster_offset += s->cluster_size)
{
- int block_index, refcount;
+ int block_index;
int64_t cluster_index = cluster_offset >> s->cluster_bits;
int64_t table_index = cluster_index >> s->refcount_block_bits;
+ int64_t refcount;
/* Load the refcount block and allocate it if needed */
if (table_index != old_table_index) {
@@ -634,10 +635,10 @@ fail:
* If the return value is non-negative, it is the new refcount of the cluster.
* If it is negative, it is -errno and indicates an error.
*/
-int qcow2_update_cluster_refcount(BlockDriverState *bs,
- int64_t cluster_index,
- int addend,
- enum qcow2_discard_type type)
+int64_t qcow2_update_cluster_refcount(BlockDriverState *bs,
+ int64_t cluster_index,
+ int addend,
+ enum qcow2_discard_type type)
{
BDRVQcowState *s = bs->opaque;
int ret;
@@ -663,7 +664,7 @@ static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size)
{
BDRVQcowState *s = bs->opaque;
uint64_t i, nb_clusters;
- int refcount;
+ int64_t refcount;
nb_clusters = size_to_clusters(s, size);
retry:
@@ -722,7 +723,8 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
BDRVQcowState *s = bs->opaque;
uint64_t cluster_index;
uint64_t i;
- int refcount, ret;
+ int64_t refcount;
+ int ret;
assert(nb_clusters >= 0);
if (nb_clusters == 0) {
@@ -878,8 +880,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
BDRVQcowState *s = bs->opaque;
uint64_t *l1_table, *l2_table, l2_offset, offset, l1_size2;
bool l1_allocated = false;
- int64_t old_offset, old_l2_offset;
- int i, j, l1_modified = 0, nb_csectors, refcount;
+ int64_t old_offset, old_l2_offset, refcount;
+ int i, j, l1_modified = 0, nb_csectors;
int ret;
l2_table = NULL;
@@ -1341,7 +1343,7 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
BDRVQcowState *s = bs->opaque;
uint64_t *l2_table = qemu_blockalign(bs, s->cluster_size);
int ret;
- int refcount;
+ int64_t refcount;
int i, j;
for (i = 0; i < s->l1_size; i++) {
@@ -1360,7 +1362,7 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
}
if ((refcount == 1) != ((l1_entry & QCOW_OFLAG_COPIED) != 0)) {
fprintf(stderr, "%s OFLAG_COPIED L2 cluster: l1_index=%d "
- "l1_entry=%" PRIx64 " refcount=%d\n",
+ "l1_entry=%" PRIx64 " refcount=%" PRId64 "\n",
fix & BDRV_FIX_ERRORS ? "Repairing" :
"ERROR",
i, l1_entry, refcount);
@@ -1403,7 +1405,7 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
}
if ((refcount == 1) != ((l2_entry & QCOW_OFLAG_COPIED) != 0)) {
fprintf(stderr, "%s OFLAG_COPIED data cluster: "
- "l2_entry=%" PRIx64 " refcount=%d\n",
+ "l2_entry=%" PRIx64 " refcount=%" PRId64 "\n",
fix & BDRV_FIX_ERRORS ? "Repairing" :
"ERROR",
l2_entry, refcount);
@@ -1628,8 +1630,8 @@ static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
uint16_t *refcount_table, int64_t nb_clusters)
{
BDRVQcowState *s = bs->opaque;
- int64_t i;
- int refcount1, refcount2, ret;
+ int64_t i, refcount1, refcount2;
+ int ret;
for (i = 0, *highest_cluster = 0; i < nb_clusters; i++) {
refcount1 = qcow2_get_refcount(bs, i);
@@ -1657,7 +1659,8 @@ static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
num_fixed = &res->corruptions_fixed;
}
- fprintf(stderr, "%s cluster %" PRId64 " refcount=%d reference=%d\n",
+ fprintf(stderr, "%s cluster %" PRId64 " refcount=%" PRId64
+ " reference=%" PRId64 "\n",
num_fixed != NULL ? "Repairing" :
refcount1 < refcount2 ? "ERROR" :
"Leaked",
diff --git a/block/qcow2.h b/block/qcow2.h
index 4d8c902..0f8eb15 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -489,10 +489,11 @@ void qcow2_signal_corruption(BlockDriverState *bs, bool fatal, int64_t offset,
int qcow2_refcount_init(BlockDriverState *bs);
void qcow2_refcount_close(BlockDriverState *bs);
-int qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index);
+int64_t qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index);
-int qcow2_update_cluster_refcount(BlockDriverState *bs, int64_t cluster_index,
- int addend, enum qcow2_discard_type type);
+int64_t qcow2_update_cluster_refcount(BlockDriverState *bs,
+ int64_t cluster_index, int addend,
+ enum qcow2_discard_type type);
int64_t qcow2_alloc_clusters(BlockDriverState *bs, uint64_t size);
int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 03/22] qcow2: Use 64 bits for refcount values
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 03/22] qcow2: Use 64 bits for refcount values Max Reitz
@ 2014-11-27 13:49 ` Stefan Hajnoczi
0 siblings, 0 replies; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-27 13:49 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 1185 bytes --]
On Thu, Nov 20, 2014 at 06:06:19PM +0100, Max Reitz wrote:
> Refcounts may have a width of up to 64 bits, so qemu should use the same
> width to represent refcount values internally.
>
> Since for instance qcow2_get_refcount() signals an error by returning a
> negative value, refcount values are generally signed to be able to
> represent those error values correctly. This limits the maximum refcount
> value supported by qemu to INT64_MAX (= 63 bits), as established in
> "qcow2: Add two new fields to BDRVQcowState".
>
> This limitation should have no implications in practice for normal valid
> images. If the MSb in a 64 bit refcount value is set, we can safely
> assume the value to be invalid (because reaching such high refcounts is
> impossible due to other limitations of the qcow2 format).
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> block/qcow2-cluster.c | 9 ++++++---
> block/qcow2-refcount.c | 37 ++++++++++++++++++++-----------------
> block/qcow2.h | 7 ++++---
> 3 files changed, 30 insertions(+), 23 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH v3 04/22] qcow2: Respect error in qcow2_alloc_bytes()
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (2 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 03/22] qcow2: Use 64 bits for refcount values Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-27 14:56 ` Stefan Hajnoczi
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 05/22] qcow2: Refcount overflow and qcow2_alloc_bytes() Max Reitz
` (17 subsequent siblings)
21 siblings, 1 reply; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
qcow2_update_cluster_refcount() may fail, and qcow2_alloc_bytes() should
mind that case.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qcow2-refcount.c | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 6e06531..be4e5fe 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -761,7 +761,8 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
{
BDRVQcowState *s = bs->opaque;
- int64_t offset, cluster_offset;
+ int64_t offset, cluster_offset, new_cluster;
+ int64_t ret;
int free_in_cluster;
BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_ALLOC_BYTES);
@@ -783,23 +784,32 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
free_in_cluster -= size;
if (free_in_cluster == 0)
s->free_byte_offset = 0;
- if (offset_into_cluster(s, offset) != 0)
- qcow2_update_cluster_refcount(bs, offset >> s->cluster_bits, 1,
- QCOW2_DISCARD_NEVER);
+ if (offset_into_cluster(s, offset) != 0) {
+ ret = qcow2_update_cluster_refcount(bs, offset >> s->cluster_bits,
+ 1, QCOW2_DISCARD_NEVER);
+ if (ret < 0) {
+ return ret;
+ }
+ }
} else {
- offset = qcow2_alloc_clusters(bs, s->cluster_size);
- if (offset < 0) {
- return offset;
+ new_cluster = qcow2_alloc_clusters(bs, s->cluster_size);
+ if (new_cluster < 0) {
+ return new_cluster;
}
cluster_offset = start_of_cluster(s, s->free_byte_offset);
- if ((cluster_offset + s->cluster_size) == offset) {
+ if ((cluster_offset + s->cluster_size) == new_cluster) {
/* we are lucky: contiguous data */
offset = s->free_byte_offset;
- qcow2_update_cluster_refcount(bs, offset >> s->cluster_bits, 1,
- QCOW2_DISCARD_NEVER);
+ ret = qcow2_update_cluster_refcount(bs, offset >> s->cluster_bits,
+ 1, QCOW2_DISCARD_NEVER);
+ if (ret < 0) {
+ qcow2_free_clusters(bs, new_cluster, s->cluster_size,
+ QCOW2_DISCARD_NEVER);
+ return ret;
+ }
s->free_byte_offset += size;
} else {
- s->free_byte_offset = offset;
+ s->free_byte_offset = new_cluster;
goto redo;
}
}
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 04/22] qcow2: Respect error in qcow2_alloc_bytes()
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 04/22] qcow2: Respect error in qcow2_alloc_bytes() Max Reitz
@ 2014-11-27 14:56 ` Stefan Hajnoczi
0 siblings, 0 replies; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-27 14:56 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 434 bytes --]
On Thu, Nov 20, 2014 at 06:06:20PM +0100, Max Reitz wrote:
> qcow2_update_cluster_refcount() may fail, and qcow2_alloc_bytes() should
> mind that case.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> block/qcow2-refcount.c | 32 +++++++++++++++++++++-----------
> 1 file changed, 21 insertions(+), 11 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH v3 05/22] qcow2: Refcount overflow and qcow2_alloc_bytes()
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (3 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 04/22] qcow2: Respect error in qcow2_alloc_bytes() Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-27 14:59 ` Stefan Hajnoczi
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 06/22] qcow2: Helper for refcount array reallocation Max Reitz
` (16 subsequent siblings)
21 siblings, 1 reply; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
qcow2_alloc_bytes() may reuse a cluster multiple times, in which case
the refcount is increased accordingly. However, if this would lead to an
overflow the function should instead just not reuse this cluster and
allocate a new one.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qcow2-refcount.c | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index be4e5fe..66c78c0 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -761,12 +761,13 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
{
BDRVQcowState *s = bs->opaque;
- int64_t offset, cluster_offset, new_cluster;
+ int64_t offset, cluster_offset, new_cluster, refcount;
int64_t ret;
int free_in_cluster;
BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_ALLOC_BYTES);
assert(size > 0 && size <= s->cluster_size);
+ redo:
if (s->free_byte_offset == 0) {
offset = qcow2_alloc_clusters(bs, s->cluster_size);
if (offset < 0) {
@@ -774,12 +775,25 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
}
s->free_byte_offset = offset;
}
- redo:
+
free_in_cluster = s->cluster_size -
offset_into_cluster(s, s->free_byte_offset);
if (size <= free_in_cluster) {
/* enough space in current cluster */
offset = s->free_byte_offset;
+
+ if (offset_into_cluster(s, offset) != 0) {
+ /* We will have to increase the refcount of this cluster; if the
+ * maximum has been reached already, this cluster cannot be used */
+ refcount = qcow2_get_refcount(bs, offset >> s->cluster_bits);
+ if (refcount < 0) {
+ return refcount;
+ } else if (refcount == s->refcount_max) {
+ s->free_byte_offset = 0;
+ goto redo;
+ }
+ }
+
s->free_byte_offset += size;
free_in_cluster -= size;
if (free_in_cluster == 0)
@@ -800,6 +814,20 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
if ((cluster_offset + s->cluster_size) == new_cluster) {
/* we are lucky: contiguous data */
offset = s->free_byte_offset;
+
+ /* Same as above: In order to reuse the cluster, the refcount has to
+ * be increased; if that will not work, we are not so lucky after
+ * all */
+ refcount = qcow2_get_refcount(bs, offset >> s->cluster_bits);
+ if (refcount < 0) {
+ qcow2_free_clusters(bs, new_cluster, s->cluster_size,
+ QCOW2_DISCARD_NEVER);
+ return refcount;
+ } else if (refcount == s->refcount_max) {
+ s->free_byte_offset = offset;
+ goto redo;
+ }
+
ret = qcow2_update_cluster_refcount(bs, offset >> s->cluster_bits,
1, QCOW2_DISCARD_NEVER);
if (ret < 0) {
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 05/22] qcow2: Refcount overflow and qcow2_alloc_bytes()
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 05/22] qcow2: Refcount overflow and qcow2_alloc_bytes() Max Reitz
@ 2014-11-27 14:59 ` Stefan Hajnoczi
0 siblings, 0 replies; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-27 14:59 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 582 bytes --]
On Thu, Nov 20, 2014 at 06:06:21PM +0100, Max Reitz wrote:
> qcow2_alloc_bytes() may reuse a cluster multiple times, in which case
> the refcount is increased accordingly. However, if this would lead to an
> overflow the function should instead just not reuse this cluster and
> allocate a new one.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> block/qcow2-refcount.c | 32 ++++++++++++++++++++++++++++++--
> 1 file changed, 30 insertions(+), 2 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH v3 06/22] qcow2: Helper for refcount array reallocation
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (4 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 05/22] qcow2: Refcount overflow and qcow2_alloc_bytes() Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-20 21:43 ` Eric Blake
2014-11-27 15:09 ` Stefan Hajnoczi
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 07/22] qcow2: Helper function for refcount modification Max Reitz
` (15 subsequent siblings)
21 siblings, 2 replies; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add a helper function for reallocating a refcount array, independent of
the refcount order. The newly allocated space is zeroed and the function
handles failed reallocations gracefully.
The helper function will always align the buffer size to a cluster
boundary; if storing the refcounts in such an array in big endian byte
order, this makes it possible to write parts of the array directly as
refcount blocks into the image file.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qcow2-refcount.c | 135 +++++++++++++++++++++++++++++++------------------
1 file changed, 86 insertions(+), 49 deletions(-)
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 66c78c0..3cd540c 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1108,6 +1108,68 @@ fail:
/* refcount checking functions */
+static size_t refcount_array_byte_size(BDRVQcowState *s, uint64_t entries)
+{
+ if (s->refcount_order < 3) {
+ /* sub-byte width */
+ int shift = 3 - s->refcount_order;
+ return (entries + (1 << shift) - 1) >> shift;
+ } else if (s->refcount_order == 3) {
+ /* byte width */
+ return entries;
+ } else {
+ /* multiple bytes wide */
+
+ /* This assertion holds because there is no way we can address more than
+ * 2^(64 - 9) clusters at once (with cluster size 512 = 2^9, and because
+ * offsets have to be representable in bytes); due to every cluster
+ * corresponding to one refcount entry and because refcount_order has to
+ * be below 7, we are far below that limit */
+ assert(!(entries >> (64 - (s->refcount_order - 3))));
+
+ return entries << (s->refcount_order - 3);
+ }
+}
+
+/**
+ * Reallocates *array so that it can hold new_size entries. *size must contain
+ * the current number of entries in *array. If the reallocation fails, *array
+ * and *size will not be modified and -errno will be returned. If the
+ * reallocation is successful, *array will be set to the new buffer and *size
+ * will be set to new_size. The size of the reallocated refcount array buffer
+ * will be aligned to a cluster boundary, and the newly allocated area will be
+ * zeroed.
+ */
+static int realloc_refcount_array(BDRVQcowState *s, uint16_t **array,
+ int64_t *size, int64_t new_size)
+{
+ /* Round to clusters so the array can be directly written to disk */
+ size_t old_byte_size = ROUND_UP(refcount_array_byte_size(s, *size),
+ s->cluster_size);
+ size_t new_byte_size = ROUND_UP(refcount_array_byte_size(s, new_size),
+ s->cluster_size);
+ uint16_t *new_ptr;
+
+ if (new_byte_size <= old_byte_size) {
+ *size = new_size;
+ return 0;
+ }
+
+ assert(new_byte_size > 0);
+
+ new_ptr = g_try_realloc(*array, new_byte_size);
+ if (!new_ptr) {
+ return -ENOMEM;
+ }
+
+ memset((void *)((uintptr_t)new_ptr + old_byte_size), 0,
+ new_byte_size - old_byte_size);
+
+ *array = new_ptr;
+ *size = new_size;
+
+ return 0;
+}
/*
* Increases the refcount for a range of clusters in a given refcount table.
@@ -1124,6 +1186,7 @@ static int inc_refcounts(BlockDriverState *bs,
{
BDRVQcowState *s = bs->opaque;
uint64_t start, last, cluster_offset, k;
+ int ret;
if (size <= 0) {
return 0;
@@ -1135,23 +1198,12 @@ static int inc_refcounts(BlockDriverState *bs,
cluster_offset += s->cluster_size) {
k = cluster_offset >> s->cluster_bits;
if (k >= *refcount_table_size) {
- int64_t old_refcount_table_size = *refcount_table_size;
- uint16_t *new_refcount_table;
-
- *refcount_table_size = k + 1;
- new_refcount_table = g_try_realloc(*refcount_table,
- *refcount_table_size *
- sizeof(**refcount_table));
- if (!new_refcount_table) {
- *refcount_table_size = old_refcount_table_size;
+ ret = realloc_refcount_array(s, refcount_table,
+ refcount_table_size, k + 1);
+ if (ret < 0) {
res->check_errors++;
- return -ENOMEM;
+ return ret;
}
- *refcount_table = new_refcount_table;
-
- memset(*refcount_table + old_refcount_table_size, 0,
- (*refcount_table_size - old_refcount_table_size) *
- sizeof(**refcount_table));
}
if (++(*refcount_table)[k] == 0) {
@@ -1518,8 +1570,7 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res,
fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);
if (fix & BDRV_FIX_ERRORS) {
- int64_t old_nb_clusters = *nb_clusters;
- uint16_t *new_refcount_table;
+ int64_t new_nb_clusters;
if (offset > INT64_MAX - s->cluster_size) {
ret = -EINVAL;
@@ -1536,22 +1587,15 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res,
goto resize_fail;
}
- *nb_clusters = size_to_clusters(s, size);
- assert(*nb_clusters >= old_nb_clusters);
+ new_nb_clusters = size_to_clusters(s, size);
+ assert(new_nb_clusters >= *nb_clusters);
- new_refcount_table = g_try_realloc(*refcount_table,
- *nb_clusters *
- sizeof(**refcount_table));
- if (!new_refcount_table) {
- *nb_clusters = old_nb_clusters;
+ ret = realloc_refcount_array(s, refcount_table,
+ nb_clusters, new_nb_clusters);
+ if (ret < 0) {
res->check_errors++;
- return -ENOMEM;
+ return ret;
}
- *refcount_table = new_refcount_table;
-
- memset(*refcount_table + old_nb_clusters, 0,
- (*nb_clusters - old_nb_clusters) *
- sizeof(**refcount_table));
if (cluster >= *nb_clusters) {
ret = -EINVAL;
@@ -1611,10 +1655,12 @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
int ret;
if (!*refcount_table) {
- *refcount_table = g_try_new0(uint16_t, *nb_clusters);
- if (*nb_clusters && *refcount_table == NULL) {
+ int64_t old_size = 0;
+ ret = realloc_refcount_array(s, refcount_table,
+ &old_size, *nb_clusters);
+ if (ret < 0) {
res->check_errors++;
- return -ENOMEM;
+ return ret;
}
}
@@ -1746,6 +1792,7 @@ static int64_t alloc_clusters_imrt(BlockDriverState *bs,
int64_t cluster = *first_free_cluster, i;
bool first_gap = true;
int contiguous_free_clusters;
+ int ret;
/* Starting at *first_free_cluster, find a range of at least cluster_count
* continuously free clusters */
@@ -1775,28 +1822,18 @@ static int64_t alloc_clusters_imrt(BlockDriverState *bs,
/* If no such range could be found, grow the in-memory refcount table
* accordingly to append free clusters at the end of the image */
if (contiguous_free_clusters < cluster_count) {
- int64_t old_imrt_nb_clusters = *imrt_nb_clusters;
- uint16_t *new_refcount_table;
-
/* contiguous_free_clusters clusters are already empty at the image end;
* we need cluster_count clusters; therefore, we have to allocate
* cluster_count - contiguous_free_clusters new clusters at the end of
* the image (which is the current value of cluster; note that cluster
* may exceed old_imrt_nb_clusters if *first_free_cluster pointed beyond
* the image end) */
- *imrt_nb_clusters = cluster + cluster_count - contiguous_free_clusters;
- new_refcount_table = g_try_realloc(*refcount_table,
- *imrt_nb_clusters *
- sizeof(**refcount_table));
- if (!new_refcount_table) {
- *imrt_nb_clusters = old_imrt_nb_clusters;
- return -ENOMEM;
- }
- *refcount_table = new_refcount_table;
-
- memset(*refcount_table + old_imrt_nb_clusters, 0,
- (*imrt_nb_clusters - old_imrt_nb_clusters) *
- sizeof(**refcount_table));
+ ret = realloc_refcount_array(s, refcount_table, imrt_nb_clusters,
+ cluster + cluster_count
+ - contiguous_free_clusters);
+ if (ret < 0) {
+ return ret;
+ }
}
/* Go back to the first free cluster */
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 06/22] qcow2: Helper for refcount array reallocation
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 06/22] qcow2: Helper for refcount array reallocation Max Reitz
@ 2014-11-20 21:43 ` Eric Blake
2014-11-21 8:45 ` Max Reitz
2014-11-27 15:09 ` Stefan Hajnoczi
1 sibling, 1 reply; 58+ messages in thread
From: Eric Blake @ 2014-11-20 21:43 UTC (permalink / raw)
To: Max Reitz, qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 1947 bytes --]
On 11/20/2014 10:06 AM, Max Reitz wrote:
> Add a helper function for reallocating a refcount array, independent of
> the refcount order. The newly allocated space is zeroed and the function
> handles failed reallocations gracefully.
>
> The helper function will always align the buffer size to a cluster
> boundary; if storing the refcounts in such an array in big endian byte
> order, this makes it possible to write parts of the array directly as
> refcount blocks into the image file.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Perhaps the changes since v2 warranted removing my earlier R-b to make
sure I review closely?
> ---
> block/qcow2-refcount.c | 135 +++++++++++++++++++++++++++++++------------------
> 1 file changed, 86 insertions(+), 49 deletions(-)
>
> +static int realloc_refcount_array(BDRVQcowState *s, uint16_t **array,
> + int64_t *size, int64_t new_size)
> +{
> + /* Round to clusters so the array can be directly written to disk */
> + size_t old_byte_size = ROUND_UP(refcount_array_byte_size(s, *size),
> + s->cluster_size);
> + size_t new_byte_size = ROUND_UP(refcount_array_byte_size(s, new_size),
> + s->cluster_size);
> + uint16_t *new_ptr;
> +
> + if (new_byte_size <= old_byte_size) {
> + *size = new_size;
> + return 0;
> + }
> +
> + assert(new_byte_size > 0);
Should this assert be moved before the early exit?
Hmm, that's my only finding, and you did incorporate improvements mostly
to comments or asserts as compared to v2. Moving the assert is small
enough, and not a show-stopper if you leave it in place, so:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 06/22] qcow2: Helper for refcount array reallocation
2014-11-20 21:43 ` Eric Blake
@ 2014-11-21 8:45 ` Max Reitz
0 siblings, 0 replies; 58+ messages in thread
From: Max Reitz @ 2014-11-21 8:45 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi
On 2014-11-20 at 22:43, Eric Blake wrote:
> On 11/20/2014 10:06 AM, Max Reitz wrote:
>> Add a helper function for reallocating a refcount array, independent of
>> the refcount order. The newly allocated space is zeroed and the function
>> handles failed reallocations gracefully.
>>
>> The helper function will always align the buffer size to a cluster
>> boundary; if storing the refcounts in such an array in big endian byte
>> order, this makes it possible to write parts of the array directly as
>> refcount blocks into the image file.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
> Perhaps the changes since v2 warranted removing my earlier R-b to make
> sure I review closely?
Well, normally I would not have taken the R-b. But you explicitly wrote:
> Code looks correct as written, whether or not you also add more
> comments, asserts, and/or shortcuts for no-op situations.
So I took that to mean "You may change the commit message as proposed
(s/independently/independent/ and add a note about the cluster
alignment), add a comment to realloc_refcount_array() about its calling
contract, an assert(new_byte_size > 0) and/or an early-out for when the
byte size of the refcount array does not increase", which is pretty
broad, but that's what you wrote. That's why I kept the R-b in.
>> ---
>> block/qcow2-refcount.c | 135 +++++++++++++++++++++++++++++++------------------
>> 1 file changed, 86 insertions(+), 49 deletions(-)
>>
>> +static int realloc_refcount_array(BDRVQcowState *s, uint16_t **array,
>> + int64_t *size, int64_t new_size)
>> +{
>> + /* Round to clusters so the array can be directly written to disk */
>> + size_t old_byte_size = ROUND_UP(refcount_array_byte_size(s, *size),
>> + s->cluster_size);
>> + size_t new_byte_size = ROUND_UP(refcount_array_byte_size(s, new_size),
>> + s->cluster_size);
>> + uint16_t *new_ptr;
>> +
>> + if (new_byte_size <= old_byte_size) {
>> + *size = new_size;
>> + return 0;
>> + }
>> +
>> + assert(new_byte_size > 0);
> Should this assert be moved before the early exit?
Could do, but it should not matter. It's important that new_byte_size >
0 after the early-out; the caller should not use new_size == 0 at all,
but I'd rather not add an assertion against a contract breach somewhere
where the code can handle that breach actually just fine.
> Hmm, that's my only finding, and you did incorporate improvements mostly
> to comments or asserts as compared to v2. Moving the assert is small
> enough, and not a show-stopper if you leave it in place, so:
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Phew :-)
Thanks!
Max
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [Qemu-devel] [PATCH v3 06/22] qcow2: Helper for refcount array reallocation
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 06/22] qcow2: Helper for refcount array reallocation Max Reitz
2014-11-20 21:43 ` Eric Blake
@ 2014-11-27 15:09 ` Stefan Hajnoczi
2014-11-27 15:11 ` Max Reitz
1 sibling, 1 reply; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-27 15:09 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 1607 bytes --]
On Thu, Nov 20, 2014 at 06:06:22PM +0100, Max Reitz wrote:
> +/**
> + * Reallocates *array so that it can hold new_size entries. *size must contain
> + * the current number of entries in *array. If the reallocation fails, *array
> + * and *size will not be modified and -errno will be returned. If the
> + * reallocation is successful, *array will be set to the new buffer and *size
> + * will be set to new_size. The size of the reallocated refcount array buffer
> + * will be aligned to a cluster boundary, and the newly allocated area will be
> + * zeroed.
> + */
> +static int realloc_refcount_array(BDRVQcowState *s, uint16_t **array,
> + int64_t *size, int64_t new_size)
> +{
> + /* Round to clusters so the array can be directly written to disk */
> + size_t old_byte_size = ROUND_UP(refcount_array_byte_size(s, *size),
> + s->cluster_size);
> + size_t new_byte_size = ROUND_UP(refcount_array_byte_size(s, new_size),
> + s->cluster_size);
> + uint16_t *new_ptr;
> +
> + if (new_byte_size <= old_byte_size) {
> + *size = new_size;
> + return 0;
> + }
Why not realloc the array to the new smaller size? ...
> +
> + assert(new_byte_size > 0);
> +
> + new_ptr = g_try_realloc(*array, new_byte_size);
> + if (!new_ptr) {
> + return -ENOMEM;
> + }
> +
> + memset((void *)((uintptr_t)new_ptr + old_byte_size), 0,
> + new_byte_size - old_byte_size);
...we just need to skip the memset in when new_byte_size is smaller
than old_byte_size.
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 06/22] qcow2: Helper for refcount array reallocation
2014-11-27 15:09 ` Stefan Hajnoczi
@ 2014-11-27 15:11 ` Max Reitz
2014-11-28 10:46 ` Stefan Hajnoczi
0 siblings, 1 reply; 58+ messages in thread
From: Max Reitz @ 2014-11-27 15:11 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
On 2014-11-27 at 16:09, Stefan Hajnoczi wrote:
> On Thu, Nov 20, 2014 at 06:06:22PM +0100, Max Reitz wrote:
>> +/**
>> + * Reallocates *array so that it can hold new_size entries. *size must contain
>> + * the current number of entries in *array. If the reallocation fails, *array
>> + * and *size will not be modified and -errno will be returned. If the
>> + * reallocation is successful, *array will be set to the new buffer and *size
>> + * will be set to new_size. The size of the reallocated refcount array buffer
>> + * will be aligned to a cluster boundary, and the newly allocated area will be
>> + * zeroed.
>> + */
>> +static int realloc_refcount_array(BDRVQcowState *s, uint16_t **array,
>> + int64_t *size, int64_t new_size)
>> +{
>> + /* Round to clusters so the array can be directly written to disk */
>> + size_t old_byte_size = ROUND_UP(refcount_array_byte_size(s, *size),
>> + s->cluster_size);
>> + size_t new_byte_size = ROUND_UP(refcount_array_byte_size(s, new_size),
>> + s->cluster_size);
>> + uint16_t *new_ptr;
>> +
>> + if (new_byte_size <= old_byte_size) {
>> + *size = new_size;
>> + return 0;
>> + }
> Why not realloc the array to the new smaller size? ...
Because such a call will actually never happen. I could replace this if
() by assert(new_byte_size >= old_byte_size); if (new_byte_size ==
old_byte_size), but as I said before, I'm not a friend of assertions
when the code can deal perfectly well with the "unsupported" case.
Max
>> +
>> + assert(new_byte_size > 0);
>> +
>> + new_ptr = g_try_realloc(*array, new_byte_size);
>> + if (!new_ptr) {
>> + return -ENOMEM;
>> + }
>> +
>> + memset((void *)((uintptr_t)new_ptr + old_byte_size), 0,
>> + new_byte_size - old_byte_size);
> ...we just need to skip the memset in when new_byte_size is smaller
> than old_byte_size.
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 06/22] qcow2: Helper for refcount array reallocation
2014-11-27 15:11 ` Max Reitz
@ 2014-11-28 10:46 ` Stefan Hajnoczi
2014-12-02 9:52 ` Max Reitz
0 siblings, 1 reply; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-28 10:46 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 1953 bytes --]
On Thu, Nov 27, 2014 at 04:11:12PM +0100, Max Reitz wrote:
> On 2014-11-27 at 16:09, Stefan Hajnoczi wrote:
> >On Thu, Nov 20, 2014 at 06:06:22PM +0100, Max Reitz wrote:
> >>+/**
> >>+ * Reallocates *array so that it can hold new_size entries. *size must contain
> >>+ * the current number of entries in *array. If the reallocation fails, *array
> >>+ * and *size will not be modified and -errno will be returned. If the
> >>+ * reallocation is successful, *array will be set to the new buffer and *size
> >>+ * will be set to new_size. The size of the reallocated refcount array buffer
> >>+ * will be aligned to a cluster boundary, and the newly allocated area will be
> >>+ * zeroed.
> >>+ */
> >>+static int realloc_refcount_array(BDRVQcowState *s, uint16_t **array,
> >>+ int64_t *size, int64_t new_size)
> >>+{
> >>+ /* Round to clusters so the array can be directly written to disk */
> >>+ size_t old_byte_size = ROUND_UP(refcount_array_byte_size(s, *size),
> >>+ s->cluster_size);
> >>+ size_t new_byte_size = ROUND_UP(refcount_array_byte_size(s, new_size),
> >>+ s->cluster_size);
> >>+ uint16_t *new_ptr;
> >>+
> >>+ if (new_byte_size <= old_byte_size) {
> >>+ *size = new_size;
> >>+ return 0;
> >>+ }
> >Why not realloc the array to the new smaller size? ...
>
> Because such a call will actually never happen. I could replace this if ()
> by assert(new_byte_size >= old_byte_size); if (new_byte_size ==
> old_byte_size), but as I said before, I'm not a friend of assertions when
> the code can deal perfectly well with the "unsupported" case.
It is simpler to put an if statement around the memset. Then the
function actually frees unused memory and readers don't wonder why you
decided not to shrink the array.
Less code and slightly better behavior.
Stefan
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 06/22] qcow2: Helper for refcount array reallocation
2014-11-28 10:46 ` Stefan Hajnoczi
@ 2014-12-02 9:52 ` Max Reitz
0 siblings, 0 replies; 58+ messages in thread
From: Max Reitz @ 2014-12-02 9:52 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
On 2014-11-28 at 11:46, Stefan Hajnoczi wrote:
> On Thu, Nov 27, 2014 at 04:11:12PM +0100, Max Reitz wrote:
>> On 2014-11-27 at 16:09, Stefan Hajnoczi wrote:
>>> On Thu, Nov 20, 2014 at 06:06:22PM +0100, Max Reitz wrote:
>>>> +/**
>>>> + * Reallocates *array so that it can hold new_size entries. *size must contain
>>>> + * the current number of entries in *array. If the reallocation fails, *array
>>>> + * and *size will not be modified and -errno will be returned. If the
>>>> + * reallocation is successful, *array will be set to the new buffer and *size
>>>> + * will be set to new_size. The size of the reallocated refcount array buffer
>>>> + * will be aligned to a cluster boundary, and the newly allocated area will be
>>>> + * zeroed.
>>>> + */
>>>> +static int realloc_refcount_array(BDRVQcowState *s, uint16_t **array,
>>>> + int64_t *size, int64_t new_size)
>>>> +{
>>>> + /* Round to clusters so the array can be directly written to disk */
>>>> + size_t old_byte_size = ROUND_UP(refcount_array_byte_size(s, *size),
>>>> + s->cluster_size);
>>>> + size_t new_byte_size = ROUND_UP(refcount_array_byte_size(s, new_size),
>>>> + s->cluster_size);
>>>> + uint16_t *new_ptr;
>>>> +
>>>> + if (new_byte_size <= old_byte_size) {
>>>> + *size = new_size;
>>>> + return 0;
>>>> + }
>>> Why not realloc the array to the new smaller size? ...
>> Because such a call will actually never happen. I could replace this if ()
>> by assert(new_byte_size >= old_byte_size); if (new_byte_size ==
>> old_byte_size), but as I said before, I'm not a friend of assertions when
>> the code can deal perfectly well with the "unsupported" case.
> It is simpler to put an if statement around the memset.
Well, I personally find an assertion simpler; and I will not drop the if
(new_byte_size == old_byte_size) because this is a very common case so I
don't want to rely on g_realloc() to detect it. Also, Eric proposed it
and I'd like to avoid a ping-pong discussion.
> Then the function actually frees unused memory
Which will never happen, though, because new_byte_size will never be
less than old_byte_size.
> and readers don't wonder why you decided not to shrink the array.
An assertion will clear up things as well.
> Less code and slightly better behavior.
Well, an assert() is one line, while an if () takes two (if () { and });
the behavior will (hopefully) not change either.
But anyway, I'll go with your proposition because, as I said, I don't
like assertions if the code can deal perfectly fine with the bad cases.
Therefore, if at some point in time we decide to use
realloc_refcount_array() to shrink a refcount array, it's better to have
planned for that case.
Max
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH v3 07/22] qcow2: Helper function for refcount modification
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (5 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 06/22] qcow2: Helper for refcount array reallocation Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-20 22:13 ` Eric Blake
` (2 more replies)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 08/22] qcow2: More helpers " Max Reitz
` (14 subsequent siblings)
21 siblings, 3 replies; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Since refcounts do not always have to be a uint16_t, all refcount blocks
and arrays in memory should not have a specific type (thus they become
pointers to void) and for accessing them, two helper functions are used
(a getter and a setter). Those functions are called indirectly through
function pointers in the BDRVQcowState so they may later be exchanged
for different refcount orders.
With the check and repair functions using this function, the refcount
array they are creating will be in big endian byte order; additionally,
using realloc_refcount_array() makes the size of this refcount array
always cluster-aligned. Both combined allow rebuild_refcount_structure()
to drop the bounce buffer which was used to convert parts of the
refcount array to big endian byte order and store them on disk. Instead,
those parts can now be written directly.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2-refcount.c | 131 ++++++++++++++++++++++++++++++-------------------
block/qcow2.h | 8 +++
2 files changed, 89 insertions(+), 50 deletions(-)
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 3cd540c..ff36924 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -32,6 +32,11 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
int64_t offset, int64_t length,
int addend, enum qcow2_discard_type type);
+static uint64_t get_refcount_ro4(const void *refcount_array, uint64_t index);
+
+static void set_refcount_ro4(void *refcount_array, uint64_t index,
+ uint64_t value);
+
/*********************************************************/
/* refcount handling */
@@ -42,6 +47,9 @@ int qcow2_refcount_init(BlockDriverState *bs)
unsigned int refcount_table_size2, i;
int ret;
+ s->get_refcount = &get_refcount_ro4;
+ s->set_refcount = &set_refcount_ro4;
+
assert(s->refcount_table_size <= INT_MAX / sizeof(uint64_t));
refcount_table_size2 = s->refcount_table_size * sizeof(uint64_t);
s->refcount_table = g_try_malloc(refcount_table_size2);
@@ -72,6 +80,19 @@ void qcow2_refcount_close(BlockDriverState *bs)
}
+static uint64_t get_refcount_ro4(const void *refcount_array, uint64_t index)
+{
+ return be16_to_cpu(((const uint16_t *)refcount_array)[index]);
+}
+
+static void set_refcount_ro4(void *refcount_array, uint64_t index,
+ uint64_t value)
+{
+ assert(!(value >> 16));
+ ((uint16_t *)refcount_array)[index] = cpu_to_be16(value);
+}
+
+
static int load_refcount_block(BlockDriverState *bs,
int64_t refcount_block_offset,
void **refcount_block)
@@ -97,7 +118,7 @@ int64_t qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index)
uint64_t refcount_table_index, block_index;
int64_t refcount_block_offset;
int ret;
- uint16_t *refcount_block;
+ void *refcount_block;
int64_t refcount;
refcount_table_index = cluster_index >> s->refcount_block_bits;
@@ -116,20 +137,24 @@ int64_t qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index)
}
ret = qcow2_cache_get(bs, s->refcount_block_cache, refcount_block_offset,
- (void**) &refcount_block);
+ &refcount_block);
if (ret < 0) {
return ret;
}
block_index = cluster_index & (s->refcount_block_size - 1);
- refcount = be16_to_cpu(refcount_block[block_index]);
+ refcount = s->get_refcount(refcount_block, block_index);
- ret = qcow2_cache_put(bs, s->refcount_block_cache,
- (void**) &refcount_block);
+ ret = qcow2_cache_put(bs, s->refcount_block_cache, &refcount_block);
if (ret < 0) {
return ret;
}
+ if (refcount < 0) {
+ /* overflow */
+ return -ERANGE;
+ }
+
return refcount;
}
@@ -169,7 +194,7 @@ static int in_same_refcount_block(BDRVQcowState *s, uint64_t offset_a,
* Returns 0 on success or -errno in error case
*/
static int alloc_refcount_block(BlockDriverState *bs,
- int64_t cluster_index, uint16_t **refcount_block)
+ int64_t cluster_index, void **refcount_block)
{
BDRVQcowState *s = bs->opaque;
unsigned int refcount_table_index;
@@ -196,7 +221,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
}
return load_refcount_block(bs, refcount_block_offset,
- (void**) refcount_block);
+ refcount_block);
}
}
@@ -256,7 +281,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
/* The block describes itself, need to update the cache */
int block_index = (new_block >> s->cluster_bits) &
(s->refcount_block_size - 1);
- (*refcount_block)[block_index] = cpu_to_be16(1);
+ s->set_refcount(*refcount_block, block_index, 1);
} else {
/* Described somewhere else. This can recurse at most twice before we
* arrive at a block that describes itself. */
@@ -274,7 +299,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
/* Initialize the new refcount block only after updating its refcount,
* update_refcount uses the refcount cache itself */
ret = qcow2_cache_get_empty(bs, s->refcount_block_cache, new_block,
- (void**) refcount_block);
+ refcount_block);
if (ret < 0) {
goto fail_block;
}
@@ -308,7 +333,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
return -EAGAIN;
}
- ret = qcow2_cache_put(bs, s->refcount_block_cache, (void**) refcount_block);
+ ret = qcow2_cache_put(bs, s->refcount_block_cache, refcount_block);
if (ret < 0) {
goto fail_block;
}
@@ -362,7 +387,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
s->cluster_size;
uint64_t table_offset = meta_offset + blocks_clusters * s->cluster_size;
uint64_t *new_table = g_try_new0(uint64_t, table_size);
- uint16_t *new_blocks = g_try_malloc0(blocks_clusters * s->cluster_size);
+ void *new_blocks = g_try_malloc0_n(blocks_clusters, s->cluster_size);
assert(table_size > 0 && blocks_clusters > 0);
if (new_table == NULL || new_blocks == NULL) {
@@ -384,7 +409,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
uint64_t table_clusters = size_to_clusters(s, table_size * sizeof(uint64_t));
int block = 0;
for (i = 0; i < table_clusters + blocks_clusters; i++) {
- new_blocks[block++] = cpu_to_be16(1);
+ s->set_refcount(new_blocks, block++, 1);
}
/* Write refcount blocks to disk */
@@ -437,7 +462,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
qcow2_free_clusters(bs, old_table_offset, old_table_size * sizeof(uint64_t),
QCOW2_DISCARD_OTHER);
- ret = load_refcount_block(bs, new_block, (void**) refcount_block);
+ ret = load_refcount_block(bs, new_block, refcount_block);
if (ret < 0) {
return ret;
}
@@ -452,7 +477,7 @@ fail_table:
g_free(new_table);
fail_block:
if (*refcount_block != NULL) {
- qcow2_cache_put(bs, s->refcount_block_cache, (void**) refcount_block);
+ qcow2_cache_put(bs, s->refcount_block_cache, refcount_block);
}
return ret;
}
@@ -532,7 +557,7 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
{
BDRVQcowState *s = bs->opaque;
int64_t start, last, cluster_offset;
- uint16_t *refcount_block = NULL;
+ void *refcount_block = NULL;
int64_t old_table_index = -1;
int ret;
@@ -583,7 +608,12 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
/* we can update the count and save it */
block_index = cluster_index & (s->refcount_block_size - 1);
- refcount = be16_to_cpu(refcount_block[block_index]);
+ refcount = s->get_refcount(refcount_block, block_index);
+ if (refcount < 0) {
+ ret = -ERANGE;
+ goto fail;
+ }
+
refcount += addend;
if (refcount < 0 || refcount > s->refcount_max) {
ret = -EINVAL;
@@ -592,7 +622,7 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
if (refcount == 0 && cluster_index < s->free_cluster_index) {
s->free_cluster_index = cluster_index;
}
- refcount_block[block_index] = cpu_to_be16(refcount);
+ s->set_refcount(refcount_block, block_index, refcount);
if (refcount == 0 && s->discard_passthrough[type]) {
update_refcount_discard(bs, cluster_offset, s->cluster_size);
@@ -608,8 +638,7 @@ fail:
/* Write last changed block to disk */
if (refcount_block) {
int wret;
- wret = qcow2_cache_put(bs, s->refcount_block_cache,
- (void**) &refcount_block);
+ wret = qcow2_cache_put(bs, s->refcount_block_cache, &refcount_block);
if (wret < 0) {
return ret < 0 ? ret : wret;
}
@@ -1140,7 +1169,7 @@ static size_t refcount_array_byte_size(BDRVQcowState *s, uint64_t entries)
* will be aligned to a cluster boundary, and the newly allocated area will be
* zeroed.
*/
-static int realloc_refcount_array(BDRVQcowState *s, uint16_t **array,
+static int realloc_refcount_array(BDRVQcowState *s, void **array,
int64_t *size, int64_t new_size)
{
/* Round to clusters so the array can be directly written to disk */
@@ -1148,7 +1177,7 @@ static int realloc_refcount_array(BDRVQcowState *s, uint16_t **array,
s->cluster_size);
size_t new_byte_size = ROUND_UP(refcount_array_byte_size(s, new_size),
s->cluster_size);
- uint16_t *new_ptr;
+ void *new_ptr;
if (new_byte_size <= old_byte_size) {
*size = new_size;
@@ -1180,12 +1209,13 @@ static int realloc_refcount_array(BDRVQcowState *s, uint16_t **array,
*/
static int inc_refcounts(BlockDriverState *bs,
BdrvCheckResult *res,
- uint16_t **refcount_table,
+ void **refcount_table,
int64_t *refcount_table_size,
int64_t offset, int64_t size)
{
BDRVQcowState *s = bs->opaque;
uint64_t start, last, cluster_offset, k;
+ int64_t refcount;
int ret;
if (size <= 0) {
@@ -1206,11 +1236,14 @@ static int inc_refcounts(BlockDriverState *bs,
}
}
- if (++(*refcount_table)[k] == 0) {
+ refcount = s->get_refcount(*refcount_table, k);
+ if (refcount == s->refcount_max) {
fprintf(stderr, "ERROR: overflow cluster offset=0x%" PRIx64
"\n", cluster_offset);
res->corruptions++;
+ continue;
}
+ s->set_refcount(*refcount_table, k, refcount + 1);
}
return 0;
@@ -1230,8 +1263,9 @@ enum {
* error occurred.
*/
static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res,
- uint16_t **refcount_table, int64_t *refcount_table_size, int64_t l2_offset,
- int flags)
+ void **refcount_table,
+ int64_t *refcount_table_size, int64_t l2_offset,
+ int flags)
{
BDRVQcowState *s = bs->opaque;
uint64_t *l2_table, l2_entry;
@@ -1348,7 +1382,7 @@ fail:
*/
static int check_refcounts_l1(BlockDriverState *bs,
BdrvCheckResult *res,
- uint16_t **refcount_table,
+ void **refcount_table,
int64_t *refcount_table_size,
int64_t l1_table_offset, int l1_size,
int flags)
@@ -1545,7 +1579,7 @@ fail:
*/
static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res,
BdrvCheckMode fix, bool *rebuild,
- uint16_t **refcount_table, int64_t *nb_clusters)
+ void **refcount_table, int64_t *nb_clusters)
{
BDRVQcowState *s = bs->opaque;
int64_t i, size;
@@ -1630,9 +1664,10 @@ resize_fail:
if (ret < 0) {
return ret;
}
- if ((*refcount_table)[cluster] != 1) {
+ if (s->get_refcount(*refcount_table, cluster) != 1) {
fprintf(stderr, "ERROR refcount block %" PRId64
- " refcount=%d\n", i, (*refcount_table)[cluster]);
+ " refcount=%" PRIu64 "\n", i,
+ s->get_refcount(*refcount_table, cluster));
res->corruptions++;
*rebuild = true;
}
@@ -1647,7 +1682,7 @@ resize_fail:
*/
static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
BdrvCheckMode fix, bool *rebuild,
- uint16_t **refcount_table, int64_t *nb_clusters)
+ void **refcount_table, int64_t *nb_clusters)
{
BDRVQcowState *s = bs->opaque;
int64_t i;
@@ -1711,7 +1746,7 @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
BdrvCheckMode fix, bool *rebuild,
int64_t *highest_cluster,
- uint16_t *refcount_table, int64_t nb_clusters)
+ void *refcount_table, int64_t nb_clusters)
{
BDRVQcowState *s = bs->opaque;
int64_t i, refcount1, refcount2;
@@ -1726,7 +1761,7 @@ static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
continue;
}
- refcount2 = refcount_table[i];
+ refcount2 = s->get_refcount(refcount_table, i);
if (refcount1 > 0 || refcount2 > 0) {
*highest_cluster = i;
@@ -1784,7 +1819,7 @@ static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
*/
static int64_t alloc_clusters_imrt(BlockDriverState *bs,
int cluster_count,
- uint16_t **refcount_table,
+ void **refcount_table,
int64_t *imrt_nb_clusters,
int64_t *first_free_cluster)
{
@@ -1801,7 +1836,7 @@ static int64_t alloc_clusters_imrt(BlockDriverState *bs,
contiguous_free_clusters < cluster_count;
cluster++)
{
- if (!(*refcount_table)[cluster]) {
+ if (!s->get_refcount(*refcount_table, cluster)) {
contiguous_free_clusters++;
if (first_gap) {
/* If this is the first free cluster found, update
@@ -1839,7 +1874,7 @@ static int64_t alloc_clusters_imrt(BlockDriverState *bs,
/* Go back to the first free cluster */
cluster -= contiguous_free_clusters;
for (i = 0; i < cluster_count; i++) {
- (*refcount_table)[cluster + i] = 1;
+ s->set_refcount(*refcount_table, cluster + i, 1);
}
return cluster << s->cluster_bits;
@@ -1855,7 +1890,7 @@ static int64_t alloc_clusters_imrt(BlockDriverState *bs,
*/
static int rebuild_refcount_structure(BlockDriverState *bs,
BdrvCheckResult *res,
- uint16_t **refcount_table,
+ void **refcount_table,
int64_t *nb_clusters)
{
BDRVQcowState *s = bs->opaque;
@@ -1863,8 +1898,8 @@ static int rebuild_refcount_structure(BlockDriverState *bs,
int64_t refblock_offset, refblock_start, refblock_index;
uint32_t reftable_size = 0;
uint64_t *on_disk_reftable = NULL;
- uint16_t *on_disk_refblock;
- int i, ret = 0;
+ void *on_disk_refblock;
+ int ret = 0;
struct {
uint64_t reftable_offset;
uint32_t reftable_clusters;
@@ -1874,7 +1909,7 @@ static int rebuild_refcount_structure(BlockDriverState *bs,
write_refblocks:
for (; cluster < *nb_clusters; cluster++) {
- if (!(*refcount_table)[cluster]) {
+ if (!s->get_refcount(*refcount_table, cluster)) {
continue;
}
@@ -1947,17 +1982,13 @@ write_refblocks:
goto fail;
}
- on_disk_refblock = qemu_blockalign0(bs->file, s->cluster_size);
- for (i = 0; i < s->refcount_block_size &&
- refblock_start + i < *nb_clusters; i++)
- {
- on_disk_refblock[i] =
- cpu_to_be16((*refcount_table)[refblock_start + i]);
- }
+ /* The size of *refcount_table is always cluster-aligned, therefore the
+ * write operation will not overflow */
+ on_disk_refblock = (void *)((uintptr_t)*refcount_table +
+ (refblock_index << s->refcount_block_bits));
ret = bdrv_write(bs->file, refblock_offset / BDRV_SECTOR_SIZE,
- (void *)on_disk_refblock, s->cluster_sectors);
- qemu_vfree(on_disk_refblock);
+ on_disk_refblock, s->cluster_sectors);
if (ret < 0) {
fprintf(stderr, "ERROR writing refblock: %s\n", strerror(-ret));
goto fail;
@@ -2052,7 +2083,7 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
BDRVQcowState *s = bs->opaque;
BdrvCheckResult pre_compare_res;
int64_t size, highest_cluster, nb_clusters;
- uint16_t *refcount_table = NULL;
+ void *refcount_table = NULL;
bool rebuild = false;
int ret;
@@ -2101,7 +2132,7 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
/* Because the old reftable has been exchanged for a new one the
* references have to be recalculated */
rebuild = false;
- memset(refcount_table, 0, nb_clusters * sizeof(uint16_t));
+ memset(refcount_table, 0, refcount_array_byte_size(s, nb_clusters));
ret = calculate_refcounts(bs, res, 0, &rebuild, &refcount_table,
&nb_clusters);
if (ret < 0) {
diff --git a/block/qcow2.h b/block/qcow2.h
index 0f8eb15..1c63221 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -213,6 +213,11 @@ typedef struct Qcow2DiscardRegion {
QTAILQ_ENTRY(Qcow2DiscardRegion) next;
} Qcow2DiscardRegion;
+typedef uint64_t Qcow2GetRefcountFunc(const void *refcount_array,
+ uint64_t index);
+typedef void Qcow2SetRefcountFunc(void *refcount_array,
+ uint64_t index, uint64_t value);
+
typedef struct BDRVQcowState {
int cluster_bits;
int cluster_size;
@@ -261,6 +266,9 @@ typedef struct BDRVQcowState {
int refcount_bits;
uint64_t refcount_max;
+ Qcow2GetRefcountFunc *get_refcount;
+ Qcow2SetRefcountFunc *set_refcount;
+
bool discard_passthrough[QCOW2_DISCARD_MAX];
int overlap_check; /* bitmask of Qcow2MetadataOverlap values */
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 07/22] qcow2: Helper function for refcount modification
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 07/22] qcow2: Helper function for refcount modification Max Reitz
@ 2014-11-20 22:13 ` Eric Blake
2014-11-27 15:21 ` Stefan Hajnoczi
2014-11-28 11:11 ` Stefan Hajnoczi
2 siblings, 0 replies; 58+ messages in thread
From: Eric Blake @ 2014-11-20 22:13 UTC (permalink / raw)
To: Max Reitz, qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 1820 bytes --]
On 11/20/2014 10:06 AM, Max Reitz wrote:
> Since refcounts do not always have to be a uint16_t, all refcount blocks
> and arrays in memory should not have a specific type (thus they become
> pointers to void) and for accessing them, two helper functions are used
> (a getter and a setter). Those functions are called indirectly through
> function pointers in the BDRVQcowState so they may later be exchanged
> for different refcount orders.
>
> With the check and repair functions using this function, the refcount
> array they are creating will be in big endian byte order; additionally,
> using realloc_refcount_array() makes the size of this refcount array
> always cluster-aligned. Both combined allow rebuild_refcount_structure()
> to drop the bounce buffer which was used to convert parts of the
> refcount array to big endian byte order and store them on disk. Instead,
> those parts can now be written directly.
Thanks, that helps.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/qcow2-refcount.c | 131 ++++++++++++++++++++++++++++++-------------------
> block/qcow2.h | 8 +++
> 2 files changed, 89 insertions(+), 50 deletions(-)
>
> @@ -2101,7 +2132,7 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
> /* Because the old reftable has been exchanged for a new one the
> * references have to be recalculated */
> rebuild = false;
> - memset(refcount_table, 0, nb_clusters * sizeof(uint16_t));
> + memset(refcount_table, 0, refcount_array_byte_size(s, nb_clusters));
Yep, that makes more sense than what you had in v2.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [Qemu-devel] [PATCH v3 07/22] qcow2: Helper function for refcount modification
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 07/22] qcow2: Helper function for refcount modification Max Reitz
2014-11-20 22:13 ` Eric Blake
@ 2014-11-27 15:21 ` Stefan Hajnoczi
2014-11-27 15:32 ` Max Reitz
2014-11-28 11:11 ` Stefan Hajnoczi
2 siblings, 1 reply; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-27 15:21 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 2629 bytes --]
On Thu, Nov 20, 2014 at 06:06:23PM +0100, Max Reitz wrote:
> @@ -116,20 +137,24 @@ int64_t qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index)
> }
>
> ret = qcow2_cache_get(bs, s->refcount_block_cache, refcount_block_offset,
> - (void**) &refcount_block);
> + &refcount_block);
> if (ret < 0) {
> return ret;
> }
>
> block_index = cluster_index & (s->refcount_block_size - 1);
> - refcount = be16_to_cpu(refcount_block[block_index]);
> + refcount = s->get_refcount(refcount_block, block_index);
>
> - ret = qcow2_cache_put(bs, s->refcount_block_cache,
> - (void**) &refcount_block);
> + ret = qcow2_cache_put(bs, s->refcount_block_cache, &refcount_block);
> if (ret < 0) {
> return ret;
> }
>
> + if (refcount < 0) {
> + /* overflow */
> + return -ERANGE;
> + }
I guess this is because QEMU uses int64_t but the maximum refcount size
is 64 bits.
If we refuse to open files with 64-bit refcounts, can we drop this
check?
> @@ -583,7 +608,12 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
> /* we can update the count and save it */
> block_index = cluster_index & (s->refcount_block_size - 1);
>
> - refcount = be16_to_cpu(refcount_block[block_index]);
> + refcount = s->get_refcount(refcount_block, block_index);
> + if (refcount < 0) {
> + ret = -ERANGE;
> + goto fail;
> + }
Here again.
> @@ -1206,11 +1236,14 @@ static int inc_refcounts(BlockDriverState *bs,
> }
> }
>
> - if (++(*refcount_table)[k] == 0) {
> + refcount = s->get_refcount(*refcount_table, k);
Here the refcount < 0 check is missing. That's why it would be simpler
to eliminate the refcount < 0 case entirely.
> @@ -1726,7 +1761,7 @@ static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
> continue;
> }
>
> - refcount2 = refcount_table[i];
> + refcount2 = s->get_refcount(refcount_table, i);
Missing here too and in other places.
> +typedef uint64_t Qcow2GetRefcountFunc(const void *refcount_array,
> + uint64_t index);
Should the return type be int64_t?
> +typedef void Qcow2SetRefcountFunc(void *refcount_array,
> + uint64_t index, uint64_t value);
Should value's type be int64_t? Just because the type is unsigned
doesn't make (uint64_t)-1ULL a valid value.
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 07/22] qcow2: Helper function for refcount modification
2014-11-27 15:21 ` Stefan Hajnoczi
@ 2014-11-27 15:32 ` Max Reitz
2014-11-28 11:26 ` Stefan Hajnoczi
0 siblings, 1 reply; 58+ messages in thread
From: Max Reitz @ 2014-11-27 15:32 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
On 2014-11-27 at 16:21, Stefan Hajnoczi wrote:
> On Thu, Nov 20, 2014 at 06:06:23PM +0100, Max Reitz wrote:
>> @@ -116,20 +137,24 @@ int64_t qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index)
>> }
>>
>> ret = qcow2_cache_get(bs, s->refcount_block_cache, refcount_block_offset,
>> - (void**) &refcount_block);
>> + &refcount_block);
>> if (ret < 0) {
>> return ret;
>> }
>>
>> block_index = cluster_index & (s->refcount_block_size - 1);
>> - refcount = be16_to_cpu(refcount_block[block_index]);
>> + refcount = s->get_refcount(refcount_block, block_index);
>>
>> - ret = qcow2_cache_put(bs, s->refcount_block_cache,
>> - (void**) &refcount_block);
>> + ret = qcow2_cache_put(bs, s->refcount_block_cache, &refcount_block);
>> if (ret < 0) {
>> return ret;
>> }
>>
>> + if (refcount < 0) {
>> + /* overflow */
>> + return -ERANGE;
>> + }
> I guess this is because QEMU uses int64_t but the maximum refcount size
> is 64 bits.
>
> If we refuse to open files with 64-bit refcounts, can we drop this
> check?
Well, I just reduced the maximum refcount order to 6... If we don't want
to open images with 64 bit refcounts, it'd be easiest to completely
disallow them in the qcow2 specification. But I don't see the need.
>> @@ -583,7 +608,12 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
>> /* we can update the count and save it */
>> block_index = cluster_index & (s->refcount_block_size - 1);
>>
>> - refcount = be16_to_cpu(refcount_block[block_index]);
>> + refcount = s->get_refcount(refcount_block, block_index);
>> + if (refcount < 0) {
>> + ret = -ERANGE;
>> + goto fail;
>> + }
> Here again.
>
>> @@ -1206,11 +1236,14 @@ static int inc_refcounts(BlockDriverState *bs,
>> }
>> }
>>
>> - if (++(*refcount_table)[k] == 0) {
>> + refcount = s->get_refcount(*refcount_table, k);
> Here the refcount < 0 check is missing. That's why it would be simpler
> to eliminate the refcount < 0 case entirely.
It's not missing. This is part of the refcount check, as are all the
following ("in other places"). The refcount check builds a refcount
array in memory all by itself, so it knows for sure there are no
overflows. The line which you omitted directly after this clips the
refcount values against s->refcount_max which is INT64_MAX for 64-bit
refcounts.
Therefore, no overflows possible in the refcount checking functions,
because the refcount checking functions don't introduce the overflows
themselves. The other overflow checks are only in place to reject faulty
images provided from the outside.
>> @@ -1726,7 +1761,7 @@ static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
>> continue;
>> }
>>
>> - refcount2 = refcount_table[i];
>> + refcount2 = s->get_refcount(refcount_table, i);
> Missing here too and in other places.
>
>> +typedef uint64_t Qcow2GetRefcountFunc(const void *refcount_array,
>> + uint64_t index);
> Should the return type be int64_t?
No. If it was, we'd have to check for errors every time we call it, but
it cannot return errors (well, if we let it return uint64_t, it might
return -ERANGE, but that's exactly what I don't want). Therefore, let it
return uint64_t so we know this function cannot fail.
>> +typedef void Qcow2SetRefcountFunc(void *refcount_array,
>> + uint64_t index, uint64_t value);
> Should value's type be int64_t? Just because the type is unsigned
> doesn't make (uint64_t)-1ULL a valid value.
Actually, it does. It's just that the implementation provided here does
not support it.
Since there is an assertion against that case in the 64-bit
implementation for this function, I don't have a problem with using
int64_t here, though. But that would break symmetry with
Qcow2GetRefcountFunc(), and I do have a reason there not to return a
signed value, as explained above.
Max
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 07/22] qcow2: Helper function for refcount modification
2014-11-27 15:32 ` Max Reitz
@ 2014-11-28 11:26 ` Stefan Hajnoczi
2014-12-02 9:54 ` Max Reitz
0 siblings, 1 reply; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-28 11:26 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 4348 bytes --]
On Thu, Nov 27, 2014 at 04:32:52PM +0100, Max Reitz wrote:
> On 2014-11-27 at 16:21, Stefan Hajnoczi wrote:
> >On Thu, Nov 20, 2014 at 06:06:23PM +0100, Max Reitz wrote:
> >>@@ -583,7 +608,12 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
> >> /* we can update the count and save it */
> >> block_index = cluster_index & (s->refcount_block_size - 1);
> >>- refcount = be16_to_cpu(refcount_block[block_index]);
> >>+ refcount = s->get_refcount(refcount_block, block_index);
> >>+ if (refcount < 0) {
> >>+ ret = -ERANGE;
> >>+ goto fail;
> >>+ }
> >Here again.
> >
> >>@@ -1206,11 +1236,14 @@ static int inc_refcounts(BlockDriverState *bs,
> >> }
> >> }
> >>- if (++(*refcount_table)[k] == 0) {
> >>+ refcount = s->get_refcount(*refcount_table, k);
> >Here the refcount < 0 check is missing. That's why it would be simpler
> >to eliminate the refcount < 0 case entirely.
>
> It's not missing. This is part of the refcount check, as are all the
> following ("in other places"). The refcount check builds a refcount array in
> memory all by itself, so it knows for sure there are no overflows. The line
> which you omitted directly after this clips the refcount values against
> s->refcount_max which is INT64_MAX for 64-bit refcounts.
>
> Therefore, no overflows possible in the refcount checking functions, because
> the refcount checking functions don't introduce the overflows themselves.
> The other overflow checks are only in place to reject faulty images provided
> from the outside.
I see, that makes sense.
> >>@@ -1726,7 +1761,7 @@ static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
> >> continue;
> >> }
> >>- refcount2 = refcount_table[i];
> >>+ refcount2 = s->get_refcount(refcount_table, i);
> >Missing here too and in other places.
> >
> >>+typedef uint64_t Qcow2GetRefcountFunc(const void *refcount_array,
> >>+ uint64_t index);
> >Should the return type be int64_t?
>
> No. If it was, we'd have to check for errors every time we call it, but it
> cannot return errors (well, if we let it return uint64_t, it might return
> -ERANGE, but that's exactly what I don't want). Therefore, let it return
> uint64_t so we know this function cannot fail.
>
> >>+typedef void Qcow2SetRefcountFunc(void *refcount_array,
> >>+ uint64_t index, uint64_t value);
> >Should value's type be int64_t? Just because the type is unsigned
> >doesn't make (uint64_t)-1ULL a valid value.
>
> Actually, it does. It's just that the implementation provided here does not
> support it.
>
> Since there is an assertion against that case in the 64-bit implementation
> for this function, I don't have a problem with using int64_t here, though.
> But that would break symmetry with Qcow2GetRefcountFunc(), and I do have a
> reason there not to return a signed value, as explained above.
Okay, so uint64_t is really a different type - it's the qcow2 spec
64-bit refcount. int64_t is the QEMU implementation's internal
representation.
This seems error-prone to me. Maybe comments would have helped, but
it's best to eliminate the problem entirely. Why not bite the bullet
and fix up qcow2?
There are two external function prototypes that need to be changed:
int64_t qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index);
int64_t qcow2_update_cluster_refcount(BlockDriverState *bs,
int64_t cluster_index, int addend,
enum qcow2_discard_type type);
/* Returns 0 on success, -errno on failure */
int qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index,
uint64_t *refcount);
int qcow2_update_cluster_refcount(BlockDriverState *bs,
int64_t cluster_index, int addend,
enum qcow2_discard_type type,
uint64_t *refcount);
Have I missed a fundamental reason why the implementation's internal
refcount type cannot be changed from int64_t to uint64_t?
It would keep the code complexity down and reduce errors.
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 07/22] qcow2: Helper function for refcount modification
2014-11-28 11:26 ` Stefan Hajnoczi
@ 2014-12-02 9:54 ` Max Reitz
0 siblings, 0 replies; 58+ messages in thread
From: Max Reitz @ 2014-12-02 9:54 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
On 2014-11-28 at 12:26, Stefan Hajnoczi wrote:
> On Thu, Nov 27, 2014 at 04:32:52PM +0100, Max Reitz wrote:
>> On 2014-11-27 at 16:21, Stefan Hajnoczi wrote:
>>> On Thu, Nov 20, 2014 at 06:06:23PM +0100, Max Reitz wrote:
>>>> @@ -583,7 +608,12 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
>>>> /* we can update the count and save it */
>>>> block_index = cluster_index & (s->refcount_block_size - 1);
>>>> - refcount = be16_to_cpu(refcount_block[block_index]);
>>>> + refcount = s->get_refcount(refcount_block, block_index);
>>>> + if (refcount < 0) {
>>>> + ret = -ERANGE;
>>>> + goto fail;
>>>> + }
>>> Here again.
>>>
>>>> @@ -1206,11 +1236,14 @@ static int inc_refcounts(BlockDriverState *bs,
>>>> }
>>>> }
>>>> - if (++(*refcount_table)[k] == 0) {
>>>> + refcount = s->get_refcount(*refcount_table, k);
>>> Here the refcount < 0 check is missing. That's why it would be simpler
>>> to eliminate the refcount < 0 case entirely.
>> It's not missing. This is part of the refcount check, as are all the
>> following ("in other places"). The refcount check builds a refcount array in
>> memory all by itself, so it knows for sure there are no overflows. The line
>> which you omitted directly after this clips the refcount values against
>> s->refcount_max which is INT64_MAX for 64-bit refcounts.
>>
>> Therefore, no overflows possible in the refcount checking functions, because
>> the refcount checking functions don't introduce the overflows themselves.
>> The other overflow checks are only in place to reject faulty images provided
>> from the outside.
> I see, that makes sense.
>
>>>> @@ -1726,7 +1761,7 @@ static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
>>>> continue;
>>>> }
>>>> - refcount2 = refcount_table[i];
>>>> + refcount2 = s->get_refcount(refcount_table, i);
>>> Missing here too and in other places.
>>>
>>>> +typedef uint64_t Qcow2GetRefcountFunc(const void *refcount_array,
>>>> + uint64_t index);
>>> Should the return type be int64_t?
>> No. If it was, we'd have to check for errors every time we call it, but it
>> cannot return errors (well, if we let it return uint64_t, it might return
>> -ERANGE, but that's exactly what I don't want). Therefore, let it return
>> uint64_t so we know this function cannot fail.
>>
>>>> +typedef void Qcow2SetRefcountFunc(void *refcount_array,
>>>> + uint64_t index, uint64_t value);
>>> Should value's type be int64_t? Just because the type is unsigned
>>> doesn't make (uint64_t)-1ULL a valid value.
>> Actually, it does. It's just that the implementation provided here does not
>> support it.
>>
>> Since there is an assertion against that case in the 64-bit implementation
>> for this function, I don't have a problem with using int64_t here, though.
>> But that would break symmetry with Qcow2GetRefcountFunc(), and I do have a
>> reason there not to return a signed value, as explained above.
> Okay, so uint64_t is really a different type - it's the qcow2 spec
> 64-bit refcount. int64_t is the QEMU implementation's internal
> representation.
>
> This seems error-prone to me. Maybe comments would have helped, but
> it's best to eliminate the problem entirely. Why not bite the bullet
> and fix up qcow2?
>
> There are two external function prototypes that need to be changed:
>
> int64_t qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index);
>
> int64_t qcow2_update_cluster_refcount(BlockDriverState *bs,
> int64_t cluster_index, int addend,
> enum qcow2_discard_type type);
>
> /* Returns 0 on success, -errno on failure */
> int qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index,
> uint64_t *refcount);
> int qcow2_update_cluster_refcount(BlockDriverState *bs,
> int64_t cluster_index, int addend,
> enum qcow2_discard_type type,
> uint64_t *refcount);
>
> Have I missed a fundamental reason why the implementation's internal
> refcount type cannot be changed from int64_t to uint64_t?
>
> It would keep the code complexity down and reduce errors.
Sounds simple enough. I'll take a look, thanks!
Max
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [Qemu-devel] [PATCH v3 07/22] qcow2: Helper function for refcount modification
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 07/22] qcow2: Helper function for refcount modification Max Reitz
2014-11-20 22:13 ` Eric Blake
2014-11-27 15:21 ` Stefan Hajnoczi
@ 2014-11-28 11:11 ` Stefan Hajnoczi
2014-12-02 9:57 ` Max Reitz
2 siblings, 1 reply; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-28 11:11 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 1043 bytes --]
On Thu, Nov 20, 2014 at 06:06:23PM +0100, Max Reitz wrote:
> @@ -1711,7 +1746,7 @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
> static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
> BdrvCheckMode fix, bool *rebuild,
> int64_t *highest_cluster,
> - uint16_t *refcount_table, int64_t nb_clusters)
> + void *refcount_table, int64_t nb_clusters)
> {
> BDRVQcowState *s = bs->opaque;
> int64_t i, refcount1, refcount2;
An -ERANGE qcow2_get_refcount() return value is treated as a check error
here and we won't be able to rebuild the refcount blocks:
refcount1 = qcow2_get_refcount(bs, i);
if (refcount1 < 0) {
fprintf(stderr, "Can't get refcount for cluster %" PRId64 ": %s\n",
i, strerror(-refcount1));
res->check_errors++;
continue;
}
We should allow rebuilding refcount blocks, -ERANGE is just another
corrupted refcount.
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 07/22] qcow2: Helper function for refcount modification
2014-11-28 11:11 ` Stefan Hajnoczi
@ 2014-12-02 9:57 ` Max Reitz
0 siblings, 0 replies; 58+ messages in thread
From: Max Reitz @ 2014-12-02 9:57 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
On 2014-11-28 at 12:11, Stefan Hajnoczi wrote:
> On Thu, Nov 20, 2014 at 06:06:23PM +0100, Max Reitz wrote:
>> @@ -1711,7 +1746,7 @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
>> static void compare_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
>> BdrvCheckMode fix, bool *rebuild,
>> int64_t *highest_cluster,
>> - uint16_t *refcount_table, int64_t nb_clusters)
>> + void *refcount_table, int64_t nb_clusters)
>> {
>> BDRVQcowState *s = bs->opaque;
>> int64_t i, refcount1, refcount2;
> An -ERANGE qcow2_get_refcount() return value is treated as a check error
> here and we won't be able to rebuild the refcount blocks:
>
> refcount1 = qcow2_get_refcount(bs, i);
> if (refcount1 < 0) {
> fprintf(stderr, "Can't get refcount for cluster %" PRId64 ": %s\n",
> i, strerror(-refcount1));
> res->check_errors++;
> continue;
> }
>
> We should allow rebuilding refcount blocks, -ERANGE is just another
> corrupted refcount.
Right, hopefully this will be fixed by implementing your proposal for
changing "uint64_t qcow2_get_refcount(...)" to "int
qcow2_get_refcount(..., uint64_t *)" etc..
Max
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH v3 08/22] qcow2: More helpers for refcount modification
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (6 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 07/22] qcow2: Helper function for refcount modification Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-20 22:20 ` Eric Blake
2014-11-27 15:31 ` Stefan Hajnoczi
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 09/22] qcow2: Open images with refcount order != 4 Max Reitz
` (13 subsequent siblings)
21 siblings, 2 replies; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add helper functions for getting and setting refcounts in a refcount
array for any possible refcount order, and choose the correct one during
refcount initialization.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2-refcount.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 122 insertions(+), 2 deletions(-)
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index ff36924..60594d8 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -32,10 +32,49 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
int64_t offset, int64_t length,
int addend, enum qcow2_discard_type type);
+static uint64_t get_refcount_ro0(const void *refcount_array, uint64_t index);
+static uint64_t get_refcount_ro1(const void *refcount_array, uint64_t index);
+static uint64_t get_refcount_ro2(const void *refcount_array, uint64_t index);
+static uint64_t get_refcount_ro3(const void *refcount_array, uint64_t index);
static uint64_t get_refcount_ro4(const void *refcount_array, uint64_t index);
+static uint64_t get_refcount_ro5(const void *refcount_array, uint64_t index);
+static uint64_t get_refcount_ro6(const void *refcount_array, uint64_t index);
+static void set_refcount_ro0(void *refcount_array, uint64_t index,
+ uint64_t value);
+static void set_refcount_ro1(void *refcount_array, uint64_t index,
+ uint64_t value);
+static void set_refcount_ro2(void *refcount_array, uint64_t index,
+ uint64_t value);
+static void set_refcount_ro3(void *refcount_array, uint64_t index,
+ uint64_t value);
static void set_refcount_ro4(void *refcount_array, uint64_t index,
uint64_t value);
+static void set_refcount_ro5(void *refcount_array, uint64_t index,
+ uint64_t value);
+static void set_refcount_ro6(void *refcount_array, uint64_t index,
+ uint64_t value);
+
+
+static Qcow2GetRefcountFunc *const get_refcount_funcs[] = {
+ &get_refcount_ro0,
+ &get_refcount_ro1,
+ &get_refcount_ro2,
+ &get_refcount_ro3,
+ &get_refcount_ro4,
+ &get_refcount_ro5,
+ &get_refcount_ro6
+};
+
+static Qcow2SetRefcountFunc *const set_refcount_funcs[] = {
+ &set_refcount_ro0,
+ &set_refcount_ro1,
+ &set_refcount_ro2,
+ &set_refcount_ro3,
+ &set_refcount_ro4,
+ &set_refcount_ro5,
+ &set_refcount_ro6
+};
/*********************************************************/
@@ -47,8 +86,10 @@ int qcow2_refcount_init(BlockDriverState *bs)
unsigned int refcount_table_size2, i;
int ret;
- s->get_refcount = &get_refcount_ro4;
- s->set_refcount = &set_refcount_ro4;
+ assert(s->refcount_order >= 0 && s->refcount_order <= 6);
+
+ s->get_refcount = get_refcount_funcs[s->refcount_order];
+ s->set_refcount = set_refcount_funcs[s->refcount_order];
assert(s->refcount_table_size <= INT_MAX / sizeof(uint64_t));
refcount_table_size2 = s->refcount_table_size * sizeof(uint64_t);
@@ -80,6 +121,59 @@ void qcow2_refcount_close(BlockDriverState *bs)
}
+static uint64_t get_refcount_ro0(const void *refcount_array, uint64_t index)
+{
+ return (((const uint8_t *)refcount_array)[index / 8] >> (index % 8)) & 0x1;
+}
+
+static void set_refcount_ro0(void *refcount_array, uint64_t index,
+ uint64_t value)
+{
+ assert(!(value >> 1));
+ ((uint8_t *)refcount_array)[index / 8] &= ~(0x1 << (index % 8));
+ ((uint8_t *)refcount_array)[index / 8] |= value << (index % 8);
+}
+
+static uint64_t get_refcount_ro1(const void *refcount_array, uint64_t index)
+{
+ return (((const uint8_t *)refcount_array)[index / 4] >> (2 * (index % 4)))
+ & 0x3;
+}
+
+static void set_refcount_ro1(void *refcount_array, uint64_t index,
+ uint64_t value)
+{
+ assert(!(value >> 2));
+ ((uint8_t *)refcount_array)[index / 4] &= ~(0x3 << (2 * (index % 4)));
+ ((uint8_t *)refcount_array)[index / 4] |= value << (2 * (index % 4));
+}
+
+static uint64_t get_refcount_ro2(const void *refcount_array, uint64_t index)
+{
+ return (((const uint8_t *)refcount_array)[index / 2] >> (4 * (index % 2)))
+ & 0xf;
+}
+
+static void set_refcount_ro2(void *refcount_array, uint64_t index,
+ uint64_t value)
+{
+ assert(!(value >> 4));
+ ((uint8_t *)refcount_array)[index / 2] &= ~(0xf << (4 * (index % 2)));
+ ((uint8_t *)refcount_array)[index / 2] |= value << (4 * (index % 2));
+}
+
+static uint64_t get_refcount_ro3(const void *refcount_array, uint64_t index)
+{
+ return ((const uint8_t *)refcount_array)[index];
+}
+
+static void set_refcount_ro3(void *refcount_array, uint64_t index,
+ uint64_t value)
+{
+ assert(!(value >> 8));
+ ((uint8_t *)refcount_array)[index] = value;
+}
+
static uint64_t get_refcount_ro4(const void *refcount_array, uint64_t index)
{
return be16_to_cpu(((const uint16_t *)refcount_array)[index]);
@@ -92,6 +186,32 @@ static void set_refcount_ro4(void *refcount_array, uint64_t index,
((uint16_t *)refcount_array)[index] = cpu_to_be16(value);
}
+static uint64_t get_refcount_ro5(const void *refcount_array, uint64_t index)
+{
+ return be32_to_cpu(((const uint32_t *)refcount_array)[index]);
+}
+
+static void set_refcount_ro5(void *refcount_array, uint64_t index,
+ uint64_t value)
+{
+ assert(!(value >> 32));
+ ((uint32_t *)refcount_array)[index] = cpu_to_be32(value);
+}
+
+static uint64_t get_refcount_ro6(const void *refcount_array, uint64_t index)
+{
+ return be64_to_cpu(((const uint64_t *)refcount_array)[index]);
+}
+
+static void set_refcount_ro6(void *refcount_array, uint64_t index,
+ uint64_t value)
+{
+ /* for 64 bit refcounts, refcount_max is INT64_MAX to prevent signed
+ * overflows (and to allow for -errno style return values) */
+ assert(!(value >> 63));
+ ((uint64_t *)refcount_array)[index] = cpu_to_be64(value);
+}
+
static int load_refcount_block(BlockDriverState *bs,
int64_t refcount_block_offset,
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 08/22] qcow2: More helpers for refcount modification
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 08/22] qcow2: More helpers " Max Reitz
@ 2014-11-20 22:20 ` Eric Blake
2014-11-27 15:31 ` Stefan Hajnoczi
1 sibling, 0 replies; 58+ messages in thread
From: Eric Blake @ 2014-11-20 22:20 UTC (permalink / raw)
To: Max Reitz, qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 572 bytes --]
On 11/20/2014 10:06 AM, Max Reitz wrote:
> Add helper functions for getting and setting refcounts in a refcount
> array for any possible refcount order, and choose the correct one during
> refcount initialization.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/qcow2-refcount.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 122 insertions(+), 2 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [Qemu-devel] [PATCH v3 08/22] qcow2: More helpers for refcount modification
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 08/22] qcow2: More helpers " Max Reitz
2014-11-20 22:20 ` Eric Blake
@ 2014-11-27 15:31 ` Stefan Hajnoczi
1 sibling, 0 replies; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-27 15:31 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 486 bytes --]
On Thu, Nov 20, 2014 at 06:06:24PM +0100, Max Reitz wrote:
> Add helper functions for getting and setting refcounts in a refcount
> array for any possible refcount order, and choose the correct one during
> refcount initialization.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/qcow2-refcount.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 122 insertions(+), 2 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH v3 09/22] qcow2: Open images with refcount order != 4
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (7 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 08/22] qcow2: More helpers " Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-27 15:32 ` Stefan Hajnoczi
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 10/22] qcow2: refcount_order parameter for qcow2_create2 Max Reitz
` (12 subsequent siblings)
21 siblings, 1 reply; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
No longer refuse to open images with a different refcount entry width
than 16 bits; only reject images with a refcount width larger than 64
bits (which is prohibited by the specification).
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qcow2.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index d70e927..18c69fb 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -677,10 +677,10 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
}
/* Check support for various header values */
- if (header.refcount_order != 4) {
- report_unsupported(bs, errp, "%d bit reference counts",
- 1 << header.refcount_order);
- ret = -ENOTSUP;
+ if (header.refcount_order > 6) {
+ error_setg(errp, "Reference count entry width too large; may not "
+ "exceed 64 bits");
+ ret = -EINVAL;
goto fail;
}
s->refcount_order = header.refcount_order;
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 09/22] qcow2: Open images with refcount order != 4
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 09/22] qcow2: Open images with refcount order != 4 Max Reitz
@ 2014-11-27 15:32 ` Stefan Hajnoczi
0 siblings, 0 replies; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-27 15:32 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 501 bytes --]
On Thu, Nov 20, 2014 at 06:06:25PM +0100, Max Reitz wrote:
> No longer refuse to open images with a different refcount entry width
> than 16 bits; only reject images with a refcount width larger than 64
> bits (which is prohibited by the specification).
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> block/qcow2.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH v3 10/22] qcow2: refcount_order parameter for qcow2_create2
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (8 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 09/22] qcow2: Open images with refcount order != 4 Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-20 22:23 ` Eric Blake
2014-11-27 16:25 ` Stefan Hajnoczi
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 11/22] iotests: Prepare for refcount_width option Max Reitz
` (11 subsequent siblings)
21 siblings, 2 replies; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add a refcount_order parameter to qcow2_create2(), use that value for
the image header and for calculating the size required for
preallocation.
For now, always pass 4.
This addition requires changes to the calculation of the file size for
the "full" and "falloc" preallocation modes. That in turn is a nice
opportunity to add a command about that calculation not necessarily
being exact (and that being intentional).
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qcow2.c | 46 +++++++++++++++++++++++++++++++++++-----------
1 file changed, 35 insertions(+), 11 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 18c69fb..39ef74a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1775,7 +1775,7 @@ static int preallocate(BlockDriverState *bs)
static int qcow2_create2(const char *filename, int64_t total_size,
const char *backing_file, const char *backing_format,
int flags, size_t cluster_size, PreallocMode prealloc,
- QemuOpts *opts, int version,
+ QemuOpts *opts, int version, int refcount_order,
Error **errp)
{
/* Calculate cluster_bits */
@@ -1808,9 +1808,21 @@ static int qcow2_create2(const char *filename, int64_t total_size,
int ret;
if (prealloc == PREALLOC_MODE_FULL || prealloc == PREALLOC_MODE_FALLOC) {
+ /* Note: The following calculation does not need to be exact; if it is a
+ * bit off, either some bytes will be "leaked" (which is fine) or we
+ * will need to increase the file size by some bytes (which is fine,
+ * too, as long as the bulk is allocated here). Therefore, using
+ * floating point arithmetic is fine. */
int64_t meta_size = 0;
uint64_t nreftablee, nrefblocke, nl1e, nl2e;
int64_t aligned_total_size = align_offset(total_size, cluster_size);
+ int refblock_bits, refblock_size;
+ /* refcount entry size in bytes */
+ double rces = (1 << refcount_order) / 8.;
+
+ /* see qcow2_open() */
+ refblock_bits = cluster_bits - (refcount_order - 3);
+ refblock_size = 1 << refblock_bits;
/* header: 1 cluster */
meta_size += cluster_size;
@@ -1835,20 +1847,20 @@ static int qcow2_create2(const char *filename, int64_t total_size,
* c = cluster size
* y1 = number of refcount blocks entries
* y2 = meta size including everything
+ * rces = refcount entry size in bytes
* then,
* y1 = (y2 + a)/c
- * y2 = y1 * sizeof(u16) + y1 * sizeof(u16) * sizeof(u64) / c + m
+ * y2 = y1 * rces + y1 * rces * sizeof(u64) / c + m
* we can get y1:
- * y1 = (a + m) / (c - sizeof(u16) - sizeof(u16) * sizeof(u64) / c)
+ * y1 = (a + m) / (c - rces - rces * sizeof(u64) / c)
*/
- nrefblocke = (aligned_total_size + meta_size + cluster_size) /
- (cluster_size - sizeof(uint16_t) -
- 1.0 * sizeof(uint16_t) * sizeof(uint64_t) / cluster_size);
- nrefblocke = align_offset(nrefblocke, cluster_size / sizeof(uint16_t));
- meta_size += nrefblocke * sizeof(uint16_t);
+ nrefblocke = (aligned_total_size + meta_size + cluster_size)
+ / (cluster_size - rces - rces * sizeof(uint64_t)
+ / cluster_size);
+ meta_size += DIV_ROUND_UP(nrefblocke, refblock_size) * cluster_size;
/* total size of refcount tables */
- nreftablee = nrefblocke * sizeof(uint16_t) / cluster_size;
+ nreftablee = nrefblocke / refblock_size;
nreftablee = align_offset(nreftablee, cluster_size / sizeof(uint64_t));
meta_size += nreftablee * sizeof(uint64_t);
@@ -1883,7 +1895,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
.l1_size = cpu_to_be32(0),
.refcount_table_offset = cpu_to_be64(cluster_size),
.refcount_table_clusters = cpu_to_be32(1),
- .refcount_order = cpu_to_be32(4),
+ .refcount_order = cpu_to_be32(refcount_order),
.header_length = cpu_to_be32(sizeof(*header)),
};
@@ -2003,6 +2015,7 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
size_t cluster_size = DEFAULT_CLUSTER_SIZE;
PreallocMode prealloc;
int version = 3;
+ int refcount_width = 16, refcount_order;
Error *local_err = NULL;
int ret;
@@ -2057,8 +2070,19 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
goto finish;
}
+ if (version < 3 && refcount_width != 16) {
+ error_setg(errp, "Different refcount widths than 16 bits require "
+ "compatibility level 1.1 or above (use compat=1.1 or "
+ "greater)");
+ ret = -EINVAL;
+ goto finish;
+ }
+
+ refcount_order = ffs(refcount_width) - 1;
+
ret = qcow2_create2(filename, size, backing_file, backing_fmt, flags,
- cluster_size, prealloc, opts, version, &local_err);
+ cluster_size, prealloc, opts, version, refcount_order,
+ &local_err);
if (local_err) {
error_propagate(errp, local_err);
}
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 10/22] qcow2: refcount_order parameter for qcow2_create2
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 10/22] qcow2: refcount_order parameter for qcow2_create2 Max Reitz
@ 2014-11-20 22:23 ` Eric Blake
2014-11-27 16:25 ` Stefan Hajnoczi
1 sibling, 0 replies; 58+ messages in thread
From: Eric Blake @ 2014-11-20 22:23 UTC (permalink / raw)
To: Max Reitz, qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 892 bytes --]
On 11/20/2014 10:06 AM, Max Reitz wrote:
> Add a refcount_order parameter to qcow2_create2(), use that value for
> the image header and for calculating the size required for
> preallocation.
>
> For now, always pass 4.
>
> This addition requires changes to the calculation of the file size for
> the "full" and "falloc" preallocation modes. That in turn is a nice
> opportunity to add a command about that calculation not necessarily
s/command/comment/
> being exact (and that being intentional).
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
But review still holds. :)
> ---
> block/qcow2.c | 46 +++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 35 insertions(+), 11 deletions(-)
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [Qemu-devel] [PATCH v3 10/22] qcow2: refcount_order parameter for qcow2_create2
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 10/22] qcow2: refcount_order parameter for qcow2_create2 Max Reitz
2014-11-20 22:23 ` Eric Blake
@ 2014-11-27 16:25 ` Stefan Hajnoczi
2014-12-02 9:56 ` Max Reitz
1 sibling, 1 reply; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-27 16:25 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 410 bytes --]
On Thu, Nov 20, 2014 at 06:06:26PM +0100, Max Reitz wrote:
> @@ -2003,6 +2015,7 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
> size_t cluster_size = DEFAULT_CLUSTER_SIZE;
> PreallocMode prealloc;
> int version = 3;
> + int refcount_width = 16, refcount_order;
It would be nice to consistently use either refcount_width or
refcount_bits throughout the code.
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [Qemu-devel] [PATCH v3 10/22] qcow2: refcount_order parameter for qcow2_create2
2014-11-27 16:25 ` Stefan Hajnoczi
@ 2014-12-02 9:56 ` Max Reitz
0 siblings, 0 replies; 58+ messages in thread
From: Max Reitz @ 2014-12-02 9:56 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
On 2014-11-27 at 17:25, Stefan Hajnoczi wrote:
> On Thu, Nov 20, 2014 at 06:06:26PM +0100, Max Reitz wrote:
>> @@ -2003,6 +2015,7 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
>> size_t cluster_size = DEFAULT_CLUSTER_SIZE;
>> PreallocMode prealloc;
>> int version = 3;
>> + int refcount_width = 16, refcount_order;
> It would be nice to consistently use either refcount_width or
> refcount_bits throughout the code.
Will (try to) use refcount_bits everywhere.
Max
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH v3 11/22] iotests: Prepare for refcount_width option
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (9 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 10/22] qcow2: refcount_order parameter for qcow2_create2 Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-28 13:06 ` Stefan Hajnoczi
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 12/22] qcow2: Allow creation with refcount order != 4 Max Reitz
` (10 subsequent siblings)
21 siblings, 1 reply; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Some tests do not work well with certain refcount widths (i.e. you
cannot create internal snapshots with refcount_width=1), so make those
widths unsupported.
Furthermore, add another filter to _filter_img_create in common.filter
which filters out the refcount_width value.
This is necessary for test 079, which does actually work with any
refcount width, but invoking qemu-img directly leads to the
refcount_width value being visible in the output; use _make_test_img
instead which will filter it out.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
tests/qemu-iotests/007 | 3 +++
tests/qemu-iotests/015 | 2 ++
tests/qemu-iotests/026 | 7 +++++++
tests/qemu-iotests/029 | 2 ++
tests/qemu-iotests/051 | 3 +++
tests/qemu-iotests/058 | 2 ++
tests/qemu-iotests/067 | 2 ++
tests/qemu-iotests/079 | 10 ++--------
tests/qemu-iotests/079.out | 38 ++++++++++----------------------------
tests/qemu-iotests/080 | 2 ++
tests/qemu-iotests/089 | 2 ++
tests/qemu-iotests/108 | 2 ++
tests/qemu-iotests/common.filter | 3 ++-
13 files changed, 41 insertions(+), 37 deletions(-)
diff --git a/tests/qemu-iotests/007 b/tests/qemu-iotests/007
index fe1a743..8d92490 100755
--- a/tests/qemu-iotests/007
+++ b/tests/qemu-iotests/007
@@ -43,6 +43,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_fmt qcow2
_supported_proto generic
_supported_os Linux
+# refcount_width must be at least 4 bits so we can create ten internal snapshots
+# (1 bit supports none, 2 bits support two, 4 bits support 14)
+_unsupported_imgopts 'refcount_width=\(1\|2\)[^0-9]'
echo
echo "creating image"
diff --git a/tests/qemu-iotests/015 b/tests/qemu-iotests/015
index 099d757..040ca22 100755
--- a/tests/qemu-iotests/015
+++ b/tests/qemu-iotests/015
@@ -43,6 +43,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_fmt qcow2
_supported_proto generic
_supported_os Linux
+# Internal snapshots are (currently) impossible with refcount_width=1
+_unsupported_imgopts 'refcount_width=1[^0-9]'
echo
echo "creating image"
diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026
index df2884b..3b7a07f 100755
--- a/tests/qemu-iotests/026
+++ b/tests/qemu-iotests/026
@@ -46,6 +46,13 @@ _supported_proto file
_supported_os Linux
_default_cache_mode "writethrough"
_supported_cache_modes "writethrough" "none"
+# The refcount table tests expect a certain minimum width for refcount entries
+# (so that the refcount table actually needs to grow); that minimum is 16 bits,
+# being the default refcount entry width.
+# 32 and 64 bits do not work either, however, due to different leaked cluster
+# count on error.
+# Thus, the only remaining option is refcount_width=16.
+_unsupported_imgopts 'refcount_width=\([^1]\|.\([^6]\|$\)\)'
echo "Errors while writing 128 kB"
echo
diff --git a/tests/qemu-iotests/029 b/tests/qemu-iotests/029
index fa46ace..1cf1bba 100755
--- a/tests/qemu-iotests/029
+++ b/tests/qemu-iotests/029
@@ -44,6 +44,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_fmt qcow2
_supported_proto generic
_supported_os Linux
+# Internal snapshots are (currently) impossible with refcount_width=1
+_unsupported_imgopts 'refcount_width=1[^0-9]'
offset_size=24
offset_l1_size=36
diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 11c858f..8e7d326 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -41,6 +41,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_fmt qcow2
_supported_proto file
_supported_os Linux
+# A compat=0.10 image is created in this test which does not support anything
+# other than refcount_width=16
+_unsupported_imgopts 'refcount_width=\([^1]\|.\([^6]\|$\)\)'
function do_run_qemu()
{
diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058
index 14584cd..3b03d8e 100755
--- a/tests/qemu-iotests/058
+++ b/tests/qemu-iotests/058
@@ -88,6 +88,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_fmt qcow2
_supported_proto file
_require_command QEMU_NBD
+# Internal snapshots are (currently) impossible with refcount_width=1
+_unsupported_imgopts 'refcount_width=1[^0-9]'
echo
echo "== preparing image =="
diff --git a/tests/qemu-iotests/067 b/tests/qemu-iotests/067
index 29cd6b5..7fd5f65 100755
--- a/tests/qemu-iotests/067
+++ b/tests/qemu-iotests/067
@@ -35,6 +35,8 @@ status=1 # failure is the default!
_supported_fmt qcow2
_supported_proto file
_supported_os Linux
+# Because anything other than 16 would change the output of query-block
+_unsupported_imgopts 'refcount_width=\([^1]\|.\([^6]\|$\)\)'
function do_run_qemu()
{
diff --git a/tests/qemu-iotests/079 b/tests/qemu-iotests/079
index 6613cfb..ade6efa 100755
--- a/tests/qemu-iotests/079
+++ b/tests/qemu-iotests/079
@@ -42,19 +42,13 @@ _supported_fmt qcow2
_supported_proto file nfs
_supported_os Linux
-function test_qemu_img()
-{
- echo qemu-img "$@" | _filter_testdir
- $QEMU_IMG "$@" 2>&1 | _filter_testdir
- echo
-}
-
echo "=== Check option preallocation and cluster_size ==="
echo
cluster_sizes="16384 32768 65536 131072 262144 524288 1048576 2097152 4194304"
for s in $cluster_sizes; do
- test_qemu_img create -f $IMGFMT -o preallocation=metadata,cluster_size=$s "$TEST_IMG" 4G
+ IMGOPTS=$(_optstr_add "$IMGOPTS" "preallocation=metadata,cluster_size=$s") \
+ _make_test_img 4G
done
# success, all done
diff --git a/tests/qemu-iotests/079.out b/tests/qemu-iotests/079.out
index ef4b8c9..6dc5d57 100644
--- a/tests/qemu-iotests/079.out
+++ b/tests/qemu-iotests/079.out
@@ -1,32 +1,14 @@
QA output created by 079
=== Check option preallocation and cluster_size ===
-qemu-img create -f qcow2 -o preallocation=metadata,cluster_size=16384 TEST_DIR/t.qcow2 4G
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=4294967296 encryption=off cluster_size=16384 preallocation='metadata' lazy_refcounts=off
-
-qemu-img create -f qcow2 -o preallocation=metadata,cluster_size=32768 TEST_DIR/t.qcow2 4G
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=4294967296 encryption=off cluster_size=32768 preallocation='metadata' lazy_refcounts=off
-
-qemu-img create -f qcow2 -o preallocation=metadata,cluster_size=65536 TEST_DIR/t.qcow2 4G
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=4294967296 encryption=off cluster_size=65536 preallocation='metadata' lazy_refcounts=off
-
-qemu-img create -f qcow2 -o preallocation=metadata,cluster_size=131072 TEST_DIR/t.qcow2 4G
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=4294967296 encryption=off cluster_size=131072 preallocation='metadata' lazy_refcounts=off
-
-qemu-img create -f qcow2 -o preallocation=metadata,cluster_size=262144 TEST_DIR/t.qcow2 4G
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=4294967296 encryption=off cluster_size=262144 preallocation='metadata' lazy_refcounts=off
-
-qemu-img create -f qcow2 -o preallocation=metadata,cluster_size=524288 TEST_DIR/t.qcow2 4G
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=4294967296 encryption=off cluster_size=524288 preallocation='metadata' lazy_refcounts=off
-
-qemu-img create -f qcow2 -o preallocation=metadata,cluster_size=1048576 TEST_DIR/t.qcow2 4G
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=4294967296 encryption=off cluster_size=1048576 preallocation='metadata' lazy_refcounts=off
-
-qemu-img create -f qcow2 -o preallocation=metadata,cluster_size=2097152 TEST_DIR/t.qcow2 4G
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=4294967296 encryption=off cluster_size=2097152 preallocation='metadata' lazy_refcounts=off
-
-qemu-img create -f qcow2 -o preallocation=metadata,cluster_size=4194304 TEST_DIR/t.qcow2 4G
-qemu-img: TEST_DIR/t.qcow2: Cluster size must be a power of two between 512 and 2048k
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=4294967296 encryption=off cluster_size=4194304 preallocation='metadata' lazy_refcounts=off
-
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4294967296 preallocation='metadata'
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4294967296 preallocation='metadata'
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4294967296 preallocation='metadata'
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4294967296 preallocation='metadata'
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4294967296 preallocation='metadata'
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4294967296 preallocation='metadata'
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4294967296 preallocation='metadata'
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4294967296 preallocation='metadata'
+qemu-img: TEST_DIR/t.IMGFMT: Cluster size must be a power of two between 512 and 2048k
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4294967296 preallocation='metadata'
*** done
diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080
index 9de337c..0fa90c1 100755
--- a/tests/qemu-iotests/080
+++ b/tests/qemu-iotests/080
@@ -42,6 +42,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_fmt qcow2
_supported_proto file
_supported_os Linux
+# Internal snapshots are (currently) impossible with refcount_width=1
+_unsupported_imgopts 'refcount_width=1[^0-9]'
header_size=104
diff --git a/tests/qemu-iotests/089 b/tests/qemu-iotests/089
index dffc977..b2da188 100755
--- a/tests/qemu-iotests/089
+++ b/tests/qemu-iotests/089
@@ -41,6 +41,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_fmt qcow2
_supported_proto file
_supported_os Linux
+# Because anything other than 16 would change the output of qemu_io -c info
+_unsupported_imgopts 'refcount_width=\([^1]\|.\([^6]\|$\)\)'
# Using an image filename containing quotation marks will render the JSON data
# below invalid. In that case, we have little choice but simply not to run this
diff --git a/tests/qemu-iotests/108 b/tests/qemu-iotests/108
index 12fc92a..88f9b3c 100755
--- a/tests/qemu-iotests/108
+++ b/tests/qemu-iotests/108
@@ -43,6 +43,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_fmt qcow2
_supported_proto file
_supported_os Linux
+# This test directly modifies a refblock so it relies on refcount_width being 16
+_unsupported_imgopts 'refcount_width=\([^1]\|.\([^6]\|$\)\)'
echo
echo '=== Repairing an image without any refcount table ==='
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index dfb9726..57c0675 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -191,7 +191,8 @@ _filter_img_create()
-e "s# block_size=[0-9]\\+##g" \
-e "s# block_state_zero=\\(on\\|off\\)##g" \
-e "s# log_size=[0-9]\\+##g" \
- -e "s/archipelago:a/TEST_DIR\//g"
+ -e "s/archipelago:a/TEST_DIR\//g" \
+ -e "s# refcount_width=[0-9]\\+##g"
}
_filter_img_info()
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 11/22] iotests: Prepare for refcount_width option
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 11/22] iotests: Prepare for refcount_width option Max Reitz
@ 2014-11-28 13:06 ` Stefan Hajnoczi
0 siblings, 0 replies; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-28 13:06 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1450 bytes --]
On Thu, Nov 20, 2014 at 06:06:27PM +0100, Max Reitz wrote:
> Some tests do not work well with certain refcount widths (i.e. you
> cannot create internal snapshots with refcount_width=1), so make those
> widths unsupported.
>
> Furthermore, add another filter to _filter_img_create in common.filter
> which filters out the refcount_width value.
>
> This is necessary for test 079, which does actually work with any
> refcount width, but invoking qemu-img directly leads to the
> refcount_width value being visible in the output; use _make_test_img
> instead which will filter it out.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> tests/qemu-iotests/007 | 3 +++
> tests/qemu-iotests/015 | 2 ++
> tests/qemu-iotests/026 | 7 +++++++
> tests/qemu-iotests/029 | 2 ++
> tests/qemu-iotests/051 | 3 +++
> tests/qemu-iotests/058 | 2 ++
> tests/qemu-iotests/067 | 2 ++
> tests/qemu-iotests/079 | 10 ++--------
> tests/qemu-iotests/079.out | 38 ++++++++++----------------------------
> tests/qemu-iotests/080 | 2 ++
> tests/qemu-iotests/089 | 2 ++
> tests/qemu-iotests/108 | 2 ++
> tests/qemu-iotests/common.filter | 3 ++-
> 13 files changed, 41 insertions(+), 37 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH v3 12/22] qcow2: Allow creation with refcount order != 4
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (10 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 11/22] iotests: Prepare for refcount_width option Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-28 13:15 ` Stefan Hajnoczi
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 13/22] progress: Allow regressing progress Max Reitz
` (9 subsequent siblings)
21 siblings, 1 reply; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add a creation option to qcow2 for setting the refcount order of images
to be created, and respect that option's value.
This breaks some test outputs, fix them.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qcow2.c | 20 ++++++++
include/block/block_int.h | 1 +
tests/qemu-iotests/049.out | 112 ++++++++++++++++++++++-----------------------
tests/qemu-iotests/082.out | 41 ++++++++++++++---
tests/qemu-iotests/085.out | 38 +++++++--------
5 files changed, 130 insertions(+), 82 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 39ef74a..58f7bf6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2070,6 +2070,17 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
goto finish;
}
+ refcount_width = qemu_opt_get_number_del(opts, BLOCK_OPT_REFCOUNT_WIDTH,
+ refcount_width);
+ if (refcount_width <= 0 || refcount_width > 64 ||
+ !is_power_of_2(refcount_width))
+ {
+ error_setg(errp, "Refcount width must be a power of two and may not "
+ "exceed 64 bits");
+ ret = -EINVAL;
+ goto finish;
+ }
+
if (version < 3 && refcount_width != 16) {
error_setg(errp, "Different refcount widths than 16 bits require "
"compatibility level 1.1 or above (use compat=1.1 or "
@@ -2709,6 +2720,9 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
} else if (!strcmp(desc->name, "lazy_refcounts")) {
lazy_refcounts = qemu_opt_get_bool(opts, "lazy_refcounts",
lazy_refcounts);
+ } else if (!strcmp(desc->name, "refcount_width")) {
+ error_report("Cannot change refcount entry width");
+ return -ENOTSUP;
} else {
/* if this assertion fails, this probably means a new option was
* added without having it covered here */
@@ -2878,6 +2892,12 @@ static QemuOptsList qcow2_create_opts = {
.help = "Postpone refcount updates",
.def_value_str = "off"
},
+ {
+ .name = BLOCK_OPT_REFCOUNT_WIDTH,
+ .type = QEMU_OPT_NUMBER,
+ .help = "Width of a reference count entry in bits",
+ .def_value_str = "16"
+ },
{ /* end of list */ }
}
};
diff --git a/include/block/block_int.h b/include/block/block_int.h
index a1c17b9..c34d610 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -56,6 +56,7 @@
#define BLOCK_OPT_ADAPTER_TYPE "adapter_type"
#define BLOCK_OPT_REDUNDANCY "redundancy"
#define BLOCK_OPT_NOCOW "nocow"
+#define BLOCK_OPT_REFCOUNT_WIDTH "refcount_width"
typedef struct BdrvTrackedRequest {
BlockDriverState *bs;
diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out
index 09ca0ae..9369c12 100644
--- a/tests/qemu-iotests/049.out
+++ b/tests/qemu-iotests/049.out
@@ -4,90 +4,90 @@ QA output created by 049
== 1. Traditional size parameter ==
qemu-img create -f qcow2 TEST_DIR/t.qcow2 1024
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 TEST_DIR/t.qcow2 1024b
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 TEST_DIR/t.qcow2 1k
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 TEST_DIR/t.qcow2 1K
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 TEST_DIR/t.qcow2 1M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1048576 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1048576 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 TEST_DIR/t.qcow2 1G
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1073741824 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1073741824 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 TEST_DIR/t.qcow2 1T
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1099511627776 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1099511627776 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 TEST_DIR/t.qcow2 1024.0
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 TEST_DIR/t.qcow2 1024.0b
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 TEST_DIR/t.qcow2 1.5k
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1536 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1536 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 TEST_DIR/t.qcow2 1.5K
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1536 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1536 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 TEST_DIR/t.qcow2 1.5M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1572864 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1572864 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 TEST_DIR/t.qcow2 1.5G
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1610612736 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1610612736 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 TEST_DIR/t.qcow2 1.5T
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1649267441664 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1649267441664 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
== 2. Specifying size via -o ==
qemu-img create -f qcow2 -o size=1024 TEST_DIR/t.qcow2
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o size=1024b TEST_DIR/t.qcow2
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o size=1k TEST_DIR/t.qcow2
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o size=1K TEST_DIR/t.qcow2
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o size=1M TEST_DIR/t.qcow2
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1048576 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1048576 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o size=1G TEST_DIR/t.qcow2
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1073741824 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1073741824 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o size=1T TEST_DIR/t.qcow2
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1099511627776 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1099511627776 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o size=1024.0 TEST_DIR/t.qcow2
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o size=1024.0b TEST_DIR/t.qcow2
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o size=1.5k TEST_DIR/t.qcow2
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1536 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1536 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o size=1.5K TEST_DIR/t.qcow2
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1536 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1536 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o size=1.5M TEST_DIR/t.qcow2
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1572864 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1572864 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o size=1.5G TEST_DIR/t.qcow2
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1610612736 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1610612736 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o size=1.5T TEST_DIR/t.qcow2
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1649267441664 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1649267441664 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
== 3. Invalid sizes ==
@@ -97,7 +97,7 @@ qemu-img: Image size must be less than 8 EiB!
qemu-img create -f qcow2 -o size=-1024 TEST_DIR/t.qcow2
qemu-img: qcow2 doesn't support shrinking images yet
qemu-img: TEST_DIR/t.qcow2: Could not resize image: Operation not supported
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=-1024 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=-1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- -1k
qemu-img: Image size must be less than 8 EiB!
@@ -105,17 +105,17 @@ qemu-img: Image size must be less than 8 EiB!
qemu-img create -f qcow2 -o size=-1k TEST_DIR/t.qcow2
qemu-img: qcow2 doesn't support shrinking images yet
qemu-img: TEST_DIR/t.qcow2: Could not resize image: Operation not supported
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=-1024 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=-1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- 1kilobyte
-qemu-img: Invalid image size specified! You may use k, M, G, T, P or E suffixes for
+qemu-img: Invalid image size specified! You may use k, M, G, T, P or E suffixes for
qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes.
qemu-img create -f qcow2 -o size=1kilobyte TEST_DIR/t.qcow2
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- foobar
-qemu-img: Invalid image size specified! You may use k, M, G, T, P or E suffixes for
+qemu-img: Invalid image size specified! You may use k, M, G, T, P or E suffixes for
qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes.
qemu-img create -f qcow2 -o size=foobar TEST_DIR/t.qcow2
@@ -125,84 +125,84 @@ qemu-img: TEST_DIR/t.qcow2: Invalid options for file format 'qcow2'
== Check correct interpretation of suffixes for cluster size ==
qemu-img create -f qcow2 -o cluster_size=1024 TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=1024 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=1024 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o cluster_size=1024b TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=1024 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=1024 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o cluster_size=1k TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=1024 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=1024 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o cluster_size=1K TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=1024 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=1024 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o cluster_size=1M TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=1048576 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=1048576 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o cluster_size=1024.0 TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=1024 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=1024 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o cluster_size=1024.0b TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=1024 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=1024 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o cluster_size=0.5k TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=512 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=512 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o cluster_size=0.5K TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=512 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=512 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o cluster_size=0.5M TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=524288 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=524288 lazy_refcounts=off refcount_width=16
== Check compat level option ==
qemu-img create -f qcow2 -o compat=0.10 TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='0.10' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='0.10' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o compat=1.1 TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='1.1' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='1.1' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o compat=0.42 TEST_DIR/t.qcow2 64M
qemu-img: TEST_DIR/t.qcow2: Invalid compatibility level: '0.42'
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='0.42' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='0.42' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o compat=foobar TEST_DIR/t.qcow2 64M
qemu-img: TEST_DIR/t.qcow2: Invalid compatibility level: 'foobar'
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='foobar' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='foobar' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
== Check preallocation option ==
qemu-img create -f qcow2 -o preallocation=off TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 preallocation='off' lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 preallocation='off' lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o preallocation=metadata TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 preallocation='metadata' lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 preallocation='metadata' lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o preallocation=1234 TEST_DIR/t.qcow2 64M
qemu-img: TEST_DIR/t.qcow2: invalid parameter value: 1234
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 preallocation='1234' lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 preallocation='1234' lazy_refcounts=off refcount_width=16
== Check encryption option ==
qemu-img create -f qcow2 -o encryption=off TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o encryption=on TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=on cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=on cluster_size=65536 lazy_refcounts=off refcount_width=16
== Check lazy_refcounts option (only with v3) ==
qemu-img create -f qcow2 -o compat=1.1,lazy_refcounts=off TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='1.1' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='1.1' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o compat=1.1,lazy_refcounts=on TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='1.1' encryption=off cluster_size=65536 lazy_refcounts=on
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='1.1' encryption=off cluster_size=65536 lazy_refcounts=on refcount_width=16
qemu-img create -f qcow2 -o compat=0.10,lazy_refcounts=off TEST_DIR/t.qcow2 64M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='0.10' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='0.10' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
qemu-img create -f qcow2 -o compat=0.10,lazy_refcounts=on TEST_DIR/t.qcow2 64M
qemu-img: TEST_DIR/t.qcow2: Lazy refcounts only supported with compatibility level 1.1 and above (use compat=1.1 or greater)
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='0.10' encryption=off cluster_size=65536 lazy_refcounts=on
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='0.10' encryption=off cluster_size=65536 lazy_refcounts=on refcount_width=16
*** done
diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
index 4b14b4f..dc8bdd3 100644
--- a/tests/qemu-iotests/082.out
+++ b/tests/qemu-iotests/082.out
@@ -3,14 +3,14 @@ QA output created by 082
=== create: Options specified more than once ===
Testing: create -f foo -f qcow2 TEST_DIR/t.qcow2 128M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
image: TEST_DIR/t.IMGFMT
file format: IMGFMT
virtual size: 128M (134217728 bytes)
cluster_size: 65536
Testing: create -f qcow2 -o cluster_size=4k -o lazy_refcounts=on TEST_DIR/t.qcow2 128M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 encryption=off cluster_size=4096 lazy_refcounts=on
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 encryption=off cluster_size=4096 lazy_refcounts=on refcount_width=16
Testing: info TEST_DIR/t.qcow2
image: TEST_DIR/t.qcow2
@@ -25,7 +25,7 @@ Format specific information:
corrupt: false
Testing: create -f qcow2 -o cluster_size=4k -o lazy_refcounts=on -o cluster_size=8k TEST_DIR/t.qcow2 128M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 encryption=off cluster_size=8192 lazy_refcounts=on
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 encryption=off cluster_size=8192 lazy_refcounts=on refcount_width=16
Testing: info TEST_DIR/t.qcow2
image: TEST_DIR/t.qcow2
@@ -40,7 +40,7 @@ Format specific information:
corrupt: false
Testing: create -f qcow2 -o cluster_size=4k,cluster_size=8k TEST_DIR/t.qcow2 128M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 encryption=off cluster_size=8192 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 encryption=off cluster_size=8192 lazy_refcounts=off refcount_width=16
image: TEST_DIR/t.IMGFMT
file format: IMGFMT
virtual size: 128M (134217728 bytes)
@@ -58,6 +58,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: create -f qcow2 -o ? TEST_DIR/t.qcow2 128M
@@ -70,6 +71,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: create -f qcow2 -o cluster_size=4k,help TEST_DIR/t.qcow2 128M
@@ -82,6 +84,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: create -f qcow2 -o cluster_size=4k,? TEST_DIR/t.qcow2 128M
@@ -94,6 +97,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: create -f qcow2 -o help,cluster_size=4k TEST_DIR/t.qcow2 128M
@@ -106,6 +110,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: create -f qcow2 -o ?,cluster_size=4k TEST_DIR/t.qcow2 128M
@@ -118,6 +123,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: create -f qcow2 -o cluster_size=4k -o help TEST_DIR/t.qcow2 128M
@@ -130,6 +136,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: create -f qcow2 -o cluster_size=4k -o ? TEST_DIR/t.qcow2 128M
@@ -142,13 +149,14 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: create -f qcow2 -o backing_file=TEST_DIR/t.qcow2,,help TEST_DIR/t.qcow2 128M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/t.qcow2,help' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/t.qcow2,help' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
Testing: create -f qcow2 -o backing_file=TEST_DIR/t.qcow2,,? TEST_DIR/t.qcow2 128M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/t.qcow2,?' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/t.qcow2,?' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
Testing: create -f qcow2 -o backing_file=TEST_DIR/t.qcow2, -o help TEST_DIR/t.qcow2 128M
qemu-img: Invalid option list: backing_file=TEST_DIR/t.qcow2,
@@ -169,6 +177,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
Testing: create -o help
Supported options:
@@ -177,7 +186,7 @@ size Virtual disk size
=== convert: Options specified more than once ===
Testing: create -f qcow2 TEST_DIR/t.qcow2 128M
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
Testing: convert -f foo -f qcow2 TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
image: TEST_DIR/t.IMGFMT.base
@@ -236,6 +245,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: convert -O qcow2 -o ? TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -248,6 +258,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: convert -O qcow2 -o cluster_size=4k,help TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -260,6 +271,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: convert -O qcow2 -o cluster_size=4k,? TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -272,6 +284,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: convert -O qcow2 -o help,cluster_size=4k TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -284,6 +297,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: convert -O qcow2 -o ?,cluster_size=4k TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -296,6 +310,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: convert -O qcow2 -o cluster_size=4k -o help TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -308,6 +323,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: convert -O qcow2 -o cluster_size=4k -o ? TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -320,6 +336,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: convert -O qcow2 -o backing_file=TEST_DIR/t.qcow2,,help TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base
@@ -347,6 +364,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
Testing: convert -o help
Supported options:
@@ -414,6 +432,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: amend -f qcow2 -o ? TEST_DIR/t.qcow2
@@ -426,6 +445,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: amend -f qcow2 -o cluster_size=4k,help TEST_DIR/t.qcow2
@@ -438,6 +458,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: amend -f qcow2 -o cluster_size=4k,? TEST_DIR/t.qcow2
@@ -450,6 +471,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: amend -f qcow2 -o help,cluster_size=4k TEST_DIR/t.qcow2
@@ -462,6 +484,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: amend -f qcow2 -o ?,cluster_size=4k TEST_DIR/t.qcow2
@@ -474,6 +497,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: amend -f qcow2 -o cluster_size=4k -o help TEST_DIR/t.qcow2
@@ -486,6 +510,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: amend -f qcow2 -o cluster_size=4k -o ? TEST_DIR/t.qcow2
@@ -498,6 +523,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
nocow Turn off copy-on-write (valid only on btrfs)
Testing: amend -f qcow2 -o backing_file=TEST_DIR/t.qcow2,,help TEST_DIR/t.qcow2
@@ -527,6 +553,7 @@ encryption Encrypt the image
cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
+refcount_width Width of a reference count entry in bits
Testing: convert -o help
Supported options:
diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out
index 0f2b17f..2e86fb7 100644
--- a/tests/qemu-iotests/085.out
+++ b/tests/qemu-iotests/085.out
@@ -11,7 +11,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
=== Create a single snapshot on virtio0 ===
-Formatting 'TEST_DIR/1-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/t.qcow2.orig' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/1-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/t.qcow2.orig' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
{"return": {}}
=== Invalid command - missing device and nodename ===
@@ -25,31 +25,31 @@ Formatting 'TEST_DIR/1-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file
=== Create several transactional group snapshots ===
-Formatting 'TEST_DIR/2-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/1-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
-Formatting 'TEST_DIR/2-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/t.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/2-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/1-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
+Formatting 'TEST_DIR/2-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/t.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
{"return": {}}
-Formatting 'TEST_DIR/3-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/2-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
-Formatting 'TEST_DIR/3-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/2-snapshot-v1.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/3-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/2-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
+Formatting 'TEST_DIR/3-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/2-snapshot-v1.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
{"return": {}}
-Formatting 'TEST_DIR/4-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/3-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
-Formatting 'TEST_DIR/4-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/3-snapshot-v1.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/4-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/3-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
+Formatting 'TEST_DIR/4-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/3-snapshot-v1.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
{"return": {}}
-Formatting 'TEST_DIR/5-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/4-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
-Formatting 'TEST_DIR/5-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/4-snapshot-v1.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/5-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/4-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
+Formatting 'TEST_DIR/5-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/4-snapshot-v1.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
{"return": {}}
-Formatting 'TEST_DIR/6-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/5-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
-Formatting 'TEST_DIR/6-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/5-snapshot-v1.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/6-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/5-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
+Formatting 'TEST_DIR/6-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/5-snapshot-v1.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
{"return": {}}
-Formatting 'TEST_DIR/7-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/6-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
-Formatting 'TEST_DIR/7-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/6-snapshot-v1.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/7-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/6-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
+Formatting 'TEST_DIR/7-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/6-snapshot-v1.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
{"return": {}}
-Formatting 'TEST_DIR/8-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/7-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
-Formatting 'TEST_DIR/8-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/7-snapshot-v1.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/8-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/7-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
+Formatting 'TEST_DIR/8-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/7-snapshot-v1.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
{"return": {}}
-Formatting 'TEST_DIR/9-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/8-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
-Formatting 'TEST_DIR/9-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/8-snapshot-v1.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/9-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/8-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
+Formatting 'TEST_DIR/9-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/8-snapshot-v1.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
{"return": {}}
-Formatting 'TEST_DIR/10-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/9-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
-Formatting 'TEST_DIR/10-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/9-snapshot-v1.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
+Formatting 'TEST_DIR/10-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/9-snapshot-v0.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
+Formatting 'TEST_DIR/10-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file='TEST_DIR/9-snapshot-v1.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off refcount_width=16
{"return": {}}
*** done
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 12/22] qcow2: Allow creation with refcount order != 4
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 12/22] qcow2: Allow creation with refcount order != 4 Max Reitz
@ 2014-11-28 13:15 ` Stefan Hajnoczi
0 siblings, 0 replies; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-28 13:15 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 726 bytes --]
On Thu, Nov 20, 2014 at 06:06:28PM +0100, Max Reitz wrote:
> Add a creation option to qcow2 for setting the refcount order of images
> to be created, and respect that option's value.
>
> This breaks some test outputs, fix them.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> block/qcow2.c | 20 ++++++++
> include/block/block_int.h | 1 +
> tests/qemu-iotests/049.out | 112 ++++++++++++++++++++++-----------------------
> tests/qemu-iotests/082.out | 41 ++++++++++++++---
> tests/qemu-iotests/085.out | 38 +++++++--------
> 5 files changed, 130 insertions(+), 82 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH v3 13/22] progress: Allow regressing progress
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (11 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 12/22] qcow2: Allow creation with refcount order != 4 Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-20 22:29 ` Eric Blake
2014-11-28 13:17 ` Stefan Hajnoczi
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 14/22] block: Add opaque value to the amend CB Max Reitz
` (8 subsequent siblings)
21 siblings, 2 replies; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Progress may regress; this should be displayed correctly by
qemu_progress_print().
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
util/qemu-progress.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/util/qemu-progress.c b/util/qemu-progress.c
index 4ee5cd0..c0fb14d 100644
--- a/util/qemu-progress.c
+++ b/util/qemu-progress.c
@@ -152,6 +152,7 @@ void qemu_progress_print(float delta, int max)
state.current = current;
if (current > (state.last_print + state.min_skip) ||
+ current < (state.last_print - state.min_skip) ||
(current == 100) || (current == 0)) {
state.last_print = state.current;
state.print();
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 13/22] progress: Allow regressing progress
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 13/22] progress: Allow regressing progress Max Reitz
@ 2014-11-20 22:29 ` Eric Blake
2014-11-28 13:17 ` Stefan Hajnoczi
1 sibling, 0 replies; 58+ messages in thread
From: Eric Blake @ 2014-11-20 22:29 UTC (permalink / raw)
To: Max Reitz, qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 963 bytes --]
On 11/20/2014 10:06 AM, Max Reitz wrote:
> Progress may regress; this should be displayed correctly by
> qemu_progress_print().
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> util/qemu-progress.c | 1 +
> 1 file changed, 1 insertion(+)
Reviewed-by: Eric Blake <eblake@redhat.com>
>
> diff --git a/util/qemu-progress.c b/util/qemu-progress.c
> index 4ee5cd0..c0fb14d 100644
> --- a/util/qemu-progress.c
> +++ b/util/qemu-progress.c
> @@ -152,6 +152,7 @@ void qemu_progress_print(float delta, int max)
> state.current = current;
>
> if (current > (state.last_print + state.min_skip) ||
> + current < (state.last_print - state.min_skip) ||
> (current == 100) || (current == 0)) {
Maybe we can drop the redundant () as long as we are touching this area
of code? But not worth a respin.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 13/22] progress: Allow regressing progress
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 13/22] progress: Allow regressing progress Max Reitz
2014-11-20 22:29 ` Eric Blake
@ 2014-11-28 13:17 ` Stefan Hajnoczi
1 sibling, 0 replies; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-28 13:17 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 328 bytes --]
On Thu, Nov 20, 2014 at 06:06:29PM +0100, Max Reitz wrote:
> Progress may regress; this should be displayed correctly by
> qemu_progress_print().
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> util/qemu-progress.c | 1 +
> 1 file changed, 1 insertion(+)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH v3 14/22] block: Add opaque value to the amend CB
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (12 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 13/22] progress: Allow regressing progress Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 15/22] qcow2: Use error_report() in qcow2_amend_options() Max Reitz
` (7 subsequent siblings)
21 siblings, 0 replies; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add an opaque value which is to be passed to the bdrv_amend_options()
status callback.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block.c | 4 ++--
block/qcow2-cluster.c | 14 ++++++++------
block/qcow2.c | 9 +++++----
block/qcow2.h | 3 ++-
include/block/block.h | 4 ++--
include/block/block_int.h | 3 ++-
qemu-img.c | 5 +++--
7 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/block.c b/block.c
index c979d51..c34b188 100644
--- a/block.c
+++ b/block.c
@@ -5790,12 +5790,12 @@ void bdrv_add_before_write_notifier(BlockDriverState *bs,
}
int bdrv_amend_options(BlockDriverState *bs, QemuOpts *opts,
- BlockDriverAmendStatusCB *status_cb)
+ BlockDriverAmendStatusCB *status_cb, void *cb_opaque)
{
if (!bs->drv->bdrv_amend_options) {
return -ENOTSUP;
}
- return bs->drv->bdrv_amend_options(bs, opts, status_cb);
+ return bs->drv->bdrv_amend_options(bs, opts, status_cb, cb_opaque);
}
/* This function will be called by the bdrv_recurse_is_first_non_filter method
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index ab43902..2daf334 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1620,7 +1620,8 @@ fail:
static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
int l1_size, int64_t *visited_l1_entries,
int64_t l1_entries,
- BlockDriverAmendStatusCB *status_cb)
+ BlockDriverAmendStatusCB *status_cb,
+ void *cb_opaque)
{
BDRVQcowState *s = bs->opaque;
bool is_active_l1 = (l1_table == s->l1_table);
@@ -1646,7 +1647,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
/* unallocated */
(*visited_l1_entries)++;
if (status_cb) {
- status_cb(bs, *visited_l1_entries, l1_entries);
+ status_cb(bs, *visited_l1_entries, l1_entries, cb_opaque);
}
continue;
}
@@ -1768,7 +1769,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
(*visited_l1_entries)++;
if (status_cb) {
- status_cb(bs, *visited_l1_entries, l1_entries);
+ status_cb(bs, *visited_l1_entries, l1_entries, cb_opaque);
}
}
@@ -1797,7 +1798,8 @@ fail:
* qcow2 version which doesn't yet support metadata zero clusters.
*/
int qcow2_expand_zero_clusters(BlockDriverState *bs,
- BlockDriverAmendStatusCB *status_cb)
+ BlockDriverAmendStatusCB *status_cb,
+ void *cb_opaque)
{
BDRVQcowState *s = bs->opaque;
uint64_t *l1_table = NULL;
@@ -1814,7 +1816,7 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs,
ret = expand_zero_clusters_in_l1(bs, s->l1_table, s->l1_size,
&visited_l1_entries, l1_entries,
- status_cb);
+ status_cb, cb_opaque);
if (ret < 0) {
goto fail;
}
@@ -1849,7 +1851,7 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs,
ret = expand_zero_clusters_in_l1(bs, l1_table, s->snapshots[i].l1_size,
&visited_l1_entries, l1_entries,
- status_cb);
+ status_cb, cb_opaque);
if (ret < 0) {
goto fail;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index 58f7bf6..39e6061 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2597,7 +2597,7 @@ static int qcow2_load_vmstate(BlockDriverState *bs, uint8_t *buf,
* have to be removed.
*/
static int qcow2_downgrade(BlockDriverState *bs, int target_version,
- BlockDriverAmendStatusCB *status_cb)
+ BlockDriverAmendStatusCB *status_cb, void *cb_opaque)
{
BDRVQcowState *s = bs->opaque;
int current_version = s->qcow_version;
@@ -2646,7 +2646,7 @@ static int qcow2_downgrade(BlockDriverState *bs, int target_version,
/* clearing autoclear features is trivial */
s->autoclear_features = 0;
- ret = qcow2_expand_zero_clusters(bs, status_cb);
+ ret = qcow2_expand_zero_clusters(bs, status_cb, cb_opaque);
if (ret < 0) {
return ret;
}
@@ -2661,7 +2661,8 @@ static int qcow2_downgrade(BlockDriverState *bs, int target_version,
}
static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
- BlockDriverAmendStatusCB *status_cb)
+ BlockDriverAmendStatusCB *status_cb,
+ void *cb_opaque)
{
BDRVQcowState *s = bs->opaque;
int old_version = s->qcow_version, new_version = old_version;
@@ -2742,7 +2743,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
return ret;
}
} else {
- ret = qcow2_downgrade(bs, new_version, status_cb);
+ ret = qcow2_downgrade(bs, new_version, status_cb, cb_opaque);
if (ret < 0) {
return ret;
}
diff --git a/block/qcow2.h b/block/qcow2.h
index 1c63221..fe12c54 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -551,7 +551,8 @@ int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
int qcow2_zero_clusters(BlockDriverState *bs, uint64_t offset, int nb_sectors);
int qcow2_expand_zero_clusters(BlockDriverState *bs,
- BlockDriverAmendStatusCB *status_cb);
+ BlockDriverAmendStatusCB *status_cb,
+ void *cb_opaque);
/* qcow2-snapshot.c functions */
int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info);
diff --git a/include/block/block.h b/include/block/block.h
index 287dcab..5fd4c81 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -272,9 +272,9 @@ int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix);
* block driver; total_work_size may change during the course of the amendment
* operation */
typedef void BlockDriverAmendStatusCB(BlockDriverState *bs, int64_t offset,
- int64_t total_work_size);
+ int64_t total_work_size, void *opaque);
int bdrv_amend_options(BlockDriverState *bs_new, QemuOpts *opts,
- BlockDriverAmendStatusCB *status_cb);
+ BlockDriverAmendStatusCB *status_cb, void *cb_opaque);
/* external snapshots */
bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index c34d610..e2167ab 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -234,7 +234,8 @@ struct BlockDriver {
BdrvCheckMode fix);
int (*bdrv_amend_options)(BlockDriverState *bs, QemuOpts *opts,
- BlockDriverAmendStatusCB *status_cb);
+ BlockDriverAmendStatusCB *status_cb,
+ void *cb_opaque);
void (*bdrv_debug_event)(BlockDriverState *bs, BlkDebugEvent event);
diff --git a/qemu-img.c b/qemu-img.c
index a42335c..e0595fe 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2869,7 +2869,8 @@ out:
}
static void amend_status_cb(BlockDriverState *bs,
- int64_t offset, int64_t total_work_size)
+ int64_t offset, int64_t total_work_size,
+ void *opaque)
{
qemu_progress_print(100.f * offset / total_work_size, 0);
}
@@ -2982,7 +2983,7 @@ static int img_amend(int argc, char **argv)
/* In case the driver does not call amend_status_cb() */
qemu_progress_print(0.f, 0);
- ret = bdrv_amend_options(bs, opts, &amend_status_cb);
+ ret = bdrv_amend_options(bs, opts, &amend_status_cb, NULL);
qemu_progress_print(100.f, 0);
if (ret < 0) {
error_report("Error while amending options: %s", strerror(-ret));
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH v3 15/22] qcow2: Use error_report() in qcow2_amend_options()
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (13 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 14/22] block: Add opaque value to the amend CB Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-28 13:21 ` Stefan Hajnoczi
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 16/22] qcow2: Use abort() instead of assert(false) Max Reitz
` (6 subsequent siblings)
21 siblings, 1 reply; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qcow2.c | 14 ++++++--------
tests/qemu-iotests/061.out | 14 +++++++-------
2 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 39e6061..4e19615 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2691,11 +2691,11 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
} else if (!strcmp(compat, "1.1")) {
new_version = 3;
} else {
- fprintf(stderr, "Unknown compatibility level %s.\n", compat);
+ error_report("Unknown compatibility level %s", compat);
return -EINVAL;
}
} else if (!strcmp(desc->name, "preallocation")) {
- fprintf(stderr, "Cannot change preallocation mode.\n");
+ error_report("Cannot change preallocation mode");
return -ENOTSUP;
} else if (!strcmp(desc->name, "size")) {
new_size = qemu_opt_get_size(opts, "size", 0);
@@ -2706,16 +2706,14 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
} else if (!strcmp(desc->name, "encryption")) {
encrypt = qemu_opt_get_bool(opts, "encryption", s->crypt_method);
if (encrypt != !!s->crypt_method) {
- fprintf(stderr, "Changing the encryption flag is not "
- "supported.\n");
+ error_report("Changing the encryption flag is not supported");
return -ENOTSUP;
}
} else if (!strcmp(desc->name, "cluster_size")) {
cluster_size = qemu_opt_get_size(opts, "cluster_size",
cluster_size);
if (cluster_size != s->cluster_size) {
- fprintf(stderr, "Changing the cluster size is not "
- "supported.\n");
+ error_report("Changing the cluster size is not supported");
return -ENOTSUP;
}
} else if (!strcmp(desc->name, "lazy_refcounts")) {
@@ -2761,8 +2759,8 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
if (s->use_lazy_refcounts != lazy_refcounts) {
if (lazy_refcounts) {
if (s->qcow_version < 3) {
- fprintf(stderr, "Lazy refcounts only supported with compatibility "
- "level 1.1 and above (use compat=1.1 or greater)\n");
+ error_report("Lazy refcounts only supported with compatibility "
+ "level 1.1 and above (use compat=1.1 or greater)");
return -EINVAL;
}
s->compatible_features |= QCOW2_COMPAT_LAZY_REFCOUNTS;
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index 9045544..2fd92ca 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -281,19 +281,19 @@ No errors were found on the image.
=== Testing invalid configurations ===
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-Lazy refcounts only supported with compatibility level 1.1 and above (use compat=1.1 or greater)
+qemu-img: Lazy refcounts only supported with compatibility level 1.1 and above (use compat=1.1 or greater)
qemu-img: Error while amending options: Invalid argument
-Lazy refcounts only supported with compatibility level 1.1 and above (use compat=1.1 or greater)
+qemu-img: Lazy refcounts only supported with compatibility level 1.1 and above (use compat=1.1 or greater)
qemu-img: Error while amending options: Invalid argument
-Unknown compatibility level 0.42.
+qemu-img: Unknown compatibility level 0.42
qemu-img: Error while amending options: Invalid argument
qemu-img: Invalid parameter 'foo'
qemu-img: Invalid options for file format 'qcow2'
-Changing the cluster size is not supported.
+qemu-img: Changing the cluster size is not supported
qemu-img: Error while amending options: Operation not supported
-Changing the encryption flag is not supported.
+qemu-img: Changing the encryption flag is not supported
qemu-img: Error while amending options: Operation not supported
-Cannot change preallocation mode.
+qemu-img: Cannot change preallocation mode
qemu-img: Error while amending options: Operation not supported
=== Testing correct handling of unset value ===
@@ -301,7 +301,7 @@ qemu-img: Error while amending options: Operation not supported
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
Should work:
Should not work:
-Changing the cluster size is not supported.
+qemu-img: Changing the cluster size is not supported
qemu-img: Error while amending options: Operation not supported
=== Testing zero expansion on inactive clusters ===
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH v3 16/22] qcow2: Use abort() instead of assert(false)
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (14 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 15/22] qcow2: Use error_report() in qcow2_amend_options() Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-28 13:21 ` Stefan Hajnoczi
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 17/22] qcow2: Split upgrade/downgrade paths for amend Max Reitz
` (5 subsequent siblings)
21 siblings, 1 reply; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qcow2.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 4e19615..e8d54ab 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2723,9 +2723,9 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
error_report("Cannot change refcount entry width");
return -ENOTSUP;
} else {
- /* if this assertion fails, this probably means a new option was
+ /* if this point is reached, this probably means a new option was
* added without having it covered here */
- assert(false);
+ abort();
}
desc++;
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH v3 17/22] qcow2: Split upgrade/downgrade paths for amend
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (15 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 16/22] qcow2: Use abort() instead of assert(false) Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-28 13:22 ` Stefan Hajnoczi
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 18/22] qcow2: Use intermediate helper CB " Max Reitz
` (4 subsequent siblings)
21 siblings, 1 reply; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
If the image version should be upgraded, that is the first we should do;
if it should be downgraded, that is the last we should do. So split the
version change block into an upgrade part at the start and a downgrade
part at the end.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qcow2.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index e8d54ab..c5c4e86 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2731,20 +2731,13 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
desc++;
}
- if (new_version != old_version) {
- if (new_version > old_version) {
- /* Upgrade */
- s->qcow_version = new_version;
- ret = qcow2_update_header(bs);
- if (ret < 0) {
- s->qcow_version = old_version;
- return ret;
- }
- } else {
- ret = qcow2_downgrade(bs, new_version, status_cb, cb_opaque);
- if (ret < 0) {
- return ret;
- }
+ /* Upgrade first (some features may require compat=1.1) */
+ if (new_version > old_version) {
+ s->qcow_version = new_version;
+ ret = qcow2_update_header(bs);
+ if (ret < 0) {
+ s->qcow_version = old_version;
+ return ret;
}
}
@@ -2758,7 +2751,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
if (s->use_lazy_refcounts != lazy_refcounts) {
if (lazy_refcounts) {
- if (s->qcow_version < 3) {
+ if (new_version < 3) {
error_report("Lazy refcounts only supported with compatibility "
"level 1.1 and above (use compat=1.1 or greater)");
return -EINVAL;
@@ -2794,6 +2787,14 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
}
}
+ /* Downgrade last (so unsupported features can be removed before) */
+ if (new_version < old_version) {
+ ret = qcow2_downgrade(bs, new_version, status_cb, cb_opaque);
+ if (ret < 0) {
+ return ret;
+ }
+ }
+
return 0;
}
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 17/22] qcow2: Split upgrade/downgrade paths for amend
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 17/22] qcow2: Split upgrade/downgrade paths for amend Max Reitz
@ 2014-11-28 13:22 ` Stefan Hajnoczi
0 siblings, 0 replies; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-28 13:22 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 574 bytes --]
On Thu, Nov 20, 2014 at 06:06:33PM +0100, Max Reitz wrote:
> If the image version should be upgraded, that is the first we should do;
> if it should be downgraded, that is the last we should do. So split the
> version change block into an upgrade part at the start and a downgrade
> part at the end.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> block/qcow2.c | 31 ++++++++++++++++---------------
> 1 file changed, 16 insertions(+), 15 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH v3 18/22] qcow2: Use intermediate helper CB for amend
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (16 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 17/22] qcow2: Split upgrade/downgrade paths for amend Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-28 14:13 ` Stefan Hajnoczi
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 19/22] qcow2: Add function for refcount order amendment Max Reitz
` (3 subsequent siblings)
21 siblings, 1 reply; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
If there is more than one time-consuming operation to be performed for
qcow2_amend_options(), we need an intermediate CB which coordinates the
progress of the individual operations and passes the result to the
original status callback.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qcow2.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 79 insertions(+), 1 deletion(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index c5c4e86..23d2d59 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2660,6 +2660,75 @@ static int qcow2_downgrade(BlockDriverState *bs, int target_version,
return 0;
}
+typedef enum Qcow2AmendOperation {
+ /* This is the value Qcow2AmendHelperCBInfo::last_operation will be
+ * statically initialized to so that the helper CB can discern the first
+ * invocation from an operation change */
+ QCOW2_NO_OPERATION = 0,
+
+ QCOW2_DOWNGRADING,
+} Qcow2AmendOperation;
+
+typedef struct Qcow2AmendHelperCBInfo {
+ /* The code coordinating the amend operations should only modify
+ * these four fields; the rest will be managed by the CB */
+ BlockDriverAmendStatusCB *original_status_cb;
+ void *original_cb_opaque;
+
+ Qcow2AmendOperation current_operation;
+
+ /* Total number of operations to perform (only set once) */
+ int total_operations;
+
+ /* The following fields are managed by the CB */
+
+ /* Number of operations completed */
+ int operations_completed;
+
+ /* Cumulative offset of all completed operations */
+ int64_t offset_completed;
+
+ Qcow2AmendOperation last_operation;
+ int64_t last_work_size;
+} Qcow2AmendHelperCBInfo;
+
+static void qcow2_amend_helper_cb(BlockDriverState *bs,
+ int64_t operation_offset,
+ int64_t operation_work_size, void *opaque)
+{
+ Qcow2AmendHelperCBInfo *info = opaque;
+ int64_t current_work_size;
+ int64_t projected_work_size;
+
+ if (info->current_operation != info->last_operation) {
+ if (info->last_operation != QCOW2_NO_OPERATION) {
+ info->offset_completed += info->last_work_size;
+ info->operations_completed++;
+ }
+
+ info->last_operation = info->current_operation;
+ }
+
+ assert(info->total_operations > 0);
+ assert(info->operations_completed < info->total_operations);
+
+ info->last_work_size = operation_work_size;
+
+ current_work_size = info->offset_completed + operation_work_size;
+
+ /* current_work_size is the total work size for (operations_completed + 1)
+ * operations (which includes this one), so multiply it by the number of
+ * operations not covered and divide it by the number of operations
+ * covered to get a projection for the operations not covered */
+ projected_work_size = current_work_size * (info->total_operations -
+ info->operations_completed - 1)
+ / (info->operations_completed + 1);
+
+ info->original_status_cb(bs, info->offset_completed + operation_offset,
+ current_work_size + projected_work_size,
+ info->original_cb_opaque);
+}
+
static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
BlockDriverAmendStatusCB *status_cb,
void *cb_opaque)
@@ -2674,6 +2743,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
bool encrypt;
int ret;
QemuOptDesc *desc = opts->list->desc;
+ Qcow2AmendHelperCBInfo helper_cb_info;
while (desc && desc->name) {
if (!qemu_opt_find(opts, desc->name)) {
@@ -2731,6 +2801,12 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
desc++;
}
+ helper_cb_info = (Qcow2AmendHelperCBInfo){
+ .original_status_cb = status_cb,
+ .original_cb_opaque = cb_opaque,
+ .total_operations = (new_version < old_version)
+ };
+
/* Upgrade first (some features may require compat=1.1) */
if (new_version > old_version) {
s->qcow_version = new_version;
@@ -2789,7 +2865,9 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
/* Downgrade last (so unsupported features can be removed before) */
if (new_version < old_version) {
- ret = qcow2_downgrade(bs, new_version, status_cb, cb_opaque);
+ helper_cb_info.current_operation = QCOW2_DOWNGRADING;
+ ret = qcow2_downgrade(bs, new_version, &qcow2_amend_helper_cb,
+ &helper_cb_info);
if (ret < 0) {
return ret;
}
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 18/22] qcow2: Use intermediate helper CB for amend
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 18/22] qcow2: Use intermediate helper CB " Max Reitz
@ 2014-11-28 14:13 ` Stefan Hajnoczi
2014-12-02 9:59 ` Max Reitz
0 siblings, 1 reply; 58+ messages in thread
From: Stefan Hajnoczi @ 2014-11-28 14:13 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1673 bytes --]
On Thu, Nov 20, 2014 at 06:06:34PM +0100, Max Reitz wrote:
> @@ -2674,6 +2743,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
> bool encrypt;
> int ret;
> QemuOptDesc *desc = opts->list->desc;
> + Qcow2AmendHelperCBInfo helper_cb_info;
>
> while (desc && desc->name) {
> if (!qemu_opt_find(opts, desc->name)) {
> @@ -2731,6 +2801,12 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
> desc++;
> }
>
> + helper_cb_info = (Qcow2AmendHelperCBInfo){
> + .original_status_cb = status_cb,
> + .original_cb_opaque = cb_opaque,
> + .total_operations = (new_version < old_version)
If you respin, another way of writing this is without total_operations
here (so it initializes to 0)...
> + };
> +
> /* Upgrade first (some features may require compat=1.1) */
> if (new_version > old_version) {
> s->qcow_version = new_version;
> @@ -2789,7 +2865,9 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
>
> /* Downgrade last (so unsupported features can be removed before) */
> if (new_version < old_version) {
> - ret = qcow2_downgrade(bs, new_version, status_cb, cb_opaque);
> + helper_cb_info.current_operation = QCOW2_DOWNGRADING;
...and then helper_cb_info.total_operations++ here.
That way the new_version < old_version check is not duplicated into the
helper_cb_info initializer.
The code is clearer because we assign current_operation and
total_operations at the same time.
Just a style suggestion, feel free to ignore if you don't like it.
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 18/22] qcow2: Use intermediate helper CB for amend
2014-11-28 14:13 ` Stefan Hajnoczi
@ 2014-12-02 9:59 ` Max Reitz
0 siblings, 0 replies; 58+ messages in thread
From: Max Reitz @ 2014-12-02 9:59 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Kevin Wolf, qemu-devel
On 2014-11-28 at 15:13, Stefan Hajnoczi wrote:
> On Thu, Nov 20, 2014 at 06:06:34PM +0100, Max Reitz wrote:
>> @@ -2674,6 +2743,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
>> bool encrypt;
>> int ret;
>> QemuOptDesc *desc = opts->list->desc;
>> + Qcow2AmendHelperCBInfo helper_cb_info;
>>
>> while (desc && desc->name) {
>> if (!qemu_opt_find(opts, desc->name)) {
>> @@ -2731,6 +2801,12 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
>> desc++;
>> }
>>
>> + helper_cb_info = (Qcow2AmendHelperCBInfo){
>> + .original_status_cb = status_cb,
>> + .original_cb_opaque = cb_opaque,
>> + .total_operations = (new_version < old_version)
> If you respin, another way of writing this is without total_operations
> here (so it initializes to 0)...
>
>> + };
>> +
>> /* Upgrade first (some features may require compat=1.1) */
>> if (new_version > old_version) {
>> s->qcow_version = new_version;
>> @@ -2789,7 +2865,9 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
>>
>> /* Downgrade last (so unsupported features can be removed before) */
>> if (new_version < old_version) {
>> - ret = qcow2_downgrade(bs, new_version, status_cb, cb_opaque);
>> + helper_cb_info.current_operation = QCOW2_DOWNGRADING;
> ...and then helper_cb_info.total_operations++ here.
>
> That way the new_version < old_version check is not duplicated into the
> helper_cb_info initializer.
>
> The code is clearer because we assign current_operation and
> total_operations at the same time.
>
> Just a style suggestion, feel free to ignore if you don't like it.
I think helper_cb_info.total_operations should be set right when
creating the object. We can only do .total_operations++ once, and that
is for the very first operation because after that is executed, we may
no longer change .total_operations; and I don't think we should treat
the first operation differently than the rest.
Max
^ permalink raw reply [flat|nested] 58+ messages in thread
* [Qemu-devel] [PATCH v3 19/22] qcow2: Add function for refcount order amendment
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (17 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 18/22] qcow2: Use intermediate helper CB " Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 20/22] qcow2: Invoke refcount order amendment function Max Reitz
` (2 subsequent siblings)
21 siblings, 0 replies; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add a function qcow2_change_refcount_order() which allows changing the
refcount order of a qcow2 image.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qcow2-refcount.c | 454 +++++++++++++++++++++++++++++++++++++++++++++++++
block/qcow2.h | 4 +
2 files changed, 458 insertions(+)
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 60594d8..d619728 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -2477,3 +2477,457 @@ int qcow2_pre_write_overlap_check(BlockDriverState *bs, int ign, int64_t offset,
return 0;
}
+
+/* A pointer to a function of this type is given to walk_over_reftable(). That
+ * function will create refblocks and pass them to a RefblockFinishOp once they
+ * are completed (@refblock). @refblock_empty is set if the refblock is
+ * completely empty.
+ *
+ * Along with the refblock, a corresponding reftable entry is passed, in the
+ * reftable @reftable (which may be reallocated) at @reftable_index.
+ *
+ * @allocated should be set to true if a new cluster has been allocated.
+ */
+typedef int (RefblockFinishOp)(BlockDriverState *bs, uint64_t **reftable,
+ uint64_t reftable_index, uint64_t *reftable_size,
+ void *refblock, bool refblock_empty,
+ bool *allocated, Error **errp);
+
+/**
+ * This "operation" for walk_over_reftable() allocates the refblock on disk (if
+ * it is not empty) and inserts its offset into the new reftable. The size of
+ * this new reftable is increased as required.
+ */
+static int alloc_refblock(BlockDriverState *bs, uint64_t **reftable,
+ uint64_t reftable_index, uint64_t *reftable_size,
+ void *refblock, bool refblock_empty, bool *allocated,
+ Error **errp)
+{
+ BDRVQcowState *s = bs->opaque;
+ int64_t offset;
+
+ if (!refblock_empty && reftable_index >= *reftable_size) {
+ uint64_t *new_reftable;
+ uint64_t new_reftable_size;
+
+ new_reftable_size = ROUND_UP(reftable_index + 1,
+ s->cluster_size / sizeof(uint64_t));
+ if (new_reftable_size > QCOW_MAX_REFTABLE_SIZE / sizeof(uint64_t)) {
+ error_setg(errp,
+ "This operation would make the refcount table grow "
+ "beyond the maximum size supported by QEMU, aborting");
+ return -ENOTSUP;
+ }
+
+ new_reftable = g_try_realloc(*reftable, new_reftable_size *
+ sizeof(uint64_t));
+ if (!new_reftable) {
+ error_setg(errp, "Failed to increase reftable buffer size");
+ return -ENOMEM;
+ }
+
+ memset(new_reftable + *reftable_size, 0,
+ (new_reftable_size - *reftable_size) * sizeof(uint64_t));
+
+ *reftable = new_reftable;
+ *reftable_size = new_reftable_size;
+ }
+
+ if (!refblock_empty && !(*reftable)[reftable_index]) {
+ offset = qcow2_alloc_clusters(bs, s->cluster_size);
+ if (offset < 0) {
+ error_setg_errno(errp, -offset, "Failed to allocate refblock");
+ return offset;
+ }
+ (*reftable)[reftable_index] = offset;
+ *allocated = true;
+ }
+
+ return 0;
+}
+
+/**
+ * This "operation" for walk_over_reftable() writes the refblock to disk at the
+ * offset specified by the new reftable's entry. It does not modify the new
+ * reftable or change any refcounts.
+ */
+static int flush_refblock(BlockDriverState *bs, uint64_t **reftable,
+ uint64_t reftable_index, uint64_t *reftable_size,
+ void *refblock, bool refblock_empty, bool *allocated,
+ Error **errp)
+{
+ BDRVQcowState *s = bs->opaque;
+ int64_t offset;
+ int ret;
+
+ if (reftable_index < *reftable_size && (*reftable)[reftable_index]) {
+ offset = (*reftable)[reftable_index];
+
+ ret = qcow2_pre_write_overlap_check(bs, 0, offset, s->cluster_size);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Overlap check failed");
+ return ret;
+ }
+
+ ret = bdrv_pwrite(bs->file, offset, refblock, s->cluster_size);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to write refblock");
+ return ret;
+ }
+ } else {
+ assert(refblock_empty);
+ }
+
+ return 0;
+}
+
+/**
+ * This function walks over the existing reftable and every referenced refblock;
+ * if @new_set_refcount is non-NULL, it is called for every refcount entry to
+ * create an equal new entry in the passed @new_refblock. Once that
+ * @new_refblock is completely filled, @operation will be called.
+ *
+ * @status_cb and @cb_opaque are used for the amend operation's status callback.
+ * @index is the index of the walk_over_reftable() calls and @total is the total
+ * number of walk_over_reftable() calls per amend operation. Both are used for
+ * calculating the parameters for the status callback.
+ *
+ * @allocated is set to true if a new cluster has been allocated.
+ */
+static int walk_over_reftable(BlockDriverState *bs, uint64_t **new_reftable,
+ uint64_t *new_reftable_index,
+ uint64_t *new_reftable_size,
+ void *new_refblock, int new_refblock_size,
+ int new_refcount_bits,
+ RefblockFinishOp *operation, bool *allocated,
+ Qcow2SetRefcountFunc *new_set_refcount,
+ BlockDriverAmendStatusCB *status_cb,
+ void *cb_opaque, int index, int total,
+ Error **errp)
+{
+ BDRVQcowState *s = bs->opaque;
+ uint64_t reftable_index;
+ bool new_refblock_empty = true;
+ int refblock_index;
+ int new_refblock_index = 0;
+ int ret;
+
+ for (reftable_index = 0; reftable_index < s->refcount_table_size;
+ reftable_index++)
+ {
+ uint64_t refblock_offset = s->refcount_table[reftable_index]
+ & REFT_OFFSET_MASK;
+
+ status_cb(bs, (uint64_t)index * s->refcount_table_size + reftable_index,
+ (uint64_t)total * s->refcount_table_size, cb_opaque);
+
+ if (refblock_offset) {
+ void *refblock;
+
+ if (offset_into_cluster(s, refblock_offset)) {
+ qcow2_signal_corruption(bs, true, -1, -1, "Refblock offset %#"
+ PRIx64 " unaligned (reftable index: %#"
+ PRIx64 ")", refblock_offset,
+ reftable_index);
+ error_setg(errp,
+ "Image is corrupt (unaligned refblock offset)");
+ return -EIO;
+ }
+
+ ret = qcow2_cache_get(bs, s->refcount_block_cache, refblock_offset,
+ &refblock);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to retrieve refblock");
+ return ret;
+ }
+
+ for (refblock_index = 0; refblock_index < s->refcount_block_size;
+ refblock_index++)
+ {
+ uint64_t refcount;
+
+ if (new_refblock_index >= new_refblock_size) {
+ /* new_refblock is now complete */
+ ret = operation(bs, new_reftable, *new_reftable_index,
+ new_reftable_size, new_refblock,
+ new_refblock_empty, allocated, errp);
+ if (ret < 0) {
+ qcow2_cache_put(bs, s->refcount_block_cache, &refblock);
+ return ret;
+ }
+
+ (*new_reftable_index)++;
+ new_refblock_index = 0;
+ new_refblock_empty = true;
+ }
+
+ refcount = s->get_refcount(refblock, refblock_index);
+ if (new_refcount_bits < 64 && refcount >> new_refcount_bits) {
+ uint64_t offset;
+
+ qcow2_cache_put(bs, s->refcount_block_cache, &refblock);
+
+ offset = ((reftable_index << s->refcount_block_bits)
+ + refblock_index) << s->cluster_bits;
+
+ error_setg(errp, "Cannot decrease refcount entry width to "
+ "%i bits: Cluster at offset %#" PRIx64 " has a "
+ "refcount of %" PRIu64, new_refcount_bits,
+ offset, refcount);
+ return -EINVAL;
+ }
+
+ if (new_set_refcount) {
+ new_set_refcount(new_refblock, new_refblock_index++, refcount);
+ } else {
+ new_refblock_index++;
+ }
+ new_refblock_empty = new_refblock_empty && refcount == 0;
+ }
+
+ ret = qcow2_cache_put(bs, s->refcount_block_cache, &refblock);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to put refblock back into "
+ "the cache");
+ return ret;
+ }
+ } else {
+ /* No refblock means every refcount is 0 */
+ for (refblock_index = 0; refblock_index < s->refcount_block_size;
+ refblock_index++)
+ {
+ if (new_refblock_index >= new_refblock_size) {
+ /* new_refblock is now complete */
+ ret = operation(bs, new_reftable, *new_reftable_index,
+ new_reftable_size, new_refblock,
+ new_refblock_empty, allocated, errp);
+ if (ret < 0) {
+ return ret;
+ }
+
+ (*new_reftable_index)++;
+ new_refblock_index = 0;
+ new_refblock_empty = true;
+ }
+
+ if (new_set_refcount) {
+ new_set_refcount(new_refblock, new_refblock_index++, 0);
+ } else {
+ new_refblock_index++;
+ }
+ }
+ }
+ }
+
+ if (new_refblock_index > 0) {
+ /* Complete the potentially existing partially filled final refblock */
+ if (new_set_refcount) {
+ for (; new_refblock_index < new_refblock_size;
+ new_refblock_index++)
+ {
+ new_set_refcount(new_refblock, new_refblock_index, 0);
+ }
+ }
+
+ ret = operation(bs, new_reftable, *new_reftable_index,
+ new_reftable_size, new_refblock, new_refblock_empty,
+ allocated, errp);
+ if (ret < 0) {
+ return ret;
+ }
+
+ (*new_reftable_index)++;
+ }
+
+ status_cb(bs, (uint64_t)(index + 1) * s->refcount_table_size,
+ (uint64_t)total * s->refcount_table_size, cb_opaque);
+
+ return 0;
+}
+
+int qcow2_change_refcount_order(BlockDriverState *bs, int refcount_order,
+ BlockDriverAmendStatusCB *status_cb,
+ void *cb_opaque, Error **errp)
+{
+ BDRVQcowState *s = bs->opaque;
+ Qcow2GetRefcountFunc *new_get_refcount;
+ Qcow2SetRefcountFunc *new_set_refcount;
+ void *new_refblock = qemu_blockalign(bs->file, s->cluster_size);
+ uint64_t *new_reftable = NULL, new_reftable_size = 0;
+ uint64_t *old_reftable, old_reftable_size, old_reftable_offset;
+ uint64_t new_reftable_index = 0;
+ uint64_t i;
+ int64_t new_reftable_offset = 0, allocated_reftable_size = 0;
+ int new_refblock_size, new_refcount_bits = 1 << refcount_order;
+ int old_refcount_order;
+ int walk_index = 0;
+ int ret;
+ bool new_allocation;
+
+ assert(s->qcow_version >= 3);
+ assert(refcount_order >= 0 && refcount_order <= 6);
+
+ /* see qcow2_open() */
+ new_refblock_size = 1 << (s->cluster_bits - (refcount_order - 3));
+
+ new_get_refcount = get_refcount_funcs[refcount_order];
+ new_set_refcount = set_refcount_funcs[refcount_order];
+
+
+ do {
+ int total_walks;
+
+ new_allocation = false;
+
+ /* At least we have to do this walk and the one which writes the
+ * refblocks; also, at least we have to do this loop here at least
+ * twice (normally), first to do the allocations, and second to
+ * determine that everything is correctly allocated, this then makes
+ * three walks in total */
+ total_walks = MAX(walk_index + 2, 3);
+
+ /* First, allocate the structures so they are present in the refcount
+ * structures */
+ ret = walk_over_reftable(bs, &new_reftable, &new_reftable_index,
+ &new_reftable_size, NULL, new_refblock_size,
+ new_refcount_bits, &alloc_refblock,
+ &new_allocation, NULL, status_cb, cb_opaque,
+ walk_index++, total_walks, errp);
+ if (ret < 0) {
+ goto done;
+ }
+
+ new_reftable_index = 0;
+
+ if (new_allocation) {
+ if (new_reftable_offset) {
+ qcow2_free_clusters(bs, new_reftable_offset,
+ allocated_reftable_size * sizeof(uint64_t),
+ QCOW2_DISCARD_NEVER);
+ }
+
+ new_reftable_offset = qcow2_alloc_clusters(bs, new_reftable_size *
+ sizeof(uint64_t));
+ if (new_reftable_offset < 0) {
+ error_setg_errno(errp, -new_reftable_offset,
+ "Failed to allocate the new reftable");
+ ret = new_reftable_offset;
+ goto done;
+ }
+ allocated_reftable_size = new_reftable_size;
+ }
+ } while (new_allocation);
+
+ /* Second, write the new refblocks */
+ ret = walk_over_reftable(bs, &new_reftable, &new_reftable_index,
+ &new_reftable_size, new_refblock,
+ new_refblock_size, new_refcount_bits,
+ &flush_refblock, &new_allocation, new_set_refcount,
+ status_cb, cb_opaque, walk_index, walk_index + 1,
+ errp);
+ if (ret < 0) {
+ goto done;
+ }
+ assert(!new_allocation);
+
+
+ /* Write the new reftable */
+ ret = qcow2_pre_write_overlap_check(bs, 0, new_reftable_offset,
+ new_reftable_size * sizeof(uint64_t));
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Overlap check failed");
+ goto done;
+ }
+
+ for (i = 0; i < new_reftable_size; i++) {
+ cpu_to_be64s(&new_reftable[i]);
+ }
+
+ ret = bdrv_pwrite(bs->file, new_reftable_offset, new_reftable,
+ new_reftable_size * sizeof(uint64_t));
+
+ for (i = 0; i < new_reftable_size; i++) {
+ be64_to_cpus(&new_reftable[i]);
+ }
+
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to write the new reftable");
+ goto done;
+ }
+
+
+ /* Empty the refcount cache */
+ ret = qcow2_cache_flush(bs, s->refcount_block_cache);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to flush the refblock cache");
+ goto done;
+ }
+
+ /* Update the image header to point to the new reftable; this only updates
+ * the fields which are relevant to qcow2_update_header(); other fields
+ * such as s->refcount_table or s->refcount_bits stay stale for now
+ * (because we have to restore everything if qcow2_update_header() fails) */
+ old_refcount_order = s->refcount_order;
+ old_reftable_size = s->refcount_table_size;
+ old_reftable_offset = s->refcount_table_offset;
+
+ s->refcount_order = refcount_order;
+ s->refcount_table_size = new_reftable_size;
+ s->refcount_table_offset = new_reftable_offset;
+
+ ret = qcow2_update_header(bs);
+ if (ret < 0) {
+ s->refcount_order = old_refcount_order;
+ s->refcount_table_size = old_reftable_size;
+ s->refcount_table_offset = old_reftable_offset;
+ error_setg_errno(errp, -ret, "Failed to update the qcow2 header");
+ goto done;
+ }
+
+ /* Now update the rest of the in-memory information */
+ old_reftable = s->refcount_table;
+ s->refcount_table = new_reftable;
+
+ s->refcount_bits = 1 << refcount_order;
+ if (refcount_order < 6) {
+ s->refcount_max = (UINT64_C(1) << s->refcount_bits) - 1;
+ } else {
+ s->refcount_max = INT64_MAX;
+ }
+
+ s->refcount_block_bits = s->cluster_bits - (refcount_order - 3);
+ s->refcount_block_size = 1 << s->refcount_block_bits;
+
+ s->get_refcount = new_get_refcount;
+ s->set_refcount = new_set_refcount;
+
+ /* For cleaning up all old refblocks and the old reftable below the "done"
+ * label */
+ new_reftable = old_reftable;
+ new_reftable_size = old_reftable_size;
+ new_reftable_offset = old_reftable_offset;
+
+done:
+ if (new_reftable) {
+ /* On success, new_reftable actually points to the old reftable (and
+ * new_reftable_size is the old reftable's size); but that is just
+ * fine */
+ for (i = 0; i < new_reftable_size; i++) {
+ uint64_t offset = new_reftable[i] & REFT_OFFSET_MASK;
+ if (offset) {
+ qcow2_free_clusters(bs, offset, s->cluster_size,
+ QCOW2_DISCARD_OTHER);
+ }
+ }
+ g_free(new_reftable);
+
+ if (new_reftable_offset > 0) {
+ qcow2_free_clusters(bs, new_reftable_offset,
+ new_reftable_size * sizeof(uint64_t),
+ QCOW2_DISCARD_OTHER);
+ }
+ }
+
+ qemu_vfree(new_refblock);
+ return ret;
+}
diff --git a/block/qcow2.h b/block/qcow2.h
index fe12c54..5b96519 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -526,6 +526,10 @@ int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset,
int qcow2_pre_write_overlap_check(BlockDriverState *bs, int ign, int64_t offset,
int64_t size);
+int qcow2_change_refcount_order(BlockDriverState *bs, int refcount_order,
+ BlockDriverAmendStatusCB *status_cb,
+ void *cb_opaque, Error **errp);
+
/* qcow2-cluster.c functions */
int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
bool exact_size);
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH v3 20/22] qcow2: Invoke refcount order amendment function
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (18 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 19/22] qcow2: Add function for refcount order amendment Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 21/22] qcow2: Point to amend function in check Max Reitz
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 22/22] iotests: Add test for different refcount widths Max Reitz
21 siblings, 0 replies; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Make use of qcow2_change_refcount_order() to support changing the
refcount order with qemu-img amend.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qcow2.c | 44 +++++++++++++++++++++++++++++++++++---------
1 file changed, 35 insertions(+), 9 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 23d2d59..aea54df 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2612,13 +2612,7 @@ static int qcow2_downgrade(BlockDriverState *bs, int target_version,
}
if (s->refcount_order != 4) {
- /* we would have to convert the image to a refcount_order == 4 image
- * here; however, since qemu (at the time of writing this) does not
- * support anything different than 4 anyway, there is no point in doing
- * so right now; however, we should error out (if qemu supports this in
- * the future and this code has not been adapted) */
- error_report("qcow2_downgrade: Image refcount orders other than 4 are "
- "currently not supported.");
+ error_report("compat=0.10 requires refcount_width=16");
return -ENOTSUP;
}
@@ -2666,6 +2660,7 @@ typedef enum Qcow2AmendOperation {
* invocation from an operation change */
QCOW2_NO_OPERATION = 0,
+ QCOW2_CHANGING_REFCOUNT_ORDER,
QCOW2_DOWNGRADING,
} Qcow2AmendOperation;
@@ -2741,6 +2736,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
const char *compat = NULL;
uint64_t cluster_size = s->cluster_size;
bool encrypt;
+ int refcount_width = s->refcount_bits;
int ret;
QemuOptDesc *desc = opts->list->desc;
Qcow2AmendHelperCBInfo helper_cb_info;
@@ -2790,8 +2786,16 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
lazy_refcounts = qemu_opt_get_bool(opts, "lazy_refcounts",
lazy_refcounts);
} else if (!strcmp(desc->name, "refcount_width")) {
- error_report("Cannot change refcount entry width");
- return -ENOTSUP;
+ refcount_width = qemu_opt_get_number(opts, "refcount_width",
+ refcount_width);
+
+ if (refcount_width <= 0 || refcount_width > 64 ||
+ !is_power_of_2(refcount_width))
+ {
+ error_report("Refcount width must be a power of two and may "
+ "not exceed 64 bits");
+ return -EINVAL;
+ }
} else {
/* if this point is reached, this probably means a new option was
* added without having it covered here */
@@ -2805,6 +2809,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
.original_status_cb = status_cb,
.original_cb_opaque = cb_opaque,
.total_operations = (new_version < old_version)
+ + (s->refcount_bits != refcount_width)
};
/* Upgrade first (some features may require compat=1.1) */
@@ -2817,6 +2822,27 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
}
}
+ if (s->refcount_bits != refcount_width) {
+ int refcount_order = ffs(refcount_width) - 1;
+ Error *local_error = NULL;
+
+ if (new_version < 3 && refcount_width != 16) {
+ error_report("Different refcount widths than 16 bits require "
+ "compatibility level 1.1 or above (use compat=1.1 or "
+ "greater)");
+ return -EINVAL;
+ }
+
+ helper_cb_info.current_operation = QCOW2_CHANGING_REFCOUNT_ORDER;
+ ret = qcow2_change_refcount_order(bs, refcount_order,
+ &qcow2_amend_helper_cb,
+ &helper_cb_info, &local_error);
+ if (ret < 0) {
+ qerror_report_err(local_error);
+ return ret;
+ }
+ }
+
if (backing_file || backing_format) {
ret = qcow2_change_backing_file(bs, backing_file ?: bs->backing_file,
backing_format ?: bs->backing_format);
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH v3 21/22] qcow2: Point to amend function in check
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (19 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 20/22] qcow2: Invoke refcount order amendment function Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 22/22] iotests: Add test for different refcount widths Max Reitz
21 siblings, 0 replies; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
If a reference count is not representable with the current refcount
order, the image check should point to qemu-img amend for increasing the
refcount order. However, qemu-img amend needs write access to the image
which cannot be provided if the image is marked corrupt; and the image
check will not mark the image consistent unless everything actually is
consistent.
Therefore, if an image is marked corrupt and the image check encounters
a reference count overflow, it cannot be fixed by using qemu-img amend
to increase the refcount order. Instead, one has to use qemu-img convert
to create a completely new copy of the image in this case.
Alternatively, we may want to give the user a way of manually removing
the corrupt flag, maybe through qemu-img amend, but this is not part of
this patch.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qcow2-refcount.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index d619728..9201d30 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1360,6 +1360,9 @@ static int inc_refcounts(BlockDriverState *bs,
if (refcount == s->refcount_max) {
fprintf(stderr, "ERROR: overflow cluster offset=0x%" PRIx64
"\n", cluster_offset);
+ fprintf(stderr, "Use qemu-img amend to increase the refcount entry "
+ "width or qemu-img convert to create a clean copy if the "
+ "image cannot be opened for writing\n");
res->corruptions++;
continue;
}
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* [Qemu-devel] [PATCH v3 22/22] iotests: Add test for different refcount widths
2014-11-20 17:06 [Qemu-devel] [PATCH v3 00/22] qcow2: Support refcount orders != 4 Max Reitz
` (20 preceding siblings ...)
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 21/22] qcow2: Point to amend function in check Max Reitz
@ 2014-11-20 17:06 ` Max Reitz
2014-11-20 23:04 ` Eric Blake
21 siblings, 1 reply; 58+ messages in thread
From: Max Reitz @ 2014-11-20 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
Add a test for conversion between different refcount widths and errors
specific to certain widths (i.e. snapshots with refcount_width=1).
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/112 | 278 +++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/112.out | 143 +++++++++++++++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 422 insertions(+)
create mode 100755 tests/qemu-iotests/112
create mode 100644 tests/qemu-iotests/112.out
diff --git a/tests/qemu-iotests/112 b/tests/qemu-iotests/112
new file mode 100755
index 0000000..b184e34
--- /dev/null
+++ b/tests/qemu-iotests/112
@@ -0,0 +1,278 @@
+#!/bin/bash
+#
+# Test cases for different refcount_widths
+#
+# Copyright (C) 2014 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 qcow2-specific low-level functionality
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+# This test will set refcount_width on its own which would conflict with the
+# manual setting; compat will be overridden as well
+_unsupported_imgopts refcount_width 'compat=0.10'
+
+function print_refcount_width()
+{
+ $QEMU_IMG info "$TEST_IMG" | sed -n '/refcount width:/ s/^ *//p'
+}
+
+echo
+echo '=== refcount_width limits ==='
+echo
+
+# Must be positive (non-zero)
+IMGOPTS="$IMGOPTS,refcount_width=0" _make_test_img 64M
+# Must be positive (non-negative)
+IMGOPTS="$IMGOPTS,refcount_width=-1" _make_test_img 64M
+# May not exceed 64
+IMGOPTS="$IMGOPTS,refcount_width=128" _make_test_img 64M
+# Must be a power of two
+IMGOPTS="$IMGOPTS,refcount_width=42" _make_test_img 64M
+
+# 1 is the minimum
+IMGOPTS="$IMGOPTS,refcount_width=1" _make_test_img 64M
+print_refcount_width
+
+# 64 is the maximum
+IMGOPTS="$IMGOPTS,refcount_width=64" _make_test_img 64M
+print_refcount_width
+
+# 16 is the default
+_make_test_img 64M
+print_refcount_width
+
+echo
+echo '=== refcount_width and compat=0.10 ==='
+echo
+
+# Should work
+IMGOPTS="$IMGOPTS,compat=0.10,refcount_width=16" _make_test_img 64M
+print_refcount_width
+
+# Should not work
+IMGOPTS="$IMGOPTS,compat=0.10,refcount_width=1" _make_test_img 64M
+IMGOPTS="$IMGOPTS,compat=0.10,refcount_width=64" _make_test_img 64M
+
+
+echo
+echo '=== Snapshot limit on refcount_width=1 ==='
+echo
+
+IMGOPTS="$IMGOPTS,refcount_width=1" _make_test_img 64M
+print_refcount_width
+
+$QEMU_IO -c 'write 0 512' "$TEST_IMG" | _filter_qemu_io
+
+# Should fail for now; in the future, this might be supported by automatically
+# copying all clusters with overflowing refcount
+$QEMU_IMG snapshot -c foo "$TEST_IMG"
+
+# The new L1 table could/should be leaked
+_check_test_img
+
+echo
+echo '=== Snapshot limit on refcount_width=2 ==='
+echo
+
+IMGOPTS="$IMGOPTS,refcount_width=2" _make_test_img 64M
+print_refcount_width
+
+$QEMU_IO -c 'write 0 512' "$TEST_IMG" | _filter_qemu_io
+
+# Should succeed
+$QEMU_IMG snapshot -c foo "$TEST_IMG"
+$QEMU_IMG snapshot -c bar "$TEST_IMG"
+# Should fail (4th reference)
+$QEMU_IMG snapshot -c baz "$TEST_IMG"
+
+# The new L1 table could/should be leaked
+_check_test_img
+
+echo
+echo '=== Compressed clusters with refcount_width=1 ==='
+echo
+
+IMGOPTS="$IMGOPTS,refcount_width=1" _make_test_img 64M
+print_refcount_width
+
+# Both should fit into a single host cluster; instead of failing to increase the
+# refcount of that cluster, qemu should just allocate a new cluster and make
+# this operation succeed
+$QEMU_IO -c 'write -P 0 -c 0 64k' \
+ -c 'write -P 1 -c 64k 64k' \
+ "$TEST_IMG" | _filter_qemu_io
+
+_check_test_img
+
+echo
+echo '=== MSb set in 64 bit refcount ==='
+echo
+
+IMGOPTS="$IMGOPTS,refcount_width=64" _make_test_img 64M
+print_refcount_width
+
+$QEMU_IO -c 'write 0 512' "$TEST_IMG" | _filter_qemu_io
+
+# Set the MSb in the refblock entry of the data cluster
+poke_file "$TEST_IMG" $((0x20028)) "\x80\x00\x00\x00\x00\x00\x00\x00"
+
+# Clear OFLAG_COPIED in the L2 entry of the data cluster
+poke_file "$TEST_IMG" $((0x40000)) "\x00\x00\x00\x00\x00\x05\x00\x00"
+
+# And now try to write to that cluster (this would normally result in a COW
+# operation; but with the refcount being invalid, this will fail)
+$QEMU_IO -c 'write 0 512' "$TEST_IMG" | _filter_qemu_io
+
+# Now try again with one refcount less (which should work)
+poke_file "$TEST_IMG" $((0x20028)) "\x7f\xff\xff\xff\xff\xff\xff\xff"
+$QEMU_IO -c 'write 0 512' "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo '=== Amend from refcount_width=16 to refcount_width=1 ==='
+echo
+
+_make_test_img 64M
+print_refcount_width
+
+$QEMU_IO -c 'write 16M 32M' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG amend -o refcount_width=1 "$TEST_IMG"
+_check_test_img
+print_refcount_width
+
+echo
+echo '=== Amend from refcount_width=1 to refcount_width=64 ==='
+echo
+
+$QEMU_IMG amend -o refcount_width=64 "$TEST_IMG"
+_check_test_img
+print_refcount_width
+
+echo
+echo '=== Amend to compat=0.10 ==='
+echo
+
+# Should not work because refcount_width needs to be 16 for compat=0.10
+$QEMU_IMG amend -o compat=0.10 "$TEST_IMG"
+print_refcount_width
+# Should work
+$QEMU_IMG amend -o compat=0.10,refcount_width=16 "$TEST_IMG"
+_check_test_img
+print_refcount_width
+
+# Get back to compat=1.1 and refcount_width=16
+$QEMU_IMG amend -o compat=1.1 "$TEST_IMG"
+print_refcount_width
+# Should not work
+$QEMU_IMG amend -o refcount_width=32,compat=0.10 "$TEST_IMG"
+print_refcount_width
+
+echo
+echo '=== Amend with snapshot ==='
+echo
+
+$QEMU_IMG snapshot -c foo "$TEST_IMG"
+# Just to have different refcounts across the image
+$QEMU_IO -c 'write 0 16M' "$TEST_IMG" | _filter_qemu_io
+
+# Should not work (may work in the future by first decreasing all refcounts so
+# they fit into the target range by copying them)
+$QEMU_IMG amend -o refcount_width=1 "$TEST_IMG"
+_check_test_img
+print_refcount_width
+
+# Should work
+$QEMU_IMG amend -o refcount_width=2 "$TEST_IMG"
+_check_test_img
+print_refcount_width
+
+echo
+echo '=== Testing too many references for check ==='
+echo
+
+IMGOPTS="$IMGOPTS,refcount_width=1" _make_test_img 64M
+print_refcount_width
+
+# This cluster should be created at 0x50000
+$QEMU_IO -c 'write 0 64k' "$TEST_IMG" | _filter_qemu_io
+# Now make the second L2 entry (the L2 table should be at 0x40000) point to that
+# cluster, so we have two references
+poke_file "$TEST_IMG" $((0x40008)) "\x80\x00\x00\x00\x00\x05\x00\x00"
+
+# This should say "please use amend"
+_check_test_img -r all
+
+# So we do that
+$QEMU_IMG amend -o refcount_width=2 "$TEST_IMG"
+print_refcount_width
+
+# And try again
+_check_test_img -r all
+
+echo
+echo '=== Multiple walks necessary during amend ==='
+echo
+
+IMGOPTS="$IMGOPTS,refcount_width=1,cluster_size=512" _make_test_img 64k
+
+# Cluster 0 is the image header, clusters 1 to 4 are used by the L1 table, a
+# single L2 table, the reftable and a single refblock. This creates 58 data
+# clusters (actually, the L2 table is created here, too), so in total there are
+# then 63 used clusters in the image. With a refcount width of 64, one refblock
+# describes 64 clusters (512 bytes / 64 bits/entry = 64 entries), so this will
+# make the first refblock in the amended image have exactly one free entry.
+$QEMU_IO -c "write 0 $((58 * 512))" "$TEST_IMG" | _filter_qemu_io
+
+# Now change the refcount width; since the first new refblock will have exactly
+# one free entry, that entry will be used to store its own reference. No other
+# refblocks are needed, so then the new reftable will be allocated; since the
+# first new refblock is completely filled up, this will require a new refblock
+# which is why the refcount width changing function will need to run through
+# everything one more time until the allocations are stable.
+# Having more walks than usual should be visible as regressing progress (from
+# 66.67 % (2/3 walks) to 50.00 % (2/4 walks)).
+$QEMU_IMG amend -o refcount_width=64 -p "$TEST_IMG" | tr '\r' '\n' \
+ | grep -A 1 '66.67'
+print_refcount_width
+
+_check_test_img
+
+
+# success, all done
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/112.out b/tests/qemu-iotests/112.out
new file mode 100644
index 0000000..1ff036f
--- /dev/null
+++ b/tests/qemu-iotests/112.out
@@ -0,0 +1,143 @@
+QA output created by 112
+
+=== refcount_width limits ===
+
+qemu-img: TEST_DIR/t.IMGFMT: Refcount width must be a power of two and may not exceed 64 bits
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+qemu-img: TEST_DIR/t.IMGFMT: Refcount width must be a power of two and may not exceed 64 bits
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 refcount_width=-1
+qemu-img: TEST_DIR/t.IMGFMT: Refcount width must be a power of two and may not exceed 64 bits
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+qemu-img: TEST_DIR/t.IMGFMT: Refcount width must be a power of two and may not exceed 64 bits
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+refcount width: 1
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+refcount width: 64
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+refcount width: 16
+
+=== refcount_width and compat=0.10 ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+refcount width: 16
+qemu-img: TEST_DIR/t.IMGFMT: Different refcount widths than 16 bits require compatibility level 1.1 or above (use compat=1.1 or greater)
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+qemu-img: TEST_DIR/t.IMGFMT: Different refcount widths than 16 bits require compatibility level 1.1 or above (use compat=1.1 or greater)
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+
+=== Snapshot limit on refcount_width=1 ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+refcount width: 1
+wrote 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-img: Could not create snapshot 'foo': -22 (Invalid argument)
+Leaked cluster 6 refcount=1 reference=0
+
+1 leaked clusters were found on the image.
+This means waste of disk space, but no harm to data.
+
+=== Snapshot limit on refcount_width=2 ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+refcount width: 2
+wrote 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-img: Could not create snapshot 'baz': -22 (Invalid argument)
+Leaked cluster 7 refcount=1 reference=0
+
+1 leaked clusters were found on the image.
+This means waste of disk space, but no harm to data.
+
+=== Compressed clusters with refcount_width=1 ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+refcount width: 1
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 65536
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+
+=== MSb set in 64 bit refcount ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+refcount width: 64
+wrote 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+write failed: Numerical result out of range
+wrote 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Amend from refcount_width=16 to refcount_width=1 ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+refcount width: 16
+wrote 33554432/33554432 bytes at offset 16777216
+32 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+refcount width: 1
+
+=== Amend from refcount_width=1 to refcount_width=64 ===
+
+No errors were found on the image.
+refcount width: 64
+
+=== Amend to compat=0.10 ===
+
+qemu-img: compat=0.10 requires refcount_width=16
+qemu-img: Error while amending options: Operation not supported
+refcount width: 64
+No errors were found on the image.
+refcount width: 16
+refcount width: 16
+qemu-img: Different refcount widths than 16 bits require compatibility level 1.1 or above (use compat=1.1 or greater)
+qemu-img: Error while amending options: Invalid argument
+refcount width: 16
+
+=== Amend with snapshot ===
+
+wrote 16777216/16777216 bytes at offset 0
+16 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-img: Cannot decrease refcount entry width to 1 bits: Cluster at offset 0x50000 has a refcount of 2
+qemu-img: Error while amending options: Invalid argument
+No errors were found on the image.
+refcount width: 16
+No errors were found on the image.
+refcount width: 2
+
+=== Testing too many references for check ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+refcount width: 1
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+ERROR: overflow cluster offset=0x50000
+Use qemu-img amend to increase the refcount entry width or qemu-img convert to create a clean copy if the image cannot be opened for writing
+
+1 errors were found on the image.
+Data may be corrupted, or further writes to the image may corrupt it.
+refcount width: 2
+ERROR cluster 5 refcount=1 reference=2
+Repairing cluster 5 refcount=1 reference=2
+Repairing OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=2
+Repairing OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=2
+The following inconsistencies were found and repaired:
+
+ 0 leaked clusters
+ 3 corruptions
+
+Double checking the fixed image now...
+No errors were found on the image.
+
+=== Multiple walks necessary during amend ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536
+wrote 29696/29696 bytes at offset 0
+29 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+ (66.67/100%)
+ (50.00/100%)
+refcount width: 64
+No errors were found on the image.
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 7dfe469..593f3dd 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -112,3 +112,4 @@
107 rw auto quick
108 rw auto quick
111 rw auto quick
+112 rw auto
--
1.9.3
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [Qemu-devel] [PATCH v3 22/22] iotests: Add test for different refcount widths
2014-11-20 17:06 ` [Qemu-devel] [PATCH v3 22/22] iotests: Add test for different refcount widths Max Reitz
@ 2014-11-20 23:04 ` Eric Blake
0 siblings, 0 replies; 58+ messages in thread
From: Eric Blake @ 2014-11-20 23:04 UTC (permalink / raw)
To: Max Reitz, qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 2947 bytes --]
On 11/20/2014 10:06 AM, Max Reitz wrote:
> Add a test for conversion between different refcount widths and errors
> specific to certain widths (i.e. snapshots with refcount_width=1).
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/112 | 278 +++++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/112.out | 143 +++++++++++++++++++++++
> tests/qemu-iotests/group | 1 +
> 3 files changed, 422 insertions(+)
> create mode 100755 tests/qemu-iotests/112
> create mode 100644 tests/qemu-iotests/112.out
>
> +echo
> +echo '=== Multiple walks necessary during amend ==='
> +echo
> +
> +IMGOPTS="$IMGOPTS,refcount_width=1,cluster_size=512" _make_test_img 64k
> +
> +# Cluster 0 is the image header, clusters 1 to 4 are used by the L1 table, a
> +# single L2 table, the reftable and a single refblock. This creates 58 data
> +# clusters (actually, the L2 table is created here, too), so in total there are
> +# then 63 used clusters in the image. With a refcount width of 64, one refblock
> +# describes 64 clusters (512 bytes / 64 bits/entry = 64 entries), so this will
> +# make the first refblock in the amended image have exactly one free entry.
> +$QEMU_IO -c "write 0 $((58 * 512))" "$TEST_IMG" | _filter_qemu_io
> +
> +# Now change the refcount width; since the first new refblock will have exactly
> +# one free entry, that entry will be used to store its own reference. No other
> +# refblocks are needed, so then the new reftable will be allocated; since the
> +# first new refblock is completely filled up, this will require a new refblock
> +# which is why the refcount width changing function will need to run through
> +# everything one more time until the allocations are stable.
> +# Having more walks than usual should be visible as regressing progress (from
> +# 66.67 % (2/3 walks) to 50.00 % (2/4 walks)).
> +$QEMU_IMG amend -o refcount_width=64 -p "$TEST_IMG" | tr '\r' '\n' \
> + | grep -A 1 '66.67'
You probably know the drill by now: 'grep -A' is a GNU extension, and
not necessarily portable to other grep. Portable would be "grep 66.67 |
head -n2", but I'm not going to insist on a rewrite since we already
depend on other GNU-isms in the testsuite. (Or put another way, if
someone runs the test on BSD and it fails, _then_ we can patch things).
> +
> +=== Multiple walks necessary during amend ===
> +
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536
> +wrote 29696/29696 bytes at offset 0
> +29 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> + (66.67/100%)
> + (50.00/100%)
> +refcount width: 64
> +No errors were found on the image.
Nicely done, here, and in the overall series.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 58+ messages in thread