[Qemu-devel] [PULL for-1.4 00/13] Block patches

[Qemu-devel] [PULL for-1.4 00/13] Block patches

[Stefan Hajnoczi]

The following changes since commit 8a55ebf01507ab73cc458cfcd5b9cb856aba0b9e:

  Merge remote-tracking branch 'afaerber/qom-cpu' into staging (2013-01-31 19:37:33 -0600)

are available in the git repository at:


  git://github.com/stefanha/qemu.git block

for you to fetch changes up to fdf263f63fad86b04032da86686a952edfe4644f:

  block/raw-posix: Build fix for O_ASYNC (2013-02-01 15:11:12 +0100)

----------------------------------------------------------------
Andreas Färber (1):
      block/raw-posix: Build fix for O_ASYNC

Kevin Wolf (7):
      qemu-iotests: Add regression test for b7ab0fea
      bochs: Fix bdrv_open() error handling
      cloop: Fix bdrv_open() error handling
      vpc: Fix bdrv_open() error handling
      dmg: Fix bdrv_open() error handling
      dmg: Use g_free instead of free
      parallels: Fix bdrv_open() error handling

Liu Yuan (1):
      sheepdog: pass vdi_id to sheep daemon for sd_close()

Othmar Pasteka (1):
      vmdk: Allow selecting SCSI adapter in image creation

Philipp Hahn (1):
      vmdk: Allow space in file name

Vishvananda Ishaya (2):
      block: Fix is_allocated_above with resized files
      block: Adds mirroring tests for resized images

 block.c                    |   4 +-
 block/bochs.c              |  22 ++++---
 block/cloop.c              |  29 ++++++---
 block/dmg.c                | 153 +++++++++++++++++++++++++++++++--------------
 block/parallels.c          |  23 ++++---
 block/raw-posix.c          |  11 +++-
 block/sheepdog.c           |   5 +-
 block/vmdk.c               |  41 ++++++++----
 block/vpc.c                |  42 +++++++++----
 include/block/block_int.h  |   1 +
 tests/qemu-iotests/041     |  48 ++++++++++++++
 tests/qemu-iotests/041.out |   4 +-
 tests/qemu-iotests/047     |  75 ++++++++++++++++++++++
 tests/qemu-iotests/047.out |  22 +++++++
 tests/qemu-iotests/group   |   1 +
 15 files changed, 379 insertions(+), 102 deletions(-)
 create mode 100755 tests/qemu-iotests/047
 create mode 100644 tests/qemu-iotests/047.out

-- 
1.8.1

[Qemu-devel] [PATCH 01/13] qemu-iotests: Add regression test for b7ab0fea

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

It turned out that the change in b7ab0fea was actually a real qcow2
corruption fix. This is a reproducer for the bug.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/047     | 75 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/047.out | 22 ++++++++++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 98 insertions(+)
 create mode 100755 tests/qemu-iotests/047
 create mode 100644 tests/qemu-iotests/047.out

diff --git a/tests/qemu-iotests/047 b/tests/qemu-iotests/047
new file mode 100755
index 0000000..0cf36b4
--- /dev/null
+++ b/tests/qemu-iotests/047
@@ -0,0 +1,75 @@
+#!/bin/bash
+#
+# Regression test for commit b7ab0fea (which was a corruption fix,
+# despite the commit message claiming otherwise)
+#
+# Copyright (C) 2013 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=kwolf@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
+
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+size=128M
+
+_make_test_img $size
+
+function qemu_io_cmds()
+{
+cat <<EOF
+write -P 0x66 0 320k
+write 320k 128k
+write -P 0x55 1M 128k
+write -P 0x88 448k 128k
+discard 320k 128k
+aio_flush
+
+write -P 0x77 0 480k
+aio_flush
+
+read -P 0x77 0 480k
+read -P 0x88 480k 96k
+read -P 0x55 1M 128k
+EOF
+}
+
+qemu_io_cmds | $QEMU_IO $TEST_IMG | _filter_qemu_io
+_check_test_img
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/047.out b/tests/qemu-iotests/047.out
new file mode 100644
index 0000000..81b2ff7
--- /dev/null
+++ b/tests/qemu-iotests/047.out
@@ -0,0 +1,22 @@
+QA output created by 047
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
+qemu-io> wrote 327680/327680 bytes at offset 0
+320 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io> wrote 131072/131072 bytes at offset 327680
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io> wrote 131072/131072 bytes at offset 1048576
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io> wrote 131072/131072 bytes at offset 458752
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io> discard 131072/131072 bytes at offset 327680
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io> qemu-io> qemu-io> wrote 491520/491520 bytes at offset 0
+480 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io> qemu-io> qemu-io> read 491520/491520 bytes at offset 0
+480 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io> read 98304/98304 bytes at offset 491520
+96 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io> read 131072/131072 bytes at offset 1048576
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io> No errors were found on the image.
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index a0307de..1bbd2bf 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -53,3 +53,4 @@
 044 rw auto
 045 rw auto
 046 rw auto aio
+047 rw auto
-- 
1.8.1

[Qemu-devel] [PATCH 02/13] block: Fix is_allocated_above with resized files

[Stefan Hajnoczi]

From: Vishvananda Ishaya <vishvananda@gmail.com>

In an image chain, if the base image is smaller than the current
image, we need to make sure to use the current images count of
unallocated blocks once we get to the end of the base image. Without
this change the code will return 0 blocks when it gets to the end
of the base image and mirror_run will fail its assertion.

Signed-off-by: Vishvananda Ishaya <vishvananda@gmail.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index ba67c0d..50dab8e 100644
--- a/block.c
+++ b/block.c
@@ -2800,7 +2800,9 @@ int coroutine_fn bdrv_co_is_allocated_above(BlockDriverState *top,
          *
          * [sector_num+x, nr_sectors] allocated.
          */
-        if (n > pnum_inter) {
+        if (n > pnum_inter &&
+            (intermediate == top ||
+             sector_num + pnum_inter < intermediate->total_sectors)) {
             n = pnum_inter;
         }
 
-- 
1.8.1

[Qemu-devel] [PATCH 03/13] block: Adds mirroring tests for resized images

[Stefan Hajnoczi]

From: Vishvananda Ishaya <vishvananda@gmail.com>

This test verifies two mirroring issues are fixed with resized images:

 * sync='top' creates an image that is the proper size
 * sync='full' doesn't cause an assertion failure and crash qemu
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/041     | 48 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/041.out |  4 ++--
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index b040820..720eeff 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -344,6 +344,54 @@ class TestMirrorNoBacking(ImageMirroringTestCase):
         self.assertTrue(self.compare_images(test_img, target_img),
                         'target image does not match source after mirroring')
 
+class TestMirrorResized(ImageMirroringTestCase):
+    backing_len = 1 * 1024 * 1024 # MB
+    image_len = 2 * 1024 * 1024 # MB
+
+    def setUp(self):
+        self.create_image(backing_img, TestMirrorResized.backing_len)
+        qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
+        qemu_img('resize', test_img, '2M')
+        self.vm = iotests.VM().add_drive(test_img)
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove(test_img)
+        os.remove(backing_img)
+        try:
+            os.remove(target_img)
+        except OSError:
+            pass
+
+    def test_complete_top(self):
+        self.assert_no_active_mirrors()
+
+        result = self.vm.qmp('drive-mirror', device='drive0', sync='top',
+                             target=target_img)
+        self.assert_qmp(result, 'return', {})
+
+        self.complete_and_wait()
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/inserted/file', target_img)
+        self.vm.shutdown()
+        self.assertTrue(self.compare_images(test_img, target_img),
+                        'target image does not match source after mirroring')
+
+    def test_complete_full(self):
+        self.assert_no_active_mirrors()
+
+        result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
+                             target=target_img)
+        self.assert_qmp(result, 'return', {})
+
+        self.complete_and_wait()
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/inserted/file', target_img)
+        self.vm.shutdown()
+        self.assertTrue(self.compare_images(test_img, target_img),
+                        'target image does not match source after mirroring')
+
 class TestReadErrors(ImageMirroringTestCase):
     image_len = 2 * 1024 * 1024 # MB
 
diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
index 84bfd63..42314e9 100644
--- a/tests/qemu-iotests/041.out
+++ b/tests/qemu-iotests/041.out
@@ -1,5 +1,5 @@
-......................
+........................
 ----------------------------------------------------------------------
-Ran 22 tests
+Ran 24 tests
 
 OK
-- 
1.8.1

[Qemu-devel] [PATCH 04/13] vmdk: Allow selecting SCSI adapter in image creation

[Stefan Hajnoczi]

From: Othmar Pasteka <pasteka@kabsi.at>

Introduce a new option "adapter_type" when converting to vmdk images.
It can be one of the following: ide (default), buslogic, lsilogic
or legacyESX (according to the vmdk spec from vmware).

In case of a non-ide adapter, heads is set to 255 instead of the 16.
The latter is used for "ide".

Also see LP#545089

Signed-off-by: Othmar Pasteka <pasteka@kabsi.at>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/vmdk.c              | 31 ++++++++++++++++++++++++++++---
 include/block/block_int.h |  1 +
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 8333afb..a8cb5c9 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1442,6 +1442,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
     int fd, idx = 0;
     char desc[BUF_SIZE];
     int64_t total_size = 0, filesize;
+    const char *adapter_type = NULL;
     const char *backing_file = NULL;
     const char *fmt = NULL;
     int flags = 0;
@@ -1453,6 +1454,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
     const char *desc_extent_line;
     char parent_desc_line[BUF_SIZE] = "";
     uint32_t parent_cid = 0xffffffff;
+    uint32_t number_heads = 16;
     const char desc_template[] =
         "# Disk DescriptorFile\n"
         "version=1\n"
@@ -1469,9 +1471,9 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
         "\n"
         "ddb.virtualHWVersion = \"%d\"\n"
         "ddb.geometry.cylinders = \"%" PRId64 "\"\n"
-        "ddb.geometry.heads = \"16\"\n"
+        "ddb.geometry.heads = \"%d\"\n"
         "ddb.geometry.sectors = \"63\"\n"
-        "ddb.adapterType = \"ide\"\n";
+        "ddb.adapterType = \"%s\"\n";
 
     if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) {
         return -EINVAL;
@@ -1480,6 +1482,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
     while (options && options->name) {
         if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
             total_size = options->value.n;
+        } else if (!strcmp(options->name, BLOCK_OPT_ADAPTER_TYPE)) {
+            adapter_type = options->value.s;
         } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
             backing_file = options->value.s;
         } else if (!strcmp(options->name, BLOCK_OPT_COMPAT6)) {
@@ -1489,6 +1493,20 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
         }
         options++;
     }
+    if (!adapter_type) {
+        adapter_type = "ide";
+    } else if (strcmp(adapter_type, "ide") &&
+               strcmp(adapter_type, "buslogic") &&
+               strcmp(adapter_type, "lsilogic") &&
+               strcmp(adapter_type, "legacyESX")) {
+        fprintf(stderr, "VMDK: Unknown adapter type: '%s'.\n", adapter_type);
+        return -EINVAL;
+    }
+    if (strcmp(adapter_type, "ide") != 0) {
+        /* that's the number of heads with which vmware operates when
+           creating, exporting, etc. vmdk files with a non-ide adapter type */
+        number_heads = 255;
+    }
     if (!fmt) {
         /* Default format to monolithicSparse */
         fmt = "monolithicSparse";
@@ -1576,7 +1594,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
             parent_desc_line,
             ext_desc_lines,
             (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
-            total_size / (int64_t)(63 * 16 * 512));
+            total_size / (int64_t)(63 * number_heads * 512), number_heads,
+                adapter_type);
     if (split || flat) {
         fd = qemu_open(filename,
                        O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
@@ -1661,6 +1680,12 @@ static QEMUOptionParameter vmdk_create_options[] = {
         .help = "Virtual disk size"
     },
     {
+        .name = BLOCK_OPT_ADAPTER_TYPE,
+        .type = OPT_STRING,
+        .help = "Virtual adapter type, can be one of "
+                "ide (default), lsilogic, buslogic or legacyESX"
+    },
+    {
         .name = BLOCK_OPT_BACKING_FILE,
         .type = OPT_STRING,
         .help = "File name of a base image"
diff --git a/include/block/block_int.h b/include/block/block_int.h
index f7279b9..eaad53e 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -56,6 +56,7 @@
 #define BLOCK_OPT_SUBFMT            "subformat"
 #define BLOCK_OPT_COMPAT_LEVEL      "compat"
 #define BLOCK_OPT_LAZY_REFCOUNTS    "lazy_refcounts"
+#define BLOCK_OPT_ADAPTER_TYPE      "adapter_type"
 
 typedef struct BdrvTrackedRequest BdrvTrackedRequest;
 
-- 
1.8.1

[Qemu-devel] [PATCH 05/13] sheepdog: pass vdi_id to sheep daemon for sd_close()

[Stefan Hajnoczi]

From: Liu Yuan <tailai.ly@taobao.com>

Sheep daemon needs vdi_id to identify which vdi is closed to release resources
such as object cache.

Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Liu Yuan <tailai.ly@taobao.com>
Reviewed-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/sheepdog.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 3e49bb8..d466b23 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -145,7 +145,7 @@ typedef struct SheepdogVdiReq {
     uint32_t id;
     uint32_t data_length;
     uint64_t vdi_size;
-    uint32_t base_vdi_id;
+    uint32_t vdi_id;
     uint32_t copies;
     uint32_t snapid;
     uint32_t pad[3];
@@ -1201,7 +1201,7 @@ static int do_sd_create(char *filename, int64_t vdi_size,
 
     memset(&hdr, 0, sizeof(hdr));
     hdr.opcode = SD_OP_NEW_VDI;
-    hdr.base_vdi_id = base_vid;
+    hdr.vdi_id = base_vid;
 
     wlen = SD_MAX_VDI_LEN;
 
@@ -1384,6 +1384,7 @@ static void sd_close(BlockDriverState *bs)
     memset(&hdr, 0, sizeof(hdr));
 
     hdr.opcode = SD_OP_RELEASE_VDI;
+    hdr.vdi_id = s->inode.vdi_id;
     wlen = strlen(s->name) + 1;
     hdr.data_length = wlen;
     hdr.flags = SD_FLAG_CMD_WRITE;
-- 
1.8.1

[Qemu-devel] [PATCH 06/13] bochs: Fix bdrv_open() error handling

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

Return -errno instead of -1 on errors. While touching the
code, fix a memory leak.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/bochs.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/block/bochs.c b/block/bochs.c
index 3737583..a6eb33d 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -114,11 +114,13 @@ static int bochs_open(BlockDriverState *bs, int flags)
     int i;
     struct bochs_header bochs;
     struct bochs_header_v1 header_v1;
+    int ret;
 
     bs->read_only = 1; // no write support yet
 
-    if (bdrv_pread(bs->file, 0, &bochs, sizeof(bochs)) != sizeof(bochs)) {
-        goto fail;
+    ret = bdrv_pread(bs->file, 0, &bochs, sizeof(bochs));
+    if (ret < 0) {
+        return ret;
     }
 
     if (strcmp(bochs.magic, HEADER_MAGIC) ||
@@ -138,9 +140,13 @@ static int bochs_open(BlockDriverState *bs, int flags)
 
     s->catalog_size = le32_to_cpu(bochs.extra.redolog.catalog);
     s->catalog_bitmap = g_malloc(s->catalog_size * 4);
-    if (bdrv_pread(bs->file, le32_to_cpu(bochs.header), s->catalog_bitmap,
-                   s->catalog_size * 4) != s->catalog_size * 4)
-	goto fail;
+
+    ret = bdrv_pread(bs->file, le32_to_cpu(bochs.header), s->catalog_bitmap,
+                     s->catalog_size * 4);
+    if (ret < 0) {
+        goto fail;
+    }
+
     for (i = 0; i < s->catalog_size; i++)
 	le32_to_cpus(&s->catalog_bitmap[i]);
 
@@ -153,8 +159,10 @@ static int bochs_open(BlockDriverState *bs, int flags)
 
     qemu_co_mutex_init(&s->lock);
     return 0;
- fail:
-    return -1;
+
+fail:
+    g_free(s->catalog_bitmap);
+    return ret;
 }
 
 static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num)
-- 
1.8.1

[Qemu-devel] [PATCH 07/13] cloop: Fix bdrv_open() error handling

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

Return -errno instead of -1 on errors. While touching the
code, fix a memory leak.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/cloop.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/block/cloop.c b/block/cloop.c
index 5a0d0d8..8fe13e9 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -57,27 +57,32 @@ static int cloop_open(BlockDriverState *bs, int flags)
 {
     BDRVCloopState *s = bs->opaque;
     uint32_t offsets_size, max_compressed_block_size = 1, i;
+    int ret;
 
     bs->read_only = 1;
 
     /* read header */
-    if (bdrv_pread(bs->file, 128, &s->block_size, 4) < 4) {
-        goto cloop_close;
+    ret = bdrv_pread(bs->file, 128, &s->block_size, 4);
+    if (ret < 0) {
+        return ret;
     }
     s->block_size = be32_to_cpu(s->block_size);
 
-    if (bdrv_pread(bs->file, 128 + 4, &s->n_blocks, 4) < 4) {
-        goto cloop_close;
+    ret = bdrv_pread(bs->file, 128 + 4, &s->n_blocks, 4);
+    if (ret < 0) {
+        return ret;
     }
     s->n_blocks = be32_to_cpu(s->n_blocks);
 
     /* read offsets */
     offsets_size = s->n_blocks * sizeof(uint64_t);
     s->offsets = g_malloc(offsets_size);
-    if (bdrv_pread(bs->file, 128 + 4 + 4, s->offsets, offsets_size) <
-            offsets_size) {
-        goto cloop_close;
+
+    ret = bdrv_pread(bs->file, 128 + 4 + 4, s->offsets, offsets_size);
+    if (ret < 0) {
+        goto fail;
     }
+
     for(i=0;i<s->n_blocks;i++) {
         s->offsets[i] = be64_to_cpu(s->offsets[i]);
         if (i > 0) {
@@ -92,7 +97,8 @@ static int cloop_open(BlockDriverState *bs, int flags)
     s->compressed_block = g_malloc(max_compressed_block_size + 1);
     s->uncompressed_block = g_malloc(s->block_size);
     if (inflateInit(&s->zstream) != Z_OK) {
-        goto cloop_close;
+        ret = -EINVAL;
+        goto fail;
     }
     s->current_block = s->n_blocks;
 
@@ -101,8 +107,11 @@ static int cloop_open(BlockDriverState *bs, int flags)
     qemu_co_mutex_init(&s->lock);
     return 0;
 
-cloop_close:
-    return -1;
+fail:
+    g_free(s->offsets);
+    g_free(s->compressed_block);
+    g_free(s->uncompressed_block);
+    return ret;
 }
 
 static inline int cloop_read_block(BlockDriverState *bs, int block_num)
-- 
1.8.1

[Qemu-devel] [PATCH 08/13] vpc: Fix bdrv_open() error handling

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

Return -errno instead of -1 on errors. While touching the
code, fix a memory leak.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/vpc.c | 42 ++++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index 7948609..82229ef 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -163,24 +163,33 @@ static int vpc_open(BlockDriverState *bs, int flags)
     struct vhd_dyndisk_header* dyndisk_header;
     uint8_t buf[HEADER_SIZE];
     uint32_t checksum;
-    int err = -1;
     int disk_type = VHD_DYNAMIC;
+    int ret;
 
-    if (bdrv_pread(bs->file, 0, s->footer_buf, HEADER_SIZE) != HEADER_SIZE)
+    ret = bdrv_pread(bs->file, 0, s->footer_buf, HEADER_SIZE);
+    if (ret < 0) {
         goto fail;
+    }
 
     footer = (struct vhd_footer*) s->footer_buf;
     if (strncmp(footer->creator, "conectix", 8)) {
         int64_t offset = bdrv_getlength(bs->file);
-        if (offset < HEADER_SIZE) {
+        if (offset < 0) {
+            ret = offset;
+            goto fail;
+        } else if (offset < HEADER_SIZE) {
+            ret = -EINVAL;
             goto fail;
         }
+
         /* If a fixed disk, the footer is found only at the end of the file */
-        if (bdrv_pread(bs->file, offset-HEADER_SIZE, s->footer_buf, HEADER_SIZE)
-                != HEADER_SIZE) {
+        ret = bdrv_pread(bs->file, offset-HEADER_SIZE, s->footer_buf,
+                         HEADER_SIZE);
+        if (ret < 0) {
             goto fail;
         }
         if (strncmp(footer->creator, "conectix", 8)) {
+            ret = -EMEDIUMTYPE;
             goto fail;
         }
         disk_type = VHD_FIXED;
@@ -203,19 +212,21 @@ static int vpc_open(BlockDriverState *bs, int flags)
 
     /* Allow a maximum disk size of approximately 2 TB */
     if (bs->total_sectors >= 65535LL * 255 * 255) {
-        err = -EFBIG;
+        ret = -EFBIG;
         goto fail;
     }
 
     if (disk_type == VHD_DYNAMIC) {
-        if (bdrv_pread(bs->file, be64_to_cpu(footer->data_offset), buf,
-                HEADER_SIZE) != HEADER_SIZE) {
+        ret = bdrv_pread(bs->file, be64_to_cpu(footer->data_offset), buf,
+                         HEADER_SIZE);
+        if (ret < 0) {
             goto fail;
         }
 
         dyndisk_header = (struct vhd_dyndisk_header *) buf;
 
         if (strncmp(dyndisk_header->magic, "cxsparse", 8)) {
+            ret = -EINVAL;
             goto fail;
         }
 
@@ -226,8 +237,10 @@ static int vpc_open(BlockDriverState *bs, int flags)
         s->pagetable = g_malloc(s->max_table_entries * 4);
 
         s->bat_offset = be64_to_cpu(dyndisk_header->table_offset);
-        if (bdrv_pread(bs->file, s->bat_offset, s->pagetable,
-                s->max_table_entries * 4) != s->max_table_entries * 4) {
+
+        ret = bdrv_pread(bs->file, s->bat_offset, s->pagetable,
+                         s->max_table_entries * 4);
+        if (ret < 0) {
             goto fail;
         }
 
@@ -265,8 +278,13 @@ static int vpc_open(BlockDriverState *bs, int flags)
     migrate_add_blocker(s->migration_blocker);
 
     return 0;
- fail:
-    return err;
+
+fail:
+    g_free(s->pagetable);
+#ifdef CACHE
+    g_free(s->pageentry_u8);
+#endif
+    return ret;
 }
 
 static int vpc_reopen_prepare(BDRVReopenState *state,
-- 
1.8.1

[Qemu-devel] [PATCH 09/13] dmg: Fix bdrv_open() error handling

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

Return -errno instead of -1 on errors and add error checks in some
places that didn't have one. Passing things by reference requires more
correct typing, replaced a few off_ts therefore - with a 32-bit off_t
this is even a fix for truncation bugs.

While touching the code, fix even some more memory leaks than in the
other drivers...

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 135 +++++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 97 insertions(+), 38 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index ac397dc..53be25d 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -57,29 +57,42 @@ static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
     return 0;
 }
 
-static off_t read_off(BlockDriverState *bs, int64_t offset)
+static int read_uint64(BlockDriverState *bs, int64_t offset, uint64_t *result)
 {
-	uint64_t buffer;
-	if (bdrv_pread(bs->file, offset, &buffer, 8) < 8)
-		return 0;
-	return be64_to_cpu(buffer);
+    uint64_t buffer;
+    int ret;
+
+    ret = bdrv_pread(bs->file, offset, &buffer, 8);
+    if (ret < 0) {
+        return ret;
+    }
+
+    *result = be64_to_cpu(buffer);
+    return 0;
 }
 
-static off_t read_uint32(BlockDriverState *bs, int64_t offset)
+static int read_uint32(BlockDriverState *bs, int64_t offset, uint32_t *result)
 {
-	uint32_t buffer;
-	if (bdrv_pread(bs->file, offset, &buffer, 4) < 4)
-		return 0;
-	return be32_to_cpu(buffer);
+    uint32_t buffer;
+    int ret;
+
+    ret = bdrv_pread(bs->file, offset, &buffer, 4);
+    if (ret < 0) {
+        return ret;
+    }
+
+    *result = be32_to_cpu(buffer);
+    return 0;
 }
 
 static int dmg_open(BlockDriverState *bs, int flags)
 {
     BDRVDMGState *s = bs->opaque;
-    off_t info_begin,info_end,last_in_offset,last_out_offset;
-    uint32_t count;
+    uint64_t info_begin,info_end,last_in_offset,last_out_offset;
+    uint32_t count, tmp;
     uint32_t max_compressed_size=1,max_sectors_per_chunk=1,i;
     int64_t offset;
+    int ret;
 
     bs->read_only = 1;
     s->n_chunks = 0;
@@ -88,21 +101,32 @@ static int dmg_open(BlockDriverState *bs, int flags)
     /* read offset of info blocks */
     offset = bdrv_getlength(bs->file);
     if (offset < 0) {
+        ret = offset;
         goto fail;
     }
     offset -= 0x1d8;
 
-    info_begin = read_off(bs, offset);
-    if (info_begin == 0) {
-	goto fail;
+    ret = read_uint64(bs, offset, &info_begin);
+    if (ret < 0) {
+        goto fail;
+    } else if (info_begin == 0) {
+        ret = -EINVAL;
+        goto fail;
     }
 
-    if (read_uint32(bs, info_begin) != 0x100) {
+    ret = read_uint32(bs, info_begin, &tmp);
+    if (ret < 0) {
+        goto fail;
+    } else if (tmp != 0x100) {
+        ret = -EINVAL;
         goto fail;
     }
 
-    count = read_uint32(bs, info_begin + 4);
-    if (count == 0) {
+    ret = read_uint32(bs, info_begin + 4, &count);
+    if (ret < 0) {
+        goto fail;
+    } else if (count == 0) {
+        ret = -EINVAL;
         goto fail;
     }
     info_end = info_begin + count;
@@ -114,12 +138,20 @@ static int dmg_open(BlockDriverState *bs, int flags)
     while (offset < info_end) {
         uint32_t type;
 
-	count = read_uint32(bs, offset);
-	if(count==0)
-	    goto fail;
+        ret = read_uint32(bs, offset, &count);
+        if (ret < 0) {
+            goto fail;
+        } else if (count == 0) {
+            ret = -EINVAL;
+            goto fail;
+        }
         offset += 4;
 
-	type = read_uint32(bs, offset);
+        ret = read_uint32(bs, offset, &type);
+        if (ret < 0) {
+            goto fail;
+        }
+
 	if (type == 0x6d697368 && count >= 244) {
 	    int new_size, chunk_count;
 
@@ -134,8 +166,11 @@ static int dmg_open(BlockDriverState *bs, int flags)
 	    s->sectors = g_realloc(s->sectors, new_size);
 	    s->sectorcounts = g_realloc(s->sectorcounts, new_size);
 
-	    for(i=s->n_chunks;i<s->n_chunks+chunk_count;i++) {
-		s->types[i] = read_uint32(bs, offset);
+            for (i = s->n_chunks; i < s->n_chunks + chunk_count; i++) {
+                ret = read_uint32(bs, offset, &s->types[i]);
+                if (ret < 0) {
+                    goto fail;
+                }
 		offset += 4;
 		if(s->types[i]!=0x80000005 && s->types[i]!=1 && s->types[i]!=2) {
 		    if(s->types[i]==0xffffffff) {
@@ -149,17 +184,31 @@ static int dmg_open(BlockDriverState *bs, int flags)
 		}
 		offset += 4;
 
-		s->sectors[i] = last_out_offset+read_off(bs, offset);
-		offset += 8;
-
-		s->sectorcounts[i] = read_off(bs, offset);
-		offset += 8;
-
-		s->offsets[i] = last_in_offset+read_off(bs, offset);
-		offset += 8;
-
-		s->lengths[i] = read_off(bs, offset);
-		offset += 8;
+                ret = read_uint64(bs, offset, &s->sectors[i]);
+                if (ret < 0) {
+                    goto fail;
+                }
+                s->sectors[i] += last_out_offset;
+                offset += 8;
+
+                ret = read_uint64(bs, offset, &s->sectorcounts[i]);
+                if (ret < 0) {
+                    goto fail;
+                }
+                offset += 8;
+
+                ret = read_uint64(bs, offset, &s->offsets[i]);
+                if (ret < 0) {
+                    goto fail;
+                }
+                s->offsets[i] += last_in_offset;
+                offset += 8;
+
+                ret = read_uint64(bs, offset, &s->lengths[i]);
+                if (ret < 0) {
+                    goto fail;
+                }
+                offset += 8;
 
 		if(s->lengths[i]>max_compressed_size)
 		    max_compressed_size = s->lengths[i];
@@ -173,15 +222,25 @@ static int dmg_open(BlockDriverState *bs, int flags)
     /* initialize zlib engine */
     s->compressed_chunk = g_malloc(max_compressed_size+1);
     s->uncompressed_chunk = g_malloc(512*max_sectors_per_chunk);
-    if(inflateInit(&s->zstream) != Z_OK)
-	goto fail;
+    if(inflateInit(&s->zstream) != Z_OK) {
+        ret = -EINVAL;
+        goto fail;
+    }
 
     s->current_chunk = s->n_chunks;
 
     qemu_co_mutex_init(&s->lock);
     return 0;
+
 fail:
-    return -1;
+    g_free(s->types);
+    g_free(s->offsets);
+    g_free(s->lengths);
+    g_free(s->sectors);
+    g_free(s->sectorcounts);
+    g_free(s->compressed_chunk);
+    g_free(s->uncompressed_chunk);
+    return ret;
 }
 
 static inline int is_sector_in_chunk(BDRVDMGState* s,
-- 
1.8.1

[Qemu-devel] [PATCH 10/13] dmg: Use g_free instead of free

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

The buffers are allocated with g_(re)alloc, so use g_free to free them.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index 53be25d..6d85801 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -355,15 +355,15 @@ static coroutine_fn int dmg_co_read(BlockDriverState *bs, int64_t sector_num,
 static void dmg_close(BlockDriverState *bs)
 {
     BDRVDMGState *s = bs->opaque;
-    if(s->n_chunks>0) {
-	free(s->types);
-	free(s->offsets);
-	free(s->lengths);
-	free(s->sectors);
-	free(s->sectorcounts);
-    }
-    free(s->compressed_chunk);
-    free(s->uncompressed_chunk);
+
+    g_free(s->types);
+    g_free(s->offsets);
+    g_free(s->lengths);
+    g_free(s->sectors);
+    g_free(s->sectorcounts);
+    g_free(s->compressed_chunk);
+    g_free(s->uncompressed_chunk);
+
     inflateEnd(&s->zstream);
 }
 
-- 
1.8.1

[Qemu-devel] [PATCH 11/13] parallels: Fix bdrv_open() error handling

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

Return -errno instead of -1 on errors. Hey, no memory leak to fix here
while we're touching it!

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/parallels.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 3773750..8688f6c 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -73,14 +73,18 @@ static int parallels_open(BlockDriverState *bs, int flags)
     BDRVParallelsState *s = bs->opaque;
     int i;
     struct parallels_header ph;
+    int ret;
 
     bs->read_only = 1; // no write support yet
 
-    if (bdrv_pread(bs->file, 0, &ph, sizeof(ph)) != sizeof(ph))
+    ret = bdrv_pread(bs->file, 0, &ph, sizeof(ph));
+    if (ret < 0) {
         goto fail;
+    }
 
     if (memcmp(ph.magic, HEADER_MAGIC, 16) ||
-	(le32_to_cpu(ph.version) != HEADER_VERSION)) {
+        (le32_to_cpu(ph.version) != HEADER_VERSION)) {
+        ret = -EMEDIUMTYPE;
         goto fail;
     }
 
@@ -90,18 +94,21 @@ static int parallels_open(BlockDriverState *bs, int flags)
 
     s->catalog_size = le32_to_cpu(ph.catalog_entries);
     s->catalog_bitmap = g_malloc(s->catalog_size * 4);
-    if (bdrv_pread(bs->file, 64, s->catalog_bitmap, s->catalog_size * 4) !=
-	s->catalog_size * 4)
-	goto fail;
+
+    ret = bdrv_pread(bs->file, 64, s->catalog_bitmap, s->catalog_size * 4);
+    if (ret < 0) {
+        goto fail;
+    }
+
     for (i = 0; i < s->catalog_size; i++)
 	le32_to_cpus(&s->catalog_bitmap[i]);
 
     qemu_co_mutex_init(&s->lock);
     return 0;
+
 fail:
-    if (s->catalog_bitmap)
-	g_free(s->catalog_bitmap);
-    return -1;
+    g_free(s->catalog_bitmap);
+    return ret;
 }
 
 static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num)
-- 
1.8.1

[Qemu-devel] [PATCH 12/13] vmdk: Allow space in file name

[Stefan Hajnoczi]

From: Philipp Hahn <hahn@univention.de>

The previous scanf() format string stopped parsing the file name on the
first white white space, which seems to be allowed at least by VMware
Workstation.

Change the format string to collect everything between the first and
second quote as the file name, disallowing line breaks.

Signed-off-by: Philipp Hahn <hahn@univention.de>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/vmdk.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index a8cb5c9..aef1abc 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -641,7 +641,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
          * RW [size in sectors] SPARSE "file-name.vmdk"
          */
         flat_offset = -1;
-        ret = sscanf(p, "%10s %" SCNd64 " %10s %511s %" SCNd64,
+        ret = sscanf(p, "%10s %" SCNd64 " %10s \"%511[^\n\r\"]\" %" SCNd64,
                 access, &sectors, type, fname, &flat_offset);
         if (ret < 4 || strcmp(access, "RW")) {
             goto next_line;
@@ -653,14 +653,6 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
             return -EINVAL;
         }
 
-        /* trim the quotation marks around */
-        if (fname[0] == '"') {
-            memmove(fname, fname + 1, strlen(fname));
-            if (strlen(fname) <= 1 || fname[strlen(fname) - 1] != '"') {
-                return -EINVAL;
-            }
-            fname[strlen(fname) - 1] = '\0';
-        }
         if (sectors <= 0 ||
             (strcmp(type, "FLAT") && strcmp(type, "SPARSE")) ||
             (strcmp(access, "RW"))) {
-- 
1.8.1

[Qemu-devel] [PATCH 13/13] block/raw-posix: Build fix for O_ASYNC

[Stefan Hajnoczi]

From: Andreas Färber <andreas.faerber@web.de>

Commit eeb6b45d48800e96f67ef2a5c80332557fd45ddb (block: raw-posix image
file reopen) broke the build on OpenIndiana.

illumos has no O_ASYNC. Exclude it from flags to be compared
and instead assert that it is not set where defined.

Cf. e61ab1da7e98357da47c54d8f893b9bd6ff2f7f9 for qemu-ga.

Cc: qemu-stable@nongnu.org (1.3.x)
Cc: Jeff Cody <jcody@redhat.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/raw-posix.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 657af95..8b6b926 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -345,11 +345,20 @@ static int raw_reopen_prepare(BDRVReopenState *state,
 
     raw_s->fd = -1;
 
-    int fcntl_flags = O_APPEND | O_ASYNC | O_NONBLOCK;
+    int fcntl_flags = O_APPEND | O_NONBLOCK;
 #ifdef O_NOATIME
     fcntl_flags |= O_NOATIME;
 #endif
 
+#ifdef O_ASYNC
+    /* Not all operating systems have O_ASYNC, and those that don't
+     * will not let us track the state into raw_s->open_flags (typically
+     * you achieve the same effect with an ioctl, for example I_SETSIG
+     * on Solaris). But we do not use O_ASYNC, so that's fine.
+     */
+    assert((s->open_flags & O_ASYNC) == 0);
+#endif
+
     if ((raw_s->open_flags & ~fcntl_flags) == (s->open_flags & ~fcntl_flags)) {
         /* dup the original fd */
         /* TODO: use qemu fcntl wrapper */
-- 
1.8.1

Re: [Qemu-devel] [PATCH 09/13] dmg: Fix bdrv_open() error handling

[Blue Swirl]

On Fri, Feb 1, 2013 at 2:28 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> From: Kevin Wolf <kwolf@redhat.com>
>
> Return -errno instead of -1 on errors and add error checks in some
> places that didn't have one. Passing things by reference requires more
> correct typing, replaced a few off_ts therefore - with a 32-bit off_t
> this is even a fix for truncation bugs.
>
> While touching the code, fix even some more memory leaks than in the
> other drivers...
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/dmg.c | 135 +++++++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 97 insertions(+), 38 deletions(-)
>
> diff --git a/block/dmg.c b/block/dmg.c
> index ac397dc..53be25d 100644
> --- a/block/dmg.c
> +++ b/block/dmg.c
> @@ -57,29 +57,42 @@ static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
>      return 0;
>  }
>
> -static off_t read_off(BlockDriverState *bs, int64_t offset)
> +static int read_uint64(BlockDriverState *bs, int64_t offset, uint64_t *result)
>  {
> -       uint64_t buffer;
> -       if (bdrv_pread(bs->file, offset, &buffer, 8) < 8)
> -               return 0;
> -       return be64_to_cpu(buffer);
> +    uint64_t buffer;
> +    int ret;
> +
> +    ret = bdrv_pread(bs->file, offset, &buffer, 8);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    *result = be64_to_cpu(buffer);
> +    return 0;
>  }
>
> -static off_t read_uint32(BlockDriverState *bs, int64_t offset)
> +static int read_uint32(BlockDriverState *bs, int64_t offset, uint32_t *result)
>  {
> -       uint32_t buffer;
> -       if (bdrv_pread(bs->file, offset, &buffer, 4) < 4)
> -               return 0;
> -       return be32_to_cpu(buffer);
> +    uint32_t buffer;
> +    int ret;
> +
> +    ret = bdrv_pread(bs->file, offset, &buffer, 4);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    *result = be32_to_cpu(buffer);
> +    return 0;
>  }
>
>  static int dmg_open(BlockDriverState *bs, int flags)
>  {
>      BDRVDMGState *s = bs->opaque;
> -    off_t info_begin,info_end,last_in_offset,last_out_offset;
> -    uint32_t count;
> +    uint64_t info_begin,info_end,last_in_offset,last_out_offset;
> +    uint32_t count, tmp;
>      uint32_t max_compressed_size=1,max_sectors_per_chunk=1,i;
>      int64_t offset;
> +    int ret;
>
>      bs->read_only = 1;
>      s->n_chunks = 0;
> @@ -88,21 +101,32 @@ static int dmg_open(BlockDriverState *bs, int flags)
>      /* read offset of info blocks */
>      offset = bdrv_getlength(bs->file);
>      if (offset < 0) {
> +        ret = offset;
>          goto fail;
>      }
>      offset -= 0x1d8;
>
> -    info_begin = read_off(bs, offset);
> -    if (info_begin == 0) {
> -       goto fail;
> +    ret = read_uint64(bs, offset, &info_begin);
> +    if (ret < 0) {
> +        goto fail;
> +    } else if (info_begin == 0) {
> +        ret = -EINVAL;
> +        goto fail;
>      }
>
> -    if (read_uint32(bs, info_begin) != 0x100) {
> +    ret = read_uint32(bs, info_begin, &tmp);
> +    if (ret < 0) {
> +        goto fail;
> +    } else if (tmp != 0x100) {
> +        ret = -EINVAL;
>          goto fail;
>      }
>
> -    count = read_uint32(bs, info_begin + 4);
> -    if (count == 0) {
> +    ret = read_uint32(bs, info_begin + 4, &count);
> +    if (ret < 0) {
> +        goto fail;
> +    } else if (count == 0) {
> +        ret = -EINVAL;
>          goto fail;
>      }
>      info_end = info_begin + count;
> @@ -114,12 +138,20 @@ static int dmg_open(BlockDriverState *bs, int flags)
>      while (offset < info_end) {
>          uint32_t type;
>
> -       count = read_uint32(bs, offset);
> -       if(count==0)
> -           goto fail;
> +        ret = read_uint32(bs, offset, &count);
> +        if (ret < 0) {
> +            goto fail;
> +        } else if (count == 0) {
> +            ret = -EINVAL;
> +            goto fail;
> +        }
>          offset += 4;
>
> -       type = read_uint32(bs, offset);
> +        ret = read_uint32(bs, offset, &type);
> +        if (ret < 0) {
> +            goto fail;
> +        }
> +
>         if (type == 0x6d697368 && count >= 244) {
>             int new_size, chunk_count;
>
> @@ -134,8 +166,11 @@ static int dmg_open(BlockDriverState *bs, int flags)
>             s->sectors = g_realloc(s->sectors, new_size);
>             s->sectorcounts = g_realloc(s->sectorcounts, new_size);
>
> -           for(i=s->n_chunks;i<s->n_chunks+chunk_count;i++) {
> -               s->types[i] = read_uint32(bs, offset);
> +            for (i = s->n_chunks; i < s->n_chunks + chunk_count; i++) {
> +                ret = read_uint32(bs, offset, &s->types[i]);
> +                if (ret < 0) {
> +                    goto fail;
> +                }
>                 offset += 4;
>                 if(s->types[i]!=0x80000005 && s->types[i]!=1 && s->types[i]!=2) {
>                     if(s->types[i]==0xffffffff) {
> @@ -149,17 +184,31 @@ static int dmg_open(BlockDriverState *bs, int flags)
>                 }
>                 offset += 4;
>
> -               s->sectors[i] = last_out_offset+read_off(bs, offset);
> -               offset += 8;
> -
> -               s->sectorcounts[i] = read_off(bs, offset);
> -               offset += 8;
> -
> -               s->offsets[i] = last_in_offset+read_off(bs, offset);
> -               offset += 8;
> -
> -               s->lengths[i] = read_off(bs, offset);
> -               offset += 8;
> +                ret = read_uint64(bs, offset, &s->sectors[i]);
> +                if (ret < 0) {
> +                    goto fail;
> +                }
> +                s->sectors[i] += last_out_offset;
> +                offset += 8;
> +
> +                ret = read_uint64(bs, offset, &s->sectorcounts[i]);
> +                if (ret < 0) {
> +                    goto fail;
> +                }
> +                offset += 8;
> +
> +                ret = read_uint64(bs, offset, &s->offsets[i]);
> +                if (ret < 0) {
> +                    goto fail;
> +                }
> +                s->offsets[i] += last_in_offset;
> +                offset += 8;
> +
> +                ret = read_uint64(bs, offset, &s->lengths[i]);
> +                if (ret < 0) {
> +                    goto fail;
> +                }
> +                offset += 8;
>
>                 if(s->lengths[i]>max_compressed_size)
>                     max_compressed_size = s->lengths[i];
> @@ -173,15 +222,25 @@ static int dmg_open(BlockDriverState *bs, int flags)
>      /* initialize zlib engine */
>      s->compressed_chunk = g_malloc(max_compressed_size+1);
>      s->uncompressed_chunk = g_malloc(512*max_sectors_per_chunk);
> -    if(inflateInit(&s->zstream) != Z_OK)
> -       goto fail;
> +    if(inflateInit(&s->zstream) != Z_OK) {

Please add a space between 'if' and '('.

> +        ret = -EINVAL;
> +        goto fail;
> +    }
>
>      s->current_chunk = s->n_chunks;
>
>      qemu_co_mutex_init(&s->lock);
>      return 0;
> +
>  fail:
> -    return -1;
> +    g_free(s->types);
> +    g_free(s->offsets);
> +    g_free(s->lengths);
> +    g_free(s->sectors);
> +    g_free(s->sectorcounts);
> +    g_free(s->compressed_chunk);
> +    g_free(s->uncompressed_chunk);
> +    return ret;
>  }
>
>  static inline int is_sector_in_chunk(BDRVDMGState* s,
> --
> 1.8.1
>
>