diff mbox

[v3,02/22] qcow2: Add refcount_width to format-specific info

Message ID 1416503198-17031-3-git-send-email-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz Nov. 20, 2014, 5:06 p.m. UTC 
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(-)

Comments

Stefan Hajnoczi Nov. 27, 2014, 1:47 p.m. UTC | #1 
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.
Max Reitz Nov. 27, 2014, 2:19 p.m. UTC | #2 
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
diff mbox

Patch

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