* [Qemu-devel] [PULL 01/16] block: add event when disk usage exceeds threshold
2015-01-16 15:36 [Qemu-devel] [PULL 00/16] Block patches Stefan Hajnoczi
@ 2015-01-16 15:36 ` Stefan Hajnoczi
2015-01-16 15:36 ` [Qemu-devel] [PULL 02/16] block/dmg: properly detect the UDIF trailer Stefan Hajnoczi
` (15 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Stefan Hajnoczi @ 2015-01-16 15:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Stefan Hajnoczi, Francesco Romani
From: Francesco Romani <fromani@redhat.com>
Managing applications, like oVirt (http://www.ovirt.org), make extensive
use of thin-provisioned disk images.
To let the guest run smoothly and be not unnecessarily paused, oVirt sets
a disk usage threshold (so called 'high water mark') based on the occupation
of the device, and automatically extends the image once the threshold
is reached or exceeded.
In order to detect the crossing of the threshold, oVirt has no choice but
aggressively polling the QEMU monitor using the query-blockstats command.
This lead to unnecessary system load, and is made even worse under scale:
deployments with hundreds of VMs are no longer rare.
To fix this, this patch adds:
* A new monitor command `block-set-write-threshold', to set a mark for
a given block device.
* A new event `BLOCK_WRITE_THRESHOLD', to report if a block device
usage exceeds the threshold.
* A new `write_threshold' field into the `BlockDeviceInfo' structure,
to report the configured threshold.
This will allow the managing application to use smarter and more
efficient monitoring, greatly reducing the need of polling.
[Updated qemu-iotests 067 output to add the new 'write_threshold'
property.
--Stefan]
Signed-off-by: Francesco Romani <fromani@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1421068273-692-1-git-send-email-fromani@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/Makefile.objs | 1 +
block/qapi.c | 3 +
block/write-threshold.c | 125 ++++++++++++++++++++++++++++++++++++++++
include/block/block_int.h | 4 ++
include/block/write-threshold.h | 64 ++++++++++++++++++++
qapi/block-core.json | 51 +++++++++++++++-
qmp-commands.hx | 32 ++++++++++
tests/Makefile | 3 +
tests/qemu-iotests/067.out | 5 ++
tests/test-write-threshold.c | 119 ++++++++++++++++++++++++++++++++++++++
10 files changed, 406 insertions(+), 1 deletion(-)
create mode 100644 block/write-threshold.c
create mode 100644 include/block/write-threshold.h
create mode 100644 tests/test-write-threshold.c
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 04b0e43..010afad 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -20,6 +20,7 @@ block-obj-$(CONFIG_GLUSTERFS) += gluster.o
block-obj-$(CONFIG_ARCHIPELAGO) += archipelago.o
block-obj-$(CONFIG_LIBSSH2) += ssh.o
block-obj-y += accounting.o
+block-obj-y += write-threshold.o
common-obj-y += stream.o
common-obj-y += commit.o
diff --git a/block/qapi.c b/block/qapi.c
index a6fd6f7..03abaab 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -24,6 +24,7 @@
#include "block/qapi.h"
#include "block/block_int.h"
+#include "block/write-threshold.h"
#include "qmp-commands.h"
#include "qapi-visit.h"
#include "qapi/qmp-output-visitor.h"
@@ -89,6 +90,8 @@ BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs)
info->iops_size = cfg.op_size;
}
+ info->write_threshold = bdrv_write_threshold_get(bs);
+
return info;
}
diff --git a/block/write-threshold.c b/block/write-threshold.c
new file mode 100644
index 0000000..c2cd517
--- /dev/null
+++ b/block/write-threshold.c
@@ -0,0 +1,125 @@
+/*
+ * QEMU System Emulator block write threshold notification
+ *
+ * Copyright Red Hat, Inc. 2014
+ *
+ * Authors:
+ * Francesco Romani <fromani@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+
+#include "block/block_int.h"
+#include "block/coroutine.h"
+#include "block/write-threshold.h"
+#include "qemu/notify.h"
+#include "qapi-event.h"
+#include "qmp-commands.h"
+
+
+uint64_t bdrv_write_threshold_get(const BlockDriverState *bs)
+{
+ return bs->write_threshold_offset;
+}
+
+bool bdrv_write_threshold_is_set(const BlockDriverState *bs)
+{
+ return bs->write_threshold_offset > 0;
+}
+
+static void write_threshold_disable(BlockDriverState *bs)
+{
+ if (bdrv_write_threshold_is_set(bs)) {
+ notifier_with_return_remove(&bs->write_threshold_notifier);
+ bs->write_threshold_offset = 0;
+ }
+}
+
+uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs,
+ const BdrvTrackedRequest *req)
+{
+ if (bdrv_write_threshold_is_set(bs)) {
+ if (req->offset > bs->write_threshold_offset) {
+ return (req->offset - bs->write_threshold_offset) + req->bytes;
+ }
+ if ((req->offset + req->bytes) > bs->write_threshold_offset) {
+ return (req->offset + req->bytes) - bs->write_threshold_offset;
+ }
+ }
+ return 0;
+}
+
+static int coroutine_fn before_write_notify(NotifierWithReturn *notifier,
+ void *opaque)
+{
+ BdrvTrackedRequest *req = opaque;
+ BlockDriverState *bs = req->bs;
+ uint64_t amount = 0;
+
+ amount = bdrv_write_threshold_exceeded(bs, req);
+ if (amount > 0) {
+ qapi_event_send_block_write_threshold(
+ bs->node_name,
+ amount,
+ bs->write_threshold_offset,
+ &error_abort);
+
+ /* autodisable to avoid flooding the monitor */
+ write_threshold_disable(bs);
+ }
+
+ return 0; /* should always let other notifiers run */
+}
+
+static void write_threshold_register_notifier(BlockDriverState *bs)
+{
+ bs->write_threshold_notifier.notify = before_write_notify;
+ notifier_with_return_list_add(&bs->before_write_notifiers,
+ &bs->write_threshold_notifier);
+}
+
+static void write_threshold_update(BlockDriverState *bs,
+ int64_t threshold_bytes)
+{
+ bs->write_threshold_offset = threshold_bytes;
+}
+
+void bdrv_write_threshold_set(BlockDriverState *bs, uint64_t threshold_bytes)
+{
+ if (bdrv_write_threshold_is_set(bs)) {
+ if (threshold_bytes > 0) {
+ write_threshold_update(bs, threshold_bytes);
+ } else {
+ write_threshold_disable(bs);
+ }
+ } else {
+ if (threshold_bytes > 0) {
+ /* avoid multiple registration */
+ write_threshold_register_notifier(bs);
+ write_threshold_update(bs, threshold_bytes);
+ }
+ /* discard bogus disable request */
+ }
+}
+
+void qmp_block_set_write_threshold(const char *node_name,
+ uint64_t threshold_bytes,
+ Error **errp)
+{
+ BlockDriverState *bs;
+ AioContext *aio_context;
+
+ bs = bdrv_find_node(node_name);
+ if (!bs) {
+ error_set(errp, QERR_DEVICE_NOT_FOUND, node_name);
+ return;
+ }
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ bdrv_write_threshold_set(bs, threshold_bytes);
+
+ aio_context_release(aio_context);
+}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 06a21dd..0c56755 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -412,6 +412,10 @@ struct BlockDriverState {
/* The error object in use for blocking operations on backing_hd */
Error *backing_blocker;
+
+ /* threshold limit for writes, in bytes. "High water mark". */
+ uint64_t write_threshold_offset;
+ NotifierWithReturn write_threshold_notifier;
};
diff --git a/include/block/write-threshold.h b/include/block/write-threshold.h
new file mode 100644
index 0000000..f1b899c
--- /dev/null
+++ b/include/block/write-threshold.h
@@ -0,0 +1,64 @@
+/*
+ * QEMU System Emulator block write threshold notification
+ *
+ * Copyright Red Hat, Inc. 2014
+ *
+ * Authors:
+ * Francesco Romani <fromani@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+#ifndef BLOCK_WRITE_THRESHOLD_H
+#define BLOCK_WRITE_THRESHOLD_H
+
+#include <stdint.h>
+
+#include "qemu/typedefs.h"
+#include "qemu-common.h"
+
+/*
+ * bdrv_write_threshold_set:
+ *
+ * Set the write threshold for block devices, in bytes.
+ * Notify when a write exceeds the threshold, meaning the device
+ * is becoming full, so it can be transparently resized.
+ * To be used with thin-provisioned block devices.
+ *
+ * Use threshold_bytes == 0 to disable.
+ */
+void bdrv_write_threshold_set(BlockDriverState *bs, uint64_t threshold_bytes);
+
+/*
+ * bdrv_write_threshold_get
+ *
+ * Get the configured write threshold, in bytes.
+ * Zero means no threshold configured.
+ */
+uint64_t bdrv_write_threshold_get(const BlockDriverState *bs);
+
+/*
+ * bdrv_write_threshold_is_set
+ *
+ * Tell if a write threshold is set for a given BDS.
+ */
+bool bdrv_write_threshold_is_set(const BlockDriverState *bs);
+
+/*
+ * bdrv_write_threshold_exceeded
+ *
+ * Return the extent of a write request that exceeded the threshold,
+ * or zero if the request is below the threshold.
+ * Return zero also if the threshold was not set.
+ *
+ * NOTE: here we assume the following holds for each request this code
+ * deals with:
+ *
+ * assert((req->offset + req->bytes) <= UINT64_MAX)
+ *
+ * Please not there is *not* an actual C assert().
+ */
+uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs,
+ const BdrvTrackedRequest *req);
+
+#endif
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 80984d1..ca9bc32 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -257,6 +257,9 @@
#
# @cache: the cache mode used for the block device (since: 2.3)
#
+# @write_threshold: configured write threshold for the device.
+# 0 if disabled. (Since 2.3)
+#
# Since: 0.14.0
#
##
@@ -271,7 +274,8 @@
'*bps_max': 'int', '*bps_rd_max': 'int',
'*bps_wr_max': 'int', '*iops_max': 'int',
'*iops_rd_max': 'int', '*iops_wr_max': 'int',
- '*iops_size': 'int', 'cache': 'BlockdevCacheInfo' } }
+ '*iops_size': 'int', 'cache': 'BlockdevCacheInfo',
+ 'write_threshold': 'int' } }
##
# @BlockDeviceIoStatus:
@@ -1910,3 +1914,48 @@
##
{ 'enum': 'PreallocMode',
'data': [ 'off', 'metadata', 'falloc', 'full' ] }
+
+##
+# @BLOCK_WRITE_THRESHOLD
+#
+# Emitted when writes on block device reaches or exceeds the
+# configured write threshold. For thin-provisioned devices, this
+# means the device should be extended to avoid pausing for
+# disk exhaustion.
+# The event is one shot. Once triggered, it needs to be
+# re-registered with another block-set-threshold command.
+#
+# @node-name: graph node name on which the threshold was exceeded.
+#
+# @amount-exceeded: amount of data which exceeded the threshold, in bytes.
+#
+# @write-threshold: last configured threshold, in bytes.
+#
+# Since: 2.3
+##
+{ 'event': 'BLOCK_WRITE_THRESHOLD',
+ 'data': { 'node-name': 'str',
+ 'amount-exceeded': 'uint64',
+ 'write-threshold': 'uint64' } }
+
+##
+# @block-set-write-threshold
+#
+# Change the write threshold for a block drive. An event will be delivered
+# if a write to this block drive crosses the configured threshold.
+# This is useful to transparently resize thin-provisioned drives without
+# the guest OS noticing.
+#
+# @node-name: graph node name on which the threshold must be set.
+#
+# @write-threshold: configured threshold for the block device, bytes.
+# Use 0 to disable the threshold.
+#
+# Returns: Nothing on success
+# If @node name is not found on the block device graph,
+# DeviceNotFound
+#
+# Since: 2.3
+##
+{ 'command': 'block-set-write-threshold',
+ 'data': { 'node-name': 'str', 'write-threshold': 'uint64' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 8957201..8a0055a 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2146,6 +2146,8 @@ Each json-object contain the following:
- "iops_size": I/O size when limiting by iops (json-int)
- "detect_zeroes": detect and optimize zero writing (json-string)
- Possible values: "off", "on", "unmap"
+ - "write_threshold": write offset threshold in bytes, a event will be
+ emitted if crossed. Zero if disabled (json-int)
- "image": the detail of the image, it is a json-object containing
the following:
- "filename": image file name (json-string)
@@ -2223,6 +2225,7 @@ Example:
"iops_wr_max": 0,
"iops_size": 0,
"detect_zeroes": "on",
+ "write_threshold": 0,
"image":{
"filename":"disks/test.qcow2",
"format":"qcow2",
@@ -3666,6 +3669,7 @@ Example:
"iops_rd_max": 0,
"iops_wr_max": 0,
"iops_size": 0,
+ "write_threshold": 0,
"image":{
"filename":"disks/test.qcow2",
"format":"qcow2",
@@ -3902,3 +3906,31 @@ Move mouse pointer to absolute coordinates (20000, 400).
<- { "return": {} }
EQMP
+
+ {
+ .name = "block-set-write-threshold",
+ .args_type = "node-name:s,write-threshold:l",
+ .mhandler.cmd_new = qmp_marshal_input_block_set_write_threshold,
+ },
+
+SQMP
+block-set-write-threshold
+------------
+
+Change the write threshold for a block drive. The threshold is an offset,
+thus must be non-negative. Default is no write threshold.
+Setting the threshold to zero disables it.
+
+Arguments:
+
+- "node-name": the node name in the block driver state graph (json-string)
+- "write-threshold": the write threshold in bytes (json-int)
+
+Example:
+
+-> { "execute": "block-set-write-threshold",
+ "arguments": { "node-name": "mydev",
+ "write-threshold": 17179869184 } }
+<- { "return": {} }
+
+EQMP
diff --git a/tests/Makefile b/tests/Makefile
index c2e2e52..29c6a48 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -66,6 +66,8 @@ check-unit-y += tests/check-qom-interface$(EXESUF)
gcov-files-check-qom-interface-y = qom/object.c
check-unit-y += tests/test-qemu-opts$(EXESUF)
gcov-files-test-qemu-opts-y = qom/test-qemu-opts.c
+check-unit-y += tests/test-write-threshold$(EXESUF)
+gcov-files-test-write-threshold-y = block/write-threshold.c
check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
@@ -354,6 +356,7 @@ tests/usb-hcd-xhci-test$(EXESUF): tests/usb-hcd-xhci-test.o $(libqos-usb-obj-y)
tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o qemu-timer.o $(qtest-obj-y)
tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a libqemustub.a
+tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o $(block-obj-y) libqemuutil.a libqemustub.a
ifeq ($(CONFIG_POSIX),y)
LIBS += -lutil
diff --git a/tests/qemu-iotests/067.out b/tests/qemu-iotests/067.out
index 13ff3cd..00b3eae 100644
--- a/tests/qemu-iotests/067.out
+++ b/tests/qemu-iotests/067.out
@@ -43,6 +43,7 @@ Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk -device virti
"drv": "qcow2",
"iops": 0,
"bps_wr": 0,
+ "write_threshold": 0,
"encrypted": false,
"bps": 0,
"bps_rd": 0,
@@ -218,6 +219,7 @@ Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk
"drv": "qcow2",
"iops": 0,
"bps_wr": 0,
+ "write_threshold": 0,
"encrypted": false,
"bps": 0,
"bps_rd": 0,
@@ -423,6 +425,7 @@ Testing:
"drv": "qcow2",
"iops": 0,
"bps_wr": 0,
+ "write_threshold": 0,
"encrypted": false,
"bps": 0,
"bps_rd": 0,
@@ -607,6 +610,7 @@ Testing:
"drv": "qcow2",
"iops": 0,
"bps_wr": 0,
+ "write_threshold": 0,
"encrypted": false,
"bps": 0,
"bps_rd": 0,
@@ -717,6 +721,7 @@ Testing:
"drv": "qcow2",
"iops": 0,
"bps_wr": 0,
+ "write_threshold": 0,
"encrypted": false,
"bps": 0,
"bps_rd": 0,
diff --git a/tests/test-write-threshold.c b/tests/test-write-threshold.c
new file mode 100644
index 0000000..d24dc17
--- /dev/null
+++ b/tests/test-write-threshold.c
@@ -0,0 +1,119 @@
+/*
+ * Test block device write threshold
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include <glib.h>
+#include <stdint.h>
+#include "block/block_int.h"
+#include "block/write-threshold.h"
+
+
+static void test_threshold_not_set_on_init(void)
+{
+ uint64_t res;
+ BlockDriverState bs;
+ memset(&bs, 0, sizeof(bs));
+
+ g_assert_false(bdrv_write_threshold_is_set(&bs));
+
+ res = bdrv_write_threshold_get(&bs);
+ g_assert_cmpint(res, ==, 0);
+}
+
+static void test_threshold_set_get(void)
+{
+ uint64_t threshold = 4 * 1024 * 1024;
+ uint64_t res;
+ BlockDriverState bs;
+ memset(&bs, 0, sizeof(bs));
+
+ bdrv_write_threshold_set(&bs, threshold);
+
+ g_assert(bdrv_write_threshold_is_set(&bs));
+
+ res = bdrv_write_threshold_get(&bs);
+ g_assert_cmpint(res, ==, threshold);
+}
+
+static void test_threshold_multi_set_get(void)
+{
+ uint64_t threshold1 = 4 * 1024 * 1024;
+ uint64_t threshold2 = 15 * 1024 * 1024;
+ uint64_t res;
+ BlockDriverState bs;
+ memset(&bs, 0, sizeof(bs));
+
+ bdrv_write_threshold_set(&bs, threshold1);
+ bdrv_write_threshold_set(&bs, threshold2);
+ res = bdrv_write_threshold_get(&bs);
+ g_assert_cmpint(res, ==, threshold2);
+}
+
+static void test_threshold_not_trigger(void)
+{
+ uint64_t amount = 0;
+ uint64_t threshold = 4 * 1024 * 1024;
+ BlockDriverState bs;
+ BdrvTrackedRequest req;
+
+ memset(&bs, 0, sizeof(bs));
+ memset(&req, 0, sizeof(req));
+ req.offset = 1024;
+ req.bytes = 1024;
+
+ bdrv_write_threshold_set(&bs, threshold);
+ amount = bdrv_write_threshold_exceeded(&bs, &req);
+ g_assert_cmpuint(amount, ==, 0);
+}
+
+
+static void test_threshold_trigger(void)
+{
+ uint64_t amount = 0;
+ uint64_t threshold = 4 * 1024 * 1024;
+ BlockDriverState bs;
+ BdrvTrackedRequest req;
+
+ memset(&bs, 0, sizeof(bs));
+ memset(&req, 0, sizeof(req));
+ req.offset = (4 * 1024 * 1024) - 1024;
+ req.bytes = 2 * 1024;
+
+ bdrv_write_threshold_set(&bs, threshold);
+ amount = bdrv_write_threshold_exceeded(&bs, &req);
+ g_assert_cmpuint(amount, >=, 1024);
+}
+
+typedef struct TestStruct {
+ const char *name;
+ void (*func)(void);
+} TestStruct;
+
+
+int main(int argc, char **argv)
+{
+ size_t i;
+ TestStruct tests[] = {
+ { "/write-threshold/not-set-on-init",
+ test_threshold_not_set_on_init },
+ { "/write-threshold/set-get",
+ test_threshold_set_get },
+ { "/write-threshold/multi-set-get",
+ test_threshold_multi_set_get },
+ { "/write-threshold/not-trigger",
+ test_threshold_not_trigger },
+ { "/write-threshold/trigger",
+ test_threshold_trigger },
+ { NULL, NULL }
+ };
+
+ g_test_init(&argc, &argv, NULL);
+ for (i = 0; tests[i].name != NULL; i++) {
+ g_test_add_func(tests[i].name, tests[i].func);
+ }
+ return g_test_run();
+}
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PULL 02/16] block/dmg: properly detect the UDIF trailer
2015-01-16 15:36 [Qemu-devel] [PULL 00/16] Block patches Stefan Hajnoczi
2015-01-16 15:36 ` [Qemu-devel] [PULL 01/16] block: add event when disk usage exceeds threshold Stefan Hajnoczi
@ 2015-01-16 15:36 ` Stefan Hajnoczi
2015-01-16 15:37 ` [Qemu-devel] [PULL 03/16] block/dmg: extract mish block decoding functionality Stefan Hajnoczi
` (14 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Stefan Hajnoczi @ 2015-01-16 15:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Peter Wu, Stefan Hajnoczi
From: Peter Wu <peter@lekensteyn.nl>
DMG files have a variable length with a UDIF trailer at the end of a
file. This UDIF trailer is essential as it describes the contents of
the image. At the moment however, the start of this trailer is almost
always incorrect as bdrv_getlength() returns a multiple of the block
size (rounded up). This results in a failure to recognize DMG files,
resulting in Invalid argument (EINVAL) errors.
As there is no API to retrieve the real file size, look for the magic
header in the last two sectors to find the start of this 512-byte UDIF
trailer (the "koly" block).
The resource fork offset ("info_begin") has its offset adjusted as the
initial value of offset does not mean "end of file" anymore, but "begin
of UDIF trailer".
[Replaced error_set(errp, ERROR_CLASS_GENERIC_ERROR, ...) with
error_setg(errp, ...) as discussed with Peter.
--Stefan]
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1420566495-13284-2-git-send-email-peter@lekensteyn.nl
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/dmg.c | 47 +++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 43 insertions(+), 4 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index e455886..cdad28f 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -131,6 +131,46 @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk,
}
}
+static int64_t dmg_find_koly_offset(BlockDriverState *file_bs, Error **errp)
+{
+ int64_t length;
+ int64_t offset = 0;
+ uint8_t buffer[515];
+ int i, ret;
+
+ /* bdrv_getlength returns a multiple of block size (512), rounded up. Since
+ * dmg images can have odd sizes, try to look for the "koly" magic which
+ * marks the begin of the UDIF trailer (512 bytes). This magic can be found
+ * in the last 511 bytes of the second-last sector or the first 4 bytes of
+ * the last sector (search space: 515 bytes) */
+ length = bdrv_getlength(file_bs);
+ if (length < 0) {
+ error_setg_errno(errp, -length,
+ "Failed to get file size while reading UDIF trailer");
+ return length;
+ } else if (length < 512) {
+ error_setg(errp, "dmg file must be at least 512 bytes long");
+ return -EINVAL;
+ }
+ if (length > 511 + 512) {
+ offset = length - 511 - 512;
+ }
+ length = length < 515 ? length : 515;
+ ret = bdrv_pread(file_bs, offset, buffer, length);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed while reading UDIF trailer");
+ return ret;
+ }
+ for (i = 0; i < length - 3; i++) {
+ if (buffer[i] == 'k' && buffer[i+1] == 'o' &&
+ buffer[i+2] == 'l' && buffer[i+3] == 'y') {
+ return offset + i;
+ }
+ }
+ error_setg(errp, "Could not locate UDIF trailer in dmg file");
+ return -EINVAL;
+}
+
static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
@@ -145,15 +185,14 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
s->n_chunks = 0;
s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
- /* read offset of info blocks */
- offset = bdrv_getlength(bs->file);
+ /* locate the UDIF trailer */
+ offset = dmg_find_koly_offset(bs->file, errp);
if (offset < 0) {
ret = offset;
goto fail;
}
- offset -= 0x1d8;
- ret = read_uint64(bs, offset, &info_begin);
+ ret = read_uint64(bs, offset + 0x28, &info_begin);
if (ret < 0) {
goto fail;
} else if (info_begin == 0) {
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PULL 03/16] block/dmg: extract mish block decoding functionality
2015-01-16 15:36 [Qemu-devel] [PULL 00/16] Block patches Stefan Hajnoczi
2015-01-16 15:36 ` [Qemu-devel] [PULL 01/16] block: add event when disk usage exceeds threshold Stefan Hajnoczi
2015-01-16 15:36 ` [Qemu-devel] [PULL 02/16] block/dmg: properly detect the UDIF trailer Stefan Hajnoczi
@ 2015-01-16 15:37 ` Stefan Hajnoczi
2015-01-16 15:37 ` [Qemu-devel] [PULL 04/16] block/dmg: extract processing of resource forks Stefan Hajnoczi
` (13 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Stefan Hajnoczi @ 2015-01-16 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Peter Wu, Stefan Hajnoczi
From: Peter Wu <peter@lekensteyn.nl>
Extract the mish block decoder such that this can be used for other
formats in the future. A new DmgHeaderState struct is introduced to
share state while decoding.
The code is kept unchanged as much as possible, a "fail" label is added
for example where a simple return would probably do. In dmg_open, the
variable "tmp" is renamed to "rsrc_data_offset" for clarity and comments
have been added explaining various data.
Note that this patch has one subtle difference with the previous
version which should not affect functionality. In the previous code,
the end of a resource was inferred from the mish block (the offsets
would be increased by the fields). In this patch, the resource length
is used instead to avoid the need to rely on the previous offsets.
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1420566495-13284-3-git-send-email-peter@lekensteyn.nl
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/dmg.c | 228 +++++++++++++++++++++++++++++++++++-------------------------
1 file changed, 133 insertions(+), 95 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index cdad28f..c571ac9 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -171,19 +171,138 @@ static int64_t dmg_find_koly_offset(BlockDriverState *file_bs, Error **errp)
return -EINVAL;
}
+/* used when building the sector table */
+typedef struct DmgHeaderState {
+ /* used internally by dmg_read_mish_block to remember offsets of blocks
+ * across calls */
+ uint64_t last_in_offset;
+ uint64_t last_out_offset;
+ /* exported for dmg_open */
+ uint32_t max_compressed_size;
+ uint32_t max_sectors_per_chunk;
+} DmgHeaderState;
+
+static int dmg_read_mish_block(BlockDriverState *bs, DmgHeaderState *ds,
+ int64_t offset, uint32_t count)
+{
+ BDRVDMGState *s = bs->opaque;
+ uint32_t type, i;
+ int ret;
+ size_t new_size;
+ uint32_t chunk_count;
+
+ ret = read_uint32(bs, offset, &type);
+ if (ret < 0) {
+ goto fail;
+ }
+
+ /* skip data that is not a valid MISH block (invalid magic or too small) */
+ if (type != 0x6d697368 || count < 244) {
+ /* assume success for now */
+ return 0;
+ }
+
+ offset += 4;
+ offset += 200;
+
+ chunk_count = (count - 204) / 40;
+ new_size = sizeof(uint64_t) * (s->n_chunks + chunk_count);
+ s->types = g_realloc(s->types, new_size / 2);
+ s->offsets = g_realloc(s->offsets, new_size);
+ s->lengths = g_realloc(s->lengths, new_size);
+ 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++) {
+ 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 && i > 0) {
+ ds->last_in_offset = s->offsets[i - 1] + s->lengths[i - 1];
+ ds->last_out_offset = s->sectors[i - 1] +
+ s->sectorcounts[i - 1];
+ }
+ chunk_count--;
+ i--;
+ offset += 36;
+ continue;
+ }
+ offset += 4;
+
+ ret = read_uint64(bs, offset, &s->sectors[i]);
+ if (ret < 0) {
+ goto fail;
+ }
+ s->sectors[i] += ds->last_out_offset;
+ offset += 8;
+
+ ret = read_uint64(bs, offset, &s->sectorcounts[i]);
+ if (ret < 0) {
+ goto fail;
+ }
+ offset += 8;
+
+ if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
+ error_report("sector count %" PRIu64 " for chunk %" PRIu32
+ " is larger than max (%u)",
+ s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
+ ret = -EINVAL;
+ goto fail;
+ }
+
+ ret = read_uint64(bs, offset, &s->offsets[i]);
+ if (ret < 0) {
+ goto fail;
+ }
+ s->offsets[i] += ds->last_in_offset;
+ offset += 8;
+
+ ret = read_uint64(bs, offset, &s->lengths[i]);
+ if (ret < 0) {
+ goto fail;
+ }
+ offset += 8;
+
+ if (s->lengths[i] > DMG_LENGTHS_MAX) {
+ error_report("length %" PRIu64 " for chunk %" PRIu32
+ " is larger than max (%u)",
+ s->lengths[i], i, DMG_LENGTHS_MAX);
+ ret = -EINVAL;
+ goto fail;
+ }
+
+ update_max_chunk_size(s, i, &ds->max_compressed_size,
+ &ds->max_sectors_per_chunk);
+ }
+ s->n_chunks += chunk_count;
+ return 0;
+
+fail:
+ return ret;
+}
+
static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
BDRVDMGState *s = bs->opaque;
- 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;
+ DmgHeaderState ds;
+ uint64_t info_begin, info_end;
+ uint32_t count, rsrc_data_offset;
int64_t offset;
int ret;
bs->read_only = 1;
s->n_chunks = 0;
s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
+ /* used by dmg_read_mish_block to keep track of the current I/O position */
+ ds.last_in_offset = 0;
+ ds.last_out_offset = 0;
+ ds.max_compressed_size = 1;
+ ds.max_sectors_per_chunk = 1;
/* locate the UDIF trailer */
offset = dmg_find_koly_offset(bs->file, errp);
@@ -200,10 +319,10 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
goto fail;
}
- ret = read_uint32(bs, info_begin, &tmp);
+ ret = read_uint32(bs, info_begin, &rsrc_data_offset);
if (ret < 0) {
goto fail;
- } else if (tmp != 0x100) {
+ } else if (rsrc_data_offset != 0x100) {
ret = -EINVAL;
goto fail;
}
@@ -215,15 +334,15 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
ret = -EINVAL;
goto fail;
}
+ /* end of resource data, ignoring the following resource map */
info_end = info_begin + count;
+ /* begin of resource data (consisting of one or more resources) */
offset = info_begin + 0x100;
- /* read offsets */
- last_in_offset = last_out_offset = 0;
+ /* read offsets (mish blocks) from one or more resources in resource data */
while (offset < info_end) {
- uint32_t type;
-
+ /* size of following resource */
ret = read_uint32(bs, offset, &count);
if (ret < 0) {
goto fail;
@@ -233,100 +352,19 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
}
offset += 4;
- ret = read_uint32(bs, offset, &type);
+ ret = dmg_read_mish_block(bs, &ds, offset, count);
if (ret < 0) {
goto fail;
}
-
- if (type == 0x6d697368 && count >= 244) {
- size_t new_size;
- uint32_t chunk_count;
-
- offset += 4;
- offset += 200;
-
- chunk_count = (count - 204) / 40;
- new_size = sizeof(uint64_t) * (s->n_chunks + chunk_count);
- s->types = g_realloc(s->types, new_size / 2);
- s->offsets = g_realloc(s->offsets, new_size);
- s->lengths = g_realloc(s->lengths, new_size);
- 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++) {
- 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 && i > 0) {
- last_in_offset = s->offsets[i - 1] + s->lengths[i - 1];
- last_out_offset = s->sectors[i - 1] +
- s->sectorcounts[i - 1];
- }
- chunk_count--;
- i--;
- offset += 36;
- continue;
- }
- offset += 4;
-
- 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;
-
- if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
- error_report("sector count %" PRIu64 " for chunk %" PRIu32
- " is larger than max (%u)",
- s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
- ret = -EINVAL;
- goto fail;
- }
-
- 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] > DMG_LENGTHS_MAX) {
- error_report("length %" PRIu64 " for chunk %" PRIu32
- " is larger than max (%u)",
- s->lengths[i], i, DMG_LENGTHS_MAX);
- ret = -EINVAL;
- goto fail;
- }
-
- update_max_chunk_size(s, i, &max_compressed_size,
- &max_sectors_per_chunk);
- }
- s->n_chunks += chunk_count;
- }
+ /* advance offset by size of resource */
+ offset += count;
}
/* initialize zlib engine */
s->compressed_chunk = qemu_try_blockalign(bs->file,
- max_compressed_size + 1);
+ ds.max_compressed_size + 1);
s->uncompressed_chunk = qemu_try_blockalign(bs->file,
- 512 * max_sectors_per_chunk);
+ 512 * ds.max_sectors_per_chunk);
if (s->compressed_chunk == NULL || s->uncompressed_chunk == NULL) {
ret = -ENOMEM;
goto fail;
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PULL 04/16] block/dmg: extract processing of resource forks
2015-01-16 15:36 [Qemu-devel] [PULL 00/16] Block patches Stefan Hajnoczi
` (2 preceding siblings ...)
2015-01-16 15:37 ` [Qemu-devel] [PULL 03/16] block/dmg: extract mish block decoding functionality Stefan Hajnoczi
@ 2015-01-16 15:37 ` Stefan Hajnoczi
2015-01-16 15:37 ` [Qemu-devel] [PULL 05/16] block/dmg: process a buffer instead of reading ints Stefan Hajnoczi
` (12 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Stefan Hajnoczi @ 2015-01-16 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Peter Wu, Stefan Hajnoczi
From: Peter Wu <peter@lekensteyn.nl>
Besides the offset, also read the resource length. This length is now
used in the extracted function to verify the end of the resource fork
against "count" from the resource fork.
Instead of relying on the value of offset to conclude whether the
resource fork is available or not (info_begin==0), check the
rsrc_fork_length instead. This would allow a dmg file to begin with a
resource fork. This seemingly unnecessary restriction was found while
trying to craft a DMG file by hand.
Other changes:
- Do not require resource data offset to be 0x100 (but check that it
is within bounds though).
- Further improve boundary checking (resource data must be within
the resource fork).
- Use correct value for resource data length (spotted by John Snow)
- Consider the resource data offset when determining info_end.
This fixes an EINVAL on the tuxpaint dmg example.
The resource fork format is documented at
https://developer.apple.com/legacy/library/documentation/mac/pdf/MoreMacintoshToolbox.pdf#page=151
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1420566495-13284-4-git-send-email-peter@lekensteyn.nl
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/dmg.c | 104 ++++++++++++++++++++++++++++++++++++++----------------------
1 file changed, 66 insertions(+), 38 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index c571ac9..04bae72 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -285,60 +285,38 @@ fail:
return ret;
}
-static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
- Error **errp)
+static int dmg_read_resource_fork(BlockDriverState *bs, DmgHeaderState *ds,
+ uint64_t info_begin, uint64_t info_length)
{
- BDRVDMGState *s = bs->opaque;
- DmgHeaderState ds;
- uint64_t info_begin, info_end;
- uint32_t count, rsrc_data_offset;
- int64_t offset;
int ret;
+ uint32_t count, rsrc_data_offset;
+ uint64_t info_end;
+ uint64_t offset;
- bs->read_only = 1;
- s->n_chunks = 0;
- s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
- /* used by dmg_read_mish_block to keep track of the current I/O position */
- ds.last_in_offset = 0;
- ds.last_out_offset = 0;
- ds.max_compressed_size = 1;
- ds.max_sectors_per_chunk = 1;
-
- /* locate the UDIF trailer */
- offset = dmg_find_koly_offset(bs->file, errp);
- if (offset < 0) {
- ret = offset;
- goto fail;
- }
-
- ret = read_uint64(bs, offset + 0x28, &info_begin);
- if (ret < 0) {
- goto fail;
- } else if (info_begin == 0) {
- ret = -EINVAL;
- goto fail;
- }
-
+ /* read offset from begin of resource fork (info_begin) to resource data */
ret = read_uint32(bs, info_begin, &rsrc_data_offset);
if (ret < 0) {
goto fail;
- } else if (rsrc_data_offset != 0x100) {
+ } else if (rsrc_data_offset > info_length) {
ret = -EINVAL;
goto fail;
}
- ret = read_uint32(bs, info_begin + 4, &count);
+ /* read length of resource data */
+ ret = read_uint32(bs, info_begin + 8, &count);
if (ret < 0) {
goto fail;
- } else if (count == 0) {
+ } else if (count == 0 || rsrc_data_offset + count > info_length) {
ret = -EINVAL;
goto fail;
}
- /* end of resource data, ignoring the following resource map */
- info_end = info_begin + count;
/* begin of resource data (consisting of one or more resources) */
- offset = info_begin + 0x100;
+ offset = info_begin + rsrc_data_offset;
+
+ /* end of resource data (there is possibly a following resource map
+ * which will be ignored). */
+ info_end = offset + count;
/* read offsets (mish blocks) from one or more resources in resource data */
while (offset < info_end) {
@@ -352,13 +330,63 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
}
offset += 4;
- ret = dmg_read_mish_block(bs, &ds, offset, count);
+ ret = dmg_read_mish_block(bs, ds, offset, count);
if (ret < 0) {
goto fail;
}
/* advance offset by size of resource */
offset += count;
}
+ return 0;
+
+fail:
+ return ret;
+}
+
+static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
+{
+ BDRVDMGState *s = bs->opaque;
+ DmgHeaderState ds;
+ uint64_t rsrc_fork_offset, rsrc_fork_length;
+ int64_t offset;
+ int ret;
+
+ bs->read_only = 1;
+ s->n_chunks = 0;
+ s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
+ /* used by dmg_read_mish_block to keep track of the current I/O position */
+ ds.last_in_offset = 0;
+ ds.last_out_offset = 0;
+ ds.max_compressed_size = 1;
+ ds.max_sectors_per_chunk = 1;
+
+ /* locate the UDIF trailer */
+ offset = dmg_find_koly_offset(bs->file, errp);
+ if (offset < 0) {
+ ret = offset;
+ goto fail;
+ }
+
+ /* offset of resource fork (RsrcForkOffset) */
+ ret = read_uint64(bs, offset + 0x28, &rsrc_fork_offset);
+ if (ret < 0) {
+ goto fail;
+ }
+ ret = read_uint64(bs, offset + 0x30, &rsrc_fork_length);
+ if (ret < 0) {
+ goto fail;
+ }
+ if (rsrc_fork_length != 0) {
+ ret = dmg_read_resource_fork(bs, &ds,
+ rsrc_fork_offset, rsrc_fork_length);
+ if (ret < 0) {
+ goto fail;
+ }
+ } else {
+ ret = -EINVAL;
+ goto fail;
+ }
/* initialize zlib engine */
s->compressed_chunk = qemu_try_blockalign(bs->file,
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PULL 05/16] block/dmg: process a buffer instead of reading ints
2015-01-16 15:36 [Qemu-devel] [PULL 00/16] Block patches Stefan Hajnoczi
` (3 preceding siblings ...)
2015-01-16 15:37 ` [Qemu-devel] [PULL 04/16] block/dmg: extract processing of resource forks Stefan Hajnoczi
@ 2015-01-16 15:37 ` Stefan Hajnoczi
2015-01-16 15:37 ` [Qemu-devel] [PULL 06/16] block/dmg: validate chunk size to avoid overflow Stefan Hajnoczi
` (11 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Stefan Hajnoczi @ 2015-01-16 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Peter Wu, Stefan Hajnoczi
From: Peter Wu <peter@lekensteyn.nl>
As the decoded plist XML is not a pointer in the file,
dmg_read_mish_block must be able to process a buffer instead of a file
pointer. Since the full buffer must be processed, let's change the
return value again to just a success flag.
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1420566495-13284-5-git-send-email-peter@lekensteyn.nl
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/dmg.c | 60 ++++++++++++++++++++++++++++++------------------------------
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index 04bae72..4f56227 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -100,6 +100,16 @@ static int read_uint32(BlockDriverState *bs, int64_t offset, uint32_t *result)
return 0;
}
+static inline uint64_t buff_read_uint64(const uint8_t *buffer, int64_t offset)
+{
+ return be64_to_cpu(*(uint64_t *)&buffer[offset]);
+}
+
+static inline uint32_t buff_read_uint32(const uint8_t *buffer, int64_t offset)
+{
+ return be32_to_cpu(*(uint32_t *)&buffer[offset]);
+}
+
/* Increase max chunk sizes, if necessary. This function is used to calculate
* the buffer sizes needed for compressed/uncompressed chunk I/O.
*/
@@ -182,20 +192,16 @@ typedef struct DmgHeaderState {
uint32_t max_sectors_per_chunk;
} DmgHeaderState;
-static int dmg_read_mish_block(BlockDriverState *bs, DmgHeaderState *ds,
- int64_t offset, uint32_t count)
+static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
+ uint8_t *buffer, uint32_t count)
{
- BDRVDMGState *s = bs->opaque;
uint32_t type, i;
int ret;
size_t new_size;
uint32_t chunk_count;
+ int64_t offset = 0;
- ret = read_uint32(bs, offset, &type);
- if (ret < 0) {
- goto fail;
- }
-
+ type = buff_read_uint32(buffer, offset);
/* skip data that is not a valid MISH block (invalid magic or too small) */
if (type != 0x6d697368 || count < 244) {
/* assume success for now */
@@ -214,10 +220,7 @@ static int dmg_read_mish_block(BlockDriverState *bs, DmgHeaderState *ds,
s->sectorcounts = g_realloc(s->sectorcounts, new_size);
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;
- }
+ s->types[i] = buff_read_uint32(buffer, offset);
offset += 4;
if (s->types[i] != 0x80000005 && s->types[i] != 1 &&
s->types[i] != 2) {
@@ -233,17 +236,11 @@ static int dmg_read_mish_block(BlockDriverState *bs, DmgHeaderState *ds,
}
offset += 4;
- ret = read_uint64(bs, offset, &s->sectors[i]);
- if (ret < 0) {
- goto fail;
- }
+ s->sectors[i] = buff_read_uint64(buffer, offset);
s->sectors[i] += ds->last_out_offset;
offset += 8;
- ret = read_uint64(bs, offset, &s->sectorcounts[i]);
- if (ret < 0) {
- goto fail;
- }
+ s->sectorcounts[i] = buff_read_uint64(buffer, offset);
offset += 8;
if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
@@ -254,17 +251,11 @@ static int dmg_read_mish_block(BlockDriverState *bs, DmgHeaderState *ds,
goto fail;
}
- ret = read_uint64(bs, offset, &s->offsets[i]);
- if (ret < 0) {
- goto fail;
- }
+ s->offsets[i] = buff_read_uint64(buffer, offset);
s->offsets[i] += ds->last_in_offset;
offset += 8;
- ret = read_uint64(bs, offset, &s->lengths[i]);
- if (ret < 0) {
- goto fail;
- }
+ s->lengths[i] = buff_read_uint64(buffer, offset);
offset += 8;
if (s->lengths[i] > DMG_LENGTHS_MAX) {
@@ -288,8 +279,10 @@ fail:
static int dmg_read_resource_fork(BlockDriverState *bs, DmgHeaderState *ds,
uint64_t info_begin, uint64_t info_length)
{
+ BDRVDMGState *s = bs->opaque;
int ret;
uint32_t count, rsrc_data_offset;
+ uint8_t *buffer = NULL;
uint64_t info_end;
uint64_t offset;
@@ -330,16 +323,23 @@ static int dmg_read_resource_fork(BlockDriverState *bs, DmgHeaderState *ds,
}
offset += 4;
- ret = dmg_read_mish_block(bs, ds, offset, count);
+ buffer = g_realloc(buffer, count);
+ ret = bdrv_pread(bs->file, offset, buffer, count);
+ if (ret < 0) {
+ goto fail;
+ }
+
+ ret = dmg_read_mish_block(s, ds, buffer, count);
if (ret < 0) {
goto fail;
}
/* advance offset by size of resource */
offset += count;
}
- return 0;
+ ret = 0;
fail:
+ g_free(buffer);
return ret;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PULL 06/16] block/dmg: validate chunk size to avoid overflow
2015-01-16 15:36 [Qemu-devel] [PULL 00/16] Block patches Stefan Hajnoczi
` (4 preceding siblings ...)
2015-01-16 15:37 ` [Qemu-devel] [PULL 05/16] block/dmg: process a buffer instead of reading ints Stefan Hajnoczi
@ 2015-01-16 15:37 ` Stefan Hajnoczi
2015-01-16 15:37 ` [Qemu-devel] [PULL 07/16] block/dmg: process XML plists Stefan Hajnoczi
` (10 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Stefan Hajnoczi @ 2015-01-16 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Peter Wu, Stefan Hajnoczi
From: Peter Wu <peter@lekensteyn.nl>
Previously the chunk size was not checked, allowing for a large memory
allocation. This patch checks whether the chunks size is within the
resource fork length, and whether the resource fork is below the
trailer of the dmg file.
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1420566495-13284-6-git-send-email-peter@lekensteyn.nl
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/dmg.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/block/dmg.c b/block/dmg.c
index 4f56227..5c2c2c2 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -317,7 +317,7 @@ static int dmg_read_resource_fork(BlockDriverState *bs, DmgHeaderState *ds,
ret = read_uint32(bs, offset, &count);
if (ret < 0) {
goto fail;
- } else if (count == 0) {
+ } else if (count == 0 || count > info_end - offset) {
ret = -EINVAL;
goto fail;
}
@@ -377,6 +377,11 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
if (ret < 0) {
goto fail;
}
+ if (rsrc_fork_offset >= offset ||
+ rsrc_fork_length > offset - rsrc_fork_offset) {
+ ret = -EINVAL;
+ goto fail;
+ }
if (rsrc_fork_length != 0) {
ret = dmg_read_resource_fork(bs, &ds,
rsrc_fork_offset, rsrc_fork_length);
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PULL 07/16] block/dmg: process XML plists
2015-01-16 15:36 [Qemu-devel] [PULL 00/16] Block patches Stefan Hajnoczi
` (5 preceding siblings ...)
2015-01-16 15:37 ` [Qemu-devel] [PULL 06/16] block/dmg: validate chunk size to avoid overflow Stefan Hajnoczi
@ 2015-01-16 15:37 ` Stefan Hajnoczi
2015-01-16 15:37 ` [Qemu-devel] [PULL 08/16] block/dmg: set virtual size to a non-zero value Stefan Hajnoczi
` (9 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Stefan Hajnoczi @ 2015-01-16 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Peter Wu, Stefan Hajnoczi
From: Peter Wu <peter@lekensteyn.nl>
The format is simple enough to avoid using a full-blown XML parser. It
assumes that all BLKX items begin with the "mish" magic word, therefore
it is not a problem if other values get matched which are not a BLKX
block.
The offsets are based on the description at
http://newosxbook.com/DMG.html
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1420566495-13284-7-git-send-email-peter@lekensteyn.nl
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/dmg.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)
diff --git a/block/dmg.c b/block/dmg.c
index 5c2c2c2..d4a7520 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -26,6 +26,7 @@
#include "qemu/bswap.h"
#include "qemu/module.h"
#include <zlib.h>
+#include <glib.h>
enum {
/* Limit chunk sizes to prevent unreasonable amounts of memory being used
@@ -343,12 +344,66 @@ fail:
return ret;
}
+static int dmg_read_plist_xml(BlockDriverState *bs, DmgHeaderState *ds,
+ uint64_t info_begin, uint64_t info_length)
+{
+ BDRVDMGState *s = bs->opaque;
+ int ret;
+ uint8_t *buffer = NULL;
+ char *data_begin, *data_end;
+
+ /* Have at least some length to avoid NULL for g_malloc. Attempt to set a
+ * safe upper cap on the data length. A test sample had a XML length of
+ * about 1 MiB. */
+ if (info_length == 0 || info_length > 16 * 1024 * 1024) {
+ ret = -EINVAL;
+ goto fail;
+ }
+
+ buffer = g_malloc(info_length + 1);
+ buffer[info_length] = '\0';
+ ret = bdrv_pread(bs->file, info_begin, buffer, info_length);
+ if (ret != info_length) {
+ ret = -EINVAL;
+ goto fail;
+ }
+
+ /* look for <data>...</data>. The data is 284 (0x11c) bytes after base64
+ * decode. The actual data element has 431 (0x1af) bytes which includes tabs
+ * and line feeds. */
+ data_end = (char *)buffer;
+ while ((data_begin = strstr(data_end, "<data>")) != NULL) {
+ gsize out_len = 0;
+
+ data_begin += 6;
+ data_end = strstr(data_begin, "</data>");
+ /* malformed XML? */
+ if (data_end == NULL) {
+ ret = -EINVAL;
+ goto fail;
+ }
+ *data_end++ = '\0';
+ g_base64_decode_inplace(data_begin, &out_len);
+ ret = dmg_read_mish_block(s, ds, (uint8_t *)data_begin,
+ (uint32_t)out_len);
+ if (ret < 0) {
+ goto fail;
+ }
+ }
+ ret = 0;
+
+fail:
+ g_free(buffer);
+ return ret;
+}
+
static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
BDRVDMGState *s = bs->opaque;
DmgHeaderState ds;
uint64_t rsrc_fork_offset, rsrc_fork_length;
+ uint64_t plist_xml_offset, plist_xml_length;
int64_t offset;
int ret;
@@ -382,12 +437,31 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
ret = -EINVAL;
goto fail;
}
+ /* offset of property list (XMLOffset) */
+ ret = read_uint64(bs, offset + 0xd8, &plist_xml_offset);
+ if (ret < 0) {
+ goto fail;
+ }
+ ret = read_uint64(bs, offset + 0xe0, &plist_xml_length);
+ if (ret < 0) {
+ goto fail;
+ }
+ if (plist_xml_offset >= offset ||
+ plist_xml_length > offset - plist_xml_offset) {
+ ret = -EINVAL;
+ goto fail;
+ }
if (rsrc_fork_length != 0) {
ret = dmg_read_resource_fork(bs, &ds,
rsrc_fork_offset, rsrc_fork_length);
if (ret < 0) {
goto fail;
}
+ } else if (plist_xml_length != 0) {
+ ret = dmg_read_plist_xml(bs, &ds, plist_xml_offset, plist_xml_length);
+ if (ret < 0) {
+ goto fail;
+ }
} else {
ret = -EINVAL;
goto fail;
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PULL 08/16] block/dmg: set virtual size to a non-zero value
2015-01-16 15:36 [Qemu-devel] [PULL 00/16] Block patches Stefan Hajnoczi
` (6 preceding siblings ...)
2015-01-16 15:37 ` [Qemu-devel] [PULL 07/16] block/dmg: process XML plists Stefan Hajnoczi
@ 2015-01-16 15:37 ` Stefan Hajnoczi
2015-01-16 15:37 ` [Qemu-devel] [PULL 09/16] block/dmg: fix sector data offset calculation Stefan Hajnoczi
` (8 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Stefan Hajnoczi @ 2015-01-16 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Peter Wu, Stefan Hajnoczi
From: Peter Wu <peter@lekensteyn.nl>
Right now the virtual size is always reported as zero which makes it
impossible to convert between formats.
After this patch, the number of sectors will be read from the trailer
("koly" block).
To verify the behavior, the output of `dmg2img foo.dmg foo.img` was
compared against `qemu-img convert -f dmg -O raw foo.dmg foo.raw`. The
tests showed that the file contents are exactly the same, except that
QEMU creates a slightly larger file (it matches the total sectors
count).
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1420566495-13284-8-git-send-email-peter@lekensteyn.nl
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/dmg.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/block/dmg.c b/block/dmg.c
index d4a7520..3f8f1cc 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -451,6 +451,14 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
ret = -EINVAL;
goto fail;
}
+ ret = read_uint64(bs, offset + 0x1ec, (uint64_t *)&bs->total_sectors);
+ if (ret < 0) {
+ goto fail;
+ }
+ if (bs->total_sectors < 0) {
+ ret = -EINVAL;
+ goto fail;
+ }
if (rsrc_fork_length != 0) {
ret = dmg_read_resource_fork(bs, &ds,
rsrc_fork_offset, rsrc_fork_length);
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PULL 09/16] block/dmg: fix sector data offset calculation
2015-01-16 15:36 [Qemu-devel] [PULL 00/16] Block patches Stefan Hajnoczi
` (7 preceding siblings ...)
2015-01-16 15:37 ` [Qemu-devel] [PULL 08/16] block/dmg: set virtual size to a non-zero value Stefan Hajnoczi
@ 2015-01-16 15:37 ` Stefan Hajnoczi
2015-01-16 15:37 ` [Qemu-devel] [PULL 10/16] block/dmg: use SectorNumber from BLKX header Stefan Hajnoczi
` (7 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Stefan Hajnoczi @ 2015-01-16 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Peter Wu, Stefan Hajnoczi
From: Peter Wu <peter@lekensteyn.nl>
This patch addresses two issues:
- The data fork offset was not taken into account, resulting in failure
to read an InstallESD.dmg file (5164763151 bytes) which had a
non-zero DataForkOffset field.
- The offset of the previous block ("partition") was unconditionally
added to the current block because older files would start the input
offset of a new block at zero. Newer files (including vlc-2.1.5.dmg,
tuxpaint-0.9.15-macosx.dmg and OS X Yosemite [MAS].dmg) failed in
reads because these files have chunk offsets, relative to the begin
of a data fork.
Now the data offset of the mish is taken into account. While we could
check that the data_offset is within the data fork, let's not do that
here as it would only result in parse failures on invalid files (rather
than gracefully handling such bad files). dmg_read will error out if
the offset is incorrect.
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1420566495-13284-9-git-send-email-peter@lekensteyn.nl
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/dmg.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index 3f8f1cc..65fc7fe 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -186,7 +186,7 @@ static int64_t dmg_find_koly_offset(BlockDriverState *file_bs, Error **errp)
typedef struct DmgHeaderState {
/* used internally by dmg_read_mish_block to remember offsets of blocks
* across calls */
- uint64_t last_in_offset;
+ uint64_t data_fork_offset;
uint64_t last_out_offset;
/* exported for dmg_open */
uint32_t max_compressed_size;
@@ -201,6 +201,8 @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
size_t new_size;
uint32_t chunk_count;
int64_t offset = 0;
+ uint64_t data_offset;
+ uint64_t in_offset = ds->data_fork_offset;
type = buff_read_uint32(buffer, offset);
/* skip data that is not a valid MISH block (invalid magic or too small) */
@@ -209,8 +211,12 @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
return 0;
}
- offset += 4;
- offset += 200;
+ /* location in data fork for (compressed) blob (in bytes) */
+ data_offset = buff_read_uint64(buffer, offset + 0x18);
+ in_offset += data_offset;
+
+ /* move to begin of chunk entries */
+ offset += 204;
chunk_count = (count - 204) / 40;
new_size = sizeof(uint64_t) * (s->n_chunks + chunk_count);
@@ -226,7 +232,6 @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
if (s->types[i] != 0x80000005 && s->types[i] != 1 &&
s->types[i] != 2) {
if (s->types[i] == 0xffffffff && i > 0) {
- ds->last_in_offset = s->offsets[i - 1] + s->lengths[i - 1];
ds->last_out_offset = s->sectors[i - 1] +
s->sectorcounts[i - 1];
}
@@ -253,7 +258,7 @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
}
s->offsets[i] = buff_read_uint64(buffer, offset);
- s->offsets[i] += ds->last_in_offset;
+ s->offsets[i] += in_offset;
offset += 8;
s->lengths[i] = buff_read_uint64(buffer, offset);
@@ -411,7 +416,7 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
s->n_chunks = 0;
s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
/* used by dmg_read_mish_block to keep track of the current I/O position */
- ds.last_in_offset = 0;
+ ds.data_fork_offset = 0;
ds.last_out_offset = 0;
ds.max_compressed_size = 1;
ds.max_sectors_per_chunk = 1;
@@ -423,6 +428,15 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
goto fail;
}
+ /* offset of data fork (DataForkOffset) */
+ ret = read_uint64(bs, offset + 0x18, &ds.data_fork_offset);
+ if (ret < 0) {
+ goto fail;
+ } else if (ds.data_fork_offset > offset) {
+ ret = -EINVAL;
+ goto fail;
+ }
+
/* offset of resource fork (RsrcForkOffset) */
ret = read_uint64(bs, offset + 0x28, &rsrc_fork_offset);
if (ret < 0) {
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PULL 10/16] block/dmg: use SectorNumber from BLKX header
2015-01-16 15:36 [Qemu-devel] [PULL 00/16] Block patches Stefan Hajnoczi
` (8 preceding siblings ...)
2015-01-16 15:37 ` [Qemu-devel] [PULL 09/16] block/dmg: fix sector data offset calculation Stefan Hajnoczi
@ 2015-01-16 15:37 ` Stefan Hajnoczi
2015-01-16 15:37 ` [Qemu-devel] [PULL 11/16] block/dmg: factor out block type check Stefan Hajnoczi
` (6 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Stefan Hajnoczi @ 2015-01-16 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Peter Wu, Stefan Hajnoczi
From: Peter Wu <peter@lekensteyn.nl>
Previously the sector table parsing relied on the previous offset of
the DMG file. Now it uses the sector number from the BLKX header
(see http://newosxbook.com/DMG.html).
The implementation of dmg2img (from vu1tur) does not base the output
sector on the location of the terminator (0xffffffff) either so it
should be safe to drop this dependency on the previous state.
(It makes somehow makes sense, a terminator should halt further
processing of a block and is perhaps used to preallocate some space.)
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1420566495-13284-10-git-send-email-peter@lekensteyn.nl
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/dmg.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index 65fc7fe..0f278c8 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -187,7 +187,6 @@ typedef struct DmgHeaderState {
/* used internally by dmg_read_mish_block to remember offsets of blocks
* across calls */
uint64_t data_fork_offset;
- uint64_t last_out_offset;
/* exported for dmg_open */
uint32_t max_compressed_size;
uint32_t max_sectors_per_chunk;
@@ -203,6 +202,7 @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
int64_t offset = 0;
uint64_t data_offset;
uint64_t in_offset = ds->data_fork_offset;
+ uint64_t out_offset;
type = buff_read_uint32(buffer, offset);
/* skip data that is not a valid MISH block (invalid magic or too small) */
@@ -211,6 +211,9 @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
return 0;
}
+ /* chunk offsets are relative to this sector number */
+ out_offset = buff_read_uint64(buffer, offset + 8);
+
/* location in data fork for (compressed) blob (in bytes) */
data_offset = buff_read_uint64(buffer, offset + 0x18);
in_offset += data_offset;
@@ -231,10 +234,6 @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
offset += 4;
if (s->types[i] != 0x80000005 && s->types[i] != 1 &&
s->types[i] != 2) {
- if (s->types[i] == 0xffffffff && i > 0) {
- ds->last_out_offset = s->sectors[i - 1] +
- s->sectorcounts[i - 1];
- }
chunk_count--;
i--;
offset += 36;
@@ -243,7 +242,7 @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
offset += 4;
s->sectors[i] = buff_read_uint64(buffer, offset);
- s->sectors[i] += ds->last_out_offset;
+ s->sectors[i] += out_offset;
offset += 8;
s->sectorcounts[i] = buff_read_uint64(buffer, offset);
@@ -417,7 +416,6 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
/* used by dmg_read_mish_block to keep track of the current I/O position */
ds.data_fork_offset = 0;
- ds.last_out_offset = 0;
ds.max_compressed_size = 1;
ds.max_sectors_per_chunk = 1;
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PULL 11/16] block/dmg: factor out block type check
2015-01-16 15:36 [Qemu-devel] [PULL 00/16] Block patches Stefan Hajnoczi
` (9 preceding siblings ...)
2015-01-16 15:37 ` [Qemu-devel] [PULL 10/16] block/dmg: use SectorNumber from BLKX header Stefan Hajnoczi
@ 2015-01-16 15:37 ` Stefan Hajnoczi
2015-01-16 15:37 ` [Qemu-devel] [PULL 12/16] block/dmg: support bzip2 block entry types Stefan Hajnoczi
` (5 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Stefan Hajnoczi @ 2015-01-16 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Peter Wu, Stefan Hajnoczi
From: Peter Wu <peter@lekensteyn.nl>
In preparation for adding bzip2 support, split the type check into a
separate function. Make all offsets relative to the begin of a chunk
such that it is easier to recognize the position without having to
add up all offsets. Some comments are added to describe the fields.
There is no functional change.
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1420566495-13284-11-git-send-email-peter@lekensteyn.nl
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/dmg.c | 36 +++++++++++++++++++++++-------------
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index 0f278c8..2bb7078 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -192,6 +192,18 @@ typedef struct DmgHeaderState {
uint32_t max_sectors_per_chunk;
} DmgHeaderState;
+static bool dmg_is_known_block_type(uint32_t entry_type)
+{
+ switch (entry_type) {
+ case 0x00000001: /* uncompressed */
+ case 0x00000002: /* zeroes */
+ case 0x80000005: /* zlib */
+ return true;
+ default:
+ return false;
+ }
+}
+
static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
uint8_t *buffer, uint32_t count)
{
@@ -231,22 +243,19 @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
for (i = s->n_chunks; i < s->n_chunks + chunk_count; i++) {
s->types[i] = buff_read_uint32(buffer, offset);
- offset += 4;
- if (s->types[i] != 0x80000005 && s->types[i] != 1 &&
- s->types[i] != 2) {
+ if (!dmg_is_known_block_type(s->types[i])) {
chunk_count--;
i--;
- offset += 36;
+ offset += 40;
continue;
}
- offset += 4;
- s->sectors[i] = buff_read_uint64(buffer, offset);
+ /* sector number */
+ s->sectors[i] = buff_read_uint64(buffer, offset + 8);
s->sectors[i] += out_offset;
- offset += 8;
- s->sectorcounts[i] = buff_read_uint64(buffer, offset);
- offset += 8;
+ /* sector count */
+ s->sectorcounts[i] = buff_read_uint64(buffer, offset + 0x10);
if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
error_report("sector count %" PRIu64 " for chunk %" PRIu32
@@ -256,12 +265,12 @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
goto fail;
}
- s->offsets[i] = buff_read_uint64(buffer, offset);
+ /* offset in (compressed) data fork */
+ s->offsets[i] = buff_read_uint64(buffer, offset + 0x18);
s->offsets[i] += in_offset;
- offset += 8;
- s->lengths[i] = buff_read_uint64(buffer, offset);
- offset += 8;
+ /* length in (compressed) data fork */
+ s->lengths[i] = buff_read_uint64(buffer, offset + 0x20);
if (s->lengths[i] > DMG_LENGTHS_MAX) {
error_report("length %" PRIu64 " for chunk %" PRIu32
@@ -273,6 +282,7 @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
update_max_chunk_size(s, i, &ds->max_compressed_size,
&ds->max_sectors_per_chunk);
+ offset += 40;
}
s->n_chunks += chunk_count;
return 0;
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PULL 12/16] block/dmg: support bzip2 block entry types
2015-01-16 15:36 [Qemu-devel] [PULL 00/16] Block patches Stefan Hajnoczi
` (10 preceding siblings ...)
2015-01-16 15:37 ` [Qemu-devel] [PULL 11/16] block/dmg: factor out block type check Stefan Hajnoczi
@ 2015-01-16 15:37 ` Stefan Hajnoczi
2015-01-16 15:37 ` [Qemu-devel] [PULL 13/16] block/dmg: improve zeroes handling Stefan Hajnoczi
` (4 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Stefan Hajnoczi @ 2015-01-16 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Peter Wu, Stefan Hajnoczi
From: Peter Wu <peter@lekensteyn.nl>
This patch adds support for bzip2-compressed block entries as introduced
with OS X 10.4 (source: https://en.wikipedia.org/wiki/Apple_Disk_Image).
It was tested against a 5.2G "OS X Yosemite" installation image which
stores the BLXX block in the XML property list (instead of resource
forks) and has over 5k chunks.
New configure entries are added (--enable-bzip2 / --disable-bzip2) to
control inclusion of bzip2 functionality (which requires linking against
libbz2). The help message suggests that this option is needed for DMG
files, but the tests are generic enough that other parts of QEMU can use
bzip2 if needed.
The identifiers are based on http://newosxbook.com/DMG.html.
The decompression routines are based on the zlib case, but as there is
no way to reset the decompression state (unlike zlib), memory is
allocated and deallocated for every decompression. This should not be
problematic as the decompression takes most of the time and as blocks
are typically about/over 1 MiB in size, only one allocation is done
every 2000 sectors.
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: John Snow <jsnow@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 1420566495-13284-12-git-send-email-peter@lekensteyn.nl
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/Makefile.objs | 1 +
block/dmg.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
configure | 31 +++++++++++++++++++++++++++++++
3 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 010afad..db2933e 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -37,5 +37,6 @@ gluster.o-libs := $(GLUSTERFS_LIBS)
ssh.o-cflags := $(LIBSSH2_CFLAGS)
ssh.o-libs := $(LIBSSH2_LIBS)
archipelago.o-libs := $(ARCHIPELAGO_LIBS)
+dmg.o-libs := $(BZIP2_LIBS)
qcow.o-libs := -lz
linux-aio.o-libs := -laio
diff --git a/block/dmg.c b/block/dmg.c
index 2bb7078..196c746 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -26,6 +26,9 @@
#include "qemu/bswap.h"
#include "qemu/module.h"
#include <zlib.h>
+#ifdef CONFIG_BZIP2
+#include <bzlib.h>
+#endif
#include <glib.h>
enum {
@@ -56,6 +59,9 @@ typedef struct BDRVDMGState {
uint8_t *compressed_chunk;
uint8_t *uncompressed_chunk;
z_stream zstream;
+#ifdef CONFIG_BZIP2
+ bz_stream bzstream;
+#endif
} BDRVDMGState;
static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
@@ -123,6 +129,7 @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk,
switch (s->types[chunk]) {
case 0x80000005: /* zlib compressed */
+ case 0x80000006: /* bzip2 compressed */
compressed_size = s->lengths[chunk];
uncompressed_sectors = s->sectorcounts[chunk];
break;
@@ -198,6 +205,9 @@ static bool dmg_is_known_block_type(uint32_t entry_type)
case 0x00000001: /* uncompressed */
case 0x00000002: /* zeroes */
case 0x80000005: /* zlib */
+#ifdef CONFIG_BZIP2
+ case 0x80000006: /* bzip2 */
+#endif
return true;
default:
return false;
@@ -563,13 +573,16 @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
if (!is_sector_in_chunk(s, s->current_chunk, sector_num)) {
int ret;
uint32_t chunk = search_chunk(s, sector_num);
+#ifdef CONFIG_BZIP2
+ uint64_t total_out;
+#endif
if (chunk >= s->n_chunks) {
return -1;
}
s->current_chunk = s->n_chunks;
- switch (s->types[chunk]) {
+ switch (s->types[chunk]) { /* block entry type */
case 0x80000005: { /* zlib compressed */
/* we need to buffer, because only the chunk as whole can be
* inflated. */
@@ -593,6 +606,34 @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
return -1;
}
break; }
+#ifdef CONFIG_BZIP2
+ case 0x80000006: /* bzip2 compressed */
+ /* we need to buffer, because only the chunk as whole can be
+ * inflated. */
+ ret = bdrv_pread(bs->file, s->offsets[chunk],
+ s->compressed_chunk, s->lengths[chunk]);
+ if (ret != s->lengths[chunk]) {
+ return -1;
+ }
+
+ ret = BZ2_bzDecompressInit(&s->bzstream, 0, 0);
+ if (ret != BZ_OK) {
+ return -1;
+ }
+ s->bzstream.next_in = (char *)s->compressed_chunk;
+ s->bzstream.avail_in = (unsigned int) s->lengths[chunk];
+ s->bzstream.next_out = (char *)s->uncompressed_chunk;
+ s->bzstream.avail_out = (unsigned int) 512 * s->sectorcounts[chunk];
+ ret = BZ2_bzDecompress(&s->bzstream);
+ total_out = ((uint64_t)s->bzstream.total_out_hi32 << 32) +
+ s->bzstream.total_out_lo32;
+ BZ2_bzDecompressEnd(&s->bzstream);
+ if (ret != BZ_STREAM_END ||
+ total_out != 512 * s->sectorcounts[chunk]) {
+ return -1;
+ }
+ break;
+#endif /* CONFIG_BZIP2 */
case 1: /* copy */
ret = bdrv_pread(bs->file, s->offsets[chunk],
s->uncompressed_chunk, s->lengths[chunk]);
diff --git a/configure b/configure
index 7539645..6ea199f 100755
--- a/configure
+++ b/configure
@@ -313,6 +313,7 @@ glx=""
zlib="yes"
lzo=""
snappy=""
+bzip2=""
guest_agent=""
guest_agent_with_vss="no"
vss_win32_sdk=""
@@ -1060,6 +1061,10 @@ for opt do
;;
--enable-snappy) snappy="yes"
;;
+ --disable-bzip2) bzip2="no"
+ ;;
+ --enable-bzip2) bzip2="yes"
+ ;;
--enable-guest-agent) guest_agent="yes"
;;
--disable-guest-agent) guest_agent="no"
@@ -1374,6 +1379,8 @@ Advanced options (experts only):
--enable-usb-redir enable usb network redirection support
--enable-lzo enable the support of lzo compression library
--enable-snappy enable the support of snappy compression library
+ --enable-bzip2 enable the support of bzip2 compression library (for
+ reading bzip2-compressed dmg images)
--disable-guest-agent disable building of the QEMU Guest Agent
--enable-guest-agent enable building of the QEMU Guest Agent
--with-vss-sdk=SDK-path enable Windows VSS support in QEMU Guest Agent
@@ -1820,6 +1827,24 @@ EOF
fi
##########################################
+# bzip2 check
+
+if test "$bzip2" != "no" ; then
+ cat > $TMPC << EOF
+#include <bzlib.h>
+int main(void) { BZ2_bzlibVersion(); return 0; }
+EOF
+ if compile_prog "" "-lbz2" ; then
+ bzip2="yes"
+ else
+ if test "$bzip2" = "yes"; then
+ feature_not_found "libbzip2" "Install libbzip2 devel"
+ fi
+ bzip2="no"
+ fi
+fi
+
+##########################################
# libseccomp check
if test "$seccomp" != "no" ; then
@@ -4340,6 +4365,7 @@ echo "vhdx $vhdx"
echo "Quorum $quorum"
echo "lzo support $lzo"
echo "snappy support $snappy"
+echo "bzip2 support $bzip2"
echo "NUMA host support $numa"
if test "$sdl_too_old" = "yes"; then
@@ -4695,6 +4721,11 @@ if test "$snappy" = "yes" ; then
echo "CONFIG_SNAPPY=y" >> $config_host_mak
fi
+if test "$bzip2" = "yes" ; then
+ echo "CONFIG_BZIP2=y" >> $config_host_mak
+ echo "BZIP2_LIBS=-lbz2" >> $config_host_mak
+fi
+
if test "$libiscsi" = "yes" ; then
echo "CONFIG_LIBISCSI=m" >> $config_host_mak
echo "LIBISCSI_CFLAGS=$libiscsi_cflags" >> $config_host_mak
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PULL 13/16] block/dmg: improve zeroes handling
2015-01-16 15:36 [Qemu-devel] [PULL 00/16] Block patches Stefan Hajnoczi
` (11 preceding siblings ...)
2015-01-16 15:37 ` [Qemu-devel] [PULL 12/16] block/dmg: support bzip2 block entry types Stefan Hajnoczi
@ 2015-01-16 15:37 ` Stefan Hajnoczi
2015-01-16 15:37 ` [Qemu-devel] [PULL 14/16] qed: check for header size overflow Stefan Hajnoczi
` (3 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Stefan Hajnoczi @ 2015-01-16 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Peter Wu, Stefan Hajnoczi
From: Peter Wu <peter@lekensteyn.nl>
Disk images may contain large all-zeroes gaps (1.66k sectors or 812 MiB
is seen in the real world). These blocks (type 2) do not need to be
extracted into a temporary buffer, there is no need to allocate memory
for these blocks nor to check its length.
(For the test image, the maximum uncompressed size is 1054371 bytes,
probably for a bzip2-compressed block.)
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1420566495-13284-13-git-send-email-peter@lekensteyn.nl
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/dmg.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index 196c746..66a4b3f 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -137,7 +137,9 @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk,
uncompressed_sectors = (s->lengths[chunk] + 511) / 512;
break;
case 2: /* zero */
- uncompressed_sectors = s->sectorcounts[chunk];
+ /* as the all-zeroes block may be large, it is treated specially: the
+ * sector is not copied from a large buffer, a simple memset is used
+ * instead. Therefore uncompressed_sectors does not need to be set. */
break;
}
@@ -267,7 +269,9 @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
/* sector count */
s->sectorcounts[i] = buff_read_uint64(buffer, offset + 0x10);
- if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
+ /* all-zeroes sector (type 2) does not need to be "uncompressed" and can
+ * therefore be unbounded. */
+ if (s->types[i] != 2 && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
error_report("sector count %" PRIu64 " for chunk %" PRIu32
" is larger than max (%u)",
s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
@@ -642,7 +646,8 @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
}
break;
case 2: /* zero */
- memset(s->uncompressed_chunk, 0, 512 * s->sectorcounts[chunk]);
+ /* see dmg_read, it is treated specially. No buffer needs to be
+ * pre-filled, the zeroes can be set directly. */
break;
}
s->current_chunk = chunk;
@@ -661,6 +666,13 @@ static int dmg_read(BlockDriverState *bs, int64_t sector_num,
if (dmg_read_chunk(bs, sector_num + i) != 0) {
return -1;
}
+ /* Special case: current chunk is all zeroes. Do not perform a memcpy as
+ * s->uncompressed_chunk may be too small to cover the large all-zeroes
+ * section. dmg_read_chunk is called to find s->current_chunk */
+ if (s->types[s->current_chunk] == 2) { /* all zeroes block entry */
+ memset(buf + i * 512, 0, 512);
+ continue;
+ }
sector_offset_in_chunk = sector_num + i - s->sectors[s->current_chunk];
memcpy(buf + i * 512,
s->uncompressed_chunk + sector_offset_in_chunk * 512, 512);
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PULL 14/16] qed: check for header size overflow
2015-01-16 15:36 [Qemu-devel] [PULL 00/16] Block patches Stefan Hajnoczi
` (12 preceding siblings ...)
2015-01-16 15:37 ` [Qemu-devel] [PULL 13/16] block/dmg: improve zeroes handling Stefan Hajnoczi
@ 2015-01-16 15:37 ` Stefan Hajnoczi
2015-01-16 15:37 ` [Qemu-devel] [PULL 15/16] qemu-iotests: add 116 invalid QED input file tests Stefan Hajnoczi
` (2 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Stefan Hajnoczi @ 2015-01-16 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Stefan Hajnoczi
Header size is denoted in clusters. The maximum cluster size is 64 MB
but there is no limit on header size. Check for uint32_t overflow in
case the header size field has a whacky value.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1421065893-18875-2-git-send-email-stefanha@redhat.com
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
block/qed.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/block/qed.c b/block/qed.c
index 80f18d8..892b13c 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -440,6 +440,11 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
s->l2_mask = s->table_nelems - 1;
s->l1_shift = s->l2_shift + ffs(s->table_nelems) - 1;
+ /* Header size calculation must not overflow uint32_t */
+ if (s->header.header_size > UINT32_MAX / s->header.cluster_size) {
+ return -EINVAL;
+ }
+
if ((s->header.features & QED_F_BACKING_FILE)) {
if ((uint64_t)s->header.backing_filename_offset +
s->header.backing_filename_size >
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PULL 15/16] qemu-iotests: add 116 invalid QED input file tests
2015-01-16 15:36 [Qemu-devel] [PULL 00/16] Block patches Stefan Hajnoczi
` (13 preceding siblings ...)
2015-01-16 15:37 ` [Qemu-devel] [PULL 14/16] qed: check for header size overflow Stefan Hajnoczi
@ 2015-01-16 15:37 ` Stefan Hajnoczi
2015-01-16 15:37 ` [Qemu-devel] [PULL 16/16] qemu-iotests: Fix supported_oses check Stefan Hajnoczi
2015-01-16 16:46 ` [Qemu-devel] [PULL 00/16] Block patches Peter Maydell
16 siblings, 0 replies; 29+ messages in thread
From: Stefan Hajnoczi @ 2015-01-16 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Stefan Hajnoczi
These tests exercise error code paths in the QED image format. The
tests are very simple, they just prove that the error path exits
cleanly.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1421065893-18875-3-git-send-email-stefanha@redhat.com
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/116 | 96 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/116.out | 37 ++++++++++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 134 insertions(+)
create mode 100755 tests/qemu-iotests/116
create mode 100644 tests/qemu-iotests/116.out
diff --git a/tests/qemu-iotests/116 b/tests/qemu-iotests/116
new file mode 100755
index 0000000..713ed48
--- /dev/null
+++ b/tests/qemu-iotests/116
@@ -0,0 +1,96 @@
+#!/bin/bash
+#
+# Test error code paths for invalid QED images
+#
+# The aim of this test is to exercise the error paths in qed_open() to ensure
+# there are no crashes with invalid input files.
+#
+# Copyright (C) 2015 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=stefanha@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 qed
+_supported_proto generic
+_supported_os Linux
+
+
+size=128M
+
+echo
+echo "== truncated header cluster =="
+_make_test_img $size
+truncate -s 512 "$TEST_IMG"
+$QEMU_IO -f "$IMGFMT" -c "read 0 $size" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== invalid header magic =="
+_make_test_img $size
+poke_file "$TEST_IMG" "0" "QEDX"
+$QEMU_IO -f "$IMGFMT" -c "read 0 $size" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== invalid cluster size =="
+_make_test_img $size
+poke_file "$TEST_IMG" "4" "\xff\xff\xff\xff"
+$QEMU_IO -f "$IMGFMT" -c "read 0 $size" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== invalid table size =="
+_make_test_img $size
+poke_file "$TEST_IMG" "8" "\xff\xff\xff\xff"
+$QEMU_IO -f "$IMGFMT" -c "read 0 $size" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== invalid header size =="
+_make_test_img $size
+poke_file "$TEST_IMG" "12" "\xff\xff\xff\xff"
+$QEMU_IO -f "$IMGFMT" -c "read 0 $size" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== invalid L1 table offset =="
+_make_test_img $size
+poke_file "$TEST_IMG" "40" "\xff\xff\xff\xff\xff\xff\xff\xff"
+$QEMU_IO -f "$IMGFMT" -c "read 0 $size" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== invalid image size =="
+_make_test_img $size
+poke_file "$TEST_IMG" "48" "\xff\xff\xff\xff\xff\xff\xff\xff"
+$QEMU_IO -f "$IMGFMT" -c "read 0 $size" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/116.out b/tests/qemu-iotests/116.out
new file mode 100644
index 0000000..b679cee
--- /dev/null
+++ b/tests/qemu-iotests/116.out
@@ -0,0 +1,37 @@
+QA output created by 116
+
+== truncated header cluster ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+qemu-io: can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
+no file open, try 'help open'
+
+== invalid header magic ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+qemu-io: can't open device TEST_DIR/t.qed: Image not in QED format
+no file open, try 'help open'
+
+== invalid cluster size ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+qemu-io: can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
+no file open, try 'help open'
+
+== invalid table size ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+qemu-io: can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
+no file open, try 'help open'
+
+== invalid header size ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+qemu-io: can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
+no file open, try 'help open'
+
+== invalid L1 table offset ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+qemu-io: can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
+no file open, try 'help open'
+
+== invalid image size ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+qemu-io: can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument
+no file open, try 'help open'
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index f8bf354..4b2b93b 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -116,3 +116,4 @@
111 rw auto quick
113 rw auto quick
114 rw auto quick
+116 rw auto quick
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PULL 16/16] qemu-iotests: Fix supported_oses check
2015-01-16 15:36 [Qemu-devel] [PULL 00/16] Block patches Stefan Hajnoczi
` (14 preceding siblings ...)
2015-01-16 15:37 ` [Qemu-devel] [PULL 15/16] qemu-iotests: add 116 invalid QED input file tests Stefan Hajnoczi
@ 2015-01-16 15:37 ` Stefan Hajnoczi
2015-01-16 16:46 ` [Qemu-devel] [PULL 00/16] Block patches Peter Maydell
16 siblings, 0 replies; 29+ messages in thread
From: Stefan Hajnoczi @ 2015-01-16 15:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Fam Zheng, Stefan Hajnoczi
From: Fam Zheng <famz@redhat.com>
There is a bug in the recently added sys.platform test, and we no longer
run python tests, because "linux2" is the value to compare here. So do a
prefix match. According to python doc [1], the way to use sys.platform
is "unless you want to test for a specific system version, it is
therefore recommended to use the following idiom":
if sys.platform.startswith('freebsd'):
# FreeBSD-specific code here...
elif sys.platform.startswith('linux'):
# Linux-specific code here...
[1]: https://docs.python.org/2.7/library/sys.html#sys.platform
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/qemu-iotests/iotests.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 87002e0..241b5ee 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -288,7 +288,7 @@ def main(supported_fmts=[], supported_oses=['linux']):
if supported_fmts and (imgfmt not in supported_fmts):
notrun('not suitable for this image format: %s' % imgfmt)
- if sys.platform not in supported_oses:
+ if True not in [sys.platform.startswith(x) for x in supported_oses]:
notrun('not suitable for this OS: %s' % sys.platform)
# We need to filter out the time taken from the output so that qemu-iotest
--
2.1.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PULL 00/16] Block patches
2015-01-16 15:36 [Qemu-devel] [PULL 00/16] Block patches Stefan Hajnoczi
` (15 preceding siblings ...)
2015-01-16 15:37 ` [Qemu-devel] [PULL 16/16] qemu-iotests: Fix supported_oses check Stefan Hajnoczi
@ 2015-01-16 16:46 ` Peter Maydell
2015-01-17 10:41 ` Peter Wu
16 siblings, 1 reply; 29+ messages in thread
From: Peter Maydell @ 2015-01-16 16:46 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: QEMU Developers
On 16 January 2015 at 15:36, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> The following changes since commit df58887b20fab8fe8a6dcca4db30cd4e4077d53a:
>
> Merge remote-tracking branch 'remotes/mjt/tags/pull-trivial-patches-2015-01-15' into staging (2015-01-15 10:08:46 +0000)
>
> are available in the git repository at:
>
> git://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to d6e2630f17fa40342af80d20c04f279fa23ea189:
>
> qemu-iotests: Fix supported_oses check (2015-01-16 15:36:34 +0000)
Hi. I'm afraid this doesn't build on all platforms, due to
dependencies on newer glib versions than our minimum
requirement.
OSX:
CC tests/test-write-threshold.o
/Users/pm215/src/qemu/tests/test-write-threshold.c:21:5: warning:
implicit declaration of function
'g_assert_false' is invalid in C99 [-Wimplicit-function-declaration]
g_assert_false(bdrv_write_threshold_is_set(&bs));
^
1 warning generated.
LINK tests/test-write-threshold
Undefined symbols for architecture x86_64:
"_g_assert_false", referenced from:
_test_threshold_not_set_on_init in test-write-threshold.o
ld: symbol(s) not found for architecture x86_64
CentOS5:
../block/dmg.o: In function `dmg_read_plist_xml':
/home/petmay01/linaro/qemu-for-merges/block/dmg.c:414: undefined
reference to `g_base64_decode_inplace'
-- PMM
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PULL 00/16] Block patches
2015-01-16 16:46 ` [Qemu-devel] [PULL 00/16] Block patches Peter Maydell
@ 2015-01-17 10:41 ` Peter Wu
2015-01-20 10:26 ` Stefan Hajnoczi
0 siblings, 1 reply; 29+ messages in thread
From: Peter Wu @ 2015-01-17 10:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Stefan Hajnoczi
On Friday 16 January 2015 16:46:39 Peter Maydell wrote:
> CentOS5:
>
> ../block/dmg.o: In function `dmg_read_plist_xml':
> /home/petmay01/linaro/qemu-for-merges/block/dmg.c:414: undefined
> reference to `g_base64_decode_inplace'
Should have paid more attention to the API docs. Can you try the
following patch? It still passes 4 dmg tests for me
(https://lekensteyn.nl/files/dmg-tests/).
--
Kind regards,
Peter
https://lekensteyn.nl
--
>From 462454e820d2fa5f8eefe7b039d6ea32e4a88d41 Mon Sep 17 00:00:00 2001
From: Peter Wu <peter@lekensteyn.nl>
Date: Sat, 17 Jan 2015 11:34:32 +0100
Subject: [PATCH] block/dmg: fix compatibility with glib 2.12
For compatibility with glib 2.12, use g_base64_decode (which
additionally requires an extra buffer allocation) instead of
g_base64_decode_inplace (which is only available since glib 2.20).
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
block/dmg.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index 4e24076..0430f55 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -403,6 +403,7 @@ static int dmg_read_plist_xml(BlockDriverState *bs, DmgHeaderState *ds,
* and line feeds. */
data_end = (char *)buffer;
while ((data_begin = strstr(data_end, "<data>")) != NULL) {
+ guchar *mish;
gsize out_len = 0;
data_begin += 6;
@@ -413,9 +414,9 @@ static int dmg_read_plist_xml(BlockDriverState *bs, DmgHeaderState *ds,
goto fail;
}
*data_end++ = '\0';
- g_base64_decode_inplace(data_begin, &out_len);
- ret = dmg_read_mish_block(s, ds, (uint8_t *)data_begin,
- (uint32_t)out_len);
+ mish = g_base64_decode(data_begin, &out_len);
+ ret = dmg_read_mish_block(s, ds, mish, (uint32_t)out_len);
+ g_free(mish);
if (ret < 0) {
goto fail;
}
--
2.2.2
^ permalink raw reply related [flat|nested] 29+ messages in thread