Re: [PATCH v2 05/21] iotests: Replace IMGOPTS by _unsupported_imgopts

2019-11-06 Thread Maxim Levitsky
pts 'compat=0.10' 'refcount_bits=1[^0-9]' > > -IMGOPTS="compat=1.1" > IMG_SIZE=128K > > case "$QEMU_DEFAULT_MACHINE" in > diff --git a/tests/qemu-iotests/098 b/tests/qemu-iotests/098 > index 1c1d1c468f..700068b328 100755 > --- a/tests/qemu-iotests/098 > +++ b/

Re: [PATCH v2 16/21] iotests: Make 091 work with data_file

2019-11-06 Thread Maxim Levitsky
MG' > No errors were found on the image. > -80/16384 = 0.49% allocated, 0.00% fragmented, 0.00% compressed clusters > -Image end offset: 5570560 > *** done Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v2 12/21] iotests: Drop IMGOPTS use in 267

2019-11-06 Thread Maxim Levitsky
e,filename="$TEST_IMG",node-name=file \ > @@ -141,7 +145,7 @@ echo > echo "=== -blockdev with NBD server on the backing file ===" > echo > > -IMGOPTS="backing_file=$TEST_IMG.base" _make_test_img $size > +_make_test_img -b "$TEST_IMG.base" $size > cat < nbd_server_start unix:$TEST_DIR/nbd > nbd_server_add -w backing-fmt Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v2 10/21] iotests: Replace IMGOPTS= by -o

2019-11-06 Thread Maxim Levitsky
_make_test_img -o preallocation=$mode $size | _filter_imgfmt > stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks > $min_blocks $size > done > > Looks good. > diff --git a/tests/qemu-iotests/190 b/tests/qemu-iotests/190 > index eb766ad09f..5890ff9cfc 100755 > --- a/tests/qemu-iotests/190 > +++ b/tests/qemu-iotests/190 > @@ -45,7 +45,7 @@ _supported_proto file > echo "== Huge file ==" > echo > > -IMGOPTS='cluster_size=2M' _make_test_img 2T > +_make_test_img -o 'cluster_size=2M' 2T > > $QEMU_IMG measure -O raw -f qcow2 "$TEST_IMG" > $QEMU_IMG measure -O qcow2 -o cluster_size=64k -f qcow2 "$TEST_IMG" > Looks good. > diff --git a/tests/qemu-iotests/191 b/tests/qemu-iotests/191 > index 528022e8d8..21c16a32cb 100755 > --- a/tests/qemu-iotests/191 > +++ b/tests/qemu-iotests/191 > @@ -51,8 +51,7 @@ echo === Preparing and starting VM === > echo > > TEST_IMG="${TEST_IMG}.base" _make_test_img $size > -IMGOPTS=$(_optstr_add "$IMGOPTS" "backing_fmt=$IMGFMT") \ > -TEST_IMG="${TEST_IMG}.mid" _make_test_img -b "${TEST_IMG}.base" > +TEST_IMG="${TEST_IMG}.mid" _make_test_img -o "backing_fmt=$IMGFMT" -b > "${TEST_IMG}.base" > _make_test_img -b "${TEST_IMG}.mid" > TEST_IMG="${TEST_IMG}.ovl2" _make_test_img -b "${TEST_IMG}.mid" > > Looks good. > diff --git a/tests/qemu-iotests/220 b/tests/qemu-iotests/220 > index 2d62c5dcac..3769f372cb 100755 > --- a/tests/qemu-iotests/220 > +++ b/tests/qemu-iotests/220 > @@ -37,6 +37,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 > _supported_fmt qcow2 > _supported_proto file > _supported_os Linux > +# To use a different refcount width but 16 bits we need compat=1.1 > +_unsupported_imgopts 'compat=0.10' > > echo "== Creating huge file ==" > > @@ -46,7 +48,7 @@ if ! truncate --size=513T "$TEST_IMG"; then > _notrun "file system on $TEST_DIR does not support large enough files" > fi > rm "$TEST_IMG" > -IMGOPTS='cluster_size=2M,refcount_bits=1' _make_test_img 513T > +_make_test_img -o 'cluster_size=2M,refcount_bits=1' 513T > > echo "== Populating refcounts ==" > # We want an image with 256M refcounts * 2M clusters = 512T referenced. > Looks good. > diff --git a/tests/qemu-iotests/243 b/tests/qemu-iotests/243 > index e563761307..2b84b896db 100755 > --- a/tests/qemu-iotests/243 > +++ b/tests/qemu-iotests/243 > @@ -40,6 +40,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 > _supported_fmt qcow2 > _supported_proto file > _supported_os Linux > +# External data files do not work with compat=0.10 > +_unsupported_imgopts 'compat=0.10' > > for mode in off metadata falloc full; do > > @@ -47,7 +49,7 @@ for mode in off metadata falloc full; do > echo "=== preallocation=$mode ===" > echo > > -IMGOPTS="preallocation=$mode" _make_test_img 64M > +_make_test_img -o "preallocation=$mode" 64M > > printf "File size: " > du -b $TEST_IMG | cut -f1 > @@ -64,7 +66,7 @@ for mode in off metadata falloc full; do > echo "=== External data file: preallocation=$mode ===" > echo > > -IMGOPTS="data_file=$TEST_IMG.data,preallocation=$mode" _make_test_img 64M > +_make_test_img -o "data_file=$TEST_IMG.data,preallocation=$mode" 64M > > echo -n "qcow2 file size: " > du -b $TEST_IMG | cut -f1 > Looks good. > diff --git a/tests/qemu-iotests/244 b/tests/qemu-iotests/244 > index 13978f93d2..0375bc12d4 100755 > --- a/tests/qemu-iotests/244 > +++ b/tests/qemu-iotests/244 > @@ -41,13 +41,15 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 > _supported_fmt qcow2 > _supported_proto file > _supported_os Linux > +# External data files do not work with compat=0.10 > +_unsupported_imgopts 'compat=0.10' > > echo > echo "=== Create and open image with external data file ===" > echo > > echo "With data file name in the image:" > -IMGOPTS="data_file=$TEST_IMG.data" _make_test_img 64M > +_make_test_img -o "data_file=$TEST_IMG.data" 64M > _check_test_img > > $QEMU_IO -c "open $TEST_IMG" -c "read -P 0 0 64k" 2>&1 | _filter_qemu_io | > _filter_testdir > @@ -104,7 +106,7 @@ echo > echo "=== Standalone image with external data file (efficient) ===" > echo > > -IMGOPTS="data_file=$TEST_IMG.data" _make_test_img 64M > +_make_test_img -o "data_file=$TEST_IMG.data" 64M > > echo -n "qcow2 file size before I/O: " > du -b $TEST_IMG | cut -f1 > @@ -154,7 +156,7 @@ echo > echo "=== Standalone image with external data file (valid raw) ===" > echo > > -IMGOPTS="data_file=$TEST_IMG.data,data_file_raw=on" _make_test_img 64M > +_make_test_img -o "data_file=$TEST_IMG.data,data_file_raw=on" 64M > > echo -n "qcow2 file size before I/O: " > du -b $TEST_IMG | cut -f1 > @@ -187,7 +189,7 @@ echo > echo "=== bdrv_co_block_status test for file and offset=0 ===" > echo > > -IMGOPTS="data_file=$TEST_IMG.data" _make_test_img 64M > +_make_test_img -o "data_file=$TEST_IMG.data" 64M > > $QEMU_IO -c 'write -P 0x11 0 1M' -f $IMGFMT "$TEST_IMG" | _filter_qemu_io > $QEMU_IO -c 'read -P 0x11 0 1M' -f $IMGFMT "$TEST_IMG" | _filter_qemu_io > Looks good. > diff --git a/tests/qemu-iotests/250 b/tests/qemu-iotests/250 > index c9c0a84a5a..670cf19076 100755 > --- a/tests/qemu-iotests/250 > +++ b/tests/qemu-iotests/250 > @@ -55,9 +55,8 @@ disk_usage() > } > > size=2100M > -IMGOPTS="cluster_size=1M,preallocation=metadata" > > -_make_test_img $size > +_make_test_img -o "cluster_size=1M,preallocation=metadata" $size > $QEMU_IO -c 'discard 0 10M' -c 'discard 2090M 10M' \ > -c 'write 2090M 10M' -c 'write 0 10M' "$TEST_IMG" | _filter_qemu_io > > Looks good. > diff --git a/tests/qemu-iotests/265 b/tests/qemu-iotests/265 > index dce6f77be3..00f2ec769e 100755 > --- a/tests/qemu-iotests/265 > +++ b/tests/qemu-iotests/265 > @@ -41,7 +41,7 @@ _supported_os Linux > echo '--- Writing to the image ---' > > # Reduce cluster size so we get more and quicker I/O > -IMGOPTS='cluster_size=4096' _make_test_img 1M > +_make_test_img -o 'cluster_size=4096' 1M > (for ((kb = 1024 - 4; kb >= 0; kb -= 4)); do \ > echo "aio_write -P 42 $((kb + 1))k 2k"; \ > done) \ Looks good as well. To make review of this patch a bit less boring, I went over all the tests to understand more or less what each test does. I hope that I didn't miss anything. Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v2 04/21] iotests: Filter refcount_order in 036

2019-11-06 Thread Maxim Levitsky
0x514649fb > -version 3 > -backing_file_offset 0x0 > -backing_file_size 0x0 > -cluster_bits 16 > -size 67108864 > -crypt_method 0 > -l1_size 1 > -l1_table_offset 0x3 > -refcount_table_offset 0x1 > -refcount_table_clusters 1 > -nb_snapshots 0 > -snapshot_offset 0x0 > incompatible_features [] > compatible_features [] > autoclear_features[] > -refcount_order4 > -header_length 104 > - > Header extension: > magic 0x6803f857 > length192 Great! Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v2 03/21] iotests: Add _filter_json_filename

2019-11-06 Thread Maxim Levitsky
t; + > +sys.stdout.write(result)' > +} > + > # make sure this script returns success > true I must admit that I haven't run tested it, but it looks like it should work. Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v2 02/21] iotests/qcow2.py: Split feature fields into bits

2019-11-06 Thread Maxim Levitsky
quot; ) > -incompatible_features 0x0 > +incompatible_features [] > Formatting 'TEST_DIR/t.IMGFMT', 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) > diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py > index d813b4fc81..91e4420b9f 100755 > --- a/tests/qemu-iotests/qcow2.py > +++ b/tests/qemu-iotests/qcow2.py > @@ -42,9 +42,9 @@ class QcowHeader: > [ uint64_t, '%#x', 'snapshot_offset' ], > > # Version 3 header fields > -[ uint64_t, '%#x', 'incompatible_features' ], > -[ uint64_t, '%#x', 'compatible_features' ], > -[ uint64_t, '%#x', 'autoclear_features' ], > +[ uint64_t, 'mask', 'incompatible_features' ], > +[ uint64_t, 'mask', 'compatible_features' ], > +[ uint64_t, 'mask', 'autoclear_features' ], > [ uint32_t, '%d', 'refcount_order' ], > [ uint32_t, '%d', 'header_length' ], > ]; > @@ -130,7 +130,17 @@ class QcowHeader: > > def dump(self): > for f in QcowHeader.fields: > -print("%-25s" % f[2], f[1] % self.__dict__[f[2]]) > +value = self.__dict__[f[2]] > +if f[1] == 'mask': > +bits = [] > +for bit in range(64): Very very minor nitpick, the type size (64 bit) is hardcoded here but I think it is OK. > +if value & (1 << bit): > +bits.append(bit) > +value_str = str(bits) > +else: > +value_str = f[1] % value > + > +print("%-25s" % f[2], value_str) > print("") > > def dump_extensions(self): This is very good idea and implementation. Thanks! Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v2 01/21] iotests/qcow2.py: Add dump-header-exts

2019-11-06 Thread Maxim Levitsky
field in > the header'], > [ 'add-header-ext', cmd_add_header_ext, 2, 'Add a header > extension' ], > [ 'add-header-ext-stdio', cmd_add_header_ext_stdio, 1, 'Add a header > extension, data from stdin' ], Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v2 0/2] block/nvme: add support for write zeros and discard

2019-11-04 Thread Maxim Levitsky
On Tue, 2019-10-29 at 09:33 -0400, John Snow wrote: > > On 10/28/19 6:35 AM, Max Reitz wrote: > > On 13.09.19 15:36, Maxim Levitsky wrote: > > > This is the second part of the patches I prepared > > > for this driver back when I worked on mdev-nvme. > > &g

Re: [PATCH v2 2/2] nbd: Allow description when creating NBD blockdev

2019-11-04 Thread Maxim Levitsky
map":"b2"}}' "return" > + "description":"some text", "bitmap":"b2"}}' "return" > $QEMU_NBD_PROG -L -k "$TEST_DIR/nbd" > > echo > diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out > index 23b34fcd202e..16d597585b4f 100644 > --- a/tests/qemu-iotests/223.out > +++ b/tests/qemu-iotests/223.out > @@ -49,6 +49,7 @@ exports available: 2 > base:allocation > qemu:dirty-bitmap:b > export: 'n2' > + description: some text >size: 4194304 >flags: 0xced ( flush fua trim zeroes df cache fast-zero ) >min block: 1 Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH v2 1/2] nbd: Don't send oversize strings

2019-11-04 Thread Maxim Levitsky
>exp->export_bitmap_context = > > > > g_strdup_printf("qemu:dirty-bitmap:%s", > > > > bitmap); > > > > +/* See BME_MAX_NAME_SIZE in block/qcow2-bitmap.c */ > > > > > > Hmm. BME_MAX_NAME_SIZE is checked only when creating persistent bitmaps. > > > But for non-persistent > > > name length is actually unlimited. So, we should either limit all bitmap > > > names to 1023 (hope, > > > this will not break existing scenarios) or error out here (or earlier) > > > instead of assertion. > > > > I'm leaning towards limiting ALL bitmaps to the same length (as we've > > already debated the idea of being able to convert an existing bitmap from > > transient to persistent). > > Agreed, but .. > > > > > > > > > We also may want QEMU_BUILD_BUG_ON(NBD_MAX_STRING_SIZE < > > > BME_MAX_NAME_SIZE + sizeof("qemu:dirty-bitmap:") - 1) > > > > Except that BME_MAX_NAME_SIZE is not (currently) in a public .h file. > > > > .. I think, than it should be new BLOCK_DIRTY_BITMAP_MAX_NAME_SIZE.. And > we'll have to note it in qapi doc.. > Should this change go through deprecation? Or we consider non-persistent > bitmaps as something not really useful? > > -- > Best regards, > Vladimir I followed upon the new patch and the comments, and it seems ok now to me, (including the comments that were already made) but I haven't checked if there are more cases of missing length checks. Best regards, Maxim Levitsky

[PATCH 1/1] nbd: add empty .bdrv_reopen_prepare

2019-09-30 Thread Maxim Levitsky
Fixes commit job / qemu-img commit, when commiting qcow2 file which is based on nbd export. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1718727 Signed-off-by: Maxim Levitsky --- block/nbd.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/block/nbd.c b/block

[PATCH 0/1] RFC: implement reopen for nbd driver

2019-09-30 Thread Maxim Levitsky
) is supposed to be enough. Sending this as RFC, since I am not sure that this is the correct solution. Best regards, Maxim Levitsky Maxim Levitsky (1): nbd: add empty .bdrv_reopen_prepare block/nbd.c | 15 +++ 1 file changed, 15 insertions(+) -- 2.17.2

Re: [Qemu-devel] [PATCH v2 00/11] RFC crypto/luks: encryption key managment using amend interface

2019-09-30 Thread Maxim Levitsky
On Fri, 2019-09-20 at 17:14 -0400, John Snow wrote: > > On 9/12/19 6:30 PM, Maxim Levitsky wrote: > > This patch series is continuation of my work to add encryption > > key managment to luks/qcow2 with luks. > > > > This is second version of this patch set. > &g

Re: [PATCH 09/18] iotests: Drop IMGOPTS use in 267

2019-09-30 Thread Maxim Levitsky
On Mon, 2019-09-30 at 17:01 +0200, Max Reitz wrote: > On 30.09.19 16:32, Maxim Levitsky wrote: > > On Mon, 2019-09-30 at 15:01 +0200, Max Reitz wrote: > > > On 29.09.19 18:33, Maxim Levitsky wrote: > > > > On Fri, 2019-09-27 at 11:42 +0200, Max Reitz wrote: >

Re: [PATCH 02/18] iotests: Replace IMGOPTS by _unsupported_imgopts

2019-09-30 Thread Maxim Levitsky
On Mon, 2019-09-30 at 14:59 +0200, Max Reitz wrote: > On 29.09.19 18:31, Maxim Levitsky wrote: > > On Fri, 2019-09-27 at 11:42 +0200, Max Reitz wrote: > > > Some tests require compat=1.1 and thus set IMGOPTS='compat=1.1' > > > globally. That is not how it should be

Re: [PATCH 09/18] iotests: Drop IMGOPTS use in 267

2019-09-30 Thread Maxim Levitsky
On Mon, 2019-09-30 at 15:01 +0200, Max Reitz wrote: > On 29.09.19 18:33, Maxim Levitsky wrote: > > On Fri, 2019-09-27 at 11:42 +0200, Max Reitz wrote: > > > Overwriting IMGOPTS means ignoring all user-supplied options, which is > > > not what we want. Replace the c

Re: [PATCH 07/18] iotests: Replace IMGOPTS= by -o

2019-09-30 Thread Maxim Levitsky
On Mon, 2019-09-30 at 15:00 +0200, Max Reitz wrote: > On 29.09.19 18:33, Maxim Levitsky wrote: > > On Fri, 2019-09-27 at 11:42 +0200, Max Reitz wrote: > > > Tests should not overwrite all user-supplied image options, but only add > > > to it (which will effectively o

Re: [PATCH 01/18] iotests: Filter refcount_order in 036

2019-09-30 Thread Maxim Levitsky
On Mon, 2019-09-30 at 16:04 +0200, Max Reitz wrote: > On 30.09.19 15:58, Maxim Levitsky wrote: > > On Mon, 2019-09-30 at 15:44 +0200, Max Reitz wrote: > > > On 30.09.19 15:40, Maxim Levitsky wrote: > > > > On Mon, 2019-09-30 at 14:43 +0200, Max Reitz wrote: >

Re: [PATCH 15/18] iotests: Make 137 work with data_file

2019-09-30 Thread Maxim Levitsky
On Mon, 2019-09-30 at 15:57 +0200, Max Reitz wrote: > On 30.09.19 15:46, Maxim Levitsky wrote: > > On Mon, 2019-09-30 at 15:38 +0200, Max Reitz wrote: > > > On 29.09.19 18:38, Maxim Levitsky wrote: > > > > On Fri, 2019-09-27 at 11:42 +0200, Max Reitz wrote: > &g

Re: [PATCH 01/18] iotests: Filter refcount_order in 036

2019-09-30 Thread Maxim Levitsky
On Mon, 2019-09-30 at 15:44 +0200, Max Reitz wrote: > On 30.09.19 15:40, Maxim Levitsky wrote: > > On Mon, 2019-09-30 at 14:43 +0200, Max Reitz wrote: > > > On 29.09.19 18:31, Maxim Levitsky wrote: > > > > On Fri, 2019-09-27 at 11:42 +0200, Max Reitz wrote: > &g

Re: [PATCH 15/18] iotests: Make 137 work with data_file

2019-09-30 Thread Maxim Levitsky
On Mon, 2019-09-30 at 15:38 +0200, Max Reitz wrote: > On 29.09.19 18:38, Maxim Levitsky wrote: > > On Fri, 2019-09-27 at 11:42 +0200, Max Reitz wrote: > > > When using an external data file, there are no refcounts for data > > > clusters. We thus have to adjust the co

Re: [PATCH 14/18] iotests: Make 110 work with data_file

2019-09-30 Thread Maxim Levitsky
On Mon, 2019-09-30 at 15:34 +0200, Max Reitz wrote: > On 29.09.19 18:34, Maxim Levitsky wrote: > > On Fri, 2019-09-27 at 11:42 +0200, Max Reitz wrote: > > > The only difference is that the json:{} filename of the image looks > > > different. We actually do n

Re: [PATCH 01/18] iotests: Filter refcount_order in 036

2019-09-30 Thread Maxim Levitsky
On Mon, 2019-09-30 at 14:43 +0200, Max Reitz wrote: > On 29.09.19 18:31, Maxim Levitsky wrote: > > On Fri, 2019-09-27 at 11:42 +0200, Max Reitz wrote: > > > This test can run just fine with other values for refcount_bits, so we > > > should filter the value

Re: [PATCH] nbd: Don't let client send oversize strings

2019-09-29 Thread Maxim Levitsky
if (strlen(info->name) > NBD_MAX_STRING_SIZE) { > +error_setg(errp, "name too long to send to server"); Maybe 'export name'? > +return -EINVAL; > +} > trace_nbd_receive_negotiate_name(info->name); > > result = nbd_start_negotiate(aio_context, ioc, tlscreds, hostname, > outioc, Why not to do the export name check when info->name is set, that is in nbd_client_connect? Best regards, Maxim Levitsky

Re: [PATCH] tests: fix I/O test for hosts defaulting to LUKSv2

2019-09-29 Thread Maxim Levitsky
t; much bigger job. This is fine for now. If there is need, I volunteer to research the area and if this is feasible, add support for LUKSv2. I haven't yet had looked at the spec. Best regards, Maxim Levitsky

Re: [PATCH 18/18] iotests: Allow check -o data_file

2019-09-29 Thread Maxim Levitsky
me:' | cut -f 2 > -d: \ > | xargs -I {} rm -f "{}" > +elif [ "$IMGFMT" = "qcow2" ]; then > +# Remove external data file > +if echo "$IMGOPTS" | grep -q 'data_file='; then > +data_file=$(echo "$IMGOPTS" | sed -e > 's/.*data_file=\([^,]*\).*/\1/' \ > + -e "s#\\\$TEST_IMG#$img#") Same here, even more evidence that it would be nice to move this to a common function. Again, I am not 100% sure that this is the same regex, but it looks like that. > +rm -f "$data_file" > +fi > fi > rm -f "$img" > } Best regards, Maxim Levitsky

Re: [PATCH 11/18] iotests: Use _rm_test_img for deleting test images

2019-09-29 Thread Maxim Levitsky
_test_img > -rm -f "$TEST_IMG.mid" > +_rm_test_img "$TEST_IMG.mid" > } > trap "_cleanup; exit \$status" 0 1 2 3 15 > > diff --git a/tests/qemu-iotests/197 b/tests/qemu-iotests/197 > index 4d3d08ad6f..95f05b0e34 100755 > --- a/tests/qemu-iotests/197 > +++ b/tests/qemu-iotests/197 > @@ -43,7 +43,7 @@ esac > _cleanup() > { > _cleanup_test_img > -rm -f "$TEST_WRAP" > +_rm_test_img "$TEST_WRAP" > rm -f "$BLKDBG_CONF" > } > trap "_cleanup; exit \$status" 0 1 2 3 15 > diff --git a/tests/qemu-iotests/200 b/tests/qemu-iotests/200 > index d904885136..a2cdd7f83d 100755 > --- a/tests/qemu-iotests/200 > +++ b/tests/qemu-iotests/200 > @@ -31,7 +31,8 @@ status=1# failure is the default! > _cleanup() > { > _cleanup_qemu > -rm -f "${TEST_IMG}" "${BACKING_IMG}" > +_rm_test_img "${TEST_IMG}" > +_rm_test_img "${BACKING_IMG}" > } > trap "_cleanup; exit \$status" 0 1 2 3 15 > > diff --git a/tests/qemu-iotests/215 b/tests/qemu-iotests/215 > index 55a1874dcd..f99bae78c7 100755 > --- a/tests/qemu-iotests/215 > +++ b/tests/qemu-iotests/215 > @@ -40,7 +40,7 @@ esac > _cleanup() > { > _cleanup_test_img > -rm -f "$TEST_WRAP" > +_rm_test_img "$TEST_WRAP" > rm -f "$BLKDBG_CONF" > } > trap "_cleanup; exit \$status" 0 1 2 3 15 > diff --git a/tests/qemu-iotests/225 b/tests/qemu-iotests/225 > index fbd7404791..c9a334c7e9 100755 > --- a/tests/qemu-iotests/225 > +++ b/tests/qemu-iotests/225 > @@ -29,7 +29,7 @@ status=1# failure is the default! > _cleanup() > { > _cleanup_test_img > -rm -f "$TEST_IMG.not_base" > +_rm_test_img "$TEST_IMG.not_base" > } > trap "_cleanup; exit \$status" 0 1 2 3 15 > > diff --git a/tests/qemu-iotests/229 b/tests/qemu-iotests/229 > index e18a464fe0..866168b236 100755 > --- a/tests/qemu-iotests/229 > +++ b/tests/qemu-iotests/229 > @@ -31,7 +31,8 @@ _cleanup() > { > _cleanup_qemu > _cleanup_test_img > -rm -f "$TEST_IMG" "$DEST_IMG" > +_rm_test_img "$TEST_IMG" > +_rm_test_img "$DEST_IMG" > } > trap "_cleanup; exit \$status" 0 1 2 3 15 > > diff --git a/tests/qemu-iotests/232 b/tests/qemu-iotests/232 > index 65b0e42063..685356ac3b 100755 > --- a/tests/qemu-iotests/232 > +++ b/tests/qemu-iotests/232 > @@ -29,7 +29,9 @@ status=1# failure is the default! > _cleanup() > { > _cleanup_test_img > -rm -f $TEST_IMG.[01234] > +for img in "$TEST_IMG".[01234]; do > +_rm_test_img "$img" > +done > } > trap "_cleanup; exit \$status" 0 1 2 3 15 > > diff --git a/tests/qemu-iotests/243 b/tests/qemu-iotests/243 > index 2b84b896db..3dc3b6a711 100755 > --- a/tests/qemu-iotests/243 > +++ b/tests/qemu-iotests/243 > @@ -29,7 +29,7 @@ status=1# failure is the default! > _cleanup() > { > _cleanup_test_img > -rm -f $TEST_IMG.data > +_rm_test_img "$TEST_IMG.data" > } > trap "_cleanup; exit \$status" 0 1 2 3 15 > > diff --git a/tests/qemu-iotests/244 b/tests/qemu-iotests/244 > index 0375bc12d4..13263292b0 100755 > --- a/tests/qemu-iotests/244 > +++ b/tests/qemu-iotests/244 > @@ -29,8 +29,8 @@ status=1# failure is the default! > _cleanup() > { > _cleanup_test_img > -rm -f $TEST_IMG.data > -rm -f $TEST_IMG.src > +_rm_test_img "$TEST_IMG.data" > +_rm_test_img "$TEST_IMG.src" > } > trap "_cleanup; exit \$status" 0 1 2 3 15 > > diff --git a/tests/qemu-iotests/247 b/tests/qemu-iotests/247 > index c853b73819..87e37b39e2 100755 > --- a/tests/qemu-iotests/247 > +++ b/tests/qemu-iotests/247 > @@ -29,7 +29,9 @@ status=1# failure is the default! > _cleanup() > { > _cleanup_test_img > -rm -f $TEST_IMG.[01234] > +for img in "$TEST_IMG".[01234]; do > +_rm_test_img "$img" > +done > } > trap "_cleanup; exit \$status" 0 1 2 3 15 > > diff --git a/tests/qemu-iotests/249 b/tests/qemu-iotests/249 > index e4650ecf6b..2b99c9789e 100755 > --- a/tests/qemu-iotests/249 > +++ b/tests/qemu-iotests/249 > @@ -30,8 +30,8 @@ status=1# failure is the default! > _cleanup() > { > _cleanup_test_img > -rm -f "$TEST_IMG.base" > -rm -f "$TEST_IMG.int" > +_rm_test_img "$TEST_IMG.base" > +_rm_test_img "$TEST_IMG.int" > } > trap "_cleanup; exit \$status" 0 1 2 3 15 > > diff --git a/tests/qemu-iotests/252 b/tests/qemu-iotests/252 > index f6c8f71444..83280c1715 100755 > --- a/tests/qemu-iotests/252 > +++ b/tests/qemu-iotests/252 > @@ -29,7 +29,7 @@ status=1# failure is the default! > _cleanup() > { > _cleanup_test_img > -rm -f "$TEST_IMG.base_new" > +_rm_test_img "$TEST_IMG.base_new" > } > trap "_cleanup; exit \$status" 0 1 2 3 15 > The patch is large, thus I can't be sure I can check all of it, but overall looks good to me. Best regards, Maxim Levitsky

Re: [PATCH 06/18] iotests: Inject space into -ocompat=0.10 in 051

2019-09-29 Thread Maxim Levitsky
cho > > -_make_test_img -ocompat=0.10 $size > +_make_test_img -o compat=0.10 $size > > run_qemu -drive file="$TEST_IMG",format=qcow2,lazy-refcounts=on > run_qemu -drive file="$TEST_IMG",format=qcow2,lazy-refcounts=off Reviewed-by: Maxim Levitsky

Re: [PATCH 10/18] iotests: Avoid qemu-img create

2019-09-29 Thread Maxim Levitsky
s/qemu-iotests/200 > index 72d431f251..d904885136 100755 > --- a/tests/qemu-iotests/200 > +++ b/tests/qemu-iotests/200 > @@ -46,8 +46,8 @@ _supported_proto file > BACKING_IMG="${TEST_DIR}/backing.img" > TEST_IMG="${TEST_DIR}/test.img" > > -${QEMU_IMG} create -f $IMGFMT "${BACKING_IMG}" 512M | _filter_img_create > -${QEMU_IMG} create -f $IMGFMT -F $IMGFMT "${TEST_IMG}" -b "${BACKING_IMG}" > 512M | _filter_img_create > +TEST_IMG="$BACKING_IMG" _make_test_img 512M > +_make_test_img -F $IMGFMT -b "$BACKING_IMG" 512M > > ${QEMU_IO} -c "write -P 0xa5 512 300M" "${BACKING_IMG}" | _filter_qemu_io > Reviewed-by: Maxim Levitsky

Re: [PATCH 16/18] iotests: Make 198 work with data_file

2019-09-29 Thread Maxim Levitsky
to reduce duplication. Best regards, Maxim Levitsky > > Signed-off-by: Max Reitz > --- > tests/qemu-iotests/198 | 6 -- > tests/qemu-iotests/198.out | 4 ++-- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/tests/qemu-iotests/198

Re: [PATCH 17/18] iotests: Disable data_file where it cannot be used

2019-09-29 Thread Maxim Levitsky
> tests/qemu-iotests/220 | 5 +++-- > tests/qemu-iotests/243 | 6 -- > tests/qemu-iotests/244 | 5 +++-- > tests/qemu-iotests/250 | 2 ++ > tests/qemu-iotests/267 | 5 +++-- > 42 files changed, 116 insertions(+), 51 deletions(-) > No iotest 267 on master branch, same as patch 9 Best regards, Maxim Levitsky

Re: [PATCH 15/18] iotests: Make 137 work with data_file

2019-09-29 Thread Maxim Levitsky
qemu-io: Parameter 'lazy-refcounts' expects 'on' or 'off' > -qcow2: Marking image as corrupt: Preventing invalid write on metadata > (overlaps with qcow2_header); further corruption events will be suppressed > +qcow2: Marking image as corrupt: Preventing invalid allocation of L2 table > at offset 0; further corruption events will be suppressed > write failed: Input/output error > *** done Best regards, Maxim Levitsky

Re: [PATCH 14/18] iotests: Make 110 work with data_file

2019-09-29 Thread Maxim Levitsky
> === Nodes without a common directory === > > -image: json:{"driver": "IMGFMT", "file": {"children": [{"driver": "file", > "filename": "TEST_DIR/t.IMGFMT"}, {"driver": "file", "filename": > "TEST_DIR/t.IMGFMT.copy"}], "driver": "quorum", "vote- > threshold": 1}} > +image: json:{ /* filtered */ } > file format: IMGFMT > virtual size: 64 MiB (67108864 bytes) > backing file: t.IMGFMT.base (cannot determine actual path) Again, maybe put that into the common.filter, so new tests won't need to copy this? Also maybe remove the image name completely from output, thus not needing the more complex regex? Best regards, Maxim Levitsky

Re: [PATCH 09/18] iotests: Drop IMGOPTS use in 267

2019-09-29 Thread Maxim Levitsky
e,filename="$TEST_IMG",node-name=file \ > @@ -141,7 +145,7 @@ echo > echo "=== -blockdev with NBD server on the backing file ===" > echo > > -IMGOPTS="backing_file=$TEST_IMG.base" _make_test_img $size > +_make_test_img -b "$TEST_IMG.base" $size > cat < nbd_server_start unix:$TEST_DIR/nbd > nbd_server_add -w backing-fmt qemu's master (pulled today), don't have iotest 267, you probably based this on some not yet merged branch. Or I made some mistake with pulling the master branch. Best regards, Maxim Levitsky

Re: [PATCH 12/18] iotests: Avoid cp/mv of test images

2019-09-29 Thread Maxim Levitsky
t.IMGFMT.2', fmt=IMGFMT size=134217728 > > === Running QEMU === > > @@ -55,10 +55,10 @@ Formatting 'TEST_DIR/10-snapshot-v1.qcow2', fmt=qcow2 > size=134217728 backing_fil > > === Create a couple of snapshots using blockdev-snapshot === > > -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 > backing_file=TEST_DIR/10-snapshot-v0.IMGFMT > +Formatting 'TEST_DIR/11-snapshot-v0.IMGFMT', fmt=IMGFMT size=134217728 > backing_file=TEST_DIR/10-snapshot-v0.IMGFMT > {"return": {}} > {"return": {}} > -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 > backing_file=TEST_DIR/11-snapshot-v0.IMGFMT > +Formatting 'TEST_DIR/12-snapshot-v0.IMGFMT', fmt=IMGFMT size=134217728 > backing_file=TEST_DIR/11-snapshot-v0.IMGFMT > {"return": {}} > {"return": {}} > Looks good. Reviewed-by: Maxim Levitsky

Re: [PATCH 13/18] iotests: Make 091 work with data_file

2019-09-29 Thread Maxim Levitsky
4304 bytes at offset 0 > Running 'qemu-img check -r all $TEST_IMG' > No errors were found on the image. > 80/16384 = 0.49% allocated, 0.00% fragmented, 0.00% compressed clusters > -Image end offset: 5570560 > *** done Best regards, Maxim Levitsky

Re: [PATCH 07/18] iotests: Replace IMGOPTS= by -o

2019-09-29 Thread Maxim Levitsky
=$mode _make_test_img $size | _filter_imgfmt > +_make_test_img -o preallocation=$mode $size | _filter_imgfmt > stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks > $min_blocks $size > done > > diff --git a/tests/qemu-iotests/190 b/tests/qemu-iotests/190 > index eb766ad09f..5890ff9cfc 100755 > --- a/tests/qemu-iotests/190 > +++ b/tests/qemu-iotests/190 > @@ -45,7 +45,7 @@ _supported_proto file > echo "== Huge file ==" > echo > > -IMGOPTS='cluster_size=2M' _make_test_img 2T > +_make_test_img -o 'cluster_size=2M' 2T > > $QEMU_IMG measure -O raw -f qcow2 "$TEST_IMG" > $QEMU_IMG measure -O qcow2 -o cluster_size=64k -f qcow2 "$TEST_IMG" > diff --git a/tests/qemu-iotests/191 b/tests/qemu-iotests/191 > index 528022e8d8..21c16a32cb 100755 > --- a/tests/qemu-iotests/191 > +++ b/tests/qemu-iotests/191 > @@ -51,8 +51,7 @@ echo === Preparing and starting VM === > echo > > TEST_IMG="${TEST_IMG}.base" _make_test_img $size > -IMGOPTS=$(_optstr_add "$IMGOPTS" "backing_fmt=$IMGFMT") \ > -TEST_IMG="${TEST_IMG}.mid" _make_test_img -b "${TEST_IMG}.base" > +TEST_IMG="${TEST_IMG}.mid" _make_test_img -o "backing_fmt=$IMGFMT" -b > "${TEST_IMG}.base" > _make_test_img -b "${TEST_IMG}.mid" > TEST_IMG="${TEST_IMG}.ovl2" _make_test_img -b "${TEST_IMG}.mid" > > diff --git a/tests/qemu-iotests/220 b/tests/qemu-iotests/220 > index 2d62c5dcac..3769f372cb 100755 > --- a/tests/qemu-iotests/220 > +++ b/tests/qemu-iotests/220 > @@ -37,6 +37,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 > _supported_fmt qcow2 > _supported_proto file > _supported_os Linux > +# To use a different refcount width but 16 bits we need compat=1.1 > +_unsupported_imgopts 'compat=0.10' > > echo "== Creating huge file ==" > > @@ -46,7 +48,7 @@ if ! truncate --size=513T "$TEST_IMG"; then > _notrun "file system on $TEST_DIR does not support large enough files" > fi > rm "$TEST_IMG" > -IMGOPTS='cluster_size=2M,refcount_bits=1' _make_test_img 513T > +_make_test_img -o 'cluster_size=2M,refcount_bits=1' 513T > > echo "== Populating refcounts ==" > # We want an image with 256M refcounts * 2M clusters = 512T referenced. > diff --git a/tests/qemu-iotests/243 b/tests/qemu-iotests/243 > index e563761307..2b84b896db 100755 > --- a/tests/qemu-iotests/243 > +++ b/tests/qemu-iotests/243 > @@ -40,6 +40,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 > _supported_fmt qcow2 > _supported_proto file > _supported_os Linux > +# External data files do not work with compat=0.10 > +_unsupported_imgopts 'compat=0.10' > > for mode in off metadata falloc full; do > > @@ -47,7 +49,7 @@ for mode in off metadata falloc full; do > echo "=== preallocation=$mode ===" > echo > > -IMGOPTS="preallocation=$mode" _make_test_img 64M > +_make_test_img -o "preallocation=$mode" 64M > > printf "File size: " > du -b $TEST_IMG | cut -f1 > @@ -64,7 +66,7 @@ for mode in off metadata falloc full; do > echo "=== External data file: preallocation=$mode ===" > echo > > -IMGOPTS="data_file=$TEST_IMG.data,preallocation=$mode" _make_test_img 64M > +_make_test_img -o "data_file=$TEST_IMG.data,preallocation=$mode" 64M > > echo -n "qcow2 file size: " > du -b $TEST_IMG | cut -f1 > diff --git a/tests/qemu-iotests/244 b/tests/qemu-iotests/244 > index 13978f93d2..0375bc12d4 100755 > --- a/tests/qemu-iotests/244 > +++ b/tests/qemu-iotests/244 > @@ -41,13 +41,15 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 > _supported_fmt qcow2 > _supported_proto file > _supported_os Linux > +# External data files do not work with compat=0.10 > +_unsupported_imgopts 'compat=0.10' > > echo > echo "=== Create and open image with external data file ===" > echo > > echo "With data file name in the image:" > -IMGOPTS="data_file=$TEST_IMG.data" _make_test_img 64M > +_make_test_img -o "data_file=$TEST_IMG.data" 64M > _check_test_img > > $QEMU_IO -c "open $TEST_IMG" -c "read -P 0 0 64k" 2>&1 | _filter_qemu_io | > _filter_testdir > @@ -104,7 +106,7 @@ echo > echo "=== Standalone image with external data file (efficient) ===" > echo > > -IMGOPTS="data_file=$TEST_IMG.data" _make_test_img 64M > +_make_test_img -o "data_file=$TEST_IMG.data" 64M > > echo -n "qcow2 file size before I/O: " > du -b $TEST_IMG | cut -f1 > @@ -154,7 +156,7 @@ echo > echo "=== Standalone image with external data file (valid raw) ===" > echo > > -IMGOPTS="data_file=$TEST_IMG.data,data_file_raw=on" _make_test_img 64M > +_make_test_img -o "data_file=$TEST_IMG.data,data_file_raw=on" 64M > > echo -n "qcow2 file size before I/O: " > du -b $TEST_IMG | cut -f1 > @@ -187,7 +189,7 @@ echo > echo "=== bdrv_co_block_status test for file and offset=0 ===" > echo > > -IMGOPTS="data_file=$TEST_IMG.data" _make_test_img 64M > +_make_test_img -o "data_file=$TEST_IMG.data" 64M > > $QEMU_IO -c 'write -P 0x11 0 1M' -f $IMGFMT "$TEST_IMG" | _filter_qemu_io > $QEMU_IO -c 'read -P 0x11 0 1M' -f $IMGFMT "$TEST_IMG" | _filter_qemu_io > diff --git a/tests/qemu-iotests/250 b/tests/qemu-iotests/250 > index c9c0a84a5a..670cf19076 100755 > --- a/tests/qemu-iotests/250 > +++ b/tests/qemu-iotests/250 > @@ -55,9 +55,8 @@ disk_usage() > } > > size=2100M > -IMGOPTS="cluster_size=1M,preallocation=metadata" > > -_make_test_img $size > +_make_test_img -o "cluster_size=1M,preallocation=metadata" $size > $QEMU_IO -c 'discard 0 10M' -c 'discard 2090M 10M' \ > -c 'write 2090M 10M' -c 'write 0 10M' "$TEST_IMG" | _filter_qemu_io > > diff --git a/tests/qemu-iotests/265 b/tests/qemu-iotests/265 > index dce6f77be3..00f2ec769e 100755 > --- a/tests/qemu-iotests/265 > +++ b/tests/qemu-iotests/265 > @@ -41,7 +41,7 @@ _supported_os Linux > echo '--- Writing to the image ---' > > # Reduce cluster size so we get more and quicker I/O > -IMGOPTS='cluster_size=4096' _make_test_img 1M > +_make_test_img -o 'cluster_size=4096' 1M > (for ((kb = 1024 - 4; kb >= 0; kb -= 4)); do \ > echo "aio_write -P 42 $((kb + 1))k 2k"; \ > done) \ Looks all right, but the patch is large, and I might have missed something. I don't know the iotests well enough to be sure. Best regards, Maxim Levitsky

Re: [PATCH 08/18] iotests: Replace IMGOPTS='' by --no-opts

2019-09-29 Thread Maxim Levitsky
, > diff --git a/tests/qemu-iotests/215 b/tests/qemu-iotests/215 > index 2eb377d682..55a1874dcd 100755 > --- a/tests/qemu-iotests/215 > +++ b/tests/qemu-iotests/215 > @@ -63,8 +63,8 @@ if [ "$IMGFMT" = "vpc" ]; then > fi > _make_test_img 4G > $QEMU_IO -c "write -P 55 3G 1k" "$TEST_IMG" | _filter_qemu_io > -IMGPROTO=file IMGFMT=qcow2 IMGOPTS= TEST_IMG_FILE="$TEST_WRAP" \ > -_make_test_img -F "$IMGFMT" -b "$TEST_IMG" | _filter_img_create > +IMGPROTO=file IMGFMT=qcow2 TEST_IMG_FILE="$TEST_WRAP" \ > +_make_test_img --no-opts -F "$IMGFMT" -b "$TEST_IMG" | _filter_img_create > $QEMU_IO -f qcow2 -c "write -z -u 1M 64k" "$TEST_WRAP" | _filter_qemu_io > > # Ensure that a read of two clusters, but where one is already allocated, Reviewed-by: Maxim Levitsky

Re: [PATCH 03/18] iotests: Drop compat=1.1 in 050

2019-09-29 Thread Maxim Levitsky
trap "_cleanup; exit \$status" 0 1 2 3 15 > _supported_fmt qcow2 qed > _supported_proto file > > -if test "$IMGFMT" = qcow2 && test $IMGOPTS = ""; then > - IMGOPTS=compat=1.1 > -fi > - > echo > echo "== Creating images ==" > Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH 05/18] iotests: Add -o and --no-opts to _make_test_img

2019-09-29 Thread Maxim Levitsky
> @@ -314,6 +319,14 @@ _make_test_img() > use_backing=1 > ;; > > +-o) > + opts_param=true > + ;; > + > +--no-opts) > +optstr="" > +;; > + > *) > misc_params=("${misc_params[@]}" "$param") > ;; Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH 04/18] iotests: Let _make_test_img parse its parameters

2019-09-29 Thread Maxim Levitsky
ame" $image_size 2>&1 > +$QEMU_IMG create $object_options -f $IMGFMT $extra_img_options -b > "$backing_file" "$img_name" "${misc_params[@]}" 2>&1 > else > -$QEMU_IMG create $object_options -f $IMGFMT $extra_img_options > "$img_name" $image_size 2>&1 > +$QEMU_IMG create $object_options -f $IMGFMT $extra_img_options > "$img_name" "${misc_params[@]}" 2>&1 > fi > ) | _filter_img_create > Look all right as far as I understand that code, and this is a very welcome change. Signed-off-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [PATCH 02/18] iotests: Replace IMGOPTS by _unsupported_imgopts

2019-09-29 Thread Maxim Levitsky
nt_bits=1[^0-9]' > > -IMGOPTS="compat=1.1" > IMG_SIZE=128K > > case "$QEMU_DEFAULT_MACHINE" in > diff --git a/tests/qemu-iotests/098 b/tests/qemu-iotests/098 > index 1c1d1c468f..2d68dc7d6c 100755 > --- a/tests/qemu-iotests/098 > +++ b/tests/qemu-iotests/098 > @@ -40,8 +40,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 > > _supported_fmt qcow2 > _supported_proto file > - > -IMGOPTS="compat=1.1" > +_unsupported_imgopts 'compat=0.10' Any idea why? I am not familiar with qcow2 well enought to know but this misses a comment with justification. > > for event in l1_update empty_image_prepare reftable_update refblock_alloc; do > Best regards, Maxim Levitsky

Re: [PATCH 01/18] iotests: Filter refcount_order in 036

2019-09-29 Thread Maxim Levitsky
unt_order(filtered) > header_length 104 > > Header extension: Minor notes: 1. Maybe put the sed command into a function to avoid duplication? 2. As I understand the test, it only checks the feature bits. So maybe instead of blacklisting some of the values, white list only the feature bits in the output? Best regards, Maxim Levitsky

Re: [PATCH v2 01/13] block-crypto: misc refactoring

2019-09-27 Thread Maxim Levitsky
On Fri, 2019-09-27 at 11:15 +0100, Daniel P. Berrangé wrote: > On Thu, Sep 26, 2019 at 12:35:15AM +0300, Maxim Levitsky wrote: > > * rename the write_func to create_write_func, > > and init_func to create_init_func > > this is preparation for other write_func that will &g

Re: [PATCH] tests: fix I/O test for hosts defaulting to LUKSv2

2019-09-27 Thread Maxim Levitsky
pping close - close device (remove mapping) I guess the documentation was not updated? I tested both by running the iotest 149, and by doing manually: dd if=/dev/zero bs=4K count=1000 of=./test cryptsetup -q -v luksFormat --type luks1 --cipher aes-xts-plain64 --key-size 512 --hash sha1 --key-slot 0 --key-file - --iter-time 10 ./test Tested-by: Maxim Levitsky Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

[PATCH v2 12/13] qcrypto-luks: more rigorous header checking

2019-09-25 Thread Maxim Levitsky
Check that keyslots don't overlap with the data, and check that keyslots don't overlap with each other. (this is done using naive O(n^2) nested loops, but since there are just 8 keyslots, this doesn't really matter. Signed-off-by: Maxim Levitsky Reviewed-by: Daniel P. Berrangé --- crypto/block

[PATCH v2 13/13] LUKS: better error message when creating too large files

2019-09-25 Thread Maxim Levitsky
, detect -EFBIG and in this case present a better error message, which is what this patch does The new error message is: qemu-img: error creating test.luks: The requested file size is too large Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534898 Signed-off-by: Maxim Levitsky Reviewed

[PATCH v2 07/13] qcrypto-luks: purge unused error codes from open callback

2019-09-25 Thread Maxim Levitsky
These values are not used by generic crypto code anyway Signed-off-by: Maxim Levitsky --- crypto/block-luks.c | 45 + 1 file changed, 13 insertions(+), 32 deletions(-) diff --git a/crypto/block-luks.c b/crypto/block-luks.c index f3bfc921b2

[PATCH v2 05/13] qcrypto-luks: pass keyslot index rather that pointer to the keyslot

2019-09-25 Thread Maxim Levitsky
Another minor refactoring Signed-off-by: Maxim Levitsky Reviewed-by: Daniel P. Berrangé --- crypto/block-luks.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crypto/block-luks.c b/crypto/block-luks.c index 9e59a791a6..b759cc8d19 100644 --- a/crypto/block-luks.c

[PATCH v2 11/13] qcrypto-luks: simplify the math used for keyslot locations

2019-09-25 Thread Maxim Levitsky
Signed-off-by: Maxim Levitsky Reviewed-by: Daniel P. Berrangé --- crypto/block-luks.c | 63 - 1 file changed, 40 insertions(+), 23 deletions(-) diff --git a/crypto/block-luks.c b/crypto/block-luks.c index 6d4e9eb348..a53d5d1916 100644 --- a/crypto

[PATCH v2 04/13] qcrypto-luks: simplify masterkey and masterkey length

2019-09-25 Thread Maxim Levitsky
Let the caller allocate masterkey Always use master key len from the header Signed-off-by: Maxim Levitsky Reviewed-by: Daniel P. Berrangé --- crypto/block-luks.c | 44 +--- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/crypto/block

[PATCH v2 10/13] qcrypto-luks: extract store key function

2019-09-25 Thread Maxim Levitsky
This function will be used later to store new keys to the luks metadata Signed-off-by: Maxim Levitsky Reviewed-by: Daniel P. Berrangé --- crypto/block-luks.c | 304 ++-- 1 file changed, 181 insertions(+), 123 deletions(-) diff --git a/crypto/block

[PATCH v2 08/13] qcrypto-luks: extract store and load header

2019-09-25 Thread Maxim Levitsky
Signed-off-by: Maxim Levitsky Reviewed-by: Daniel P. Berrangé --- crypto/block-luks.c | 155 ++-- 1 file changed, 93 insertions(+), 62 deletions(-) diff --git a/crypto/block-luks.c b/crypto/block-luks.c index b8f9b9c20a..47371edf13 100644 --- a/crypto

[PATCH v2 06/13] qcrypto-luks: use the parsed encryption settings in QCryptoBlockLUKS

2019-09-25 Thread Maxim Levitsky
Prior to that patch, the parsed encryption settings were already stored into the QCryptoBlockLUKS but not used anywhere but in qcrypto_block_luks_get_info Using them simplifies the code Signed-off-by: Maxim Levitsky Reviewed-by: Daniel P. Berrangé --- crypto/block-luks.c | 169

[PATCH v2 09/13] qcrypto-luks: extract check and parse header

2019-09-25 Thread Maxim Levitsky
This is just to make qcrypto_block_luks_open more reasonable in size. Signed-off-by: Maxim Levitsky Reviewed-by: Daniel P. Berrangé --- crypto/block-luks.c | 223 +--- 1 file changed, 125 insertions(+), 98 deletions(-) diff --git a/crypto/block-luks.c b

[PATCH v2 02/13] qcrypto-luks: rename some fields in QCryptoBlockLUKSHeader

2019-09-25 Thread Maxim Levitsky
* key_bytes -> master_key_len * payload_offset = payload_offset_sector (to emphasise that this isn't byte offset) * key_offset -> key_offset_sector - same as above for luks slots Signed-off-by: Maxim Levitsky Reviewed-by: Daniel P. Berrangé --- crypto/block-luks.

[PATCH v2 03/13] qcrypto-luks: don't overwrite cipher_mode in header

2019-09-25 Thread Maxim Levitsky
This way we can store the header we loaded, which will be used in key management code Signed-off-by: Maxim Levitsky Reviewed-by: Daniel P. Berrangé --- crypto/block-luks.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crypto/block-luks.c b/crypto/block-luks.c

[PATCH v2 01/13] block-crypto: misc refactoring

2019-09-25 Thread Maxim Levitsky
* rename the write_func to create_write_func, and init_func to create_init_func this is preparation for other write_func that will be used to update the encryption keys. No functional changes Signed-off-by: Maxim Levitsky Reviewed-by: Daniel P. Berrangé --- block/crypto.c | 12

[PATCH v2 00/13] crypto/luks: preparation for encryption key managment

2019-09-25 Thread Maxim Levitsky
an open bug that waits to be closed. Its also is form of refactoring, and thus I guess it makes sense to include it here. Best regards, Maxim Levitsky Maxim Levitsky (13): block-crypto: misc refactoring qcrypto-luks: rename some fields in QCryptoBlockLUKSHeader qcrypto-luks: don't

[PATCH] qemu-pr-helper: fix crash in mpath_reconstruct_sense

2019-09-23 Thread Maxim Levitsky
. However qemu-pr-helper should not crash in this case. Signed-off-by: Maxim Levitsky --- scsi/qemu-pr-helper.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c index a8a74d1dba..debb18f4aa 100644 --- a/scsi/qemu-pr-helper.c +++ b

Re: [PATCH v2 01/11] qcrypto: add suport for amend options

2019-09-23 Thread Maxim Levitsky
On Mon, 2019-09-23 at 08:08 -0500, Eric Blake wrote: > On 9/12/19 5:30 PM, Maxim Levitsky wrote: > > This adds the qcrypto_amend_options and corresponding > > crypto driver callbacks for the for encrypted > > grammar is off, did you miss a word where that double space is? &

Re: [Qemu-devel] [PATCH v2 00/11] RFC crypto/luks: encryption key managment using amend interface

2019-09-22 Thread Maxim Levitsky
On Fri, 2019-09-20 at 17:14 -0400, John Snow wrote: > > On 9/12/19 6:30 PM, Maxim Levitsky wrote: > > This patch series is continuation of my work to add encryption > > key managment to luks/qcow2 with luks. > > > > This is second version of this patch set. > &g

Re: [Qemu-block] [PATCH 7/8] block: Pass truncate exact=true where reasonable

2019-09-18 Thread Maxim Levitsky
turn offset; > } > > -ret = blk_truncate(blk, offset, false, PREALLOC_MODE_OFF, _err); > +/* > + * qemu-io is a debugging tool, so let us be strict here and pass > + * exact=true. It is better to err on the "emit more errors" side > + * than to be overly permissive. > + */ Also agree. > +ret = blk_truncate(blk, offset, true, PREALLOC_MODE_OFF, _err); > if (ret < 0) { > error_report_err(local_err); > return ret; Other than few nitpicks which probably aren't important, Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [Qemu-block] [PATCH 4/8] block: Add @exact parameter to bdrv_co_truncate()

2019-09-18 Thread Maxim Levitsky
ementations evaluate it, so that > this patch does not change existing behavior. Future patches take care > of that. > > Suggested-by: Maxim Levitsky > Signed-off-by: Max Reitz > --- > include/block/block.h | 6 +++--- > include/block/block_int.h | 17 +

Re: [Qemu-block] [PATCH 8/8] Revert "qemu-img: Check post-truncation size"

2019-09-18 Thread Maxim Levitsky
ed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [Qemu-block] [PATCH 6/8] block: Let format drivers pass @exact

2019-09-18 Thread Maxim Levitsky
> @@ -387,7 +387,7 @@ static int coroutine_fn raw_co_truncate(BlockDriverState > *bs, int64_t offset, > > s->size = offset; > offset += s->offset; > -return bdrv_co_truncate(bs->file, offset, false, prealloc, errp); > +return bdrv_co_truncate(bs->file, offset, exact, prealloc, errp); > } > > static void raw_eject(BlockDriverState *bs, bool eject_flag) Looks all right. Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [Qemu-block] [PATCH 3/8] block: Do not truncate file node when formatting

2019-09-18 Thread Maxim Levitsky
-} > - > /* Write the header */ > QEMU_BUILD_BUG_ON((1 << MIN_CLUSTER_BITS) < sizeof(*header)); > header = g_malloc0(cluster_size); As long as bdrv_co_create_ops still creates the underlying file with bdrv_create_file or so, I also don't see a reason to truncate it to 0 afterward. Especially for block devices... So, Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [Qemu-block] [PATCH 5/8] block: Evaluate @exact in protocol drivers

2019-09-18 Thread Maxim Levitsky
amp;& exact) { > + error_setg(errp, "Cannot resize iSCSI devices"); > +return -ENOTSUP; > +} else if (offset > cur_length) { > error_setg(errp, "Cannot grow iSCSI devices"); > return -EINVAL; > } Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [Qemu-block] [PATCH 2/8] block/cor: Drop cor_co_truncate()

2019-09-18 Thread Maxim Levitsky
> .bdrv_child_perm= cor_child_perm, > > .bdrv_getlength = cor_getlength, > -.bdrv_co_truncate = cor_co_truncate, > > .bdrv_co_preadv = cor_co_preadv, > .bdrv_co_pwritev= cor_co_pwritev, Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [Qemu-block] [PATCH 1/8] block: Handle filter truncation like native impl.

2019-09-18 Thread Maxim Levitsky
t I myself don't mind this :-) > ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); > if (ret < 0) { > error_setg_errno(errp, -ret, "Could not refresh total sector count"); Looks all right to me, although I don't know the block filters well yet. Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky

Re: [Qemu-block] [PATCH v7 0/3] Fix qcow2+luks corruption introduced by commit 8ac0f15f335

2019-09-16 Thread Maxim Levitsky
On Mon, 2019-09-16 at 16:59 +0300, Maxim Levitsky wrote: > On Mon, 2019-09-16 at 15:39 +0200, Max Reitz wrote: > > On 15.09.19 22:36, Maxim Levitsky wrote: > > > Commit 8ac0f15f335 accidently broke the COW of non changed areas > > > of newly allocated clusters, wh

Re: [Qemu-block] [PATCH v7 0/3] Fix qcow2+luks corruption introduced by commit 8ac0f15f335

2019-09-16 Thread Maxim Levitsky
On Mon, 2019-09-16 at 15:39 +0200, Max Reitz wrote: > On 15.09.19 22:36, Maxim Levitsky wrote: > > Commit 8ac0f15f335 accidently broke the COW of non changed areas > > of newly allocated clusters, when the write spans multiple clusters, > > and needs COW both prio

[Qemu-block] [PATCH v7 3/3] qemu-iotests: Add test for bz #1745922

2019-09-15 Thread Maxim Levitsky
Signed-off-by: Maxim Levitsky Tested-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/263 | 91 ++ tests/qemu-iotests/263.out | 40 + tests/qemu-iotests/group | 1 + 3 files changed, 132 insertions(+) create mode 100755 tests

[Qemu-block] [PATCH v7 0/3] Fix qcow2+luks corruption introduced by commit 8ac0f15f335

2019-09-15 Thread Maxim Levitsky
regards, Maxim Levitsky Maxim Levitsky (3): block/qcow2: Fix corruption introduced by commit 8ac0f15f335 block/qcow2: refactor encryption code qemu-iotests: Add test for bz #1745922 block/qcow2-cluster.c | 40 ++--- block/qcow2-threads.c | 63

[Qemu-block] [PATCH v7 1/3] block/qcow2: Fix corruption introduced by commit 8ac0f15f335

2019-09-15 Thread Maxim Levitsky
these assumptions. In the bugreport that was triggered by rebasing a luks image to new, zero filled base, which lot of such writes, and causes some files with zero areas to contain garbage there instead. But as described above it can happen elsewhere as well Signed-off-by: Maxim Levitsky Reviewed

[Qemu-block] [PATCH v7 2/3] block/qcow2: refactor encryption code

2019-09-15 Thread Maxim Levitsky
{encrypt|decrypt} arguments to prevent the bug fixed in former commit from hopefully happening again. Signed-off-by: Maxim Levitsky --- block/qcow2-cluster.c | 41 ++-- block/qcow2-threads.c | 63 +-- block/qcow2.c | 5

Re: [Qemu-block] [Qemu-devel] [PATCH v6 2/3] block/qcow2: refactor threaded encryption code

2019-09-13 Thread Maxim Levitsky
On Fri, 2019-09-13 at 17:51 +, Vladimir Sementsov-Ogievskiy wrote: > 13.09.2019 20:27, Maxim Levitsky wrote: > > Change the qcow2_co_encrypt to just receive full host and > > guest offsets and in pariticular remove the > > offset_in_cluster parameter of do_perform_c

[Qemu-block] [PATCH v6 2/3] block/qcow2: refactor threaded encryption code

2019-09-13 Thread Maxim Levitsky
it with qcow2_co_encrypt Also document the qcow2_co_encrypt arguments to prevent that bug from happening again Signed-off-by: Maxim Levitsky Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/qcow2-cluster.c | 35 +-- block/qcow2-threads.c | 81

[Qemu-block] [PATCH v6 3/3] qemu-iotests: Add test for bz #1745922

2019-09-13 Thread Maxim Levitsky
Signed-off-by: Maxim Levitsky Tested-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/263 | 91 ++ tests/qemu-iotests/263.out | 40 + tests/qemu-iotests/group | 1 + 3 files changed, 132 insertions(+) create mode 100755 tests

[Qemu-block] [PATCH v6 1/3] Fix qcow2+luks corruption introduced by commit 8ac0f15f335

2019-09-13 Thread Maxim Levitsky
. In the bugreport that was triggered by rebasing a luks image to new, zero filled base, which lot of such writes, and causes some files with zero areas to contain garbage there instead. But as described above it can happen elsewhere as well Signed-off-by: Maxim Levitsky Reviewed-by: Vladimir Sementsov

[Qemu-block] [PATCH v6 0/3] Fix qcow2+luks corruption introduced by commit 8ac0f15f335

2019-09-13 Thread Maxim Levitsky
everything. V5: reworked the patches so one of them fixes the bug only and other one is just refactoring V6: removed do_perform_cow_encrypt Best regards, Maxim Levitsky Maxim Levitsky (3): Fix qcow2+luks corruption introduced by commit 8ac0f15f335 block/qcow2: refactor threaded

Re: [Qemu-block] [PATCH v5 3/3] qemu-iotests: Add test for bz #1745922

2019-09-13 Thread Maxim Levitsky
On Fri, 2019-09-13 at 16:57 +, Vladimir Sementsov-Ogievskiy wrote: > 13.09.2019 19:39, Maxim Levitsky wrote: > > On Fri, 2019-09-13 at 16:27 +, Vladimir Sementsov-Ogievskiy wrote: > > > 13.09.2019 18:28, Maxim Levitsky wrote: > > > >

Re: [Qemu-block] [PATCH v5 3/3] qemu-iotests: Add test for bz #1745922

2019-09-13 Thread Maxim Levitsky
On Fri, 2019-09-13 at 16:27 +, Vladimir Sementsov-Ogievskiy wrote: > 13.09.2019 18:28, Maxim Levitsky wrote: > > Signed-off-by: Maxim Levitsky > > --- > > tests/qemu-iotests/263 | 91 ++ > > test

Re: [Qemu-block] [PATCH v5 2/3] block/qcow2: refactor threaded encryption code

2019-09-13 Thread Maxim Levitsky
On Fri, 2019-09-13 at 16:11 +, Vladimir Sementsov-Ogievskiy wrote: > 13.09.2019 18:28, Maxim Levitsky wrote: > > Change do_perform_cow_encrypt and its callee qcow2_co_encrypt > > to just receive full host and guest offsets and in pariticular > > remove the offset

[Qemu-block] [PATCH v5 2/3] block/qcow2: refactor threaded encryption code

2019-09-13 Thread Maxim Levitsky
the qcow2_co_encrypt arguments to prevent that bug from happening again Signed-off-by: Maxim Levitsky --- block/qcow2-cluster.c | 30 - block/qcow2-threads.c | 62 ++- block/qcow2.c | 5 ++-- block/qcow2.h | 8 +++--- 4

[Qemu-block] [PATCH v5 3/3] qemu-iotests: Add test for bz #1745922

2019-09-13 Thread Maxim Levitsky
Signed-off-by: Maxim Levitsky --- tests/qemu-iotests/263 | 91 ++ tests/qemu-iotests/263.out | 40 + tests/qemu-iotests/group | 2 + 3 files changed, 133 insertions(+) create mode 100755 tests/qemu-iotests/263 create mode 100644 tests

[Qemu-block] [PATCH v5 1/3] Fix qcow2+luks corruption introduced by commit 8ac0f15f335

2019-09-13 Thread Maxim Levitsky
. In the bugreport that was triggered by rebasing a luks image to new, zero filled base, which lot of such writes, and causes some files with zero areas to contain garbage there instead. But as described above it can happen elsewhere as well Signed-off-by: Maxim Levitsky --- block/qcow2-cluster.c | 7

[Qemu-block] [PATCH v5 0/3] Fix qcow2+luks corruption introduced by commit 8ac0f15f335

2019-09-13 Thread Maxim Levitsky
everything. V5: reworked the patches so one of them fixes the bug only and other one is just refactoring Best regards, Maxim Levitsky Maxim Levitsky (3): Fix qcow2+luks corruption introduced by commit 8ac0f15f335 block/qcow2: refactor threaded encryption code qemu-iotests: Add

Re: [Qemu-block] [Qemu-devel] [PATCH v4 1/3] block/qcow2: refactoring of threaded encryption code

2019-09-13 Thread Maxim Levitsky
On Fri, 2019-09-13 at 16:24 +0200, Kevin Wolf wrote: > Am 13.09.2019 um 16:07 hat Maxim Levitsky geschrieben: > > On Fri, 2019-09-13 at 14:01 +, Vladimir Sementsov-Ogievskiy wrote: > > > 13.09.2019 15:59, Maxim Levitsky wrote: > > > > This commit tries to

Re: [Qemu-block] [Qemu-devel] [PATCH v4 1/3] block/qcow2: refactoring of threaded encryption code

2019-09-13 Thread Maxim Levitsky
On Fri, 2019-09-13 at 14:01 +, Vladimir Sementsov-Ogievskiy wrote: > 13.09.2019 15:59, Maxim Levitsky wrote: > > This commit tries to clarify few function arguments, > > and add comments describing the encrypt/decrypt interface > > > > Signed-off-by: Maxim Levitsky

[Qemu-block] [PATCH v2 1/2] block/nvme: add support for write zeros

2019-09-13 Thread Maxim Levitsky
Signed-off-by: Maxim Levitsky --- block/nvme.c | 72 +++- block/trace-events | 1 + include/block/nvme.h | 19 +++- 3 files changed, 90 insertions(+), 2 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index 5be3a39b63..d95265fae4

[Qemu-block] [PATCH v2 0/2] block/nvme: add support for write zeros and discard

2019-09-13 Thread Maxim Levitsky
This is the second part of the patches I prepared for this driver back when I worked on mdev-nvme. V2: addressed review feedback, no major changes Best regards, Maxim Levitsky Maxim Levitsky (2): block/nvme: add support for write zeros block/nvme: add support for discard block

[Qemu-block] [PATCH v2 2/2] block/nvme: add support for discard

2019-09-13 Thread Maxim Levitsky
Signed-off-by: Maxim Levitsky --- block/nvme.c | 83 ++ block/trace-events | 2 ++ 2 files changed, 85 insertions(+) diff --git a/block/nvme.c b/block/nvme.c index d95265fae4..c17edd6aae 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -112,6

Re: [Qemu-block] [Qemu-devel] [PATCH 1/2] block/nvme: add support for write zeros

2019-09-13 Thread Maxim Levitsky
working without a unit test, a spec, or > some instructions to help me verify any of this does what it looks like > it does. > > On 8/25/19 3:15 AM, Maxim Levitsky wrote: > > Signed-off-by: Maxim Levitsky > > --- > > block/nvme.c | 72 +++

Re: [Qemu-block] [PATCH v3 1/3] block/qcow2: refactoring of threaded encryption code

2019-09-13 Thread Maxim Levitsky
On Fri, 2019-09-13 at 12:37 +0200, Max Reitz wrote: > On 13.09.19 00:37, Maxim Levitsky wrote: > > This commit tries to clarify few function arguments, > > and add comments describing the encrypt/decrypt interface > > > > Signed-off-by: Maxim Levitsky > > --

[Qemu-block] [PATCH v4 2/3] block/qcow2: fix the corruption when rebasing luks encrypted files

2019-09-13 Thread Maxim Levitsky
, and causes some files with zero areas to contain garbage there instead. But as described above it can happen elsewhere as well Signed-off-by: Maxim Levitsky Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- block/qcow2-cluster.c | 30 +- 1 file

[Qemu-block] [PATCH v4 3/3] qemu-iotests: Add test for bz #1745922

2019-09-13 Thread Maxim Levitsky
Signed-off-by: Maxim Levitsky --- tests/qemu-iotests/263 | 91 ++ tests/qemu-iotests/263.out | 40 + tests/qemu-iotests/group | 1 + 3 files changed, 132 insertions(+) create mode 100755 tests/qemu-iotests/263 create mode 100644 tests

[Qemu-block] [PATCH v4 1/3] block/qcow2: refactoring of threaded encryption code

2019-09-13 Thread Maxim Levitsky
This commit tries to clarify few function arguments, and add comments describing the encrypt/decrypt interface Signed-off-by: Maxim Levitsky --- block/qcow2-cluster.c | 9 --- block/qcow2-threads.c | 62 ++- block/qcow2.c | 5 ++-- block

[Qemu-block] [PATCH v4 0/3] Fix qcow2+luks corruption introduced by commit 8ac0f15f335

2019-09-13 Thread Maxim Levitsky
everything. Best regards, Maxim Levitsky Maxim Levitsky (3): block/qcow2: refactoring of threaded encryption code block/qcow2: fix the corruption when rebasing luks encrypted files qemu-iotests: Add test for bz #1745922 block/qcow2-cluster.c | 29 +++- block/qcow2

<    2   3   4   5   6   7   8   9   10   >