[Qemu-devel] [PATCH for-2.12 00/12] block: Follow-up for .bdrv_co_create (part 1)

[Qemu-devel] [PATCH for-2.12 00/12] block: Follow-up for .bdrv_co_create (part 1)

[Kevin Wolf]

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.

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       |   6 +-
 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/211     | 256 +++++++++++++++++++++++++++++++++
 tests/qemu-iotests/211.out |  98 +++++++++++++
 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 +
 14 files changed, 1365 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

-- 
2.13.6

[Qemu-devel] [PATCH for-2.12 01/12] vdi: Change 'static' create option to 'preallocation' in QMP

[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.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/block-core.json |  6 ++----
 block/vdi.c          | 24 ++++++++++++++++++++++--
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5b0ad1a8b7..f6673f2b13 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3834,16 +3834,14 @@
 #
 # @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 (default: off)
 #
 # Since: 2.12
 ##
 { 'struct': 'BlockdevCreateOptionsVdi',
   'data': { 'file':             'BlockdevRef',
             'size':             'size',
-            '*static':          'bool' } }
+            '*preallocation':   'PreallocMode' } }
 
 ##
 # @BlockdevVhdxSubformat:
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));
-- 
2.13.6

[Qemu-devel] [PATCH for-2.12 02/12] vdi: Fix build with CONFIG_VDI_DEBUG

[Kevin Wolf]

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 <kwolf@redhat.com>
---
 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) {
-- 
2.13.6

[Qemu-devel] [PATCH for-2.12 03/12] qemu-iotests: Test vdi image creation with QMP

[Kevin Wolf]

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/211     | 256 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/211.out |  98 +++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 355 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..6bf1ca784c
--- /dev/null
+++ b/tests/qemu-iotests/211
@@ -0,0 +1,256 @@
+#!/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
+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 <<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. 2^63 - 512 (generally valid, but with the crypto header the file will
+#                exceed 63 bits)
+# 4. 0x1fffff8000000 (maximum image size for VDI)
+# 5. (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": 9223372036854775296
+  }
+}
+{ "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
diff --git a/tests/qemu-iotests/211.out b/tests/qemu-iotests/211.out
new file mode 100644
index 0000000000..a570cec04f
--- /dev/null
+++ b/tests/qemu-iotests/211.out
@@ -0,0 +1,98 @@
+QA output created by 211
+
+=== Successful image creation (defaults) ===
+
+Testing:
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 128M (134217728 bytes)
+[{ "start": 0, "length": 134217728, "depth": 0, "zero": true, "data": false}]
+
+=== Successful image creation (explicit defaults) ===
+
+Testing:
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 64M (67108864 bytes)
+[{ "start": 0, "length": 67108864, "depth": 0, "zero": true, "data": false}]
+
+=== Successful image creation (with non-default options) ===
+
+Testing:
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 32M (33554432 bytes)
+[{ "start": 0, "length": 3072, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 3072, "length": 33551360, "depth": 0, "zero": true, "data": true, "offset": OFFSET}]
+
+=== Invalid BlockdevRef ===
+
+Testing:
+QMP_VERSION
+{"return": {}}
+{"error": {"class": "GenericError", "desc": "Cannot find device=this doesn't exist nor node_name=this doesn't exist"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+
+=== Zero size ===
+
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 0 (0 bytes)
+
+=== Maximum size ===
+
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 512T (562949819203584 bytes)
+
+=== Invalid sizes ===
+
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0
+QMP_VERSION
+{"return": {}}
+{"error": {"class": "GenericError", "desc": "Unsupported VDI image size (size is 0xfffffffffffffe00, max supported is 0x1fffff8000000)"}}
+{"error": {"class": "GenericError", "desc": "Unsupported VDI image size (size is 0x8000000000000000, max supported is 0x1fffff8000000)"}}
+{"error": {"class": "GenericError", "desc": "Unsupported VDI image size (size is 0x7ffffffffffffe00, max supported is 0x1fffff8000000)"}}
+{"error": {"class": "GenericError", "desc": "Unsupported VDI image size (size is 0x1fffff8000001, max supported is 0x1fffff8000000)"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 0c2983f4cd..0d11d40aec 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -209,3 +209,4 @@
 208 rw auto quick
 209 rw auto quick
 210 rw auto
+211 rw auto quick
-- 
2.13.6

[Qemu-devel] [PATCH for-2.12 04/12] qemu-iotests: Enable 025 for luks

[Kevin Wolf]

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 <kwolf@redhat.com>
---
 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"
-- 
2.13.6

[Qemu-devel] [PATCH for-2.12 05/12] luks: Turn another invalid assertion into check

[Kevin Wolf]

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 <kwolf@redhat.com>
---
 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;
 
-- 
2.13.6

[Qemu-devel] [PATCH for-2.12 06/12] qemu-iotests: Test invalid resize on luks

[Kevin Wolf]

This tests that the .bdrv_truncate implementation for luks doesn't crash
for invalid image sizes.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/210 | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 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" <<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
-- 
2.13.6

[Qemu-devel] [PATCH for-2.12 07/12] parallels: Check maximum cluster size on create

[Kevin Wolf]

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 <kwolf@redhat.com>
---
 block/parallels.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/block/parallels.c b/block/parallels.c
index 2da5e56a9d..e4ca018c2e 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 >= UINT64_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;
-- 
2.13.6

[Qemu-devel] [PATCH for-2.12 08/12] qemu-iotests: Test parallels image creation with QMP

[Kevin Wolf]

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 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..112a8420ce
--- /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 <http://www.gnu.org/licenses/>.
+#
+
+# 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 <<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 crypto 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
diff --git a/tests/qemu-iotests/212.out b/tests/qemu-iotests/212.out
new file mode 100644
index 0000000000..587de6fad0
--- /dev/null
+++ b/tests/qemu-iotests/212.out
@@ -0,0 +1,111 @@
+QA output created by 212
+
+=== Successful image creation (defaults) ===
+
+Testing:
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 128M (134217728 bytes)
+
+=== Successful image creation (explicit defaults) ===
+
+Testing:
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 64M (67108864 bytes)
+
+=== Successful image creation (with non-default options) ===
+
+Testing:
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 32M (33554432 bytes)
+
+=== Invalid BlockdevRef ===
+
+Testing:
+QMP_VERSION
+{"return": {}}
+{"error": {"class": "GenericError", "desc": "Cannot find device=this doesn't exist nor node_name=this doesn't exist"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+
+=== Zero size ===
+
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 0 (0 bytes)
+
+=== Maximum size ===
+
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 4096T (4503599627369984 bytes)
+
+=== Invalid sizes ===
+
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0
+QMP_VERSION
+{"return": {}}
+{"error": {"class": "GenericError", "desc": "Image size must be a multiple of 512 bytes"}}
+{"error": {"class": "GenericError", "desc": "Image size is too large for this cluster size"}}
+{"error": {"class": "GenericError", "desc": "Image size is too large for this cluster size"}}
+{"error": {"class": "GenericError", "desc": "Image size is too large for this cluster size"}}
+{"error": {"class": "GenericError", "desc": "Image size is too large for this cluster size"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+
+=== Invalid cluster size ===
+
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0
+QMP_VERSION
+{"return": {}}
+{"error": {"class": "GenericError", "desc": "Cluster size must be a multiple of 512 bytes"}}
+{"error": {"class": "GenericError", "desc": "Cluster size must be a multiple of 512 bytes"}}
+{"error": {"class": "GenericError", "desc": "Cluster size is too large"}}
+{"error": {"class": "GenericError", "desc": "Cluster size is too large"}}
+{"error": {"class": "GenericError", "desc": "Cluster size is too large"}}
+{"error": {"class": "GenericError", "desc": "Image size is too large for this cluster size"}}
+{"error": {"class": "GenericError", "desc": "Image size is too large for this cluster size"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 0d11d40aec..f706346f86 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -210,3 +210,4 @@
 209 rw auto quick
 210 rw auto
 211 rw auto quick
+212 rw auto quick
-- 
2.13.6

[Qemu-devel] [PATCH for-2.12 09/12] vhdx: Require power-of-two block size on create

[Kevin Wolf]

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 <kwolf@redhat.com>
---
 block/vhdx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/block/vhdx.c b/block/vhdx.c
index f1b97f4b49..ca211a3196 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);
-- 
2.13.6

[Qemu-devel] [PATCH for-2.12 10/12] vhdx: Don't use error_setg_errno() with constant errno

[Kevin Wolf]

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 <kwolf@redhat.com>
---
 block/vhdx.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index ca211a3196..0e48179b81 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;
     }
 
-- 
2.13.6

[Qemu-devel] [PATCH for-2.12 11/12] vhdx: Check for 4 GB maximum log size on creation

[Kevin Wolf]

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 <kwolf@redhat.com>
---
 block/vhdx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/block/vhdx.c b/block/vhdx.c
index 0e48179b81..a1a0302799 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) {
-- 
2.13.6

[Qemu-devel] [PATCH for-2.12 12/12] qemu-iotests: Test vhdx image creation with QMP

[Kevin Wolf]

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 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..7e44accb06
--- /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 <http://www.gnu.org/licenses/>.
+#
+
+# 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 <<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 crypto header the file will
+#                exceed 63 bits)
+# 4. 2^46 + 1 (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": 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
diff --git a/tests/qemu-iotests/213.out b/tests/qemu-iotests/213.out
new file mode 100644
index 0000000000..8e8fc29cbc
--- /dev/null
+++ b/tests/qemu-iotests/213.out
@@ -0,0 +1,121 @@
+QA output created by 213
+
+=== Successful image creation (defaults) ===
+
+Testing:
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 128M (134217728 bytes)
+
+=== Successful image creation (explicit defaults) ===
+
+Testing:
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 64M (67108864 bytes)
+
+=== Successful image creation (with non-default options) ===
+
+Testing:
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 32M (33554432 bytes)
+
+=== Invalid BlockdevRef ===
+
+Testing:
+QMP_VERSION
+{"return": {}}
+{"error": {"class": "GenericError", "desc": "Cannot find device=this doesn't exist nor node_name=this doesn't exist"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+
+=== Zero size ===
+
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 0 (0 bytes)
+
+=== Maximum size ===
+
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 64T (70368744177664 bytes)
+
+=== Invalid sizes ===
+
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0
+QMP_VERSION
+{"return": {}}
+{"error": {"class": "GenericError", "desc": "Image size too large; max of 64TB"}}
+{"error": {"class": "GenericError", "desc": "Image size too large; max of 64TB"}}
+{"error": {"class": "GenericError", "desc": "Image size too large; max of 64TB"}}
+{"error": {"class": "GenericError", "desc": "Image size too large; max of 64TB"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+
+=== Invalid block size ===
+
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0
+QMP_VERSION
+{"return": {}}
+{"error": {"class": "GenericError", "desc": "Block size must be a multiple of 1 MB"}}
+{"error": {"class": "GenericError", "desc": "Block size must be a multiple of 1 MB"}}
+{"error": {"class": "GenericError", "desc": "Block size must be a power of two"}}
+{"error": {"class": "GenericError", "desc": "Block size must not exceed 268435456"}}
+{"error": {"class": "GenericError", "desc": "Block size must be a multiple of 1 MB"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+
+=== Invalid log size ===
+
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0
+QMP_VERSION
+{"return": {}}
+{"error": {"class": "GenericError", "desc": "Log size must be a multiple of 1 MB"}}
+{"error": {"class": "GenericError", "desc": "Log size must be a multiple of 1 MB"}}
+{"error": {"class": "GenericError", "desc": "Log size must be smaller than 4 GB"}}
+{"error": {"class": "GenericError", "desc": "Log size must be a multiple of 1 MB"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index f706346f86..52a80f3f9e 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -211,3 +211,4 @@
 210 rw auto
 211 rw auto quick
 212 rw auto quick
+213 rw auto quick
-- 
2.13.6

Re: [Qemu-devel] [PATCH for-2.12 01/12] vdi: Change 'static' create option to 'preallocation' in QMP

[Eric Blake]

On 03/20/2018 12:36 PM, Kevin Wolf wrote:
> 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.

The x- naming of the command means we don't HAVE to get this patch in 
for 2.12, but I agree that sooner is better than later and that doing 
this in 2.12 is worth the effort.

> 
> This doesn't implement any new functionality, so the only supported
> preallocation modes are 'off' and 'metadata' for now.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   qapi/block-core.json |  6 ++----
>   block/vdi.c          | 24 ++++++++++++++++++++++--
>   2 files changed, 24 insertions(+), 6 deletions(-)
> 

> +++ b/qapi/block-core.json
> @@ -3834,16 +3834,14 @@
>   #
>   # @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 (default: off)

Do we want to document that 'full' is not supported yet?  Or is just 
relying on the error message good enough?

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-2.12 02/12] vdi: Fix build with CONFIG_VDI_DEBUG

[Eric Blake]

On 03/20/2018 12:36 PM, Kevin Wolf wrote:
> 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 <kwolf@redhat.com>
> ---
>   block/vdi.c | 22 ++++++++++------------
>   1 file changed, 10 insertions(+), 12 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-2.12 03/12] qemu-iotests: Test vdi image creation with QMP

[Eric Blake]

On 03/20/2018 12:36 PM, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   tests/qemu-iotests/211     | 256 +++++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/211.out |  98 +++++++++++++++++
>   tests/qemu-iotests/group   |   1 +
>   3 files changed, 355 insertions(+)
>   create mode 100755 tests/qemu-iotests/211
>   create mode 100644 tests/qemu-iotests/211.out
> 

> +
> +# creator
> +owner=kwolf@redhat.com
> +
> +seq=`basename $0`
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +status=1	# failure is the default!

More of the useless boilerplate we've been meaning to clean up.  Oh well.


> +# 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 crypto header the file will
> +#                exceed 63 bits)
> +# 4. 0x1fffff8000000 (maximum image size for VDI)
> +# 5. (one byte more than maximum image size for VDI)

You list 5 cases...

> +
> +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": 562949819203585
> +  }

...but only four x-blockdev-create.  Why?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-2.12 04/12] qemu-iotests: Enable 025 for luks

[Eric Blake]

On 03/20/2018 12:36 PM, Kevin Wolf wrote:
> 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 <kwolf@redhat.com>
> ---
>   tests/qemu-iotests/025 | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-2.12 05/12] luks: Turn another invalid assertion into check

[Eric Blake]

On 03/20/2018 12:36 PM, Kevin Wolf wrote:
> 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 <kwolf@redhat.com>
> ---
>   block/crypto.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-2.12 06/12] qemu-iotests: Test invalid resize on luks

[Eric Blake]

On 03/20/2018 12:36 PM, Kevin Wolf wrote:
> This tests that the .bdrv_truncate implementation for luks doesn't crash
> for invalid image sizes.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   tests/qemu-iotests/210 | 37 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 37 insertions(+)

Missing a change to 210.out?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-2.12 07/12] parallels: Check maximum cluster size on create

[Eric Blake]

On 03/20/2018 12:36 PM, Kevin Wolf wrote:
> 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 <kwolf@redhat.com>
> ---
>   block/parallels.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/block/parallels.c b/block/parallels.c
> index 2da5e56a9d..e4ca018c2e 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 >= UINT64_MAX / MAX_PARALLELS_IMAGE_FACTOR) {

INT64_MAX is probably a saner starting point for the division...

> +        error_setg(errp, "Cluster size is too large");
> +        return -EINVAL;
> +    }
>       if (total_size >= MAX_PARALLELS_IMAGE_FACTOR * cl_size) {

since total_size still has to fit within off_t (63 bits, not 64)

>           error_setg(errp, "Image size is too large for this cluster size");
>           return -E2BIG;
> 

With that change,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-2.12 08/12] qemu-iotests: Test parallels image creation with QMP

[Eric Blake]

On 03/20/2018 12:36 PM, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   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
> 

> +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 crypto header the file will
> +#                exceed 63 bits)

Is this part of the comment stale copy-and-paste? There's no crypto 
header, and the real max is much smaller...

> +# 5. 2^52 (512 bytes more than maximum image size)
> +

> +echo
> +echo "=== Invalid cluster size ==="
> +echo
> +
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-2.12 09/12] vhdx: Require power-of-two block size on create

[Eric Blake]

On 03/20/2018 12:36 PM, Kevin Wolf wrote:
> 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 <kwolf@redhat.com>
> ---
>   block/vhdx.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

> diff --git a/block/vhdx.c b/block/vhdx.c
> index f1b97f4b49..ca211a3196 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);
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-2.12 10/12] vhdx: Don't use error_setg_errno() with constant errno

[Eric Blake]

On 03/20/2018 12:36 PM, Kevin Wolf wrote:
> 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().

Well, it DOES let you get the libc-specific spelling of the strerror for 
that errno (not all libc report the same string) - but you're right that 
it didn't add much here.

> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block/vhdx.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-2.12 11/12] vhdx: Check for 4 GB maximum log size on creation

[Eric Blake]

On 03/20/2018 12:36 PM, Kevin Wolf wrote:
> 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 <kwolf@redhat.com>
> ---
>   block/vhdx.c | 4 ++++
>   1 file changed, 4 insertions(+)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/block/vhdx.c b/block/vhdx.c
> index 0e48179b81..a1a0302799 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) {
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-2.12 12/12] qemu-iotests: Test vhdx image creation with QMP

[Eric Blake]

On 03/20/2018 12:36 PM, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   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
> 

> +
> +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 crypto header the file will
> +#                exceed 63 bits)

Same comments as before on whether this comment is slightly stale after 
copy-and-paste.

> +# 4. 2^46 + 1 (512 bytes more than maximum image size)

Does this image format require 512-byte alignment?  If so, are you 
missing a test of unaligned sizes?  If not, why not just 1 byte more 
than the maximum?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-2.12 05/12] luks: Turn another invalid assertion into check

[Daniel P. Berrangé]

On Tue, Mar 20, 2018 at 06:36:25PM +0100, Kevin Wolf wrote:
> 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 <kwolf@redhat.com>
> ---
>  block/crypto.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH for-2.12 06/12] qemu-iotests: Test invalid resize on luks

[Daniel P. Berrangé]

On Tue, Mar 20, 2018 at 06:36:26PM +0100, Kevin Wolf wrote:
> This tests that the .bdrv_truncate implementation for luks doesn't crash
> for invalid image sizes.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  tests/qemu-iotests/210 | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH for-2.12 04/12] qemu-iotests: Enable 025 for luks

[Daniel P. Berrangé]

On Tue, Mar 20, 2018 at 06:36:24PM +0100, Kevin Wolf wrote:
> 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 <kwolf@redhat.com>
> ---
>  tests/qemu-iotests/025 | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH for-2.12 09/12] vhdx: Require power-of-two block size on create

[Jeff Cody]

On Tue, Mar 20, 2018 at 06:36:29PM +0100, Kevin Wolf wrote:
> Images with a non-power-of-two block size are invalid and cannot be
> opened. Reject such block sizes when creating an image.
> 

Good catch.

Reviewed-by: Jeff Cody <jcody@redhat.com>


> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/vhdx.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/block/vhdx.c b/block/vhdx.c
> index f1b97f4b49..ca211a3196 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);
> -- 
> 2.13.6
> 

Re: [Qemu-devel] [PATCH for-2.12 10/12] vhdx: Don't use error_setg_errno() with constant errno

[Jeff Cody]

On Tue, Mar 20, 2018 at 06:36:30PM +0100, Kevin Wolf wrote:
> 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 <kwolf@redhat.com>
> ---
>  block/vhdx.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/block/vhdx.c b/block/vhdx.c
> index ca211a3196..0e48179b81 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;
>      }
>  
> -- 
> 2.13.6
> 

Reviewed-by: Jeff Cody <jcody@redhat.com>

Re: [Qemu-devel] [PATCH for-2.12 11/12] vhdx: Check for 4 GB maximum log size on creation

[Jeff Cody]

On Tue, Mar 20, 2018 at 06:36:31PM +0100, Kevin Wolf wrote:
> 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.
> 

It's a uint32 in the on-disk header per spec, so I agree the implied max is
UINT32_MAX

Reviewed-by: Jeff Cody <jcody@redhat.com>

> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/vhdx.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/block/vhdx.c b/block/vhdx.c
> index 0e48179b81..a1a0302799 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) {
> -- 
> 2.13.6
> 

Re: [Qemu-devel] [PATCH for-2.12 08/12] qemu-iotests: Test parallels image creation with QMP

[Kevin Wolf]

Am 20.03.2018 um 19:42 hat Eric Blake geschrieben:
> On 03/20/2018 12:36 PM, Kevin Wolf wrote:
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >   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
> > 
> 
> > +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 crypto header the file will
> > +#                exceed 63 bits)
> 
> Is this part of the comment stale copy-and-paste? There's no crypto header,
> and the real max is much smaller...

Yeah... Not sure if the case is all that useful, but it can't hurt, so
I'll just s/crypto header/image header/ rather than removing it.

Kevin

Re: [Qemu-devel] [PATCH for-2.12 06/12] qemu-iotests: Test invalid resize on luks

[Kevin Wolf]

Am 20.03.2018 um 19:33 hat Eric Blake geschrieben:
> On 03/20/2018 12:36 PM, Kevin Wolf wrote:
> > This tests that the .bdrv_truncate implementation for luks doesn't crash
> > for invalid image sizes.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >   tests/qemu-iotests/210 | 37 +++++++++++++++++++++++++++++++++++++
> >   1 file changed, 37 insertions(+)
> 
> Missing a change to 210.out?

Obviously. I wonder how that happened... Thanks!

Kevin

Re: [Qemu-devel] [PATCH for-2.12 12/12] qemu-iotests: Test vhdx image creation with QMP

[Kevin Wolf]

Am 20.03.2018 um 19:53 hat Eric Blake geschrieben:
> On 03/20/2018 12:36 PM, Kevin Wolf wrote:
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >   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
> > 
> 
> > +
> > +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 crypto header the file will
> > +#                exceed 63 bits)
> 
> Same comments as before on whether this comment is slightly stale after
> copy-and-paste.

Will do the same thing as there ("image header").

> > +# 4. 2^46 + 1 (512 bytes more than maximum image size)
> 
> Does this image format require 512-byte alignment?  If so, are you missing a
> test of unaligned sizes?  If not, why not just 1 byte more than the maximum?

The comment is wrong, the code already does just 1 byte more.

Kevin