From b93d6d2468ba81b9e373066004f2084efbdcc9d6 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 23 Jan 2013 16:52:49 +0100 Subject: [PATCH 01/13] qemu-iotests: Add regression test for b7ab0fea 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 Signed-off-by: Stefan Hajnoczi --- 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 0000000000..0cf36b434f --- /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 . +# + +# 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 < 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 a0307de06b..1bbd2bf929 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 From 63ba17d39f1a8d262b31ea6a07dd3eb45d5a41e2 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Thu, 24 Jan 2013 10:02:08 -0800 Subject: [PATCH 02/13] block: Fix is_allocated_above with resized files 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 Signed-off-by: Stefan Hajnoczi --- block.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/block.c b/block.c index ba67c0def2..50dab8e595 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; } From a04eca108e5efe8a09fe82f7079fcd1568ffc8d7 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Fri, 25 Jan 2013 10:57:20 -0800 Subject: [PATCH 03/13] block: Adds mirroring tests for resized images 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 Signed-off-by: Stefan Hajnoczi --- 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 b040820c51..720eeff921 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 84bfd63fba..42314e9c00 100644 --- a/tests/qemu-iotests/041.out +++ b/tests/qemu-iotests/041.out @@ -1,5 +1,5 @@ -...................... +........................ ---------------------------------------------------------------------- -Ran 22 tests +Ran 24 tests OK From 7f2039f61113f11be92112adf31b6052e04d986f Mon Sep 17 00:00:00 2001 From: Othmar Pasteka Date: Wed, 30 Jan 2013 00:26:52 +0100 Subject: [PATCH 04/13] vmdk: Allow selecting SCSI adapter in image creation 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 Signed-off-by: Stefan Hajnoczi --- 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 8333afb5e3..a8cb5c972d 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, @@ -1660,6 +1679,12 @@ static QEMUOptionParameter vmdk_create_options[] = { .type = OPT_SIZE, .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, diff --git a/include/block/block_int.h b/include/block/block_int.h index f7279b978a..eaad53e426 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; From 6f74c260b45a8f94007929c800d95c2303f1a7ec Mon Sep 17 00:00:00 2001 From: Liu Yuan Date: Tue, 29 Jan 2013 17:14:16 +0800 Subject: [PATCH 05/13] sheepdog: pass vdi_id to sheep daemon for sd_close() Sheep daemon needs vdi_id to identify which vdi is closed to release resources such as object cache. Cc: MORITA Kazutaka Cc: Kevin Wolf Cc: Stefan Hajnoczi Signed-off-by: Liu Yuan Reviewed-by: MORITA Kazutaka Signed-off-by: Stefan Hajnoczi --- block/sheepdog.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 3e49bb83bb..d466b232d7 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; From 5b7d7dfd198f06ec5edd0c857291c5035c5c060f Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 25 Jan 2013 17:07:27 +0100 Subject: [PATCH 06/13] bochs: Fix bdrv_open() error handling Return -errno instead of -1 on errors. While touching the code, fix a memory leak. Signed-off-by: Kevin Wolf Signed-off-by: Stefan Hajnoczi --- block/bochs.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/block/bochs.c b/block/bochs.c index 37375834e9..a6eb33da42 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) From 1a60657f5729bac57e70802eb17e67ad793400fd Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 25 Jan 2013 17:07:28 +0100 Subject: [PATCH 07/13] cloop: Fix bdrv_open() error handling Return -errno instead of -1 on errors. While touching the code, fix a memory leak. Signed-off-by: Kevin Wolf Signed-off-by: Stefan Hajnoczi --- block/cloop.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/block/cloop.c b/block/cloop.c index 5a0d0d805f..8fe13e92a1 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;in_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) From 59294e465953ffb07d42dc61c827bb98cc0ca423 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 25 Jan 2013 17:07:29 +0100 Subject: [PATCH 08/13] vpc: Fix bdrv_open() error handling Return -errno instead of -1 on errors. While touching the code, fix a memory leak. Signed-off-by: Kevin Wolf Signed-off-by: Stefan Hajnoczi --- block/vpc.c | 42 ++++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/block/vpc.c b/block/vpc.c index 7948609e50..82229ef5a0 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, From 69d34a360dfe773e17e72c76d15931c9b9d190f6 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 25 Jan 2013 17:07:30 +0100 Subject: [PATCH 09/13] dmg: Fix bdrv_open() error handling 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 Signed-off-by: Stefan Hajnoczi --- block/dmg.c | 133 +++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 96 insertions(+), 37 deletions(-) diff --git a/block/dmg.c b/block/dmg.c index ac397dc8f7..53be25d787 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; - } - - if (read_uint32(bs, info_begin) != 0x100) { + ret = read_uint64(bs, offset, &info_begin); + if (ret < 0) { + goto fail; + } else if (info_begin == 0) { + ret = -EINVAL; goto fail; } - count = read_uint32(bs, info_begin + 4); - if (count == 0) { + ret = read_uint32(bs, info_begin, &tmp); + if (ret < 0) { + goto fail; + } else if (tmp != 0x100) { + ret = -EINVAL; + goto fail; + } + + 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;in_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; + ret = read_uint64(bs, offset, &s->sectors[i]); + if (ret < 0) { + goto fail; + } + s->sectors[i] += last_out_offset; + offset += 8; - s->sectorcounts[i] = read_off(bs, offset); - offset += 8; + ret = read_uint64(bs, offset, &s->sectorcounts[i]); + if (ret < 0) { + goto fail; + } + offset += 8; - s->offsets[i] = last_in_offset+read_off(bs, offset); - offset += 8; + ret = read_uint64(bs, offset, &s->offsets[i]); + if (ret < 0) { + goto fail; + } + s->offsets[i] += last_in_offset; + offset += 8; - s->lengths[i] = read_off(bs, 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, From 4f8aa2e19f88fe0dfaf8240ae666de7fd18dd1d0 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 25 Jan 2013 17:07:31 +0100 Subject: [PATCH 10/13] dmg: Use g_free instead of free The buffers are allocated with g_(re)alloc, so use g_free to free them. Signed-off-by: Kevin Wolf Signed-off-by: Stefan Hajnoczi --- block/dmg.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/block/dmg.c b/block/dmg.c index 53be25d787..6d85801a84 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); } From 46536235d80a012cc4286b71426cafad0c7f41f0 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 25 Jan 2013 17:07:32 +0100 Subject: [PATCH 11/13] parallels: Fix bdrv_open() error handling Return -errno instead of -1 on errors. Hey, no memory leak to fix here while we're touching it! Signed-off-by: Kevin Wolf Signed-off-by: Stefan Hajnoczi --- block/parallels.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/block/parallels.c b/block/parallels.c index 377375046f..8688f6ca4c 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) From cd9234757528a1b7155a75ec2eedb375f71e99fa Mon Sep 17 00:00:00 2001 From: Philipp Hahn Date: Tue, 29 Jan 2013 22:50:31 +0100 Subject: [PATCH 12/13] vmdk: Allow space in file name 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 Reviewed-by: Markus Armbruster Signed-off-by: Stefan Hajnoczi --- block/vmdk.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index a8cb5c972d..aef1abcb4f 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, §ors, 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"))) { From fdf263f63fad86b04032da86686a952edfe4644f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Thu, 31 Jan 2013 15:40:14 +0100 Subject: [PATCH 13/13] block/raw-posix: Build fix for O_ASYNC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Suggested-by: Paolo Bonzini Signed-off-by: Andreas Färber Signed-off-by: Stefan Hajnoczi --- 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 657af95637..8b6b92608b 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 */