* [Qemu-devel] [PATCH 01/13] qemu-iotests: Add regression test for b7ab0fea
2013-02-01 14:27 [Qemu-devel] [PULL for-1.4 00/13] Block patches Stefan Hajnoczi
@ 2013-02-01 14:27 ` Stefan Hajnoczi
2013-02-01 14:27 ` [Qemu-devel] [PATCH 02/13] block: Fix is_allocated_above with resized files Stefan Hajnoczi
` (11 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 14:27 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 02/13] block: Fix is_allocated_above with resized files
2013-02-01 14:27 [Qemu-devel] [PULL for-1.4 00/13] Block patches Stefan Hajnoczi
2013-02-01 14:27 ` [Qemu-devel] [PATCH 01/13] qemu-iotests: Add regression test for b7ab0fea Stefan Hajnoczi
@ 2013-02-01 14:27 ` Stefan Hajnoczi
2013-02-01 14:27 ` [Qemu-devel] [PATCH 03/13] block: Adds mirroring tests for resized images Stefan Hajnoczi
` (10 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 14:27 UTC (permalink / raw)
To: qemu-devel; +Cc: Anthony Liguori, Vishvananda Ishaya, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 03/13] block: Adds mirroring tests for resized images
2013-02-01 14:27 [Qemu-devel] [PULL for-1.4 00/13] Block patches Stefan Hajnoczi
2013-02-01 14:27 ` [Qemu-devel] [PATCH 01/13] qemu-iotests: Add regression test for b7ab0fea Stefan Hajnoczi
2013-02-01 14:27 ` [Qemu-devel] [PATCH 02/13] block: Fix is_allocated_above with resized files Stefan Hajnoczi
@ 2013-02-01 14:27 ` Stefan Hajnoczi
2013-02-01 14:27 ` [Qemu-devel] [PATCH 04/13] vmdk: Allow selecting SCSI adapter in image creation Stefan Hajnoczi
` (9 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 14:27 UTC (permalink / raw)
To: qemu-devel; +Cc: Anthony Liguori, Vishvananda Ishaya, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 04/13] vmdk: Allow selecting SCSI adapter in image creation
2013-02-01 14:27 [Qemu-devel] [PULL for-1.4 00/13] Block patches Stefan Hajnoczi
` (2 preceding siblings ...)
2013-02-01 14:27 ` [Qemu-devel] [PATCH 03/13] block: Adds mirroring tests for resized images Stefan Hajnoczi
@ 2013-02-01 14:27 ` Stefan Hajnoczi
2013-02-01 14:27 ` [Qemu-devel] [PATCH 05/13] sheepdog: pass vdi_id to sheep daemon for sd_close() Stefan Hajnoczi
` (8 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 14:27 UTC (permalink / raw)
To: qemu-devel; +Cc: Othmar Pasteka, Anthony Liguori, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 05/13] sheepdog: pass vdi_id to sheep daemon for sd_close()
2013-02-01 14:27 [Qemu-devel] [PULL for-1.4 00/13] Block patches Stefan Hajnoczi
` (3 preceding siblings ...)
2013-02-01 14:27 ` [Qemu-devel] [PATCH 04/13] vmdk: Allow selecting SCSI adapter in image creation Stefan Hajnoczi
@ 2013-02-01 14:27 ` Stefan Hajnoczi
2013-02-01 14:27 ` [Qemu-devel] [PATCH 06/13] bochs: Fix bdrv_open() error handling Stefan Hajnoczi
` (7 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 14:27 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi, Liu Yuan,
MORITA Kazutaka
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 06/13] bochs: Fix bdrv_open() error handling
2013-02-01 14:27 [Qemu-devel] [PULL for-1.4 00/13] Block patches Stefan Hajnoczi
` (4 preceding siblings ...)
2013-02-01 14:27 ` [Qemu-devel] [PATCH 05/13] sheepdog: pass vdi_id to sheep daemon for sd_close() Stefan Hajnoczi
@ 2013-02-01 14:27 ` Stefan Hajnoczi
2013-02-01 14:27 ` [Qemu-devel] [PATCH 07/13] cloop: " Stefan Hajnoczi
` (6 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 14:27 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 07/13] cloop: Fix bdrv_open() error handling
2013-02-01 14:27 [Qemu-devel] [PULL for-1.4 00/13] Block patches Stefan Hajnoczi
` (5 preceding siblings ...)
2013-02-01 14:27 ` [Qemu-devel] [PATCH 06/13] bochs: Fix bdrv_open() error handling Stefan Hajnoczi
@ 2013-02-01 14:27 ` Stefan Hajnoczi
2013-02-01 14:27 ` [Qemu-devel] [PATCH 08/13] vpc: " Stefan Hajnoczi
` (5 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 14:27 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 08/13] vpc: Fix bdrv_open() error handling
2013-02-01 14:27 [Qemu-devel] [PULL for-1.4 00/13] Block patches Stefan Hajnoczi
` (6 preceding siblings ...)
2013-02-01 14:27 ` [Qemu-devel] [PATCH 07/13] cloop: " Stefan Hajnoczi
@ 2013-02-01 14:27 ` Stefan Hajnoczi
2013-02-01 14:28 ` [Qemu-devel] [PATCH 09/13] dmg: " Stefan Hajnoczi
` (4 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 14:27 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 09/13] dmg: Fix bdrv_open() error handling
2013-02-01 14:27 [Qemu-devel] [PULL for-1.4 00/13] Block patches Stefan Hajnoczi
` (7 preceding siblings ...)
2013-02-01 14:27 ` [Qemu-devel] [PATCH 08/13] vpc: " Stefan Hajnoczi
@ 2013-02-01 14:28 ` Stefan Hajnoczi
2013-02-02 12:47 ` Blue Swirl
2013-02-01 14:28 ` [Qemu-devel] [PATCH 10/13] dmg: Use g_free instead of free Stefan Hajnoczi
` (3 subsequent siblings)
12 siblings, 1 reply; 15+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 14:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH 09/13] dmg: Fix bdrv_open() error handling
2013-02-01 14:28 ` [Qemu-devel] [PATCH 09/13] dmg: " Stefan Hajnoczi
@ 2013-02-02 12:47 ` Blue Swirl
0 siblings, 0 replies; 15+ messages in thread
From: Blue Swirl @ 2013-02-02 12:47 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Kevin Wolf, Anthony Liguori, qemu-devel
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
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 10/13] dmg: Use g_free instead of free
2013-02-01 14:27 [Qemu-devel] [PULL for-1.4 00/13] Block patches Stefan Hajnoczi
` (8 preceding siblings ...)
2013-02-01 14:28 ` [Qemu-devel] [PATCH 09/13] dmg: " Stefan Hajnoczi
@ 2013-02-01 14:28 ` Stefan Hajnoczi
2013-02-01 14:28 ` [Qemu-devel] [PATCH 11/13] parallels: Fix bdrv_open() error handling Stefan Hajnoczi
` (2 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 14:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 11/13] parallels: Fix bdrv_open() error handling
2013-02-01 14:27 [Qemu-devel] [PULL for-1.4 00/13] Block patches Stefan Hajnoczi
` (9 preceding siblings ...)
2013-02-01 14:28 ` [Qemu-devel] [PATCH 10/13] dmg: Use g_free instead of free Stefan Hajnoczi
@ 2013-02-01 14:28 ` Stefan Hajnoczi
2013-02-01 14:28 ` [Qemu-devel] [PATCH 12/13] vmdk: Allow space in file name Stefan Hajnoczi
2013-02-01 14:28 ` [Qemu-devel] [PATCH 13/13] block/raw-posix: Build fix for O_ASYNC Stefan Hajnoczi
12 siblings, 0 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 14:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Anthony Liguori, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 12/13] vmdk: Allow space in file name
2013-02-01 14:27 [Qemu-devel] [PULL for-1.4 00/13] Block patches Stefan Hajnoczi
` (10 preceding siblings ...)
2013-02-01 14:28 ` [Qemu-devel] [PATCH 11/13] parallels: Fix bdrv_open() error handling Stefan Hajnoczi
@ 2013-02-01 14:28 ` Stefan Hajnoczi
2013-02-01 14:28 ` [Qemu-devel] [PATCH 13/13] block/raw-posix: Build fix for O_ASYNC Stefan Hajnoczi
12 siblings, 0 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 14:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Anthony Liguori, Stefan Hajnoczi, Philipp Hahn
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, §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"))) {
--
1.8.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 13/13] block/raw-posix: Build fix for O_ASYNC
2013-02-01 14:27 [Qemu-devel] [PULL for-1.4 00/13] Block patches Stefan Hajnoczi
` (11 preceding siblings ...)
2013-02-01 14:28 ` [Qemu-devel] [PATCH 12/13] vmdk: Allow space in file name Stefan Hajnoczi
@ 2013-02-01 14:28 ` Stefan Hajnoczi
12 siblings, 0 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 14:28 UTC (permalink / raw)
To: qemu-devel
Cc: Anthony Liguori, Jeff Cody, Andreas Färber, Stefan Hajnoczi,
1.3.x
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
^ permalink raw reply related [flat|nested] 15+ messages in thread