The following changes since commit 3fbd3405d2b0604ea530fc7a1828f19da1e95ff9:

Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2019-08-17' into staging (2019-08-19 14:14:09 +0100)

https://github.com/XanClic/qemu.git tags/pull-block-2019-08-19

for you to fetch changes up to fa27c478102a6b5d1c6b02c005607ad9404b915f:

doc: Preallocation does not require writing zeroes (2019-08-19 17:13:26 +0200)

- preallocation=falloc/full support for LUKS

- Various minor fixes

Max Reitz (16):

qemu-img: Fix bdrv_has_zero_init() use in convert

mirror: Fix bdrv_has_zero_init() use

block: Add bdrv_has_zero_init_truncate()

block: Implement .bdrv_has_zero_init_truncate()

block: Use bdrv_has_zero_init_truncate()

qcow2: Fix .bdrv_has_zero_init()

vdi: Fix .bdrv_has_zero_init()

vhdx: Fix .bdrv_has_zero_init()

iotests: Convert to preallocated encrypted qcow2

iotests: Test convert -n to pre-filled image

iotests: Full mirror to existing non-zero image

vdi: Make block_status recurse for fixed images

vmdk: Make block_status recurse for flat extents

vpc: Do not return RAW from block_status

iotests: Fix 141 when run with qed

doc: Preallocation does not require writing zeroes

Maxim Levitsky (1):

LUKS: support preallocation

qapi/block-core.json | 15 +++++---

include/block/block.h | 1 +

include/block/block_int.h | 9 +++++

block.c | 21 +++++++++++

block/crypto.c | 30 ++++++++++++++--

block/file-posix.c | 1 +

block/file-win32.c | 1 +

block/gluster.c | 4 +++

block/mirror.c | 11 ++++--

block/nfs.c | 1 +

block/parallels.c | 2 +-

block/qcow2.c | 30 +++++++++++++++-

block/qed.c | 1 +

block/raw-format.c | 6 ++++

block/rbd.c | 1 +

block/sheepdog.c | 1 +

block/ssh.c | 1 +

block/vdi.c | 16 +++++++--

block/vhdx.c | 28 +++++++++++++--

block/vmdk.c | 3 ++

block/vpc.c | 2 +-

blockdev.c | 16 +++++++--

qemu-img.c | 11 ++++--

tests/test-block-iothread.c | 2 +-

docs/qemu-block-drivers.texi | 4 +--

qemu-img.texi | 4 +--

tests/qemu-iotests/041 | 62 +++++++++++++++++++++++++++++---

tests/qemu-iotests/041.out | 4 +--

tests/qemu-iotests/122 | 17 +++++++++

tests/qemu-iotests/122.out | 8 +++++

tests/qemu-iotests/141 | 9 +++--

tests/qemu-iotests/141.out | 5 ---

tests/qemu-iotests/188 | 20 ++++++++++-

tests/qemu-iotests/188.out | 4 +++

tests/qemu-iotests/common.filter | 5 +++

35 files changed, 313 insertions(+), 43 deletions(-)

When preallocating an encrypted qcow2 image, it just lets the protocol

driver write data and then does not mark the clusters as zero.

Therefore, reading this image will yield effectively random data.

As such, we have not fulfilled the promise of always writing zeroes when

preallocating an image in a while. It seems that nobody has really

cared, so change the documentation to conform to qemu's actual behavior.

Signed-off-by: Max Reitz <mreitz@redhat.com>

Message-id: 20190711132935.13070-1-mreitz@redhat.com

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

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

Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

qapi/block-core.json | 9 +++++----

docs/qemu-block-drivers.texi | 4 ++--

qemu-img.texi | 4 ++--

3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json

--- a/qapi/block-core.json

+++ b/qapi/block-core.json

@@ -XXX,XX +XXX,XX @@

# @off: no preallocation

# @metadata: preallocate only for metadata

# @falloc: like @full preallocation but allocate disk space by

-# posix_fallocate() rather than writing zeros.

-# @full: preallocate all data by writing zeros to device to ensure disk

-# space is really available. @full preallocation also sets up

-# metadata correctly.

+# posix_fallocate() rather than writing data.

+# @full: preallocate all data by writing it to the device to ensure

+# disk space is really available. This data may or may not be

+# zero, depending on the image format and storage.

+# @full preallocation also sets up metadata correctly.

#

# Since: 2.2

##

diff --git a/docs/qemu-block-drivers.texi b/docs/qemu-block-drivers.texi

index XXXXXXX..XXXXXXX 100644

--- a/docs/qemu-block-drivers.texi

+++ b/docs/qemu-block-drivers.texi

@@ -XXX,XX +XXX,XX @@ Supported options:

@item preallocation

Preallocation mode (allowed values: @code{off}, @code{falloc}, @code{full}).

@code{falloc} mode preallocates space for image by calling posix_fallocate().

-@code{full} mode preallocates space for image by writing zeros to underlying

-storage.

+@code{full} mode preallocates space for image by writing data to underlying

+storage. This data may or may not be zero, depending on the storage location.

@end table

@item qcow2

diff --git a/qemu-img.texi b/qemu-img.texi

index XXXXXXX..XXXXXXX 100644

--- a/qemu-img.texi

+++ b/qemu-img.texi

@@ -XXX,XX +XXX,XX @@ Supported options:

@item preallocation

Preallocation mode (allowed values: @code{off}, @code{falloc}, @code{full}).

@code{falloc} mode preallocates space for image by calling posix_fallocate().

-@code{full} mode preallocates space for image by writing zeros to underlying

-storage.

+@code{full} mode preallocates space for image by writing data to underlying

+storage. This data may or may not be zero, depending on the storage location.

@end table

@item qcow2

bdrv_has_zero_init() only has meaning for newly created images or image

areas. If the mirror job itself did not create the image, it cannot

rely on bdrv_has_zero_init()'s result to carry any meaning.

This is the case for drive-mirror with mode=existing and always for

blockdev-mirror.

Note that we only have to zero-initialize the target with sync=full,

because other modes actually do not promise that the target will contain

the same data as the source after the job -- sync=top only promises to

copy anything allocated in the top layer, and sync=none will only copy

new I/O. (Which is how mirror has always handled it.)

Signed-off-by: Max Reitz <mreitz@redhat.com>

Message-id: 20190724171239.8764-3-mreitz@redhat.com

Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

include/block/block_int.h | 2 ++

block/mirror.c | 11 ++++++++---

blockdev.c | 16 +++++++++++++---

tests/test-block-iothread.c | 2 +-

4 files changed, 24 insertions(+), 7 deletions(-)

@@ -XXX,XX +XXX,XX @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs,

bool has_replaces, const char *replaces,

enum MirrorSyncMode sync,

BlockMirrorBackingMode backing_mode,

+ bool zero_target,

bool has_speed, int64_t speed,

bool has_granularity, uint32_t granularity,

bool has_buf_size, int64_t buf_size,

@@ -XXX,XX +XXX,XX @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs,

*/

mirror_start(job_id, bs, target,

has_replaces ? replaces : NULL, job_flags,

- speed, granularity, buf_size, sync, backing_mode,

+ speed, granularity, buf_size, sync, backing_mode, zero_target,

on_source_error, on_target_error, unmap, filter_node_name,

copy_mode, errp);

}

@@ -XXX,XX +XXX,XX @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)

int flags;

int64_t size;

const char *format = arg->format;

+ bool zero_target;

int ret;

bs = qmp_get_root_bs(arg->device, errp);

@@ -XXX,XX +XXX,XX @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)

goto out;

}

+ zero_target = (arg->sync == MIRROR_SYNC_MODE_FULL &&

+ (arg->mode == NEW_IMAGE_MODE_EXISTING ||

+ !bdrv_has_zero_init(target_bs)));

+

ret = bdrv_try_set_aio_context(target_bs, aio_context, errp);

if (ret < 0) {

bdrv_unref(target_bs);

@@ -XXX,XX +XXX,XX @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)

blockdev_mirror_common(arg->has_job_id ? arg->job_id : NULL, bs, target_bs,

arg->has_replaces, arg->replaces, arg->sync,

- backing_mode, arg->has_speed, arg->speed,

+ backing_mode, zero_target,

+ arg->has_speed, arg->speed,

arg->has_granularity, arg->granularity,

arg->has_buf_size, arg->buf_size,

arg->has_on_source_error, arg->on_source_error,

@@ -XXX,XX +XXX,XX @@ void qmp_blockdev_mirror(bool has_job_id, const char *job_id,

AioContext *aio_context;

BlockMirrorBackingMode backing_mode = MIRROR_LEAVE_BACKING_CHAIN;

Error *local_err = NULL;

+ bool zero_target;

int ret;

bs = qmp_get_root_bs(device, errp);

@@ -XXX,XX +XXX,XX @@ void qmp_blockdev_mirror(bool has_job_id, const char *job_id,

Signed-off-by: Max Reitz <mreitz@redhat.com>

Message-id: 20190724171239.8764-11-mreitz@redhat.com

Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

tests/qemu-iotests/122 | 17 +++++++++++++++++

tests/qemu-iotests/122.out | 8 ++++++++

2 files changed, 25 insertions(+)

diff --git a/tests/qemu-iotests/122 b/tests/qemu-iotests/122

--- a/tests/qemu-iotests/122

+++ b/tests/qemu-iotests/122

@@ -XXX,XX +XXX,XX @@ for min_sparse in 4k 8k; do

$QEMU_IMG map --output=json "$TEST_IMG".orig | _filter_qemu_img_map

done

+echo '=== -n to a non-zero image ==='

+echo

+# Keep source zero

+_make_test_img 64M

+# Output is not zero, but has bdrv_has_zero_init() == 1

+TEST_IMG="$TEST_IMG".orig _make_test_img 64M

+$QEMU_IO -c "write -P 42 0 64k" "$TEST_IMG".orig | _filter_qemu_io

+# Convert with -n, which should not assume that the target is zeroed

+$QEMU_IMG convert -O $IMGFMT -n "$TEST_IMG" "$TEST_IMG".orig

+

+$QEMU_IMG compare "$TEST_IMG" "$TEST_IMG".orig

+

# success, all done

echo '*** done'

rm -f $seq.full

diff --git a/tests/qemu-iotests/122.out b/tests/qemu-iotests/122.out

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/122.out

+++ b/tests/qemu-iotests/122.out

@@ -XXX,XX +XXX,XX @@ convert -c -S 8k

{ "start": 9216, "length": 8192, "depth": 0, "zero": true, "data": false},

{ "start": 17408, "length": 1024, "depth": 0, "zero": false, "data": true},

{ "start": 18432, "length": 67090432, "depth": 0, "zero": true, "data": false}]

+

+=== -n to a non-zero image ===

+

+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864

+Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=67108864

+wrote 65536/65536 bytes at offset 0

+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

+Images are identical.

*** done

Fixes: 69f47505ee66afaa513305de0c1895a224e52c45

Signed-off-by: Max Reitz <mreitz@redhat.com>

Message-id: 20190725155512.9827-3-mreitz@redhat.com

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: John Snow <jsnow@redhat.com>

block/vmdk.c | 3 +++

1 file changed, 3 insertions(+)

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn vmdk_co_block_status(BlockDriverState *bs,

if (!extent->compressed) {

ret |= BDRV_BLOCK_OFFSET_VALID;

*map = cluster_offset + index_in_cluster;

+ if (extent->flat) {

+ ret |= BDRV_BLOCK_RECURSE;

+ }

}

*file = extent->file->bs;

break;

Add a test case for converting an empty image (which only returns zeroes

when read) to a preallocated encrypted qcow2 image.

qcow2_has_zero_init() should return 0 then, thus forcing qemu-img

convert to create zero clusters.

Signed-off-by: Max Reitz <mreitz@redhat.com>

Acked-by: Stefano Garzarella <sgarzare@redhat.com>

Tested-by: Stefano Garzarella <sgarzare@redhat.com>

Message-id: 20190724171239.8764-10-mreitz@redhat.com

Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

tests/qemu-iotests/188 | 20 +++++++++++++++++++-

tests/qemu-iotests/188.out | 4 ++++

2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/188 b/tests/qemu-iotests/188

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/188

+++ b/tests/qemu-iotests/188

@@ -XXX,XX +XXX,XX @@ SECRETALT="secret,id=sec0,data=platypus"

_make_test_img --object $SECRET -o "encrypt.format=luks,encrypt.key-secret=sec0,encrypt.iter-time=10" $size

-IMGSPEC="driver=$IMGFMT,file.filename=$TEST_IMG,encrypt.key-secret=sec0"

+IMGSPEC="driver=$IMGFMT,encrypt.key-secret=sec0,file.filename=$TEST_IMG"

QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT

@@ -XXX,XX +XXX,XX @@ echo

echo "== verify open failure with wrong password =="

$QEMU_IO --object $SECRETALT -c "read -P 0xa 0 $size" --image-opts $IMGSPEC | _filter_qemu_io | _filter_testdir

+_cleanup_test_img

+

+echo

+echo "== verify that has_zero_init returns false when preallocating =="

+

+# Empty source file

+if [ -n "$TEST_IMG_FILE" ]; then

+ TEST_IMG_FILE="${TEST_IMG_FILE}.orig" _make_test_img $size

+else

+ TEST_IMG="${TEST_IMG}.orig" _make_test_img $size

+fi

+

+$QEMU_IMG convert -O "$IMGFMT" --object $SECRET \

+ -o "encrypt.format=luks,encrypt.key-secret=sec0,encrypt.iter-time=10,preallocation=metadata" \

+ "${TEST_IMG}.orig" "$TEST_IMG"

+

+$QEMU_IMG compare --object $SECRET --image-opts "${IMGSPEC}.orig" "$IMGSPEC"

+

# success, all done

echo "*** done"

diff --git a/tests/qemu-iotests/188.out b/tests/qemu-iotests/188.out

--- a/tests/qemu-iotests/188.out

+++ b/tests/qemu-iotests/188.out

@@ -XXX,XX +XXX,XX @@ read 16777216/16777216 bytes at offset 0

== verify open failure with wrong password ==

qemu-io: can't open: Invalid password, cannot unlock any keyslot

+

+== verify that has_zero_init returns false when preallocating ==

+Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=16777216

+Images are identical.

No .bdrv_has_zero_init() implementation returns 1 if growing the file

would add non-zero areas (at least with PREALLOC_MODE_OFF), so using it

in lieu of this new function was always safe.

But on the other hand, it is possible that growing an image that is not

zero-initialized would still add a zero-initialized area, like when

using nonpreallocating truncation on a preallocated image. For callers

that care only about truncation, not about creation with potential

preallocation, this new function is useful.

Alternatively, we could have added a PreallocMode parameter to

bdrv_has_zero_init(). But the only user would have been qemu-img

convert, which does not have a plain PreallocMode value right now -- it

would have to parse the creation option to obtain it. Therefore, the

simpler solution is to let bdrv_has_zero_init() inquire the

preallocation status and add the new bdrv_has_zero_init_truncate() that

presupposes PREALLOC_MODE_OFF.

Signed-off-by: Max Reitz <mreitz@redhat.com>

Message-id: 20190724171239.8764-4-mreitz@redhat.com

Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

include/block/block.h | 1 +

include/block/block_int.h | 7 +++++++

block.c | 21 +++++++++++++++++++++

3 files changed, 29 insertions(+)

diff --git a/include/block/block.h b/include/block/block.h

--- a/include/block/block.h

+++ b/include/block/block.h

@@ -XXX,XX +XXX,XX @@ int bdrv_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes);

int bdrv_co_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes);

int bdrv_has_zero_init_1(BlockDriverState *bs);

int bdrv_has_zero_init(BlockDriverState *bs);

+int bdrv_has_zero_init_truncate(BlockDriverState *bs);

bool bdrv_unallocated_blocks_are_zero(BlockDriverState *bs);

bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs);

int bdrv_block_status(BlockDriverState *bs, int64_t offset,

diff --git a/include/block/block_int.h b/include/block/block_int.h

index XXXXXXX..XXXXXXX 100644

--- a/include/block/block_int.h

+++ b/include/block/block_int.h

@@ -XXX,XX +XXX,XX @@ struct BlockDriver {

/*

* Returns 1 if newly created images are guaranteed to contain only

* zeros, 0 otherwise.

+ * Must return 0 if .bdrv_has_zero_init_truncate() returns 0.

*/

int (*bdrv_has_zero_init)(BlockDriverState *bs);

+ /*

+ * Returns 1 if new areas added by growing the image with

+ * PREALLOC_MODE_OFF contain only zeros, 0 otherwise.

+ */

+ int (*bdrv_has_zero_init_truncate)(BlockDriverState *bs);

+

/* Remove fd handlers, timers, and other event loop callbacks so the event

* loop is no longer in use. Called with no in-flight requests and in

* depth-first traversal order with parents before child nodes.

diff --git a/block.c b/block.c

index XXXXXXX..XXXXXXX 100644

--- a/block.c

+++ b/block.c

@@ -XXX,XX +XXX,XX @@ int bdrv_has_zero_init(BlockDriverState *bs)

+int bdrv_has_zero_init_truncate(BlockDriverState *bs)

+ if (!bs->drv) {

+ return 0;

+ }

+

+ if (bs->backing) {

+ /* Depends on the backing image length, but better safe than sorry */

+ return 0;

+ }

+ if (bs->drv->bdrv_has_zero_init_truncate) {

+ return bs->drv->bdrv_has_zero_init_truncate(bs);

+ }

+ if (bs->file && bs->drv->is_filter) {

+ return bdrv_has_zero_init_truncate(bs->file->bs);

+ }

+

+ /* safe default */

bool bdrv_unallocated_blocks_are_zero(BlockDriverState *bs)