[Qemu-devel] [PATCH v2 0/3] qcow2: fix autoclear image header update

[Qemu-devel] [PATCH v2 0/3] qcow2: fix autoclear image header update

[Stefan Hajnoczi]

The code path to clear unknown autoclear feature bits when opening a qcow2
image file was broken.  This series includes the fix along with the qemu-iotest
to verify the issue and prevent regressions in the future.

v2:
 * Add qemu-iotest [Kevin]

Stefan Hajnoczi (3):
  qcow2: fix autoclear image header update
  qemu-iotests: add qcow2.py set-feature-bit command
  qemu-iotests: add 036 autoclear feature bit test

 block/qcow2.c               |   17 ++++++-----
 tests/qemu-iotests/036      |   68 +++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/036.out  |   52 +++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group    |    1 +
 tests/qemu-iotests/qcow2.py |   23 +++++++++++++++
 5 files changed, 153 insertions(+), 8 deletions(-)
 create mode 100755 tests/qemu-iotests/036
 create mode 100644 tests/qemu-iotests/036.out

-- 
1.7.10

[Qemu-devel] [PATCH v2 1/3] qcow2: fix autoclear image header update

[Stefan Hajnoczi]

The autoclear feature bits can be used for qcow2 file format features
that are safe to "drop" by old programs that do not understand the
feature.  Upon opening the image file unknown autoclear feature bits are
cleared and the image file header is rewritten, but this was happening
too early in the code when critical header fields were not yet loaded.

Process autoclear feature bits after all necessary header information
has been loaded.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block/qcow2.c |   17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index d66de58..aa3f176 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -298,14 +298,6 @@ static int qcow2_open(BlockDriverState *bs, int flags)
         goto fail;
     }
 
-    if (!bs->read_only && s->autoclear_features != 0) {
-        s->autoclear_features = 0;
-        ret = qcow2_update_header(bs);
-        if (ret < 0) {
-            goto fail;
-        }
-    }
-
     /* Check support for various header values */
     if (header.refcount_order != 4) {
         report_unsupported(bs, "%d bit reference counts",
@@ -411,6 +403,15 @@ static int qcow2_open(BlockDriverState *bs, int flags)
         goto fail;
     }
 
+    /* Clear unknown autoclear feature bits */
+    if (!bs->read_only && s->autoclear_features != 0) {
+        s->autoclear_features = 0;
+        ret = qcow2_update_header(bs);
+        if (ret < 0) {
+            goto fail;
+        }
+    }
+
     /* Initialise locks */
     qemu_co_mutex_init(&s->lock);
 
-- 
1.7.10

[Qemu-devel] [PATCH v2 2/3] qemu-iotests: add qcow2.py set-feature-bit command

[Stefan Hajnoczi]

This new command sets feature bits in the image file header:

  qcow2.py set-feature-bit incompatible|compatible|autoclear <bit>

The bit number must be in the range [0, 64).

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 tests/qemu-iotests/qcow2.py |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index e27196a..97f3770 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -181,10 +181,33 @@ def cmd_del_header_ext(fd, magic):
 
     h.update(fd)
 
+def cmd_set_feature_bit(fd, group, bit):
+    try:
+        bit = int(bit, 0)
+        if bit < 0 or bit >= 64:
+            raise ValueError
+    except:
+        print "'%s' is not a valid bit number in range [0, 64)" % bit
+        sys.exit(1)
+
+    h = QcowHeader(fd)
+    if group == 'incompatible':
+        h.incompatible_features |= 1 << bit
+    elif group == 'compatible':
+        h.compatible_features |= 1 << bit
+    elif group == 'autoclear':
+        h.autoclear_features |= 1 << bit
+    else:
+        print "'%s' is not a valid group, try 'incompatible', 'compatible', or 'autoclear'" % group
+        sys.exit(1)
+
+    h.update(fd)
+
 cmds = [
     [ 'dump-header',    cmd_dump_header,    0, 'Dump image header and header extensions' ],
     [ 'add-header-ext', cmd_add_header_ext, 2, 'Add a header extension' ],
     [ 'del-header-ext', cmd_del_header_ext, 1, 'Delete a header extension' ],
+    [ 'set-feature-bit', cmd_set_feature_bit, 2, 'Set a feature bit'],
 ]
 
 def main(filename, cmd, args):
-- 
1.7.10

[Qemu-devel] [PATCH v2 3/3] qemu-iotests: add 036 autoclear feature bit test

[Stefan Hajnoczi]

This new test validates the autoclear feature bit behavior.  When QEMU
opens a qcow2v3 image file with an unknown autoclear feature bit the bit
should be cleared in the image file header.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 tests/qemu-iotests/036     |   68 ++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/036.out |   52 +++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |    1 +
 3 files changed, 121 insertions(+)
 create mode 100755 tests/qemu-iotests/036
 create mode 100644 tests/qemu-iotests/036.out

diff --git a/tests/qemu-iotests/036 b/tests/qemu-iotests/036
new file mode 100755
index 0000000..329533e
--- /dev/null
+++ b/tests/qemu-iotests/036
@@ -0,0 +1,68 @@
+#!/bin/bash
+#
+# Test that qcow2 unknown autoclear feature bits are cleared
+#
+# Copyright (C) 2011 Red Hat, Inc.
+# Copyright IBM, Corp. 2010
+#
+# Based on test 031.
+#
+# 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=stefanha@linux.vnet.ibm.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
+. ./common.pattern
+
+# This tests qcow2-specific low-level functionality
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+# Only qcow2v3 and later supports feature bits
+IMGOPTS="compat=1.1"
+
+echo === Create image with unknown autoclear feature bit ===
+echo
+_make_test_img 64M
+./qcow2.py $TEST_IMG set-feature-bit autoclear 63
+./qcow2.py $TEST_IMG dump-header
+
+echo
+echo === Repair image ===
+echo
+$QEMU_IMG check -r all $TEST_IMG
+./qcow2.py $TEST_IMG dump-header
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/036.out b/tests/qemu-iotests/036.out
new file mode 100644
index 0000000..6953e37
--- /dev/null
+++ b/tests/qemu-iotests/036.out
@@ -0,0 +1,52 @@
+QA output created by 036
+=== Create image with unknown autoclear feature bit ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+magic                     0x514649fb
+version                   3
+backing_file_offset       0x0
+backing_file_size         0x0
+cluster_bits              16
+size                      67108864
+crypt_method              0
+l1_size                   1
+l1_table_offset           0x30000
+refcount_table_offset     0x10000
+refcount_table_clusters   1
+nb_snapshots              0
+snapshot_offset           0x0
+incompatible_features     0x0
+compatible_features       0x0
+autoclear_features        0x8000000000000000
+refcount_order            4
+header_length             104
+
+
+=== Repair image ===
+
+No errors were found on the image.
+magic                     0x514649fb
+version                   3
+backing_file_offset       0x0
+backing_file_size         0x0
+cluster_bits              16
+size                      67108864
+crypt_method              0
+l1_size                   1
+l1_table_offset           0x30000
+refcount_table_offset     0x10000
+refcount_table_clusters   1
+nb_snapshots              0
+snapshot_offset           0x0
+incompatible_features     0x0
+compatible_features       0x0
+autoclear_features        0x0
+refcount_order            4
+header_length             104
+
+Header extension:
+magic                     0x6803f857
+length                    0
+data                      ''
+
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 36ebf1a..f9b0f5b 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -42,3 +42,4 @@
 033 rw auto
 034 rw auto backing
 035 rw auto quick
+036 rw auto quick
-- 
1.7.10

Re: [Qemu-devel] [PATCH v2 2/3] qemu-iotests: add qcow2.py set-feature-bit command

[Kevin Wolf]

Am 14.06.2012 14:58, schrieb Stefan Hajnoczi:
> This new command sets feature bits in the image file header:
> 
>   qcow2.py set-feature-bit incompatible|compatible|autoclear <bit>
> 
> The bit number must be in the range [0, 64).
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  tests/qemu-iotests/qcow2.py |   23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
> index e27196a..97f3770 100755
> --- a/tests/qemu-iotests/qcow2.py
> +++ b/tests/qemu-iotests/qcow2.py
> @@ -181,10 +181,33 @@ def cmd_del_header_ext(fd, magic):
>  
>      h.update(fd)
>  
> +def cmd_set_feature_bit(fd, group, bit):
> +    try:
> +        bit = int(bit, 0)
> +        if bit < 0 or bit >= 64:
> +            raise ValueError
> +    except:
> +        print "'%s' is not a valid bit number in range [0, 64)" % bit

Heh, open intervals on integers are nice. ;-)

Thanks, applied all to the block branch.

Kevin