From 82dc8b411040fa8a7418a012ff39b8d06f68e639 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 11 Jan 2016 19:07:50 +0100 Subject: block: Fix .bdrv_open flags bdrv_common_open() modified bs->open_flags after inferring the set of options to pass to the driver's .bdrv_open callback. This means that the cache options were correctly set in bs->open_flags (and therefore correctly displayed in 'info block'), but the image would actually be opened with the default cache mode instead. This patch removes the flags parameter to bdrv_common_open() (except for BDRV_O_NO_BACKING it's the same as bs->open_flags anyway, and having two names for the same thing is confusing), and moves the assignment of open_flags down to immediately before calling into the block drivers. In all other places, bs->open_flags is now used consistently. Signed-off-by: Kevin Wolf Tested-by: Christian Borntraeger Reviewed-by: Denis V. Lunev Reviewed-by: Stefan Hajnoczi --- block.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/block.c b/block.c index 54c37f9629..1a137167d9 100644 --- a/block.c +++ b/block.c @@ -905,7 +905,7 @@ static QemuOptsList bdrv_runtime_opts = { * Removes all processed options from *options. */ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file, - QDict *options, int flags, Error **errp) + QDict *options, Error **errp) { int ret, open_flags; const char *filename; @@ -943,7 +943,8 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file, goto fail_opts; } - trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name); + trace_bdrv_open_common(bs, filename ?: "", bs->open_flags, + drv->format_name); node_name = qemu_opt_get(opts, "node-name"); bdrv_assign_node_name(bs, node_name, &local_err); @@ -955,8 +956,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file, bs->request_alignment = 512; bs->zero_beyond_eof = true; - open_flags = bdrv_open_flags(bs, flags); - bs->read_only = !(open_flags & BDRV_O_RDWR); + bs->read_only = !(bs->open_flags & BDRV_O_RDWR); if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) { error_setg(errp, @@ -969,7 +969,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file, } assert(bs->copy_on_read == 0); /* bdrv_new() and bdrv_close() make it so */ - if (flags & BDRV_O_COPY_ON_READ) { + if (bs->open_flags & BDRV_O_COPY_ON_READ) { if (!bs->read_only) { bdrv_enable_copy_on_read(bs); } else { @@ -994,6 +994,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file, bdrv_set_enable_write_cache(bs, bs->open_flags & BDRV_O_CACHE_WB); /* Open the image, either directly or using a protocol */ + open_flags = bdrv_open_flags(bs, bs->open_flags); if (drv->bdrv_file_open) { assert(file == NULL); assert(!drv->bdrv_needs_filename || filename != NULL); @@ -1656,7 +1657,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, assert(!(flags & BDRV_O_PROTOCOL) || !file); /* Open the image */ - ret = bdrv_open_common(bs, file, options, flags, &local_err); + ret = bdrv_open_common(bs, file, options, &local_err); if (ret < 0) { goto fail; } -- cgit v1.2.3 From 972b543c6b63579aee590b738d21af09f01569f7 Mon Sep 17 00:00:00 2001 From: Christian Borntraeger Date: Mon, 14 Dec 2015 16:41:19 +0100 Subject: block/raw-posix: avoid bogus fixup for cylinders on DASD disks large volume DASD that have > 64k cylinders do claim to have 0xFFFE cylinders as special value in the old 16 bit field. We want to pass this "token" along to the guest, instead of calculating the real number. Otherwise qemu might fail with "cyls must be between 1 and 65535" Cc: qemu-stable@nongnu.org Acked-by: Cornelia Huck Signed-off-by: Christian Borntraeger Reviewed-by: Markus Armbruster Signed-off-by: Kevin Wolf --- block/raw-posix.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index 076d0708a7..816bdf7bea 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -779,7 +779,6 @@ static int hdev_probe_geometry(BlockDriverState *bs, HDGeometry *geo) { BDRVRawState *s = bs->opaque; struct hd_geometry ioctl_geo = {0}; - uint32_t blksize; /* If DASD, get its geometry */ if (check_for_dasd(s->fd) < 0) { @@ -799,12 +798,6 @@ static int hdev_probe_geometry(BlockDriverState *bs, HDGeometry *geo) } geo->heads = ioctl_geo.heads; geo->sectors = ioctl_geo.sectors; - if (!probe_physical_blocksize(s->fd, &blksize)) { - /* overwrite cyls: HDIO_GETGEO result is incorrect for big drives */ - geo->cylinders = bdrv_nb_sectors(bs) / (blksize / BDRV_SECTOR_SIZE) - / (geo->heads * geo->sectors); - return 0; - } geo->cylinders = ioctl_geo.cylinders; return 0; -- cgit v1.2.3 From 25ad8e6e95770d4f01844dc2542d861e73b174b2 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 13 Jan 2016 16:37:41 +0800 Subject: qemu-img: Speed up comparing empty/zero images Two empty raw files are always compared by actually reading data even if there is no data, because BDRV_BLOCK_ZERO is considered "allocated" in bdrv_is_allocated_above(). That is inefficient. Use bdrv_get_block_status_above() for more information, and skip the consecutive zero sectors. This brings a huge speed up in comparing sparse/empty raw images: $ qemu-img create a 1G $ time ~/build/master/bin/qemu-img compare a a Images are identical. real 0m6.583s user 0m0.191s sys 0m6.367s $ time qemu-img compare a a Images are identical. real 0m0.033s user 0m0.003s sys 0m0.031s Signed-off-by: Fam Zheng Reviewed-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- qemu-img.c | 45 ++++++++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index a5949e6b05..67739439c5 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1071,28 +1071,50 @@ static int img_compare(int argc, char **argv) } for (;;) { + int64_t status1, status2; nb_sectors = sectors_to_process(total_sectors, sector_num); if (nb_sectors <= 0) { break; } - allocated1 = bdrv_is_allocated_above(bs1, NULL, sector_num, nb_sectors, - &pnum1); - if (allocated1 < 0) { + status1 = bdrv_get_block_status_above(bs1, NULL, sector_num, + total_sectors1 - sector_num, + &pnum1); + if (status1 < 0) { ret = 3; error_report("Sector allocation test failed for %s", filename1); goto out; } + allocated1 = status1 & BDRV_BLOCK_ALLOCATED; - allocated2 = bdrv_is_allocated_above(bs2, NULL, sector_num, nb_sectors, - &pnum2); - if (allocated2 < 0) { + status2 = bdrv_get_block_status_above(bs2, NULL, sector_num, + total_sectors2 - sector_num, + &pnum2); + if (status2 < 0) { ret = 3; error_report("Sector allocation test failed for %s", filename2); goto out; } - nb_sectors = MIN(pnum1, pnum2); + allocated2 = status2 & BDRV_BLOCK_ALLOCATED; + if (pnum1) { + nb_sectors = MIN(nb_sectors, pnum1); + } + if (pnum2) { + nb_sectors = MIN(nb_sectors, pnum2); + } - if (allocated1 == allocated2) { + if (strict) { + if ((status1 & ~BDRV_BLOCK_OFFSET_MASK) != + (status2 & ~BDRV_BLOCK_OFFSET_MASK)) { + ret = 1; + qprintf(quiet, "Strict mode: Offset %" PRId64 + " block status mismatch!\n", + sectors_to_bytes(sector_num)); + goto out; + } + } + if ((status1 & BDRV_BLOCK_ZERO) && (status2 & BDRV_BLOCK_ZERO)) { + nb_sectors = MIN(pnum1, pnum2); + } else if (allocated1 == allocated2) { if (allocated1) { ret = blk_read(blk1, sector_num, buf1, nb_sectors); if (ret < 0) { @@ -1120,13 +1142,6 @@ static int img_compare(int argc, char **argv) } } } else { - if (strict) { - ret = 1; - qprintf(quiet, "Strict mode: Offset %" PRId64 - " allocation mismatch!\n", - sectors_to_bytes(sector_num)); - goto out; - } if (allocated1) { ret = check_empty_sectors(blk1, sector_num, nb_sectors, -- cgit v1.2.3 From a174da3613f548d58433f64cf3c99dd302c6154a Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 10 Dec 2015 20:27:17 -0700 Subject: qemu-iotests: Reduce racy output in 028 On my machine, './check -qcow2 028' was failing about 80% of the time, due to a race in how many times the repeated attempts to run 'info block-jobs' could occur before the job was done, showing up as a failure of fewer '(qemu) ' prompts than in the expected output. Silence the output during the repetitions, then add a final clean command to keep the expected output useful; once patched, I was finally able to run the test 20 times in a row with no failures. Signed-off-by: Eric Blake Reviewed-by: John Snow Signed-off-by: Kevin Wolf --- tests/qemu-iotests/028 | 6 ++++-- tests/qemu-iotests/028.out | 3 --- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/qemu-iotests/028 b/tests/qemu-iotests/028 index 009510d0d1..4909b9bc88 100755 --- a/tests/qemu-iotests/028 +++ b/tests/qemu-iotests/028 @@ -114,10 +114,12 @@ h=$QEMU_HANDLE QEMU_COMM_TIMEOUT=1 # Silence output since it contains the disk image path and QEMU's readline -# character echoing makes it very hard to filter the output +# character echoing makes it very hard to filter the output. Plus, there +# is no telling how many times the command will repeat before succeeding. _send_qemu_cmd $h "drive_backup disk ${TEST_IMG}.copy" "(qemu)" >/dev/null _send_qemu_cmd $h "" "Formatting" | _filter_img_create -qemu_cmd_repeat=20 _send_qemu_cmd $h "info block-jobs" "No active jobs" +qemu_cmd_repeat=20 _send_qemu_cmd $h "info block-jobs" "No active jobs" >/dev/null +_send_qemu_cmd $h "info block-jobs" "No active jobs" _send_qemu_cmd $h 'quit' "" # Base image sectors diff --git a/tests/qemu-iotests/028.out b/tests/qemu-iotests/028.out index 279029d8d6..acd2870bae 100644 --- a/tests/qemu-iotests/028.out +++ b/tests/qemu-iotests/028.out @@ -469,10 +469,7 @@ No errors were found on the image. block-backup Formatting 'TEST_DIR/t.IMGFMT.copy', fmt=IMGFMT size=4294968832 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT -(qemu) (qemu) iininfinfoinfo info binfo blinfo bloinfo blocinfo blockinfo block-info block-jinfo block-joinfo block-jobinfo block-jobs -Type backup, device disk: Completed 0 of 4294968832 bytes, speed limit 0 bytes/s -iininfinfoinfo info binfo blinfo bloinfo blocinfo blockinfo block-info block-jinfo block-joinfo block-jobinfo block-jobs No active jobs === IO: pattern 195 read 512/512 bytes at offset 3221194240 -- cgit v1.2.3 From 80c71a241ae3cd3b89527865ba730b2fa1f9e46f Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 18 Jan 2016 18:01:42 +0000 Subject: block: Clean up includes Clean up includes so that osdep.h is included first and headers which it implies are not included manually. This commit was created with scripts/clean-includes. Signed-off-by: Peter Maydell Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/accounting.c | 1 + block/archipelago.c | 2 +- block/backup.c | 4 +--- block/blkdebug.c | 1 + block/blkverify.c | 2 +- block/block-backend.c | 1 + block/bochs.c | 1 + block/cloop.c | 1 + block/commit.c | 1 + block/curl.c | 1 + block/dmg.c | 1 + block/gluster.c | 1 + block/io.c | 1 + block/iscsi.c | 2 +- block/linux-aio.c | 1 + block/mirror.c | 1 + block/nbd-client.c | 1 + block/nbd.c | 3 +-- block/nfs.c | 2 +- block/null.c | 1 + block/parallels.c | 1 + block/qapi.c | 1 + block/qcow.c | 1 + block/qcow2-cache.c | 3 +-- block/qcow2-cluster.c | 1 + block/qcow2-refcount.c | 1 + block/qcow2-snapshot.c | 1 + block/qcow2.c | 1 + block/qed-check.c | 1 + block/qed-cluster.c | 1 + block/qed-gencb.c | 1 + block/qed-l2-cache.c | 1 + block/qed-table.c | 1 + block/qed.c | 1 + block/quorum.c | 1 + block/raw-posix.c | 3 +-- block/raw-win32.c | 1 + block/raw_bsd.c | 1 + block/rbd.c | 2 +- block/sheepdog.c | 1 + block/snapshot.c | 1 + block/ssh.c | 4 +--- block/stream.c | 1 + block/throttle-groups.c | 1 + block/vdi.c | 1 + block/vhdx-endian.c | 1 + block/vhdx-log.c | 1 + block/vhdx.c | 1 + block/vmdk.c | 1 + block/vpc.c | 1 + block/vvfat.c | 2 +- block/win32-aio.c | 1 + block/write-threshold.c | 1 + hw/block/block.c | 1 + hw/block/cdrom.c | 1 + hw/block/dataplane/virtio-blk.c | 1 + hw/block/ecc.c | 1 + hw/block/fdc.c | 1 + hw/block/hd-geometry.c | 1 + hw/block/m25p80.c | 1 + hw/block/nvme.c | 1 + hw/block/onenand.c | 1 + hw/block/pflash_cfi01.c | 1 + hw/block/pflash_cfi02.c | 1 + hw/block/tc58128.c | 1 + hw/block/virtio-blk.c | 1 + hw/block/xen_disk.c | 12 +----------- qemu-img.c | 2 +- qemu-io-cmds.c | 1 + qemu-io.c | 5 +---- 70 files changed, 70 insertions(+), 34 deletions(-) diff --git a/block/accounting.c b/block/accounting.c index 185025ec1e..3f457c4e73 100644 --- a/block/accounting.c +++ b/block/accounting.c @@ -23,6 +23,7 @@ * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "block/accounting.h" #include "block/block_int.h" #include "qemu/timer.h" diff --git a/block/archipelago.c b/block/archipelago.c index 855655c6bd..0507589063 100644 --- a/block/archipelago.c +++ b/block/archipelago.c @@ -50,6 +50,7 @@ * */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "block/block_int.h" #include "qemu/error-report.h" @@ -59,7 +60,6 @@ #include "qapi/qmp/qjson.h" #include "qemu/atomic.h" -#include #include #include diff --git a/block/backup.c b/block/backup.c index 705bb77661..00cafdbe2b 100644 --- a/block/backup.c +++ b/block/backup.c @@ -11,9 +11,7 @@ * */ -#include -#include -#include +#include "qemu/osdep.h" #include "trace.h" #include "block/block.h" diff --git a/block/blkdebug.c b/block/blkdebug.c index 86b143dc2d..f85c54bdc8 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -22,6 +22,7 @@ * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "qemu/config-file.h" #include "block/block_int.h" diff --git a/block/blkverify.c b/block/blkverify.c index 1d754496bc..2a885cc08d 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -7,7 +7,7 @@ * See the COPYING file in the top-level directory. */ -#include +#include "qemu/osdep.h" #include "qemu/sockets.h" /* for EINPROGRESS on Windows */ #include "block/block_int.h" #include "qapi/qmp/qdict.h" diff --git a/block/block-backend.c b/block/block-backend.c index e81375955f..efd61464da 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -10,6 +10,7 @@ * or later. See the COPYING.LIB file in the top-level directory. */ +#include "qemu/osdep.h" #include "sysemu/block-backend.h" #include "block/block_int.h" #include "block/blockjob.h" diff --git a/block/bochs.c b/block/bochs.c index 18949b9d4f..8b953bb44c 100644 --- a/block/bochs.c +++ b/block/bochs.c @@ -22,6 +22,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "block/block_int.h" #include "qemu/module.h" diff --git a/block/cloop.c b/block/cloop.c index 4190ae06d7..41bdee8d7f 100644 --- a/block/cloop.c +++ b/block/cloop.c @@ -21,6 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "block/block_int.h" #include "qemu/module.h" diff --git a/block/commit.c b/block/commit.c index a5d02aa560..446a3aeadd 100644 --- a/block/commit.c +++ b/block/commit.c @@ -12,6 +12,7 @@ * */ +#include "qemu/osdep.h" #include "trace.h" #include "block/block_int.h" #include "block/blockjob.h" diff --git a/block/curl.c b/block/curl.c index 89941826ed..1507e0ac34 100644 --- a/block/curl.c +++ b/block/curl.c @@ -21,6 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "qemu/error-report.h" #include "block/block_int.h" diff --git a/block/dmg.c b/block/dmg.c index 546a6f5330..1018fd158e 100644 --- a/block/dmg.c +++ b/block/dmg.c @@ -21,6 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "block/block_int.h" #include "qemu/bswap.h" diff --git a/block/gluster.c b/block/gluster.c index 0857c14645..65077a0d0a 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -7,6 +7,7 @@ * See the COPYING file in the top-level directory. * */ +#include "qemu/osdep.h" #include #include "block/block_int.h" #include "qemu/uri.h" diff --git a/block/io.c b/block/io.c index 63e3678036..707c04ba48 100644 --- a/block/io.c +++ b/block/io.c @@ -22,6 +22,7 @@ * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "trace.h" #include "sysemu/block-backend.h" #include "block/blockjob.h" diff --git a/block/iscsi.c b/block/iscsi.c index 3acb052b1f..bffd707b8b 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -23,7 +23,7 @@ * THE SOFTWARE. */ -#include "config-host.h" +#include "qemu/osdep.h" #include #include diff --git a/block/linux-aio.c b/block/linux-aio.c index 88b0520a8b..805757e02e 100644 --- a/block/linux-aio.c +++ b/block/linux-aio.c @@ -7,6 +7,7 @@ * This work is licensed under the terms of the GNU GPL, version 2 or later. * See the COPYING file in the top-level directory. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "block/aio.h" #include "qemu/queue.h" diff --git a/block/mirror.c b/block/mirror.c index f201f2b18a..e9e151c341 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -11,6 +11,7 @@ * */ +#include "qemu/osdep.h" #include "trace.h" #include "block/blockjob.h" #include "block/block_int.h" diff --git a/block/nbd-client.c b/block/nbd-client.c index b7fd17a115..568c56cbb6 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -26,6 +26,7 @@ * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "nbd-client.h" #include "qemu/sockets.h" diff --git a/block/nbd.c b/block/nbd.c index 416f42b903..1a90bc7855 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -26,6 +26,7 @@ * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "block/nbd-client.h" #include "qemu/uri.h" #include "block/block_int.h" @@ -36,8 +37,6 @@ #include "qapi/qmp/qint.h" #include "qapi/qmp/qstring.h" -#include -#include #define EN_OPTSTR ":exportname=" diff --git a/block/nfs.c b/block/nfs.c index fd79f89945..5eb8c133b9 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -22,7 +22,7 @@ * THE SOFTWARE. */ -#include "config-host.h" +#include "qemu/osdep.h" #include #include "qemu-common.h" diff --git a/block/null.c b/block/null.c index 7d083233fb..d90165dea7 100644 --- a/block/null.c +++ b/block/null.c @@ -10,6 +10,7 @@ * See the COPYING file in the top-level directory. */ +#include "qemu/osdep.h" #include "block/block_int.h" #define NULL_OPT_LATENCY "latency-ns" diff --git a/block/parallels.c b/block/parallels.c index e4a56a5141..ee390815dc 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -27,6 +27,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "block/block_int.h" #include "qemu/module.h" diff --git a/block/qapi.c b/block/qapi.c index 58d3975001..a49c118ba0 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -22,6 +22,7 @@ * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "block/qapi.h" #include "block/block_int.h" #include "block/throttle-groups.h" diff --git a/block/qcow.c b/block/qcow.c index 635085e27b..afed18fe98 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -21,6 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "block/block_int.h" #include "qemu/module.h" diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c index 86dd7f2bd9..0fe8edae41 100644 --- a/block/qcow2-cache.c +++ b/block/qcow2-cache.c @@ -23,7 +23,7 @@ */ /* Needed for CONFIG_MADVISE */ -#include "config-host.h" +#include "qemu/osdep.h" #if defined(CONFIG_MADVISE) || defined(CONFIG_POSIX_MADVISE) #include @@ -31,7 +31,6 @@ #include "block/block_int.h" #include "qemu-common.h" -#include "qemu/osdep.h" #include "qcow2.h" #include "trace.h" diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 34112c3abb..3e887e9ab0 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -22,6 +22,7 @@ * THE SOFTWARE. */ +#include "qemu/osdep.h" #include #include "qemu-common.h" diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index af493f8bfe..52a0a9ffc3 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -22,6 +22,7 @@ * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "block/block_int.h" #include "block/qcow2.h" diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index def720164d..13f88d1b8b 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -22,6 +22,7 @@ * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "block/block_int.h" #include "block/qcow2.h" diff --git a/block/qcow2.c b/block/qcow2.c index d992e7fac7..e4e2754ca2 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -21,6 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "block/block_int.h" #include "qemu/module.h" diff --git a/block/qed-check.c b/block/qed-check.c index 36ecd290d6..622f308976 100644 --- a/block/qed-check.c +++ b/block/qed-check.c @@ -11,6 +11,7 @@ * */ +#include "qemu/osdep.h" #include "qed.h" typedef struct { diff --git a/block/qed-cluster.c b/block/qed-cluster.c index f64b2af8f7..c24e75616a 100644 --- a/block/qed-cluster.c +++ b/block/qed-cluster.c @@ -12,6 +12,7 @@ * */ +#include "qemu/osdep.h" #include "qed.h" /** diff --git a/block/qed-gencb.c b/block/qed-gencb.c index b817a8bf50..faf8ecc840 100644 --- a/block/qed-gencb.c +++ b/block/qed-gencb.c @@ -11,6 +11,7 @@ * */ +#include "qemu/osdep.h" #include "qed.h" void *gencb_alloc(size_t len, BlockCompletionFunc *cb, void *opaque) diff --git a/block/qed-l2-cache.c b/block/qed-l2-cache.c index e9b2aae44d..5cba794650 100644 --- a/block/qed-l2-cache.c +++ b/block/qed-l2-cache.c @@ -50,6 +50,7 @@ * table will be deleted in favor of the existing cache entry. */ +#include "qemu/osdep.h" #include "trace.h" #include "qed.h" diff --git a/block/qed-table.c b/block/qed-table.c index f4219b8acc..802945f5e5 100644 --- a/block/qed-table.c +++ b/block/qed-table.c @@ -12,6 +12,7 @@ * */ +#include "qemu/osdep.h" #include "trace.h" #include "qemu/sockets.h" /* for EINPROGRESS on Windows */ #include "qed.h" diff --git a/block/qed.c b/block/qed.c index 31f4cc9e60..093d6e57d7 100644 --- a/block/qed.c +++ b/block/qed.c @@ -12,6 +12,7 @@ * */ +#include "qemu/osdep.h" #include "qemu/timer.h" #include "trace.h" #include "qed.h" diff --git a/block/quorum.c b/block/quorum.c index 6793f126c5..a5ae4b812b 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -13,6 +13,7 @@ * See the COPYING file in the top-level directory. */ +#include "qemu/osdep.h" #include "block/block_int.h" #include "qapi/qmp/qbool.h" #include "qapi/qmp/qdict.h" diff --git a/block/raw-posix.c b/block/raw-posix.c index 816bdf7bea..6df3067ddf 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -21,6 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "qemu/error-report.h" #include "qemu/timer.h" @@ -51,8 +52,6 @@ #include #endif #ifdef __linux__ -#include -#include #include #include #include diff --git a/block/raw-win32.c b/block/raw-win32.c index 2d0907a822..21a6cb89d7 100644 --- a/block/raw-win32.c +++ b/block/raw-win32.c @@ -21,6 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "qemu/timer.h" #include "block/block_int.h" diff --git a/block/raw_bsd.c b/block/raw_bsd.c index 915d6fd0e6..bcaee115e1 100644 --- a/block/raw_bsd.c +++ b/block/raw_bsd.c @@ -26,6 +26,7 @@ * IN THE SOFTWARE. */ +#include "qemu/osdep.h" #include "block/block_int.h" #include "qemu/option.h" diff --git a/block/rbd.c b/block/rbd.c index a60a19d58d..51b64f3fed 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -11,7 +11,7 @@ * GNU GPL, version 2 or (at your option) any later version. */ -#include +#include "qemu/osdep.h" #include "qemu-common.h" #include "qemu/error-report.h" diff --git a/block/sheepdog.c b/block/sheepdog.c index 6986be8151..ff89298b13 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -12,6 +12,7 @@ * GNU GPL, version 2 or (at your option) any later version. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "qemu/uri.h" #include "qemu/error-report.h" diff --git a/block/snapshot.c b/block/snapshot.c index 2d86b88a28..17a27b57ad 100644 --- a/block/snapshot.c +++ b/block/snapshot.c @@ -22,6 +22,7 @@ * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "block/snapshot.h" #include "block/block_int.h" #include "qapi/qmp/qerror.h" diff --git a/block/ssh.c b/block/ssh.c index af025c08a0..04deeba1ad 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -22,9 +22,7 @@ * THE SOFTWARE. */ -#include -#include -#include +#include "qemu/osdep.h" #include #include diff --git a/block/stream.c b/block/stream.c index 25af7eff62..cafaa07a01 100644 --- a/block/stream.c +++ b/block/stream.c @@ -11,6 +11,7 @@ * */ +#include "qemu/osdep.h" #include "trace.h" #include "block/block_int.h" #include "block/blockjob.h" diff --git a/block/throttle-groups.c b/block/throttle-groups.c index 13b5baa5d7..4920e09495 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -22,6 +22,7 @@ * along with this program; if not, see . */ +#include "qemu/osdep.h" #include "block/throttle-groups.h" #include "qemu/queue.h" #include "qemu/thread.h" diff --git a/block/vdi.c b/block/vdi.c index 17f435fad6..61bcd54575 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -49,6 +49,7 @@ * so this seems to be reasonable. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "block/block_int.h" #include "qemu/module.h" diff --git a/block/vhdx-endian.c b/block/vhdx-endian.c index 0640d3f4a9..da33cd38ef 100644 --- a/block/vhdx-endian.c +++ b/block/vhdx-endian.c @@ -15,6 +15,7 @@ * */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "block/block_int.h" #include "block/vhdx.h" diff --git a/block/vhdx-log.c b/block/vhdx-log.c index ab86416def..369076126e 100644 --- a/block/vhdx-log.c +++ b/block/vhdx-log.c @@ -17,6 +17,7 @@ * See the COPYING.LIB file in the top-level directory. * */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "block/block_int.h" #include "qemu/error-report.h" diff --git a/block/vhdx.c b/block/vhdx.c index 2fe9a5e0cf..72042e9082 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -15,6 +15,7 @@ * */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "block/block_int.h" #include "qemu/module.h" diff --git a/block/vmdk.c b/block/vmdk.c index 2b5cb00ef1..6b8596c362 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -23,6 +23,7 @@ * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "block/block_int.h" #include "qapi/qmp/qerror.h" diff --git a/block/vpc.c b/block/vpc.c index 299d373092..d852f966a0 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -22,6 +22,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "block/block_int.h" #include "qemu/module.h" diff --git a/block/vvfat.c b/block/vvfat.c index b184eca6fc..2ea5a4ab0b 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -22,7 +22,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ -#include +#include "qemu/osdep.h" #include #include "qemu-common.h" #include "block/block_int.h" diff --git a/block/win32-aio.c b/block/win32-aio.c index bbf2f01c12..2d509a9a7b 100644 --- a/block/win32-aio.c +++ b/block/win32-aio.c @@ -21,6 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "qemu/timer.h" #include "block/block_int.h" diff --git a/block/write-threshold.c b/block/write-threshold.c index 0fe38917c5..cc2ca71835 100644 --- a/block/write-threshold.c +++ b/block/write-threshold.c @@ -10,6 +10,7 @@ * See the COPYING.LIB file in the top-level directory. */ +#include "qemu/osdep.h" #include "block/block_int.h" #include "qemu/coroutine.h" #include "block/write-threshold.h" diff --git a/hw/block/block.c b/hw/block/block.c index f7243e5b94..960df2b9d0 100644 --- a/hw/block/block.c +++ b/hw/block/block.c @@ -7,6 +7,7 @@ * later. See the COPYING file in the top-level directory. */ +#include "qemu/osdep.h" #include "sysemu/blockdev.h" #include "sysemu/block-backend.h" #include "hw/block/block.h" diff --git a/hw/block/cdrom.c b/hw/block/cdrom.c index 4e1019c890..da937fe33a 100644 --- a/hw/block/cdrom.c +++ b/hw/block/cdrom.c @@ -25,6 +25,7 @@ /* ??? Most of the ATAPI emulation is still in ide.c. It should be moved here. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "hw/scsi/scsi.h" diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index b8ce6cd5f3..bc34046fb5 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -12,6 +12,7 @@ * */ +#include "qemu/osdep.h" #include "trace.h" #include "qemu/iov.h" #include "qemu/thread.h" diff --git a/hw/block/ecc.c b/hw/block/ecc.c index 10bb233089..48311d2609 100644 --- a/hw/block/ecc.c +++ b/hw/block/ecc.c @@ -11,6 +11,7 @@ * GNU GPL, version 2 or (at your option) any later version. */ +#include "qemu/osdep.h" #include "hw/hw.h" #include "hw/block/flash.h" diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 858f5f7ce7..6711c6ac1a 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -27,6 +27,7 @@ * way. There are changes in DOR register and DMA is not available. */ +#include "qemu/osdep.h" #include "hw/hw.h" #include "hw/block/fdc.h" #include "qemu/error-report.h" diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c index b187878fac..6d02192dbb 100644 --- a/hw/block/hd-geometry.c +++ b/hw/block/hd-geometry.c @@ -30,6 +30,7 @@ * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "sysemu/block-backend.h" #include "hw/block/block.h" #include "trace.h" diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index efc43dde6a..4bbf90d461 100644 --- a/hw/block/m25p80.c +++ b/hw/block/m25p80.c @@ -21,6 +21,7 @@ * with this program; if not, see . */ +#include "qemu/osdep.h" #include "hw/hw.h" #include "sysemu/block-backend.h" #include "sysemu/blockdev.h" diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 169e4fa7a5..a5fedb2906 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -20,6 +20,7 @@ * -device nvme,drive=,serial=,id= */ +#include "qemu/osdep.h" #include #include #include diff --git a/hw/block/onenand.c b/hw/block/onenand.c index 58eff508bf..91896851f5 100644 --- a/hw/block/onenand.c +++ b/hw/block/onenand.c @@ -18,6 +18,7 @@ * with this program; if not, see . */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "hw/hw.h" #include "hw/block/flash.h" diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c index 2ba6c77293..a4c4fa1c69 100644 --- a/hw/block/pflash_cfi01.c +++ b/hw/block/pflash_cfi01.c @@ -36,6 +36,7 @@ * It does not implement much more ... */ +#include "qemu/osdep.h" #include "hw/hw.h" #include "hw/block/flash.h" #include "sysemu/block-backend.h" diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c index 074a005f69..aaa697adbb 100644 --- a/hw/block/pflash_cfi02.c +++ b/hw/block/pflash_cfi02.c @@ -35,6 +35,7 @@ * It does not implement multiple sectors erase */ +#include "qemu/osdep.h" #include "hw/hw.h" #include "hw/block/flash.h" #include "qemu/timer.h" diff --git a/hw/block/tc58128.c b/hw/block/tc58128.c index 728f1c3b68..7909d5041e 100644 --- a/hw/block/tc58128.c +++ b/hw/block/tc58128.c @@ -1,3 +1,4 @@ +#include "qemu/osdep.h" #include "hw/hw.h" #include "hw/sh4/sh.h" #include "hw/loader.h" diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 51f867b513..11bedff6d6 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -11,6 +11,7 @@ * */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "qemu/iov.h" #include "qemu/error-report.h" diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index a48e726f4a..571f651008 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -19,18 +19,8 @@ * GNU GPL, version 2 or (at your option) any later version. */ -#include -#include -#include -#include -#include -#include -#include -#include -#include +#include "qemu/osdep.h" #include -#include -#include #include #include diff --git a/qemu-img.c b/qemu-img.c index 67739439c5..33e451c101 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -21,6 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "qapi-visit.h" #include "qapi/qmp-output-visitor.h" #include "qapi/qmp/qerror.h" @@ -28,7 +29,6 @@ #include "qemu-common.h" #include "qemu/option.h" #include "qemu/error-report.h" -#include "qemu/osdep.h" #include "sysemu/sysemu.h" #include "sysemu/block-backend.h" #include "block/block_int.h" diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index 18fc2bdc10..e929d24a49 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -8,6 +8,7 @@ * See the COPYING file in the top-level directory. */ +#include "qemu/osdep.h" #include "qemu-io.h" #include "sysemu/block-backend.h" #include "block/block.h" diff --git a/qemu-io.c b/qemu-io.c index d47228a963..d593f19642 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -7,10 +7,7 @@ * This work is licensed under the terms of the GNU GPL, version 2 or later. * See the COPYING file in the top-level directory. */ -#include -#include -#include -#include +#include "qemu/osdep.h" #include #include -- cgit v1.2.3 From 1a4828c7934f592c146bc3dc979e78df430545cf Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 2 Dec 2015 19:11:04 +0100 Subject: qcow2: Write feature table only for v3 images Version 2 images don't have feature bits, so writing a feature table to those images is kind of pointless. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/qcow2.c | 48 ++++++++++++++++++++++++---------------------- tests/qemu-iotests/031.out | 12 +----------- tests/qemu-iotests/061.out | 15 --------------- 3 files changed, 26 insertions(+), 49 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index e4e2754ca2..20e4057ca2 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1895,31 +1895,33 @@ int qcow2_update_header(BlockDriverState *bs) } /* Feature table */ - Qcow2Feature features[] = { - { - .type = QCOW2_FEAT_TYPE_INCOMPATIBLE, - .bit = QCOW2_INCOMPAT_DIRTY_BITNR, - .name = "dirty bit", - }, - { - .type = QCOW2_FEAT_TYPE_INCOMPATIBLE, - .bit = QCOW2_INCOMPAT_CORRUPT_BITNR, - .name = "corrupt bit", - }, - { - .type = QCOW2_FEAT_TYPE_COMPATIBLE, - .bit = QCOW2_COMPAT_LAZY_REFCOUNTS_BITNR, - .name = "lazy refcounts", - }, - }; + if (s->qcow_version >= 3) { + Qcow2Feature features[] = { + { + .type = QCOW2_FEAT_TYPE_INCOMPATIBLE, + .bit = QCOW2_INCOMPAT_DIRTY_BITNR, + .name = "dirty bit", + }, + { + .type = QCOW2_FEAT_TYPE_INCOMPATIBLE, + .bit = QCOW2_INCOMPAT_CORRUPT_BITNR, + .name = "corrupt bit", + }, + { + .type = QCOW2_FEAT_TYPE_COMPATIBLE, + .bit = QCOW2_COMPAT_LAZY_REFCOUNTS_BITNR, + .name = "lazy refcounts", + }, + }; - ret = header_ext_add(buf, QCOW2_EXT_MAGIC_FEATURE_TABLE, - features, sizeof(features), buflen); - if (ret < 0) { - goto fail; + ret = header_ext_add(buf, QCOW2_EXT_MAGIC_FEATURE_TABLE, + features, sizeof(features), buflen); + if (ret < 0) { + goto fail; + } + buf += ret; + buflen -= ret; } - buf += ret; - buflen -= ret; /* Keep unknown header extensions */ QLIST_FOREACH(uext, &s->unknown_header_ext, next) { diff --git a/tests/qemu-iotests/031.out b/tests/qemu-iotests/031.out index fce3ce0984..f065404561 100644 --- a/tests/qemu-iotests/031.out +++ b/tests/qemu-iotests/031.out @@ -52,11 +52,6 @@ autoclear_features 0x0 refcount_order 4 header_length 72 -Header extension: -magic 0x6803f857 -length 144 -data - Header extension: magic 0x12345678 length 31 @@ -68,7 +63,7 @@ No errors were found on the image. magic 0x514649fb version 2 -backing_file_offset 0x128 +backing_file_offset 0x90 backing_file_size 0x17 cluster_bits 16 size 67108864 @@ -90,11 +85,6 @@ magic 0xe2792aca length 11 data 'host_device' -Header extension: -magic 0x6803f857 -length 144 -data - Header extension: magic 0x12345678 length 31 diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out index 57aae28e53..d604682da9 100644 --- a/tests/qemu-iotests/061.out +++ b/tests/qemu-iotests/061.out @@ -43,11 +43,6 @@ autoclear_features 0x0 refcount_order 4 header_length 72 -Header extension: -magic 0x6803f857 -length 144 -data - read 131072/131072 bytes at offset 0 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) No errors were found on the image. @@ -105,11 +100,6 @@ autoclear_features 0x0 refcount_order 4 header_length 72 -Header extension: -magic 0x6803f857 -length 144 -data - read 131072/131072 bytes at offset 0 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) No errors were found on the image. @@ -155,11 +145,6 @@ autoclear_features 0x0 refcount_order 4 header_length 72 -Header extension: -magic 0x6803f857 -length 144 -data - No errors were found on the image. === Testing version upgrade and resize === -- cgit v1.2.3 From b527c9b392a87abff698ca435da0dfa2bd6324a2 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 2 Dec 2015 18:34:39 +0100 Subject: qcow2: Write full header on image creation When creating a qcow2 image, we didn't necessarily call qcow2_update_header(), but could end up with the basic header that qcow2_create2() created manually. One thing that this basic header lacks is the feature table. Let's make sure that it's always present. This requires a few updates to test cases as well. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/qcow2.c | 7 +++++++ tests/qemu-iotests/031.out | 5 +++++ tests/qemu-iotests/036 | 2 ++ tests/qemu-iotests/036.out | 5 +++++ tests/qemu-iotests/061.out | 20 ++++++++++++++++++++ 5 files changed, 39 insertions(+) diff --git a/block/qcow2.c b/block/qcow2.c index 20e4057ca2..037afbf528 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2239,6 +2239,13 @@ static int qcow2_create2(const char *filename, int64_t total_size, abort(); } + /* Create a full header (including things like feature table) */ + ret = qcow2_update_header(bs); + if (ret < 0) { + error_setg_errno(errp, -ret, "Could not update qcow2 header"); + goto out; + } + /* Okay, now that we have a valid image, let's give it the right size */ ret = bdrv_truncate(bs, total_size); if (ret < 0) { diff --git a/tests/qemu-iotests/031.out b/tests/qemu-iotests/031.out index f065404561..7f5050b816 100644 --- a/tests/qemu-iotests/031.out +++ b/tests/qemu-iotests/031.out @@ -115,6 +115,11 @@ autoclear_features 0x0 refcount_order 4 header_length 104 +Header extension: +magic 0x6803f857 +length 144 +data + Header extension: magic 0x12345678 length 31 diff --git a/tests/qemu-iotests/036 b/tests/qemu-iotests/036 index 392f1ef3e6..c4cc91b8af 100755 --- a/tests/qemu-iotests/036 +++ b/tests/qemu-iotests/036 @@ -57,6 +57,7 @@ _make_test_img 64M $PYTHON qcow2.py "$TEST_IMG" set-feature-bit incompatible 63 # Without feature table +$PYTHON qcow2.py "$TEST_IMG" del-header-ext 0x6803f857 $PYTHON qcow2.py "$TEST_IMG" dump-header _img_info @@ -73,6 +74,7 @@ $PYTHON qcow2.py "$TEST_IMG" set-feature-bit incompatible 62 $PYTHON qcow2.py "$TEST_IMG" set-feature-bit incompatible 63 # Without feature table +$PYTHON qcow2.py "$TEST_IMG" del-header-ext 0x6803f857 _img_info # With feature table containing bit 63 diff --git a/tests/qemu-iotests/036.out b/tests/qemu-iotests/036.out index 5616e37b3f..f443635b25 100644 --- a/tests/qemu-iotests/036.out +++ b/tests/qemu-iotests/036.out @@ -56,6 +56,11 @@ autoclear_features 0x8000000000000000 refcount_order 4 header_length 104 +Header extension: +magic 0x6803f857 +length 144 +data + === Repair image === diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out index d604682da9..a03732e19c 100644 --- a/tests/qemu-iotests/061.out +++ b/tests/qemu-iotests/061.out @@ -24,6 +24,11 @@ autoclear_features 0x0 refcount_order 4 header_length 104 +Header extension: +magic 0x6803f857 +length 144 +data + magic 0x514649fb version 2 backing_file_offset 0x0 @@ -76,6 +81,11 @@ autoclear_features 0x0 refcount_order 4 header_length 104 +Header extension: +magic 0x6803f857 +length 144 +data + ERROR cluster 5 refcount=0 reference=1 ERROR cluster 6 refcount=0 reference=1 Rebuilding refcount structure @@ -126,6 +136,11 @@ autoclear_features 0x40000000000 refcount_order 4 header_length 104 +Header extension: +magic 0x6803f857 +length 144 +data + magic 0x514649fb version 2 backing_file_offset 0x0 @@ -228,6 +243,11 @@ autoclear_features 0x0 refcount_order 4 header_length 104 +Header extension: +magic 0x6803f857 +length 144 +data + ERROR cluster 5 refcount=0 reference=1 ERROR cluster 6 refcount=0 reference=1 Rebuilding refcount structure -- cgit v1.2.3 From 09e0c771e47e02278c264bafff6bfc0771732d72 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 16 Dec 2015 14:00:36 +0100 Subject: block: Assert no write requests under BDRV_O_INCOMING As long as BDRV_O_INCOMING is set, the image file is only opened so we have a file descriptor for it. We're definitely not supposed to modify the image, it's still owned by the migration source. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/io.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block/io.c b/block/io.c index 707c04ba48..23729942bc 100644 --- a/block/io.c +++ b/block/io.c @@ -1301,6 +1301,7 @@ static int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs, if (bs->read_only) { return -EPERM; } + assert(!(bs->open_flags & BDRV_O_INCOMING)); ret = bdrv_check_byte_request(bs, offset, bytes); if (ret < 0) { @@ -2462,6 +2463,7 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, } else if (bs->read_only) { return -EPERM; } + assert(!(bs->open_flags & BDRV_O_INCOMING)); /* Do nothing if disabled. */ if (!(bs->open_flags & BDRV_O_UNMAP)) { -- cgit v1.2.3 From 23c88b24721ee907639705bf9de03dcd6b1e66ad Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 16 Dec 2015 16:05:21 +0100 Subject: block: Fix error path in bdrv_invalidate_cache() We can only clear BDRV_O_INCOMING if the caches were actually invalidated. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block.c b/block.c index 1a137167d9..6ac3191e08 100644 --- a/block.c +++ b/block.c @@ -3272,12 +3272,14 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp) bdrv_invalidate_cache(bs->file->bs, &local_err); } if (local_err) { + bs->open_flags |= BDRV_O_INCOMING; error_propagate(errp, local_err); return; } ret = refresh_total_sectors(bs, bs->total_sectors); if (ret < 0) { + bs->open_flags |= BDRV_O_INCOMING; error_setg_errno(errp, -ret, "Could not refresh total sector count"); return; } -- cgit v1.2.3 From 04c01a5c8f006b6e45fa5be8ea857efe7d9c41f9 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 13 Jan 2016 15:56:06 +0100 Subject: block: Rename BDRV_O_INCOMING to BDRV_O_INACTIVE Instead of covering only the state of images on the migration destination before the migration is completed, the flag will also cover the state of images on the migration source after completion. This common state implies that the image is technically still open, but no writes will happen and any cached contents will be reloaded from disk if and when the image leaves this state. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 10 +++++----- block/io.c | 4 ++-- block/qcow2.c | 6 +++--- block/qed.c | 4 ++-- include/block/block.h | 2 +- nbd/server.c | 2 +- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/block.c b/block.c index 6ac3191e08..95b2967192 100644 --- a/block.c +++ b/block.c @@ -1191,7 +1191,7 @@ static int bdrv_fill_options(QDict **options, const char *filename, } if (runstate_check(RUN_STATE_INMIGRATE)) { - *flags |= BDRV_O_INCOMING; + *flags |= BDRV_O_INACTIVE; } return 0; @@ -3261,10 +3261,10 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp) return; } - if (!(bs->open_flags & BDRV_O_INCOMING)) { + if (!(bs->open_flags & BDRV_O_INACTIVE)) { return; } - bs->open_flags &= ~BDRV_O_INCOMING; + bs->open_flags &= ~BDRV_O_INACTIVE; if (bs->drv->bdrv_invalidate_cache) { bs->drv->bdrv_invalidate_cache(bs, &local_err); @@ -3272,14 +3272,14 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp) bdrv_invalidate_cache(bs->file->bs, &local_err); } if (local_err) { - bs->open_flags |= BDRV_O_INCOMING; + bs->open_flags |= BDRV_O_INACTIVE; error_propagate(errp, local_err); return; } ret = refresh_total_sectors(bs, bs->total_sectors); if (ret < 0) { - bs->open_flags |= BDRV_O_INCOMING; + bs->open_flags |= BDRV_O_INACTIVE; error_setg_errno(errp, -ret, "Could not refresh total sector count"); return; } diff --git a/block/io.c b/block/io.c index 23729942bc..5bb353a8ca 100644 --- a/block/io.c +++ b/block/io.c @@ -1301,7 +1301,7 @@ static int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs, if (bs->read_only) { return -EPERM; } - assert(!(bs->open_flags & BDRV_O_INCOMING)); + assert(!(bs->open_flags & BDRV_O_INACTIVE)); ret = bdrv_check_byte_request(bs, offset, bytes); if (ret < 0) { @@ -2463,7 +2463,7 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, } else if (bs->read_only) { return -EPERM; } - assert(!(bs->open_flags & BDRV_O_INCOMING)); + assert(!(bs->open_flags & BDRV_O_INACTIVE)); /* Do nothing if disabled. */ if (!(bs->open_flags & BDRV_O_UNMAP)) { diff --git a/block/qcow2.c b/block/qcow2.c index 037afbf528..340ae8f8a3 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1141,7 +1141,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, } /* Clear unknown autoclear feature bits */ - if (!bs->read_only && !(flags & BDRV_O_INCOMING) && s->autoclear_features) { + if (!bs->read_only && !(flags & BDRV_O_INACTIVE) && s->autoclear_features) { s->autoclear_features = 0; ret = qcow2_update_header(bs); if (ret < 0) { @@ -1154,7 +1154,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, qemu_co_mutex_init(&s->lock); /* Repair image if dirty */ - if (!(flags & (BDRV_O_CHECK | BDRV_O_INCOMING)) && !bs->read_only && + if (!(flags & (BDRV_O_CHECK | BDRV_O_INACTIVE)) && !bs->read_only && (s->incompatible_features & QCOW2_INCOMPAT_DIRTY)) { BdrvCheckResult result = {0}; @@ -1693,7 +1693,7 @@ static void qcow2_close(BlockDriverState *bs) /* else pre-write overlap checks in cache_destroy may crash */ s->l1_table = NULL; - if (!(bs->open_flags & BDRV_O_INCOMING)) { + if (!(bs->open_flags & BDRV_O_INACTIVE)) { int ret1, ret2; ret1 = qcow2_cache_flush(bs, s->l2_table_cache); diff --git a/block/qed.c b/block/qed.c index 093d6e57d7..0c870cd90f 100644 --- a/block/qed.c +++ b/block/qed.c @@ -478,7 +478,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags, * feature is no longer valid. */ if ((s->header.autoclear_features & ~QED_AUTOCLEAR_FEATURE_MASK) != 0 && - !bdrv_is_read_only(bs->file->bs) && !(flags & BDRV_O_INCOMING)) { + !bdrv_is_read_only(bs->file->bs) && !(flags & BDRV_O_INACTIVE)) { s->header.autoclear_features &= QED_AUTOCLEAR_FEATURE_MASK; ret = qed_write_header_sync(s); @@ -506,7 +506,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags, * aid data recovery from an otherwise inconsistent image. */ if (!bdrv_is_read_only(bs->file->bs) && - !(flags & BDRV_O_INCOMING)) { + !(flags & BDRV_O_INACTIVE)) { BdrvCheckResult result = {0}; ret = qed_check(s, &result, true); diff --git a/include/block/block.h b/include/block/block.h index c96923df99..2b7d33c78e 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -84,7 +84,7 @@ typedef struct HDGeometry { #define BDRV_O_NO_BACKING 0x0100 /* don't open the backing file */ #define BDRV_O_NO_FLUSH 0x0200 /* disable flushing on this disk */ #define BDRV_O_COPY_ON_READ 0x0400 /* copy read backing sectors into image */ -#define BDRV_O_INCOMING 0x0800 /* consistency hint for incoming migration */ +#define BDRV_O_INACTIVE 0x0800 /* consistency hint for migration handoff */ #define BDRV_O_CHECK 0x1000 /* open solely for consistency check */ #define BDRV_O_ALLOW_RDWR 0x2000 /* allow reopen to change from r/o to r/w */ #define BDRV_O_UNMAP 0x4000 /* execute guest UNMAP/TRIM operations */ diff --git a/nbd/server.c b/nbd/server.c index eead339a2c..2265cb0680 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -668,7 +668,7 @@ NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size, blk_add_aio_context_notifier(blk, blk_aio_attached, blk_aio_detach, exp); /* * NBD exports are used for non-shared storage migration. Make sure - * that BDRV_O_INCOMING is cleared and the image is ready for write + * that BDRV_O_INACTIVE is cleared and the image is ready for write * access since the export could be available before migration handover. */ blk_invalidate_cache(blk, NULL); -- cgit v1.2.3 From 76b1c7fe1cbc45f46b2cccd471369ccd4b49b6fd Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 22 Dec 2015 14:07:08 +0100 Subject: block: Inactivate BDS when migration completes So far, live migration with shared storage meant that the image is in a not-really-ready don't-touch-me state on the destination while the source is still actively using it, but after completing the migration, the image was fully opened on both sides. This is bad. This patch adds a block driver callback to inactivate images on the source before completing the migration. Inactivation means that it goes to a state as if it was just live migrated to the qemu instance on the source (i.e. BDRV_O_INACTIVE is set). You're then supposed to continue either on the source or on the destination, which takes ownership of the image. A typical migration looks like this now with respect to disk images: 1. Destination qemu is started, the image is opened with BDRV_O_INACTIVE. The image is fully opened on the source. 2. Migration is about to complete. The source flushes the image and inactivates it. Now both sides have the image opened with BDRV_O_INACTIVE and are expecting the other side to still modify it. 3. One side (the destination on success) continues and calls bdrv_invalidate_all() in order to take ownership of the image again. This removes BDRV_O_INACTIVE on the resuming side; the flag remains set on the other side. This ensures that the same image isn't written to by both instances (unless both are resumed, but then you get what you deserve). This is important because .bdrv_close for non-BDRV_O_INACTIVE images could write to the image file, which is definitely forbidden while another host is using the image. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: John Snow --- block.c | 34 ++++++++++++++++++++++++++++++++++ include/block/block.h | 1 + include/block/block_int.h | 1 + migration/migration.c | 7 +++++++ qmp.c | 12 ++++++++++++ 5 files changed, 55 insertions(+) diff --git a/block.c b/block.c index 95b2967192..5709d3ddb4 100644 --- a/block.c +++ b/block.c @@ -3303,6 +3303,40 @@ void bdrv_invalidate_cache_all(Error **errp) } } +static int bdrv_inactivate(BlockDriverState *bs) +{ + int ret; + + if (bs->drv->bdrv_inactivate) { + ret = bs->drv->bdrv_inactivate(bs); + if (ret < 0) { + return ret; + } + } + + bs->open_flags |= BDRV_O_INACTIVE; + return 0; +} + +int bdrv_inactivate_all(void) +{ + BlockDriverState *bs; + int ret; + + QTAILQ_FOREACH(bs, &bdrv_states, device_list) { + AioContext *aio_context = bdrv_get_aio_context(bs); + + aio_context_acquire(aio_context); + ret = bdrv_inactivate(bs); + aio_context_release(aio_context); + if (ret < 0) { + return ret; + } + } + + return 0; +} + /**************************************************************/ /* removable device support */ diff --git a/include/block/block.h b/include/block/block.h index 2b7d33c78e..25f36dcc74 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -369,6 +369,7 @@ BlockAIOCB *bdrv_aio_ioctl(BlockDriverState *bs, /* Invalidate any cached metadata used by image formats */ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp); void bdrv_invalidate_cache_all(Error **errp); +int bdrv_inactivate_all(void); /* Ensure contents are flushed to disk. */ int bdrv_flush(BlockDriverState *bs); diff --git a/include/block/block_int.h b/include/block/block_int.h index 256609dd3d..428fa3397e 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -172,6 +172,7 @@ struct BlockDriver { * Invalidate any cached meta-data. */ void (*bdrv_invalidate_cache)(BlockDriverState *bs, Error **errp); + int (*bdrv_inactivate)(BlockDriverState *bs); /* * Flushes all data that was already written to the OS all the way down to diff --git a/migration/migration.c b/migration/migration.c index bc611e453b..aaca451cf8 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1422,7 +1422,11 @@ static int postcopy_start(MigrationState *ms, bool *old_vm_running) *old_vm_running = runstate_is_running(); global_state_store(); ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE); + if (ret < 0) { + goto fail; + } + ret = bdrv_inactivate_all(); if (ret < 0) { goto fail; } @@ -1541,6 +1545,9 @@ static void migration_completion(MigrationState *s, int current_active_state, if (!ret) { ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE); + if (ret >= 0) { + ret = bdrv_inactivate_all(); + } if (ret >= 0) { qemu_file_set_rate_limit(s->file, INT64_MAX); qemu_savevm_state_complete_precopy(s->file, false); diff --git a/qmp.c b/qmp.c index 3ff6db79b9..53affe2cfd 100644 --- a/qmp.c +++ b/qmp.c @@ -192,6 +192,18 @@ void qmp_cont(Error **errp) } } + /* Continuing after completed migration. Images have been inactivated to + * allow the destination to take control. Need to get control back now. */ + if (runstate_check(RUN_STATE_FINISH_MIGRATE) || + runstate_check(RUN_STATE_POSTMIGRATE)) + { + bdrv_invalidate_cache_all(&local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + } + if (runstate_check(RUN_STATE_INMIGRATE)) { autostart = 1; } else { -- cgit v1.2.3 From ec6d891224f708b2cda2a1edf68ffc0ff1316fca Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 22 Dec 2015 16:04:57 +0100 Subject: qcow2: Implement .bdrv_inactivate The callback has to ensure that closing or flushing the image afterwards wouldn't cause a write access to the image files. This means that just the caches have to be written out, which is part of the existing .bdrv_close implementation. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/qcow2.c | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 340ae8f8a3..bfc80ea2ff 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1686,6 +1686,32 @@ fail: return ret; } +static int qcow2_inactivate(BlockDriverState *bs) +{ + BDRVQcow2State *s = bs->opaque; + int ret, result = 0; + + ret = qcow2_cache_flush(bs, s->l2_table_cache); + if (ret) { + result = ret; + error_report("Failed to flush the L2 table cache: %s", + strerror(-ret)); + } + + ret = qcow2_cache_flush(bs, s->refcount_block_cache); + if (ret) { + result = ret; + error_report("Failed to flush the refcount block cache: %s", + strerror(-ret)); + } + + if (result == 0) { + qcow2_mark_clean(bs); + } + + return result; +} + static void qcow2_close(BlockDriverState *bs) { BDRVQcow2State *s = bs->opaque; @@ -1694,23 +1720,7 @@ static void qcow2_close(BlockDriverState *bs) s->l1_table = NULL; if (!(bs->open_flags & BDRV_O_INACTIVE)) { - int ret1, ret2; - - ret1 = qcow2_cache_flush(bs, s->l2_table_cache); - ret2 = qcow2_cache_flush(bs, s->refcount_block_cache); - - if (ret1) { - error_report("Failed to flush the L2 table cache: %s", - strerror(-ret1)); - } - if (ret2) { - error_report("Failed to flush the refcount block cache: %s", - strerror(-ret2)); - } - - if (!ret1 && !ret2) { - qcow2_mark_clean(bs); - } + qcow2_inactivate(bs); } cache_clean_timer_del(bs); @@ -3340,6 +3350,7 @@ BlockDriver bdrv_qcow2 = { .bdrv_refresh_limits = qcow2_refresh_limits, .bdrv_invalidate_cache = qcow2_invalidate_cache, + .bdrv_inactivate = qcow2_inactivate, .create_opts = &qcow2_create_opts, .bdrv_check = qcow2_check, -- cgit v1.2.3 From 140fd5a69cf19460b8daa8a9bb83bd869f6db14d Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 22 Dec 2015 16:10:32 +0100 Subject: qcow2: Fix BDRV_O_INACTIVE handling in qcow2_invalidate_cache() What qcow2_invalidate_cache() should do is close the image with BDRV_O_INACTIVE set and reopen it with the flag cleared. In fact, it used to do exactly the opposite: qcow2_close() relied on bs->open_flags, which is already updated to have cleared BDRV_O_INACTIVE at this point, whereas qcow2_open() was called with s->flags, which has the flag still set. Fix this. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/qcow2.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/block/qcow2.c b/block/qcow2.c index bfc80ea2ff..0f8c485c2c 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1719,7 +1719,7 @@ static void qcow2_close(BlockDriverState *bs) /* else pre-write overlap checks in cache_destroy may crash */ s->l1_table = NULL; - if (!(bs->open_flags & BDRV_O_INACTIVE)) { + if (!(s->flags & BDRV_O_INACTIVE)) { qcow2_inactivate(bs); } @@ -1770,6 +1770,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) memset(s, 0, sizeof(BDRVQcow2State)); options = qdict_clone_shallow(bs->options); + flags &= ~BDRV_O_INACTIVE; ret = qcow2_open(bs, options, flags, &local_err); QDECREF(options); if (local_err) { -- cgit v1.2.3 From 191fb11bdfbf53b33068268ce995a5c84045a8d9 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 22 Dec 2015 16:14:10 +0100 Subject: qcow2: Make image inaccessible after failed qcow2_invalidate_cache() If qcow2_invalidate_cache() fails, we are in a state where qcow2_close() has already been completed, but the image hasn't been reopened yet. Calling into any qcow2 function for an image in this state will cause crashes. The real solution would be to get rid of the close/open pair and instead do an atomic reset of the involved data structures, but this isn't trivial, so let's just make the image inaccessible for now. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/qcow2.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/block/qcow2.c b/block/qcow2.c index 0f8c485c2c..fd8436c5f8 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1764,6 +1764,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) bdrv_invalidate_cache(bs->file->bs, &local_err); if (local_err) { error_propagate(errp, local_err); + bs->drv = NULL; return; } @@ -1776,9 +1777,11 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) if (local_err) { error_propagate(errp, local_err); error_prepend(errp, "Could not reopen qcow2 layer: "); + bs->drv = NULL; return; } else if (ret < 0) { error_setg_errno(errp, -ret, "Could not reopen qcow2 layer"); + bs->drv = NULL; return; } -- cgit v1.2.3 From d62d9dc4b814950dcc8bd261a3e2e9300d9065e6 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 17 Sep 2015 13:04:10 +0800 Subject: vmdk: Create streamOptimized as version 3 VMware products accept only version 3 for streamOptimized, let's bump the version. Reported-by: Radoslav Gerganov Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vmdk.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/block/vmdk.c b/block/vmdk.c index 6b8596c362..698679d12c 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1663,7 +1663,13 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, } magic = cpu_to_be32(VMDK4_MAGIC); memset(&header, 0, sizeof(header)); - header.version = zeroed_grain ? 2 : 1; + if (compress) { + header.version = 3; + } else if (zeroed_grain) { + header.version = 2; + } else { + header.version = 1; + } header.flags = VMDK4_FLAG_RGD | VMDK4_FLAG_NL_DETECT | (compress ? VMDK4_FLAG_COMPRESS | VMDK4_FLAG_MARKER : 0) | (zeroed_grain ? VMDK4_FLAG_ZERO_GRAIN : 0); -- cgit v1.2.3 From 972606c4db826f286f7f475551180502859f49b9 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 20 Jan 2016 12:21:20 +0800 Subject: blockdev: Error out on negative throttling option values extract_common_blockdev_options() uses qemu_opt_get_number() to parse the bps/iops numbers to uint64_t, then converts to double and stores in ThrottleConfig. The actual parsing is done by strtoull() in parse_option_number(). Negative numbers are wrapped to large positive ones, and stored. We used to reject negative numbers since 7d81c1413c9, but this regressed when the option parsing code was changed later. Now fix this again. This time, define an arbitrary large upper limit (1e15), and check the values so both negative and impractically big numbers are caught and reported. Signed-off-by: Fam Zheng Reviewed-by: Markus Armbruster Reviewed-by: Alberto Garcia Signed-off-by: Kevin Wolf --- blockdev.c | 3 ++- include/qemu/throttle.h | 2 ++ util/throttle.c | 16 ++++++---------- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/blockdev.c b/blockdev.c index 1392fffaaa..07cfe25e1e 100644 --- a/blockdev.c +++ b/blockdev.c @@ -348,7 +348,8 @@ static bool check_throttle_config(ThrottleConfig *cfg, Error **errp) } if (!throttle_is_valid(cfg)) { - error_setg(errp, "bps/iops/maxs values must be 0 or greater"); + error_setg(errp, "bps/iops/max values must be within [0, %lld]", + THROTTLE_VALUE_MAX); return false; } diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h index 12faaad959..d0c98ed25b 100644 --- a/include/qemu/throttle.h +++ b/include/qemu/throttle.h @@ -29,6 +29,8 @@ #include "qemu-common.h" #include "qemu/timer.h" +#define THROTTLE_VALUE_MAX 1000000000000000LL + typedef enum { THROTTLE_BPS_TOTAL, THROTTLE_BPS_READ, diff --git a/util/throttle.c b/util/throttle.c index 1113671ecf..af4bc95ba3 100644 --- a/util/throttle.c +++ b/util/throttle.c @@ -282,22 +282,18 @@ bool throttle_conflicting(ThrottleConfig *cfg) */ bool throttle_is_valid(ThrottleConfig *cfg) { - bool invalid = false; int i; for (i = 0; i < BUCKETS_COUNT; i++) { - if (cfg->buckets[i].avg < 0) { - invalid = true; + if (cfg->buckets[i].avg < 0 || + cfg->buckets[i].max < 0 || + cfg->buckets[i].avg > THROTTLE_VALUE_MAX || + cfg->buckets[i].max > THROTTLE_VALUE_MAX) { + return false; } } - for (i = 0; i < BUCKETS_COUNT; i++) { - if (cfg->buckets[i].max < 0) { - invalid = true; - } - } - - return !invalid; + return true; } /* check if bps_max/iops_max is used without bps/iops -- cgit v1.2.3 From e9b155501da2638e4e319af9960d35da1bc8662b Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 20 Jan 2016 12:21:21 +0800 Subject: iotests: Test that throttle values ranges Signed-off-by: Fam Zheng Reviewed-by: Markus Armbruster Reviewed-by: Alberto Garcia Signed-off-by: Kevin Wolf --- tests/qemu-iotests/051 | 18 ++++++++++++++++++ tests/qemu-iotests/051.out | 39 +++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/051.pc.out | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+) diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051 index d91f80bb8e..7bfe9fffe1 100755 --- a/tests/qemu-iotests/051 +++ b/tests/qemu-iotests/051 @@ -262,6 +262,24 @@ run_qemu -drive file="$TEST_IMG",bps_wr_max=1234,throttling.bps-write-max=5678 run_qemu -drive file="$TEST_IMG",iops_size=1234,throttling.iops-size=5678 run_qemu -drive file="$TEST_IMG",readonly=on,read-only=off +echo +echo === Catching negative/large throttling values === +echo + +run_qemu -drive file="$TEST_IMG",iops=-1 +run_qemu -drive file="$TEST_IMG",bps=-2 +run_qemu -drive file="$TEST_IMG",bps_rd=-3 +run_qemu -drive file="$TEST_IMG",bps_rd_max=-3 +run_qemu -drive file="$TEST_IMG",throttling.iops-total=-4 +run_qemu -drive file="$TEST_IMG",throttling.bps-total=-5 +# These are accepted +run_qemu -drive file="$TEST_IMG",bps=0 +run_qemu -drive file="$TEST_IMG",bps=1 +run_qemu -drive file="$TEST_IMG",bps=1000000000000000 +# While these are not +run_qemu -drive file="$TEST_IMG",bps=1000000000000001 +run_qemu -drive file="$TEST_IMG",bps=9999999999999999 + echo echo === Parsing protocol from file name === echo diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index bf886ce1d7..0f8a8d3562 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -285,6 +285,45 @@ Testing: -drive file=TEST_DIR/t.qcow2,readonly=on,read-only=off QEMU_PROG: -drive file=TEST_DIR/t.qcow2,readonly=on,read-only=off: 'read-only' and its alias 'readonly' can't be used at the same time +=== Catching negative/large throttling values === + +Testing: -drive file=TEST_DIR/t.qcow2,iops=-1 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops=-1: bps/iops/max values must be within [0, 1000000000000000] + +Testing: -drive file=TEST_DIR/t.qcow2,bps=-2 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps=-2: bps/iops/max values must be within [0, 1000000000000000] + +Testing: -drive file=TEST_DIR/t.qcow2,bps_rd=-3 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_rd=-3: bps/iops/max values must be within [0, 1000000000000000] + +Testing: -drive file=TEST_DIR/t.qcow2,bps_rd_max=-3 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_rd_max=-3: bps/iops/max values must be within [0, 1000000000000000] + +Testing: -drive file=TEST_DIR/t.qcow2,throttling.iops-total=-4 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,throttling.iops-total=-4: bps/iops/max values must be within [0, 1000000000000000] + +Testing: -drive file=TEST_DIR/t.qcow2,throttling.bps-total=-5 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,throttling.bps-total=-5: bps/iops/max values must be within [0, 1000000000000000] + +Testing: -drive file=TEST_DIR/t.qcow2,bps=0 +QEMU X.Y.Z monitor - type 'help' for more information +(qemu) qququiquit + +Testing: -drive file=TEST_DIR/t.qcow2,bps=1 +QEMU X.Y.Z monitor - type 'help' for more information +(qemu) qququiquit + +Testing: -drive file=TEST_DIR/t.qcow2,bps=1000000000000000 +QEMU X.Y.Z monitor - type 'help' for more information +(qemu) qququiquit + +Testing: -drive file=TEST_DIR/t.qcow2,bps=1000000000000001 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps=1000000000000001: bps/iops/max values must be within [0, 1000000000000000] + +Testing: -drive file=TEST_DIR/t.qcow2,bps=9999999999999999 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps=9999999999999999: bps/iops/max values must be within [0, 1000000000000000] + + === Parsing protocol from file name === Testing: -hda foo:bar diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out index a5dfc33499..85fc05d05a 100644 --- a/tests/qemu-iotests/051.pc.out +++ b/tests/qemu-iotests/051.pc.out @@ -379,6 +379,45 @@ Testing: -drive file=TEST_DIR/t.qcow2,readonly=on,read-only=off QEMU_PROG: -drive file=TEST_DIR/t.qcow2,readonly=on,read-only=off: 'read-only' and its alias 'readonly' can't be used at the same time +=== Catching negative/large throttling values === + +Testing: -drive file=TEST_DIR/t.qcow2,iops=-1 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops=-1: bps/iops/max values must be within [0, 1000000000000000] + +Testing: -drive file=TEST_DIR/t.qcow2,bps=-2 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps=-2: bps/iops/max values must be within [0, 1000000000000000] + +Testing: -drive file=TEST_DIR/t.qcow2,bps_rd=-3 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_rd=-3: bps/iops/max values must be within [0, 1000000000000000] + +Testing: -drive file=TEST_DIR/t.qcow2,bps_rd_max=-3 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_rd_max=-3: bps/iops/max values must be within [0, 1000000000000000] + +Testing: -drive file=TEST_DIR/t.qcow2,throttling.iops-total=-4 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,throttling.iops-total=-4: bps/iops/max values must be within [0, 1000000000000000] + +Testing: -drive file=TEST_DIR/t.qcow2,throttling.bps-total=-5 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,throttling.bps-total=-5: bps/iops/max values must be within [0, 1000000000000000] + +Testing: -drive file=TEST_DIR/t.qcow2,bps=0 +QEMU X.Y.Z monitor - type 'help' for more information +(qemu) qququiquit + +Testing: -drive file=TEST_DIR/t.qcow2,bps=1 +QEMU X.Y.Z monitor - type 'help' for more information +(qemu) qququiquit + +Testing: -drive file=TEST_DIR/t.qcow2,bps=1000000000000000 +QEMU X.Y.Z monitor - type 'help' for more information +(qemu) qququiquit + +Testing: -drive file=TEST_DIR/t.qcow2,bps=1000000000000001 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps=1000000000000001: bps/iops/max values must be within [0, 1000000000000000] + +Testing: -drive file=TEST_DIR/t.qcow2,bps=9999999999999999 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps=9999999999999999: bps/iops/max values must be within [0, 1000000000000000] + + === Parsing protocol from file name === Testing: -hda foo:bar -- cgit v1.2.3