From 71b74b25444ed3ddfe7b025d059ca882916a4b02 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Sat, 10 Mar 2018 15:45:54 -0600 Subject: iotests: 163 is not quick Testing on ext4, most 'quick' qcow2 tests took less than 5 seconds, but 163 took more than 20. Let's remove it from the quick set. Signed-off-by: Eric Blake Signed-off-by: Kevin Wolf --- tests/qemu-iotests/group | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index efe0e958f2..0c2983f4cd 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -167,7 +167,7 @@ 159 rw auto quick 160 rw auto quick 162 auto quick -163 rw auto quick +163 rw auto 165 rw auto quick 169 rw auto quick 170 rw auto quick -- cgit v1.2.3 From cb83d2efe1f591cdc7ff2e8fbc67544155f264d6 Mon Sep 17 00:00:00 2001 From: Fabiano Rosas Date: Mon, 12 Mar 2018 19:07:49 -0300 Subject: block/replication: Remove protocol_name field The protocol_name field is used when selecting a driver via protocol syntax (i.e. :). Drivers that are only selected explicitly (e.g. driver=replication,mode=primary,...) should not have a protocol_name. This patch removes the protocol_name field from the brdv_replication structure so that attempts to invoke this driver using protocol syntax will fail gracefully: $ qemu-img info replication:foo qemu-img: Could not open 'replication:': Unknown protocol 'replication' Buglink: https://bugs.launchpad.net/qemu/+bug/1726733 Signed-off-by: Fabiano Rosas Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- block/replication.c | 1 - replication.h | 1 - 2 files changed, 2 deletions(-) diff --git a/block/replication.c b/block/replication.c index f98ef094b9..6c0c7186d9 100644 --- a/block/replication.c +++ b/block/replication.c @@ -703,7 +703,6 @@ static void replication_stop(ReplicationState *rs, bool failover, Error **errp) BlockDriver bdrv_replication = { .format_name = "replication", - .protocol_name = "replication", .instance_size = sizeof(BDRVReplicationState), .bdrv_open = replication_open, diff --git a/replication.h b/replication.h index 8faefe005f..4c8354de23 100644 --- a/replication.h +++ b/replication.h @@ -67,7 +67,6 @@ typedef struct ReplicationState ReplicationState; * * BlockDriver bdrv_replication = { * .format_name = "replication", - * .protocol_name = "replication", * .instance_size = sizeof(BDRVReplicationState), * * .bdrv_open = replication_open, -- cgit v1.2.3 From 65d2c3e2f64b116f6a25d38cde5919d38f36d26d Mon Sep 17 00:00:00 2001 From: Fabiano Rosas Date: Mon, 12 Mar 2018 19:07:50 -0300 Subject: block/quorum: Remove protocol-related fields The quorum driver is not a protocol so it should implement bdrv_open instead of bdrv_file_open and not provide a protocol_name. Attempts to invoke this driver using protocol syntax (i.e. quorum:) will now fail gracefully: $ qemu-img info quorum:foo qemu-img: Could not open 'quorum:foo': Unknown protocol 'quorum' Signed-off-by: Fabiano Rosas Reviewed-by: Max Reitz Reviewed-by: Alberto Garcia Signed-off-by: Kevin Wolf --- block/quorum.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/block/quorum.c b/block/quorum.c index 14333c18aa..cfe484a945 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -1098,11 +1098,10 @@ static void quorum_refresh_filename(BlockDriverState *bs, QDict *options) static BlockDriver bdrv_quorum = { .format_name = "quorum", - .protocol_name = "quorum", .instance_size = sizeof(BDRVQuorumState), - .bdrv_file_open = quorum_open, + .bdrv_open = quorum_open, .bdrv_close = quorum_close, .bdrv_refresh_filename = quorum_refresh_filename, -- cgit v1.2.3 From a7328ba55f4535613a54b4ad65a3eb8b1a47cbb7 Mon Sep 17 00:00:00 2001 From: Fabiano Rosas Date: Mon, 12 Mar 2018 19:07:51 -0300 Subject: block/throttle: Remove protocol-related fields The throttle driver is not a protocol so it should implement bdrv_open instead of bdrv_file_open and not provide a protocol_name. Attempts to invoke this driver using protocol syntax (i.e. throttle:) will now fail gracefully: $ qemu-img info throttle:foo qemu-img: Could not open 'throttle:foo': Unknown protocol 'throttle' Signed-off-by: Fabiano Rosas Reviewed-by: Max Reitz Reviewed-by: Alberto Garcia Signed-off-by: Kevin Wolf --- block/throttle.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/block/throttle.c b/block/throttle.c index 5f4d43d0fc..95ed06acd8 100644 --- a/block/throttle.c +++ b/block/throttle.c @@ -215,10 +215,9 @@ static void coroutine_fn throttle_co_drain_end(BlockDriverState *bs) static BlockDriver bdrv_throttle = { .format_name = "throttle", - .protocol_name = "throttle", .instance_size = sizeof(ThrottleGroupMember), - .bdrv_file_open = throttle_open, + .bdrv_open = throttle_open, .bdrv_close = throttle_close, .bdrv_co_flush = throttle_co_flush, -- cgit v1.2.3 From 8140e786f045a074928070640e20914873aeb396 Mon Sep 17 00:00:00 2001 From: Fabiano Rosas Date: Mon, 12 Mar 2018 19:07:52 -0300 Subject: block/blkreplay: Remove protocol-related fields The blkreplay driver is not a protocol so it should implement bdrv_open instead of bdrv_file_open and not provide a protocol_name. Attempts to invoke this driver using protocol syntax (i.e. blkreplay:) will now fail gracefully: $ qemu-img info blkreplay:foo qemu-img: Could not open 'blkreplay:foo': Unknown protocol 'blkreplay' Signed-off-by: Fabiano Rosas Reviewed-by: Pavel Dovgalyuk Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- block/blkreplay.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/block/blkreplay.c b/block/blkreplay.c index 61e44a1949..fe5a9b4a98 100755 --- a/block/blkreplay.c +++ b/block/blkreplay.c @@ -129,10 +129,9 @@ static int coroutine_fn blkreplay_co_flush(BlockDriverState *bs) static BlockDriver bdrv_blkreplay = { .format_name = "blkreplay", - .protocol_name = "blkreplay", .instance_size = 0, - .bdrv_file_open = blkreplay_open, + .bdrv_open = blkreplay_open, .bdrv_close = blkreplay_close, .bdrv_child_perm = bdrv_filter_default_perms, .bdrv_getlength = blkreplay_getlength, -- cgit v1.2.3 From 1e486cf30a3817b9136b333d31e7e77379fd6a90 Mon Sep 17 00:00:00 2001 From: Fabiano Rosas Date: Mon, 12 Mar 2018 19:07:53 -0300 Subject: include/block/block_int: Document protocol related functions Clarify that: - for protocols the brdv_file_open function is used instead of bdrv_open; - when protocol_name is set, a driver should expect to be given only a filename and no other options. Signed-off-by: Fabiano Rosas Signed-off-by: Kevin Wolf --- include/block/block_int.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/block/block_int.h b/include/block/block_int.h index 27e17addba..c4dd1d4bb8 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -126,6 +126,8 @@ struct BlockDriver { int (*bdrv_open)(BlockDriverState *bs, QDict *options, int flags, Error **errp); + + /* Protocol drivers should implement this instead of bdrv_open */ int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags, Error **errp); void (*bdrv_close)(BlockDriverState *bs); @@ -251,6 +253,12 @@ struct BlockDriver { */ int coroutine_fn (*bdrv_co_flush_to_os)(BlockDriverState *bs); + /* + * Drivers setting this field must be able to work with just a plain + * filename with ':' as a prefix, and no other options. + * Options may be extracted from the filename by implementing + * bdrv_parse_filename. + */ const char *protocol_name; int (*bdrv_truncate)(BlockDriverState *bs, int64_t offset, PreallocMode prealloc, Error **errp); -- cgit v1.2.3 From abf754fe406ecaf5b5018a6f34b2eba569352673 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Wed, 21 Mar 2018 15:38:52 +0200 Subject: qcow2: Reset free_cluster_index when allocating a new refcount block When we try to allocate new clusters we first look for available ones starting from s->free_cluster_index and once we find them we increase their reference counts. Before we get to call update_refcount() to do this last step s->free_cluster_index is already pointing to the next cluster after the ones we are trying to allocate. During update_refcount() it may happen however that we also need to allocate a new refcount block in order to store the refcounts of these new clusters (and to complicate things further that may also require us to grow the refcount table). After all this we don't know if the clusters that we originally tried to allocate are still available, so we return -EAGAIN to ask the caller to restart the search for free clusters. This is what can happen in a common scenario: 1) We want to allocate a new cluster and we see that cluster N is free. 2) We try to increase N's refcount but all refcount blocks are full, so we allocate a new one at N+1 (where s->free_cluster_index was pointing at). 3) Once we're done we return -EAGAIN to look again for a free cluster, but now s->free_cluster_index points at N+2, so that's the one we allocate. Cluster N remains unallocated and we have a hole in the qcow2 file. This can be reproduced easily: qemu-img create -f qcow2 -o cluster_size=512 hd.qcow2 1M qemu-io -c 'write 0 124k' hd.qcow2 After this the image has 132608 bytes (256 clusters), and the refcount block is full. If we write 512 more bytes it should allocate two new clusters: the data cluster itself and a new refcount block. qemu-io -c 'write 124k 512' hd.qcow2 However the image has now three new clusters (259 in total), and the first one of them is empty (and unallocated): dd if=hd.qcow2 bs=512c skip=256 count=1 | hexdump -C If we write larger amounts of data in the last step instead of the 512 bytes used in this example we can create larger holes in the qcow2 file. What this patch does is reset s->free_cluster_index to its previous value when alloc_refcount_block() returns -EAGAIN. This way the caller will try to allocate again the original clusters if they are still free. The output of iotest 026 also needs to be updated because now that images have no holes some tests fail at a different point and the number of leaked clusters is different. Signed-off-by: Alberto Garcia Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 7 +++++++ tests/qemu-iotests/026.out | 6 +++--- tests/qemu-iotests/121 | 20 ++++++++++++++++++++ tests/qemu-iotests/121.out | 10 ++++++++++ 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 362deaf303..6b8b63514a 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -839,6 +839,13 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs, qcow2_cache_put(s->refcount_block_cache, &refcount_block); } ret = alloc_refcount_block(bs, cluster_index, &refcount_block); + /* If the caller needs to restart the search for free clusters, + * try the same ones first to see if they're still free. */ + if (ret == -EAGAIN) { + if (s->free_cluster_index > (start >> s->cluster_bits)) { + s->free_cluster_index = (start >> s->cluster_bits); + } + } if (ret < 0) { goto fail; } diff --git a/tests/qemu-iotests/026.out b/tests/qemu-iotests/026.out index 86a50a2e13..8e89416a86 100644 --- a/tests/qemu-iotests/026.out +++ b/tests/qemu-iotests/026.out @@ -533,7 +533,7 @@ Failed to flush the L2 table cache: No space left on device Failed to flush the refcount block cache: No space left on device write failed: No space left on device -11 leaked clusters were found on the image. +10 leaked clusters were found on the image. This means waste of disk space, but no harm to data. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 @@ -561,7 +561,7 @@ Failed to flush the L2 table cache: No space left on device Failed to flush the refcount block cache: No space left on device write failed: No space left on device -11 leaked clusters were found on the image. +10 leaked clusters were found on the image. This means waste of disk space, but no harm to data. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 @@ -589,7 +589,7 @@ Failed to flush the L2 table cache: No space left on device Failed to flush the refcount block cache: No space left on device write failed: No space left on device -11 leaked clusters were found on the image. +10 leaked clusters were found on the image. This means waste of disk space, but no harm to data. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 diff --git a/tests/qemu-iotests/121 b/tests/qemu-iotests/121 index 1307b4e327..6d6f55a5dc 100755 --- a/tests/qemu-iotests/121 +++ b/tests/qemu-iotests/121 @@ -93,6 +93,26 @@ $QEMU_IO -c 'write 63M 130K' "$TEST_IMG" | _filter_qemu_io _check_test_img +echo +echo '=== Allocating a new refcount block must not leave holes in the image ===' +echo + +IMGOPTS='cluster_size=512,refcount_bits=16' _make_test_img 1M + +# This results in an image with 256 used clusters: the qcow2 header, +# the refcount table, one refcount block, the L1 table, four L2 tables +# and 248 data clusters +$QEMU_IO -c 'write 0 124k' "$TEST_IMG" | _filter_qemu_io + +# 256 clusters of 512 bytes each give us a 128K image +stat -c "size=%s (expected 131072)" $TEST_IMG + +# All 256 entries of the refcount block are used, so writing a new +# data cluster also allocates a new refcount block +$QEMU_IO -c 'write 124k 512' "$TEST_IMG" | _filter_qemu_io + +# Two more clusters, the image size should be 129K now +stat -c "size=%s (expected 132096)" $TEST_IMG # success, all done echo diff --git a/tests/qemu-iotests/121.out b/tests/qemu-iotests/121.out index 5961a44cd9..613d56185e 100644 --- a/tests/qemu-iotests/121.out +++ b/tests/qemu-iotests/121.out @@ -20,4 +20,14 @@ wrote 133120/133120 bytes at offset 66060288 130 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) No errors were found on the image. +=== Allocating a new refcount block must not leave holes in the image === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 +wrote 126976/126976 bytes at offset 0 +124 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +size=131072 (expected 131072) +wrote 512/512 bytes at offset 126976 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +size=132096 (expected 132096) + *** done -- cgit v1.2.3 From 61fa64871dcd823633aac129762d424a8a19e6ee Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 20 Mar 2018 15:08:00 +0100 Subject: vdi: Change 'static' create option to 'preallocation' in QMP What static=on really does is what we call metadata preallocation for other block drivers. While we can still change the QMP interface, make it more consistent by using 'preallocation' for VDI, too. This doesn't implement any new functionality, so the only supported preallocation modes are 'off' and 'metadata' for now. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/vdi.c | 24 ++++++++++++++++++++++-- qapi/block-core.json | 7 +++---- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/block/vdi.c b/block/vdi.c index d939b034c4..73c059e69d 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -728,7 +728,7 @@ static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options, int ret = 0; uint64_t bytes = 0; uint32_t blocks; - uint32_t image_type = VDI_TYPE_DYNAMIC; + uint32_t image_type; VdiHeader header; size_t i; size_t bmap_size; @@ -744,9 +744,22 @@ static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options, /* Validate options and set default values */ bytes = vdi_opts->size; - if (vdi_opts->q_static) { + + if (!vdi_opts->has_preallocation) { + vdi_opts->preallocation = PREALLOC_MODE_OFF; + } + switch (vdi_opts->preallocation) { + case PREALLOC_MODE_OFF: + image_type = VDI_TYPE_DYNAMIC; + break; + case PREALLOC_MODE_METADATA: image_type = VDI_TYPE_STATIC; + break; + default: + error_setg(errp, "Preallocation mode not supported for vdi"); + return -EINVAL; } + #ifndef CONFIG_VDI_STATIC_IMAGE if (image_type == VDI_TYPE_STATIC) { ret = -ENOTSUP; @@ -874,6 +887,7 @@ static int coroutine_fn vdi_co_create_opts(const char *filename, QemuOpts *opts, BlockdevCreateOptions *create_options = NULL; BlockDriverState *bs_file = NULL; uint64_t block_size = DEFAULT_CLUSTER_SIZE; + bool is_static = false; Visitor *v; Error *local_err = NULL; int ret; @@ -895,6 +909,9 @@ static int coroutine_fn vdi_co_create_opts(const char *filename, QemuOpts *opts, goto done; } #endif + if (qemu_opt_get_bool_del(opts, BLOCK_OPT_STATIC, false)) { + is_static = true; + } qdict = qemu_opts_to_qdict_filtered(opts, NULL, &vdi_create_opts, true); @@ -913,6 +930,9 @@ static int coroutine_fn vdi_co_create_opts(const char *filename, QemuOpts *opts, qdict_put_str(qdict, "driver", "vdi"); qdict_put_str(qdict, "file", bs_file->node_name); + if (is_static) { + qdict_put_str(qdict, "preallocation", "metadata"); + } /* Get the QAPI object */ v = qobject_input_visitor_new_keyval(QOBJECT(qdict)); diff --git a/qapi/block-core.json b/qapi/block-core.json index 1088ab0c78..c50517bff3 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -3943,16 +3943,15 @@ # # @file Node to create the image format on # @size Size of the virtual disk in bytes -# @static Whether to create a statically (true) or -# dynamically (false) allocated image -# (default: false, i.e. dynamic) +# @preallocation Preallocation mode for the new image (allowed values: off, +# metadata; default: off) # # Since: 2.12 ## { 'struct': 'BlockdevCreateOptionsVdi', 'data': { 'file': 'BlockdevRef', 'size': 'size', - '*static': 'bool' } } + '*preallocation': 'PreallocMode' } } ## # @BlockdevVhdxSubformat: -- cgit v1.2.3 From 95a14d51b24ea8adeb5d863517727916fee63f05 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 20 Mar 2018 14:41:53 +0100 Subject: vdi: Fix build with CONFIG_VDI_DEBUG Use qemu_uuid_unparse() instead of uuid_unparse() to make vdi.c compile again when CONFIG_VDI_DEBUG is set. In order to prevent future bitrot, replace '#ifdef CONFIG_VDI_DEBUG' by 'if (VDI_DEBUG)' so that the compiler always sees the code. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/vdi.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/block/vdi.c b/block/vdi.c index 73c059e69d..4a2d1ff88d 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -235,7 +235,6 @@ static void vdi_header_to_le(VdiHeader *header) qemu_uuid_bswap(&header->uuid_parent); } -#if defined(CONFIG_VDI_DEBUG) static void vdi_header_print(VdiHeader *header) { char uuid[37]; @@ -257,16 +256,15 @@ static void vdi_header_print(VdiHeader *header) logout("block extra 0x%04x\n", header->block_extra); logout("blocks tot. 0x%04x\n", header->blocks_in_image); logout("blocks all. 0x%04x\n", header->blocks_allocated); - uuid_unparse(header->uuid_image, uuid); + qemu_uuid_unparse(&header->uuid_image, uuid); logout("uuid image %s\n", uuid); - uuid_unparse(header->uuid_last_snap, uuid); + qemu_uuid_unparse(&header->uuid_last_snap, uuid); logout("uuid snap %s\n", uuid); - uuid_unparse(header->uuid_link, uuid); + qemu_uuid_unparse(&header->uuid_link, uuid); logout("uuid link %s\n", uuid); - uuid_unparse(header->uuid_parent, uuid); + qemu_uuid_unparse(&header->uuid_parent, uuid); logout("uuid parent %s\n", uuid); } -#endif static int coroutine_fn vdi_co_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix) @@ -387,9 +385,9 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags, } vdi_header_to_cpu(&header); -#if defined(CONFIG_VDI_DEBUG) - vdi_header_print(&header); -#endif + if (VDI_DEBUG) { + vdi_header_print(&header); + } if (header.disk_size > VDI_DISK_SIZE_MAX) { error_setg(errp, "Unsupported VDI image size (size is 0x%" PRIx64 @@ -825,9 +823,9 @@ static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options, qemu_uuid_generate(&header.uuid_image); qemu_uuid_generate(&header.uuid_last_snap); /* There is no need to set header.uuid_link or header.uuid_parent here. */ -#if defined(CONFIG_VDI_DEBUG) - vdi_header_print(&header); -#endif + if (VDI_DEBUG) { + vdi_header_print(&header); + } vdi_header_to_le(&header); ret = blk_pwrite(blk, offset, &header, sizeof(header), 0); if (ret < 0) { -- cgit v1.2.3 From b7de0777dcae2b39f9245cf600ab96e517d83fa3 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 20 Mar 2018 13:33:52 +0100 Subject: qemu-iotests: Test vdi image creation with QMP Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- tests/qemu-iotests/211 | 246 +++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/211.out | 97 ++++++++++++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 344 insertions(+) create mode 100755 tests/qemu-iotests/211 create mode 100644 tests/qemu-iotests/211.out diff --git a/tests/qemu-iotests/211 b/tests/qemu-iotests/211 new file mode 100755 index 0000000000..1edec26517 --- /dev/null +++ b/tests/qemu-iotests/211 @@ -0,0 +1,246 @@ +#!/bin/bash +# +# Test VDI and file image creation +# +# Copyright (C) 2018 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# creator +owner=kwolf@redhat.com + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +status=1 # failure is the default! + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +_supported_fmt vdi +_supported_proto file +_supported_os Linux + +function do_run_qemu() +{ + echo Testing: "$@" + $QEMU -nographic -qmp stdio -serial none "$@" + echo +} + +function run_qemu() +{ + do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp \ + | _filter_qemu | _filter_imgfmt \ + | _filter_actual_image_size +} + +echo +echo "=== Successful image creation (defaults) ===" +echo + +size=$((128 * 1024 * 1024)) + +run_qemu < Date: Tue, 20 Mar 2018 15:31:06 +0100 Subject: qemu-iotests: Enable 025 for luks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We want to test resizing even for luks. The only change that is needed is to explicitly zero out new space for luks because it's undefined. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Daniel P. Berrangé --- tests/qemu-iotests/025 | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/025 b/tests/qemu-iotests/025 index f5e672e6b3..70dd5f10aa 100755 --- a/tests/qemu-iotests/025 +++ b/tests/qemu-iotests/025 @@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.filter . ./common.pattern -_supported_fmt raw qcow2 qed +_supported_fmt raw qcow2 qed luks _supported_proto file sheepdog rbd nfs _supported_os Linux @@ -62,6 +62,13 @@ length EOF _check_test_img +# bdrv_truncate() doesn't zero the new space, so we need to do that explicitly. +# We still want to test automatic zeroing for other formats even though +# bdrv_truncate() doesn't guarantee it. +if [ "$IMGFMT" == "luks" ]; then + $QEMU_IO -c "write -z $small_size $((big_size - small_size))" "$TEST_IMG" > /dev/null +fi + echo echo "=== Verifying image size after reopen" $QEMU_IO -c "length" "$TEST_IMG" -- cgit v1.2.3 From 120bc742c028419c25ed7aa49497b490ce96536c Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 20 Mar 2018 16:38:51 +0100 Subject: luks: Turn another invalid assertion into check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit e39e959e fixed an invalid assertion in the .bdrv_length implementation, but left a similar assertion in place for .bdrv_truncate. Instead of crashing when the user requests a too large image size, fail gracefully. A file size of exactly INT64_MAX caused failure before, but is actually legal. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Daniel P. Berrangé --- block/crypto.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/block/crypto.c b/block/crypto.c index e0b8856f74..bc6c7e3795 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -357,7 +357,11 @@ static int block_crypto_truncate(BlockDriverState *bs, int64_t offset, BlockCrypto *crypto = bs->opaque; uint64_t payload_offset = qcrypto_block_get_payload_offset(crypto->block); - assert(payload_offset < (INT64_MAX - offset)); + + if (payload_offset > INT64_MAX - offset) { + error_setg(errp, "The requested file size is too large"); + return -EFBIG; + } offset += payload_offset; -- cgit v1.2.3 From 50880f25c88f2a629bd68a5fb1a46aa9bf0a2543 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 20 Mar 2018 16:42:12 +0100 Subject: qemu-iotests: Test invalid resize on luks This tests that the .bdrv_truncate implementation for luks doesn't crash for invalid image sizes. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- tests/qemu-iotests/210 | 37 +++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/210.out | 16 ++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/tests/qemu-iotests/210 b/tests/qemu-iotests/210 index 96a5213e77..e607c0d296 100755 --- a/tests/qemu-iotests/210 +++ b/tests/qemu-iotests/210 @@ -204,6 +204,43 @@ run_qemu -blockdev driver=file,filename="$TEST_IMG_FILE",node-name=node0 \ { "execute": "quit" } EOF +echo +echo "=== Resize image with invalid sizes ===" +echo + +run_qemu -blockdev driver=file,filename="$TEST_IMG_FILE",node-name=node0 \ + -blockdev driver=luks,file=node0,key-secret=keysec0,node-name=node1 \ + -object secret,id=keysec0,data="foo" <0 size"}} +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}} + +image: json:{"driver": "IMGFMT", "file": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "key-secret": "keysec0"} +file format: IMGFMT +virtual size: 0 (0 bytes) *** done -- cgit v1.2.3 From 2332d82589ef9e9f7e065ec1f759a2c164ad4932 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 20 Mar 2018 17:07:58 +0100 Subject: parallels: Check maximum cluster size on create It's unclear what the real maximum cluster size is for the Parallels format, but let's at least make sure that we don't get integer overflows in our .bdrv_co_create implementation. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/parallels.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/block/parallels.c b/block/parallels.c index e2515dec81..799215e079 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -526,6 +526,11 @@ static int coroutine_fn parallels_co_create(BlockdevCreateOptions* opts, cl_size = DEFAULT_CLUSTER_SIZE; } + /* XXX What is the real limit here? This is an insanely large maximum. */ + if (cl_size >= INT64_MAX / MAX_PARALLELS_IMAGE_FACTOR) { + error_setg(errp, "Cluster size is too large"); + return -EINVAL; + } if (total_size >= MAX_PARALLELS_IMAGE_FACTOR * cl_size) { error_setg(errp, "Image size is too large for this cluster size"); return -E2BIG; -- cgit v1.2.3 From e8f6ea6fb6c1fd9a778abebc66806203f2bf667b Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 20 Mar 2018 13:33:52 +0100 Subject: qemu-iotests: Test parallels image creation with QMP Signed-off-by: Kevin Wolf --- tests/qemu-iotests/212 | 326 +++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/212.out | 111 +++++++++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 438 insertions(+) create mode 100755 tests/qemu-iotests/212 create mode 100644 tests/qemu-iotests/212.out diff --git a/tests/qemu-iotests/212 b/tests/qemu-iotests/212 new file mode 100755 index 0000000000..e5a1ba77ce --- /dev/null +++ b/tests/qemu-iotests/212 @@ -0,0 +1,326 @@ +#!/bin/bash +# +# Test parallels and file image creation +# +# Copyright (C) 2018 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# creator +owner=kwolf@redhat.com + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +status=1 # failure is the default! + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +_supported_fmt parallels +_supported_proto file +_supported_os Linux + +function do_run_qemu() +{ + echo Testing: "$@" + $QEMU -nographic -qmp stdio -serial none "$@" + echo +} + +function run_qemu() +{ + do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp \ + | _filter_qemu | _filter_imgfmt \ + | _filter_actual_image_size +} + +echo +echo "=== Successful image creation (defaults) ===" +echo + +size=$((128 * 1024 * 1024)) + +run_qemu < Date: Tue, 20 Mar 2018 18:09:15 +0100 Subject: vhdx: Require power-of-two block size on create Images with a non-power-of-two block size are invalid and cannot be opened. Reject such block sizes when creating an image. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Jeff Cody --- block/vhdx.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/block/vhdx.c b/block/vhdx.c index d2c54b7891..6a5e48eb69 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -1877,6 +1877,10 @@ static int coroutine_fn vhdx_co_create(BlockdevCreateOptions *opts, error_setg_errno(errp, EINVAL, "Block size must be a multiple of 1 MB"); return -EINVAL; } + if (!is_power_of_2(block_size)) { + error_setg(errp, "Block size must be a power of two"); + return -EINVAL; + } if (block_size > VHDX_BLOCK_SIZE_MAX) { error_setg_errno(errp, EINVAL, "Block size must not exceed %d", VHDX_BLOCK_SIZE_MAX); -- cgit v1.2.3 From 0fcc38e7d0fb102ab9e9a9734459bdff89d3121f Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 20 Mar 2018 18:11:09 +0100 Subject: vhdx: Don't use error_setg_errno() with constant errno error_setg_errno() is meant for cases where we got an errno from the OS that can add useful extra information to an error message. It's pointless if we pass a constant errno, these cases should use plain error_setg(). Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Jeff Cody --- block/vhdx.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/block/vhdx.c b/block/vhdx.c index 6a5e48eb69..4a087d8708 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -1822,7 +1822,7 @@ static int coroutine_fn vhdx_co_create(BlockdevCreateOptions *opts, /* Validate options and set default values */ image_size = vhdx_opts->size; if (image_size > VHDX_MAX_IMAGE_SIZE) { - error_setg_errno(errp, EINVAL, "Image size too large; max of 64TB"); + error_setg(errp, "Image size too large; max of 64TB"); return -EINVAL; } @@ -1832,7 +1832,7 @@ static int coroutine_fn vhdx_co_create(BlockdevCreateOptions *opts, log_size = vhdx_opts->log_size; } if (log_size < MiB || (log_size % MiB) != 0) { - error_setg_errno(errp, EINVAL, "Log size must be a multiple of 1 MB"); + error_setg(errp, "Log size must be a multiple of 1 MB"); return -EINVAL; } @@ -1874,7 +1874,7 @@ static int coroutine_fn vhdx_co_create(BlockdevCreateOptions *opts, } if (block_size < MiB || (block_size % MiB) != 0) { - error_setg_errno(errp, EINVAL, "Block size must be a multiple of 1 MB"); + error_setg(errp, "Block size must be a multiple of 1 MB"); return -EINVAL; } if (!is_power_of_2(block_size)) { @@ -1882,8 +1882,7 @@ static int coroutine_fn vhdx_co_create(BlockdevCreateOptions *opts, return -EINVAL; } if (block_size > VHDX_BLOCK_SIZE_MAX) { - error_setg_errno(errp, EINVAL, "Block size must not exceed %d", - VHDX_BLOCK_SIZE_MAX); + error_setg(errp, "Block size must not exceed %d", VHDX_BLOCK_SIZE_MAX); return -EINVAL; } -- cgit v1.2.3 From 6f16f7c5625fde5634cc4c40857b66cdee09dc17 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 20 Mar 2018 18:24:54 +0100 Subject: vhdx: Check for 4 GB maximum log size on creation It's unclear what the real maximum is, but we use an uint32_t to store the log size in vhdx_co_create(), so we should check that the given value fits in 32 bits. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Jeff Cody --- block/vhdx.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/block/vhdx.c b/block/vhdx.c index 4a087d8708..6ac0424f61 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -1829,6 +1829,10 @@ static int coroutine_fn vhdx_co_create(BlockdevCreateOptions *opts, if (!vhdx_opts->has_log_size) { log_size = DEFAULT_LOG_SIZE; } else { + if (vhdx_opts->log_size > UINT32_MAX) { + error_setg(errp, "Log size must be smaller than 4 GB"); + return -EINVAL; + } log_size = vhdx_opts->log_size; } if (log_size < MiB || (log_size % MiB) != 0) { -- cgit v1.2.3 From 0b7e7f66813a7e346e12d47be977a32a530a6316 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 20 Mar 2018 13:33:52 +0100 Subject: qemu-iotests: Test vhdx image creation with QMP Signed-off-by: Kevin Wolf --- tests/qemu-iotests/213 | 349 +++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/213.out | 121 ++++++++++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 471 insertions(+) create mode 100755 tests/qemu-iotests/213 create mode 100644 tests/qemu-iotests/213.out diff --git a/tests/qemu-iotests/213 b/tests/qemu-iotests/213 new file mode 100755 index 0000000000..3a00a0f6d6 --- /dev/null +++ b/tests/qemu-iotests/213 @@ -0,0 +1,349 @@ +#!/bin/bash +# +# Test vhdx and file image creation +# +# Copyright (C) 2018 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# creator +owner=kwolf@redhat.com + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +status=1 # failure is the default! + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +_supported_fmt vhdx +_supported_proto file +_supported_os Linux + +function do_run_qemu() +{ + echo Testing: "$@" + $QEMU -nographic -qmp stdio -serial none "$@" + echo +} + +function run_qemu() +{ + do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp \ + | _filter_qemu | _filter_imgfmt \ + | _filter_actual_image_size +} + +echo +echo "=== Successful image creation (defaults) ===" +echo + +size=$((128 * 1024 * 1024)) + +run_qemu <