block: Follow-up for .bdrv_co_create (part 1)
v2 cover block: Follow-up for .bdrv_co_create (part 1)
This series adds qemu-iotests for a few more block drivers (yet more to come in another series) and fixes a few things that previous review and these tests brought up.
The only major design change is that I converted the vdi block driver from a boolean 'static' create option to the standard 'preallocation' one that other drivers are using. This seems like a good move to make while the interface isn't stable yet.
v2:
- Patch 1: Mention allowed values for 'preallocation' [Eric]
- Patch 3: Fixed comments, removed 2^63-512 case [Eric]
- Patch 6: Added missing reference output change [Eric]
- Patch 7: s/UINT64_MAX/INT64_MAX/ [Eric]
- Patches 8 and 12: Fixed comments [Eric]
Kevin Wolf (12):
vdi: Change 'static' create option to 'preallocation' in QMP
vdi: Fix build with CONFIG_VDI_DEBUG
qemu-iotests: Test vdi image creation with QMP
qemu-iotests: Enable 025 for luks
luks: Turn another invalid assertion into check
qemu-iotests: Test invalid resize on luks
parallels: Check maximum cluster size on create
qemu-iotests: Test parallels image creation with QMP
vhdx: Require power-of-two block size on create
vhdx: Don't use error_setg_errno() with constant errno
vhdx: Check for 4 GB maximum log size on creation
qemu-iotests: Test vhdx image creation with QMP
qapi/block-core.json | 7 +- block/crypto.c | 6 +- block/parallels.c | 5 + block/vdi.c | 46 ++++-- block/vhdx.c | 17 ++- tests/qemu-iotests/025 | 9 +- tests/qemu-iotests/210 | 37 +++++ tests/qemu-iotests/210.out | 16 +++ tests/qemu-iotests/211 | 246 ++++++++++++++++++++++++++++++++ tests/qemu-iotests/211.out | 97 +++++++++++++ tests/qemu-iotests/212 | 326 ++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/212.out | 111 ++++++++++++++ tests/qemu-iotests/213 | 349 +++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/213.out | 121 ++++++++++++++++ tests/qemu-iotests/group | 3 + 15 files changed, 1371 insertions(+), 25 deletions(-) create mode 100755 tests/qemu-iotests/211 create mode 100644 tests/qemu-iotests/211.out create mode 100755 tests/qemu-iotests/212 create mode 100644 tests/qemu-iotests/212.out create mode 100755 tests/qemu-iotests/213 create mode 100644 tests/qemu-iotests/213.out
Signature · 1 line
2.13.6
PATCH v2 1/12 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.
--- qapi/block-core.json | 7 +++---- block/vdi.c | 24 ++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 6 deletions(-)
qapi/block-core.json +3 -4
@@ -3834,16 +3834,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, +# full; default: off) # # Since: 2.12 ## { 'struct': 'BlockdevCreateOptionsVdi', 'data': { 'file': 'BlockdevRef', 'size': 'size', - '*static': 'bool' } } + '*preallocation': 'PreallocMode' } } ## # @BlockdevVhdxSubformat:
block/vdi.c +22 -2
@@ -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));
Signature · 1 line
2.13.6
Re: [Qemu-devel] [PATCH for-2.12 v2 01/12] vdi: Change 'static' create option to 'preallocation' in QMP
Quoted 6 lines · Kevin Wolf
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.
Disagrees with:
qapi/block-core.json line 3838
Quoted 9 lines
Signed-off-by Kevin Wolf <[email protected]>--- qapi/block-core.json | 7 +++---- block/vdi.c | 24 ++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 6 deletions(-)qapi/block-core.json +2
-0+# @preallocation Preallocation mode for the new image (allowed values: off, +# full; default: off)
this.
block/vdi.c static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options, · line 759
Quoted 12 lines
block/vdi.c +11
-0+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" );
but matches this. Fix your QMP documentation with s/full/metadata/ and you can have:
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
PATCH v2 2/12 vdi: Fix build with CONFIG_VDI_DEBUG
In reply to Kevin WolfUse 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.
--- block/vdi.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-)
block/vdi.c +10 -12
@@ -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 ) {
Signature · 1 line
2.13.6
PATCH v2 3/12 qemu-iotests: Test vdi image creation with QMP
In reply to Kevin Wolf--- 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
tests/qemu-iotests/211 new file +246 -0
@@ -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 <http://www.gnu.org/licenses/>. +# + +# creator [email protected] + +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 <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "file", + "filename": "$TEST_IMG", + "size": 0 + } +} +{ "execute": "blockdev-add", + "arguments": { + "driver": "file", + "node-name": "imgfile", + "filename": "$TEST_IMG" + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "imgfile", + "size": $size + } +} +{ "execute": "quit" } +EOF + +_img_info --format-specific | _filter_img_info --format-specific +$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map + +echo +echo "=== Successful image creation (explicit defaults) ===" +echo + +# Choose a different size to show that we got a new image +size=$((64 * 1024 * 1024)) + +run_qemu <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "file", + "filename": "$TEST_IMG", + "size": 0 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": { + "driver": "file", + "filename": "$TEST_IMG" + }, + "size": $size, + "preallocation": "off" + } +} +{ "execute": "quit" } +EOF + +_img_info --format-specific | _filter_img_info --format-specific +$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map + +echo +echo "=== Successful image creation (with non-default options) ===" +echo + +# Choose a different size to show that we got a new image +size=$((32 * 1024 * 1024)) + +run_qemu <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "file", + "filename": "$TEST_IMG", + "size": 0 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": { + "driver": "file", + "filename": "$TEST_IMG" + }, + "size": $size, + "preallocation": "metadata" + } +} +{ "execute": "quit" } +EOF + +_img_info --format-specific | _filter_img_info --format-specific +$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map + +echo +echo "=== Invalid BlockdevRef ===" +echo + +run_qemu <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "this doesn't exist", + "size": $size + } +} +{ "execute": "quit" } +EOF + +echo +echo "=== Zero size ===" +echo + +run_qemu -blockdev driver=file,filename="$TEST_IMG",node-name=node0 <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 0 + } +} +{ "execute": "quit" } +EOF + +_img_info | _filter_img_info + +echo +echo "=== Maximum size ===" +echo + +run_qemu -blockdev driver=file,filename="$TEST_IMG",node-name=node0 <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 562949819203584 + } +} +{ "execute": "quit" } +EOF + +_img_info | _filter_img_info + +echo +echo "=== Invalid sizes ===" +echo + +# TODO Negative image sizes aren't handled correctly, but this is a problem +# with QAPI's implementation of the 'size' type and affects other commands as +# well. Once this is fixed, we may want to add a test case here. + +# 1. 2^64 - 512 +# 2. 2^63 = 8 EB (qemu-img enforces image sizes less than this) +# 3. 0x1fffff8000001 (one byte more than maximum image size for VDI) + +run_qemu -blockdev driver=file,filename="$TEST_IMG",node-name=node0 <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 18446744073709551104 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 9223372036854775808 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 562949819203585 + } +} +{ "execute": "quit" } +EOF + +# success, all done +echo "*** done" +rm -f $seq.full +status=0
tests/qemu-iotests/211.out new file +97 -0
97 lines not on this page · raw message
tests/qemu-iotests/group +1 -0
@@ -209,3 +209,4 @@ 208 rw auto quick 209 rw auto quick 210 rw auto +211 rw auto quick
Signature · 1 line
2.13.6
Re: [Qemu-devel] [PATCH for-2.12 v2 03/12] qemu-iotests: Test vdi image creation with QMP
Quoted 9 lines · Kevin Wolf
Signed-off-by Kevin Wolf <[email protected]>--- 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
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
PATCH v2 4/12 qemu-iotests: Enable 025 for luks
In reply to Kevin WolfWe 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.
--- tests/qemu-iotests/025 | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
tests/qemu-iotests/025 +8 -1
@@ -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"
Signature · 1 line
2.13.6
PATCH v2 5/12 luks: Turn another invalid assertion into check
In reply to Kevin WolfCommit 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.
--- block/crypto.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
block/crypto.c +5 -1
@@ -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;
Signature · 1 line
2.13.6
PATCH v2 6/12 qemu-iotests: Test invalid resize on luks
In reply to Kevin WolfThis tests that the .bdrv_truncate implementation for luks doesn't crash for invalid image sizes.
--- tests/qemu-iotests/210 | 37 +++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/210.out | 16 ++++++++++++++++ 2 files changed, 53 insertions(+)
tests/qemu-iotests/210 +37 -0
@@ -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" <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "block_resize", + "arguments": { + "node-name": "node1", + "size": 9223372036854775296 + } +} +{ "execute": "block_resize", + "arguments": { + "node-name": "node1", + "size": 9223372036854775808 + } +} +{ "execute": "block_resize", + "arguments": { + "node-name": "node1", + "size": 18446744073709551104 + } +} +{ "execute": "block_resize", + "arguments": { + "node-name": "node1", + "size": -9223372036854775808 + } +} +{ "execute": "quit" } +EOF + +_img_info | _filter_img_info + # success, all done echo "*** done" rm -f $seq.full
tests/qemu-iotests/210.out +16 -0
@@ -133,4 +133,20 @@ QMP_VERSION {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}} + +=== Resize image with invalid sizes === + +Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0 -blockdev driver=IMGFMT,file=node0,key-secret=keysec0,node-name=node1 -object secret,id=keysec0,data=foo +QMP_VERSION +{"return": {}} +{"error": {"class": "GenericError", "desc": "The requested file size is too large"}} +{"error": {"class": "GenericError", "desc": "Invalid parameter type for 'size', expected: integer"}} +{"error": {"class": "GenericError", "desc": "Invalid parameter type for 'size', expected: integer"}} +{"error": {"class": "GenericError", "desc": "Parameter 'size' expects a >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
Signature · 1 line
2.13.6
Re: [Qemu-devel] [PATCH for-2.12 v2 06/12] qemu-iotests: Test invalid resize on luks
Quoted 9 lines · Kevin Wolf
This tests that the .bdrv_truncate implementation for luks doesn't crash for invalid image sizes.
Signed-off-by Kevin Wolf <[email protected]>--- tests/qemu-iotests/210 | 37 +++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/210.out | 16 ++++++++++++++++ 2 files changed, 53 insertions(+)
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
PATCH v2 7/12 parallels: Check maximum cluster size on create
In reply to Kevin WolfIt'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.
--- block/parallels.c | 5 +++++ 1 file changed, 5 insertions(+)
block/parallels.c +5 -0
@@ -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 ;
Signature · 1 line
2.13.6
PATCH v2 8/12 qemu-iotests: Test parallels image creation with QMP
In reply to 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
tests/qemu-iotests/212 new file +326 -0
@@ -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 <http://www.gnu.org/licenses/>. +# + +# creator [email protected] + +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 <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "file", + "filename": "$TEST_IMG", + "size": 0 + } +} +{ "execute": "blockdev-add", + "arguments": { + "driver": "file", + "node-name": "imgfile", + "filename": "$TEST_IMG" + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "imgfile", + "size": $size + } +} +{ "execute": "quit" } +EOF + +_img_info --format-specific | _filter_img_info --format-specific + +echo +echo "=== Successful image creation (explicit defaults) ===" +echo + +# Choose a different size to show that we got a new image +size=$((64 * 1024 * 1024)) + +run_qemu <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "file", + "filename": "$TEST_IMG", + "size": 0 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": { + "driver": "file", + "filename": "$TEST_IMG" + }, + "size": $size, + "cluster-size": 1048576 + } +} +{ "execute": "quit" } +EOF + +_img_info --format-specific | _filter_img_info --format-specific + +echo +echo "=== Successful image creation (with non-default options) ===" +echo + +# Choose a different size to show that we got a new image +size=$((32 * 1024 * 1024)) + +run_qemu <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "file", + "filename": "$TEST_IMG", + "size": 0 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": { + "driver": "file", + "filename": "$TEST_IMG" + }, + "size": $size, + "cluster-size": 65536 + } +} +{ "execute": "quit" } +EOF + +_img_info --format-specific | _filter_img_info --format-specific + +echo +echo "=== Invalid BlockdevRef ===" +echo + +run_qemu <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "this doesn't exist", + "size": $size + } +} +{ "execute": "quit" } +EOF + +echo +echo "=== Zero size ===" +echo + +run_qemu -blockdev driver=file,filename="$TEST_IMG",node-name=node0 <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 0 + } +} +{ "execute": "quit" } +EOF + +_img_info | _filter_img_info + +echo +echo "=== Maximum size ===" +echo + +run_qemu -blockdev driver=file,filename="$TEST_IMG",node-name=node0 <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 4503599627369984 + } +} +{ "execute": "quit" } +EOF + +_img_info | _filter_img_info + +echo +echo "=== Invalid sizes ===" +echo + +# TODO Negative image sizes aren't handled correctly, but this is a problem +# with QAPI's implementation of the 'size' type and affects other commands as +# well. Once this is fixed, we may want to add a test case here. + +# 1. Misaligned image size +# 2. 2^64 - 512 +# 3. 2^63 = 8 EB (qemu-img enforces image sizes less than this) +# 4. 2^63 - 512 (generally valid, but with the image header the file will +# exceed 63 bits) +# 5. 2^52 (512 bytes more than maximum image size) + +run_qemu -blockdev driver=file,filename="$TEST_IMG",node-name=node0 <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 1234 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 18446744073709551104 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 9223372036854775808 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 9223372036854775296 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 4503599627370497 + } +} +{ "execute": "quit" } +EOF + +echo +echo "=== Invalid cluster size ===" +echo + +run_qemu -blockdev driver=file,filename="$TEST_IMG",node-name=node0 <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 67108864, + "cluster-size": 1234 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 67108864, + "cluster-size": 128 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 67108864, + "cluster-size": 4294967296 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 67108864, + "cluster-size": 9223372036854775808 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 67108864, + "cluster-size": 18446744073709551104 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 67108864, + "cluster-size": 0 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 281474976710656, + "cluster-size": 512 + } +} +{ "execute": "quit" } +EOF + + +# success, all done +echo "*** done" +rm -f $seq.full +status=0
tests/qemu-iotests/212.out new file +111 -0
111 lines not on this page · raw message
tests/qemu-iotests/group +1 -0
@@ -210,3 +210,4 @@ 209 rw auto quick 210 rw auto 211 rw auto quick +212 rw auto quick
Signature · 1 line
2.13.6
PATCH v2 9/12 vhdx: Require power-of-two block size on create
In reply to Kevin WolfImages with a non-power-of-two block size are invalid and cannot be opened. Reject such block sizes when creating an image.
--- block/vhdx.c | 4 ++++ 1 file changed, 4 insertions(+)
block/vhdx.c +4 -0
@@ -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 );
Signature · 1 line
2.13.6
PATCH v2 10/12 vhdx: Don't use error_setg_errno() with constant errno
In reply to Kevin Wolferror_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().
--- block/vhdx.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
block/vhdx.c +4 -5
@@ -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 ; }
Signature · 1 line
2.13.6
PATCH v2 11/12 vhdx: Check for 4 GB maximum log size on creation
In reply to Kevin WolfIt'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.
--- block/vhdx.c | 4 ++++ 1 file changed, 4 insertions(+)
block/vhdx.c +4 -0
@@ -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 ) {
Signature · 1 line
2.13.6
PATCH v2 12/12 qemu-iotests: Test vhdx image creation with QMP
In reply to 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
tests/qemu-iotests/213 new file +349 -0
@@ -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 <http://www.gnu.org/licenses/>. +# + +# creator [email protected] + +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 <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "file", + "filename": "$TEST_IMG", + "size": 0 + } +} +{ "execute": "blockdev-add", + "arguments": { + "driver": "file", + "node-name": "imgfile", + "filename": "$TEST_IMG" + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "imgfile", + "size": $size + } +} +{ "execute": "quit" } +EOF + +_img_info --format-specific | _filter_img_info --format-specific + +echo +echo "=== Successful image creation (explicit defaults) ===" +echo + +# Choose a different size to show that we got a new image +size=$((64 * 1024 * 1024)) + +run_qemu <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "file", + "filename": "$TEST_IMG", + "size": 0 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": { + "driver": "file", + "filename": "$TEST_IMG" + }, + "size": $size, + "log-size": 1048576, + "block-size": 8388608, + "subformat": "dynamic", + "block-state-zero": true + } +} +{ "execute": "quit" } +EOF + +_img_info --format-specific | _filter_img_info --format-specific + +echo +echo "=== Successful image creation (with non-default options) ===" +echo + +# Choose a different size to show that we got a new image +size=$((32 * 1024 * 1024)) + +run_qemu <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "file", + "filename": "$TEST_IMG", + "size": 0 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": { + "driver": "file", + "filename": "$TEST_IMG" + }, + "size": $size, + "log-size": 8388608, + "block-size": 268435456, + "subformat": "fixed", + "block-state-zero": false + } +} +{ "execute": "quit" } +EOF + +_img_info --format-specific | _filter_img_info --format-specific + +echo +echo "=== Invalid BlockdevRef ===" +echo + +run_qemu <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "this doesn't exist", + "size": $size + } +} +{ "execute": "quit" } +EOF + +echo +echo "=== Zero size ===" +echo + +run_qemu -blockdev driver=file,filename="$TEST_IMG",node-name=node0 <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 0 + } +} +{ "execute": "quit" } +EOF + +_img_info | _filter_img_info + +echo +echo "=== Maximum size ===" +echo + +run_qemu -blockdev driver=file,filename="$TEST_IMG",node-name=node0 <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 70368744177664 + } +} +{ "execute": "quit" } +EOF + +_img_info | _filter_img_info + +echo +echo "=== Invalid sizes ===" +echo + +# TODO Negative image sizes aren't handled correctly, but this is a problem +# with QAPI's implementation of the 'size' type and affects other commands as +# well. Once this is fixed, we may want to add a test case here. + +# 1. 2^64 - 512 +# 2. 2^63 = 8 EB (qemu-img enforces image sizes less than this) +# 3. 2^63 - 512 (generally valid, but with the image header the file will +# exceed 63 bits) +# 4. 2^46 + 1 (one byte more than maximum image size) + +run_qemu -blockdev driver=file,filename="$TEST_IMG",node-name=node0 <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 18446744073709551104 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 9223372036854775808 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 9223372036854775296 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 70368744177665 + } +} +{ "execute": "quit" } +EOF + +echo +echo "=== Invalid block size ===" +echo + +run_qemu -blockdev driver=file,filename="$TEST_IMG",node-name=node0 <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 67108864, + "block-size": 1234567 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 67108864, + "block-size": 128 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 67108864, + "block-size": 3145728 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 67108864, + "block-size": 536870912 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 67108864, + "block-size": 0 + } +} +{ "execute": "quit" } +EOF + +echo +echo "=== Invalid log size ===" +echo + +run_qemu -blockdev driver=file,filename="$TEST_IMG",node-name=node0 <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 67108864, + "log-size": 1234567 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 67108864, + "log-size": 128 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 67108864, + "log-size": 4294967296 + } +} +{ "execute": "x-blockdev-create", + "arguments": { + "driver": "$IMGFMT", + "file": "node0", + "size": 67108864, + "log-size": 0 + } +} +{ "execute": "quit" } +EOF + + +# success, all done +echo "*** done" +rm -f $seq.full +status=0
tests/qemu-iotests/213.out new file +121 -0
121 lines not on this page · raw message
tests/qemu-iotests/group +1 -0
@@ -211,3 +211,4 @@ 210 rw auto 211 rw auto quick 212 rw auto quick +213 rw auto quick
Signature · 1 line
2.13.6
Kevin Wolf
AppliedAm 21.03.2018 um 18:37 hat Kevin Wolf geschrieben:
Quoted 9 lines
This series adds qemu-iotests for a few more block drivers (yet more to come in another series) and fixes a few things that previous review and these tests brought up.
The only major design change is that I converted the vdi block driver from a boolean 'static' create option to the standard 'preallocation' one that other drivers are using. This seems like a good move to make while the interface isn't stable yet.
Quoted 6 lines
v2:
- Patch 1: Mention allowed values for 'preallocation' [Eric]
- Patch 3: Fixed comments, removed 2^63-512 case [Eric]
- Patch 6: Added missing reference output change [Eric]
- Patch 7: s/UINT64_MAX/INT64_MAX/ [Eric]
- Patches 8 and 12: Fixed comments [Eric]
Applied to the block branch.
Kevin