[Qemu-devel] [PATCH 0/2] qcow2: Fix refcount table size calculation

[Qemu-devel] [PATCH 0/2] qcow2: Fix refcount table size calculation

[Kevin Wolf]

Kevin Wolf (2):
  qcow2: Fix refcount table size calculation
  qemu-iotests: qcow2: Test growing large refcount table

 block/qcow2-refcount.c        |    3 +-
 tests/qemu-iotests/044        |  117 +++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/044.out    |    6 ++
 tests/qemu-iotests/group      |    1 +
 tests/qemu-iotests/iotests.py |    6 ++-
 tests/qemu-iotests/qcow2.py   |    9 ++--
 6 files changed, 136 insertions(+), 6 deletions(-)
 create mode 100755 tests/qemu-iotests/044
 create mode 100644 tests/qemu-iotests/044.out

-- 
1.7.6.5

[Qemu-devel] [PATCH 1/2] qcow2: Fix refcount table size calculation

[Kevin Wolf]

A missing factor for the refcount table entry size in the calculation
could mean that too little memory was allocated for the in-memory
representation of the table, resulting in a buffer overflow.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-refcount.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 5e3f915..96224d1 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -301,7 +301,8 @@ static int alloc_refcount_block(BlockDriverState *bs,
     uint64_t last_table_size;
     uint64_t blocks_clusters;
     do {
-        uint64_t table_clusters = size_to_clusters(s, table_size);
+        uint64_t table_clusters =
+            size_to_clusters(s, table_size * sizeof(uint64_t));
         blocks_clusters = 1 +
             ((table_clusters + refcount_block_clusters - 1)
             / refcount_block_clusters);
-- 
1.7.6.5

[Qemu-devel] [PATCH 2/2] qemu-iotests: qcow2: Test growing large refcount table

[Kevin Wolf]

Actually writing all the content with 512 byte sector size would take
forever, therefore build the image file with a Python script and use
qemu-io for the last write that actually triggers the refcount table
growth.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/044        |  117 +++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/044.out    |    6 ++
 tests/qemu-iotests/group      |    1 +
 tests/qemu-iotests/iotests.py |    6 ++-
 tests/qemu-iotests/qcow2.py   |    9 ++--
 5 files changed, 134 insertions(+), 5 deletions(-)
 create mode 100755 tests/qemu-iotests/044
 create mode 100644 tests/qemu-iotests/044.out

diff --git a/tests/qemu-iotests/044 b/tests/qemu-iotests/044
new file mode 100755
index 0000000..11ea0f4
--- /dev/null
+++ b/tests/qemu-iotests/044
@@ -0,0 +1,117 @@
+#!/usr/bin/env python
+#
+# Tests growing a large refcount table.
+#
+# Copyright (C) 2012 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/>.
+#
+
+import time
+import os
+import qcow2
+from qcow2 import QcowHeader
+import iotests
+from iotests import qemu_img, qemu_img_verbose, qemu_io
+import struct
+import subprocess
+
+test_img = os.path.join(iotests.test_dir, 'test.img')
+
+class TestRefcountTableGrowth(iotests.QMPTestCase):
+    '''Abstract base class for image mirroring test cases'''
+
+    def preallocate(self, name):
+        fd = open(name, "r+b")
+        try:
+            off_reftable = 512
+            off_refblock = off_reftable + (512 * 512)
+            off_l1       = off_refblock + (512 * 512 * 64)
+            off_l2       = off_l1 + (512 * 512 * 4 * 8)
+            off_data     = off_l2 + (512 * 512 * 4 * 512)
+
+            # Write a new header
+            h = QcowHeader(fd)
+            h.refcount_table_offset = off_reftable
+            h.refcount_table_clusters = 512
+            h.l1_table_offset = off_l1
+            h.l1_size = 512 * 512 * 4
+            h.update(fd)
+
+            # Write a refcount table
+            fd.seek(off_reftable)
+
+            for i in xrange(0, h.refcount_table_clusters):
+                sector = ''.join(struct.pack('>Q',
+                    off_refblock + i * 64 * 512 + j * 512)
+                    for j in xrange(0, 64))
+                fd.write(sector)
+
+            # Write the refcount blocks
+            assert(fd.tell() == off_refblock)
+            sector = ''.join(struct.pack('>H', 1) for j in xrange(0, 64 * 256))
+            for block in xrange(0, h.refcount_table_clusters):
+                fd.write(sector)
+
+            # Write the L1 table
+            assert(fd.tell() == off_l1)
+            assert(off_l2 + 512 * h.l1_size == off_data)
+            table = ''.join(struct.pack('>Q', (1 << 63) | off_l2 + 512 * j)
+                for j in xrange(0, h.l1_size))
+            fd.write(table)
+
+            # Write the L2 tables
+            assert(fd.tell() == off_l2)
+            img_file_size = h.refcount_table_clusters * 64 * 256 * 512
+            remaining = img_file_size - off_data
+
+            off = off_data
+            while remaining > 1024 * 512:
+                pytable = list((1 << 63) | off + 512 * j
+                    for j in xrange(0, 1024))
+                table = struct.pack('>1024Q', *pytable)
+                fd.write(table)
+                remaining = remaining - 1024 * 512
+                off = off + 1024 * 512
+
+            table = ''.join(struct.pack('>Q', (1 << 63) | off + 512 * j)
+                for j in xrange(0, remaining / 512))
+            fd.write(table)
+
+
+            # Data
+            fd.truncate(img_file_size)
+
+
+        finally:
+            fd.close()
+
+
+    def setUp(self):
+        qemu_img('create', '-f', iotests.imgfmt, '-o', 'cluster_size=512', test_img, '16G')
+        self.preallocate(test_img)
+        pass
+
+
+    def tearDown(self):
+        os.remove(test_img)
+        pass
+
+    def test_grow_refcount_table(self):
+        qemu_io('-c', 'write 3800M 1M', test_img)
+        qemu_img_verbose('check' , test_img)
+        pass
+
+if __name__ == '__main__':
+    iotests.main(supported_fmts=['qcow2'])
diff --git a/tests/qemu-iotests/044.out b/tests/qemu-iotests/044.out
new file mode 100644
index 0000000..7a40071
--- /dev/null
+++ b/tests/qemu-iotests/044.out
@@ -0,0 +1,6 @@
+No errors were found on the image.
+.
+----------------------------------------------------------------------
+Ran 1 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index ac86f54..a4a9044 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -50,3 +50,4 @@
 041 rw auto backing
 042 rw auto quick
 043 rw auto backing
+044 rw auto
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 735c674..b2eaf20 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -42,6 +42,10 @@ def qemu_img(*args):
     devnull = open('/dev/null', 'r+')
     return subprocess.call(qemu_img_args + list(args), stdin=devnull, stdout=devnull)
 
+def qemu_img_verbose(*args):
+    '''Run qemu-img without supressing its output and return the exit code'''
+    return subprocess.call(qemu_img_args + list(args))
+
 def qemu_io(*args):
     '''Run qemu-io and return the stdout data'''
     args = qemu_io_args + list(args)
@@ -182,4 +186,4 @@ def main(supported_fmts=[]):
     try:
         unittest.main(testRunner=MyTestRunner)
     finally:
-        sys.stderr.write(re.sub(r'Ran (\d+) test[s] in [\d.]+s', r'Ran \1 tests', output.getvalue()))
+        sys.stderr.write(re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', output.getvalue()))
diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index 97f3770..fecf5b9 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -233,8 +233,9 @@ def usage():
     for name, handler, num_args, desc in cmds:
         print "    %-20s - %s" % (name, desc)
 
-if len(sys.argv) < 3:
-    usage()
-    sys.exit(1)
+if __name__ == '__main__':
+    if len(sys.argv) < 3:
+        usage()
+        sys.exit(1)
 
-main(sys.argv[1], sys.argv[2], sys.argv[3:])
+    main(sys.argv[1], sys.argv[2], sys.argv[3:])
-- 
1.7.6.5

Re: [Qemu-devel] [PATCH 1/2] qcow2: Fix refcount table size calculation

[Michael Tokarev]

On 26.10.2012 23:42, Kevin Wolf wrote:
> A missing factor for the refcount table entry size in the calculation
> could mean that too little memory was allocated for the in-memory
> representation of the table, resulting in a buffer overflow.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-and-tested-by: Michael Tokarev <mjt@tls.msk.ru>

(but the thing is rather trivial and obvious :)
(this fixes https://bugs.launchpad.net/qemu/+bug/1071236 fwiw --
maybe we should add some references to bugs when the work/patch
is after a bugreport)

This fix is applicable to -stable, at least to 1.2 and 1.1 versions.
For 0.15, while the patch applies, qcow2 driver has other bug(s)
which prevents the testcase (with qemu-img create) from working:

 $ ./qemu-img-0.15 create -f qcow2 -o cluster_size=512,preallocation=metadata disk.img 4G
 Formatting 'disk.img', fmt=qcow2 size=4294967296 encryption=off cluster_size=512 preallocation='metadata'
 qemu-img: disk.img: error while creating qcow2: Unknown error 1652533248

Thanks,

/mjt