From: Denis Plotnikov <dplotnikov@virtuozzo.com>

The patch allows to provide a pattern file for write

command. There was no similar ability before.

Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>

Message-id: 20190820164616.4072-1-dplotnikov@virtuozzo.com

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

[mreitz: Keep optstring in alphabetical order]

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

---

qemu-io-cmds.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++---

1 file changed, 93 insertions(+), 6 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c

index XXXXXXX..XXXXXXX 100644

--- a/qemu-io-cmds.c

+++ b/qemu-io-cmds.c

@@ -XXX,XX +XXX,XX @@ static void qemu_io_free(void *p)

qemu_vfree(p);

}

+/*

+ * qemu_io_alloc_from_file()

+ *

+ * Allocates the buffer and populates it with the content of the given file

+ * up to @len bytes. If the file length is less than @len, then the buffer

+ * is populated with the file content cyclically.

+ *

+ * @blk - the block backend where the buffer content is going to be written to

+ * @len - the buffer length

+ * @file_name - the file to read the content from

+ *

+ * Returns: the buffer pointer on success

+ * NULL on error

+ */

+static void *qemu_io_alloc_from_file(BlockBackend *blk, size_t len,

+ const char *file_name)

+{

+ char *buf, *buf_origin;

+ FILE *f = fopen(file_name, "r");

+ int pattern_len;

+

+ if (!f) {

+ perror(file_name);

+ return NULL;

+ }

+

+ if (qemuio_misalign) {

+ len += MISALIGN_OFFSET;

+ }

+

+ buf_origin = buf = blk_blockalign(blk, len);

+

+ if (qemuio_misalign) {

+ buf_origin += MISALIGN_OFFSET;

+ buf += MISALIGN_OFFSET;

+ len -= MISALIGN_OFFSET;

+ }

+

+ pattern_len = fread(buf_origin, 1, len, f);

+

+ if (ferror(f)) {

+ perror(file_name);

+ goto error;

+ }

+

+ if (pattern_len == 0) {

+ fprintf(stderr, "%s: file is empty\n", file_name);

+ goto error;

+ }

+

+ fclose(f);

+

+ if (len > pattern_len) {

+ len -= pattern_len;

+ buf += pattern_len;

+

+ while (len > 0) {

+ size_t len_to_copy = MIN(pattern_len, len);

+

+ memcpy(buf, buf_origin, len_to_copy);

+

+ len -= len_to_copy;

+ buf += len_to_copy;

+ }

+ }

+

+ return buf_origin;

+

+error:

+ qemu_io_free(buf_origin);

+ return NULL;

+}

+

static void dump_buffer(const void *buffer, int64_t offset, int64_t len)

{

uint64_t i;

@@ -XXX,XX +XXX,XX @@ static void write_help(void)

" -n, -- with -z, don't allow slow fallback\n"

" -p, -- ignored for backwards compatibility\n"

" -P, -- use different pattern to fill file\n"

+" -s, -- use a pattern file to fill the write buffer\n"

" -C, -- report statistics in a machine parsable format\n"

" -q, -- quiet mode, do not show I/O statistics\n"

" -u, -- with -z, allow unmapping\n"

@@ -XXX,XX +XXX,XX @@ static const cmdinfo_t write_cmd = {

.perm = BLK_PERM_WRITE,

.argmin = 2,

.argmax = -1,

- .args = "[-bcCfnquz] [-P pattern] off len",

+ .args = "[-bcCfnquz] [-P pattern | -s source_file] off len",

.oneline = "writes a number of bytes at a specified offset",

.help = write_help,

};

@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)

{

struct timespec t1, t2;

bool Cflag = false, qflag = false, bflag = false;

- bool Pflag = false, zflag = false, cflag = false;

+ bool Pflag = false, zflag = false, cflag = false, sflag = false;

int flags = 0;

int c, cnt, ret;

char *buf = NULL;

@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)

/* Some compilers get confused and warn if this is not initialized. */

int64_t total = 0;

int pattern = 0xcd;

+ const char *file_name = NULL;

- while ((c = getopt(argc, argv, "bcCfnpP:quz")) != -1) {

+ while ((c = getopt(argc, argv, "bcCfnpP:qs:uz")) != -1) {

switch (c) {

case 'b':

bflag = true;

@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)

case 'q':

qflag = true;

break;

+ case 's':

+ sflag = true;

+ file_name = optarg;

+ break;

case 'u':

flags |= BDRV_REQ_MAY_UNMAP;

break;

@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)

return -EINVAL;

}

- if (zflag && Pflag) {

- printf("-z and -P cannot be specified at the same time\n");

+ if (zflag + Pflag + sflag > 1) {

+ printf("Only one of -z, -P, and -s "

+ "can be specified at the same time\n");

return -EINVAL;

}

@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)

}

if (!zflag) {

- buf = qemu_io_alloc(blk, count, pattern);

+ if (sflag) {

+ buf = qemu_io_alloc_from_file(blk, count, file_name);

+ if (!buf) {

+ return -EINVAL;

+ }

+ } else {

+ buf = qemu_io_alloc(blk, count, pattern);

+ }

}

clock_gettime(CLOCK_MONOTONIC, &t1);

--

2.21.0

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

It's wrong to OR shared permissions. It may lead to crash on further

permission updates.

Also, no needs to consider previously calculated permissions, as at

this point we already bind all new parents and bdrv_get_cumulative_perm

result is enough. So fix the bug by just set permissions by

bdrv_get_cumulative_perm result.

Bug was introduced in long ago 234ac1a9025, in 2.9.

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

Message-id: 20190824100740.61635-1-vsementsov@virtuozzo.com

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

---

block.c | 5 ++---

1 file changed, 2 insertions(+), 3 deletions(-)

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

index XXXXXXX..XXXXXXX 100644

--- a/block.c

+++ b/block.c

@@ -XXX,XX +XXX,XX @@ void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,

{

BdrvChild *c, *next;

GSList *list = NULL, *p;

- uint64_t old_perm, old_shared;

uint64_t perm = 0, shared = BLK_PERM_ALL;

int ret;

@@ -XXX,XX +XXX,XX @@ void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,

bdrv_unref(from);

}

- bdrv_get_cumulative_perm(to, &old_perm, &old_shared);

- bdrv_set_perm(to, old_perm | perm, old_shared | shared);

+ bdrv_get_cumulative_perm(to, &perm, &shared);

+ bdrv_set_perm(to, perm, shared);

out:

g_slist_free(list);

--

2.21.0

From: Nir Soffer <nirsof@gmail.com>

When creating an image with preallocation "off" or "falloc", the first

block of the image is typically not allocated. When using Gluster

storage backed by XFS filesystem, reading this block using direct I/O

succeeds regardless of request length, fooling alignment detection.

In this case we fallback to a safe value (4096) instead of the optimal

value (512), which may lead to unneeded data copying when aligning

requests. Allocating the first block avoids the fallback.

Since we allocate the first block even with preallocation=off, we no

longer create images with zero disk size:

$ ./qemu-img create -f raw test.raw 1g

Formatting 'test.raw', fmt=raw size=1073741824

$ ls -lhs test.raw

4.0K -rw-r--r--. 1 nsoffer nsoffer 1.0G Aug 16 23:48 test.raw

And converting the image requires additional cluster:

$ ./qemu-img measure -f raw -O qcow2 test.raw

required size: 458752

fully allocated size: 1074135040

When using format like vmdk with multiple files per image, we allocate

one block per file:

$ ./qemu-img create -f vmdk -o subformat=twoGbMaxExtentFlat test.vmdk 4g

Formatting 'test.vmdk', fmt=vmdk size=4294967296 compat6=off hwversion=undefined subformat=twoGbMaxExtentFlat

$ ls -lhs test*.vmdk

4.0K -rw-r--r--. 1 nsoffer nsoffer 2.0G Aug 27 03:23 test-f001.vmdk

4.0K -rw-r--r--. 1 nsoffer nsoffer 2.0G Aug 27 03:23 test-f002.vmdk

4.0K -rw-r--r--. 1 nsoffer nsoffer 353 Aug 27 03:23 test.vmdk

I did quick performance test for copying disks with qemu-img convert to

new raw target image to Gluster storage with sector size of 512 bytes:

for i in $(seq 10); do

rm -f dst.raw

sleep 10

time ./qemu-img convert -f raw -O raw -t none -T none src.raw dst.raw

done

Here is a table comparing the total time spent:

Type Before(s) After(s) Diff(%)

---------------------------------------

real 530.028 469.123 -11.4

user 17.204 10.768 -37.4

sys 17.881 7.011 -60.7

We can see very clear improvement in CPU usage.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>

Message-id: 20190827010528.8818-2-nsoffer@redhat.com

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

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

---

block/file-posix.c | 51 +++++++++++++++++++

tests/qemu-iotests/059.out | 2 +-

tests/qemu-iotests/{150.out => 150.out.qcow2} | 0

tests/qemu-iotests/150.out.raw | 12 +++++

tests/qemu-iotests/175 | 19 ++++---

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

tests/qemu-iotests/178.out.qcow2 | 4 +-

tests/qemu-iotests/221.out | 12 +++--

tests/qemu-iotests/253.out | 12 +++--

9 files changed, 99 insertions(+), 21 deletions(-)

rename tests/qemu-iotests/{150.out => 150.out.qcow2} (100%)

create mode 100644 tests/qemu-iotests/150.out.raw

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

index XXXXXXX..XXXXXXX 100644

--- a/block/file-posix.c

+++ b/block/file-posix.c

@@ -XXX,XX +XXX,XX @@ static int handle_aiocb_discard(void *opaque)

return ret;

}

+/*

+ * Help alignment probing by allocating the first block.

+ *

+ * When reading with direct I/O from unallocated area on Gluster backed by XFS,

+ * reading succeeds regardless of request length. In this case we fallback to

+ * safe alignment which is not optimal. Allocating the first block avoids this

+ * fallback.

+ *

+ * fd may be opened with O_DIRECT, but we don't know the buffer alignment or

+ * request alignment, so we use safe values.

+ *

+ * Returns: 0 on success, -errno on failure. Since this is an optimization,

+ * caller may ignore failures.

+ */

+static int allocate_first_block(int fd, size_t max_size)

+{

+ size_t write_size = (max_size < MAX_BLOCKSIZE)

+ ? BDRV_SECTOR_SIZE

+ : MAX_BLOCKSIZE;

+ size_t max_align = MAX(MAX_BLOCKSIZE, getpagesize());

+ void *buf;

+ ssize_t n;

+ int ret;

+

+ buf = qemu_memalign(max_align, write_size);

+ memset(buf, 0, write_size);

+

+ do {

+ n = pwrite(fd, buf, write_size, 0);

+ } while (n == -1 && errno == EINTR);

+

+ ret = (n == -1) ? -errno : 0;

+

+ qemu_vfree(buf);

+ return ret;

+}

+

static int handle_aiocb_truncate(void *opaque)

{

RawPosixAIOData *aiocb = opaque;

@@ -XXX,XX +XXX,XX @@ static int handle_aiocb_truncate(void *opaque)

/* posix_fallocate() doesn't set errno. */

error_setg_errno(errp, -result,

"Could not preallocate new data");

+ } else if (current_length == 0) {

+ /*

+ * posix_fallocate() uses fallocate() if the filesystem

+ * supports it, or fallback to manually writing zeroes. If

+ * fallocate() was used, unaligned reads from the fallocated

+ * area in raw_probe_alignment() will succeed, hence we need to

+ * allocate the first block.

+ *

+ * Optimize future alignment probing; ignore failures.

+ */

+ allocate_first_block(fd, offset);

}

} else {

result = 0;

@@ -XXX,XX +XXX,XX @@ static int handle_aiocb_truncate(void *opaque)

if (ftruncate(fd, offset) != 0) {

result = -errno;

error_setg_errno(errp, -result, "Could not resize file");

+ } else if (current_length == 0 && offset > current_length) {

+ /* Optimize future alignment probing; ignore failures. */

+ allocate_first_block(fd, offset);

}

return result;

default:

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

index XXXXXXX..XXXXXXX 100644

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

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

@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824000 subformat=twoGbMax

image: TEST_DIR/t.vmdk

file format: vmdk

virtual size: 0.977 TiB (1073741824000 bytes)

-disk size: 16 KiB

+disk size: 1.97 MiB

Format specific information:

cid: XXXXXXXX

parent cid: XXXXXXXX

diff --git a/tests/qemu-iotests/150.out b/tests/qemu-iotests/150.out.qcow2

similarity index 100%

rename from tests/qemu-iotests/150.out

rename to tests/qemu-iotests/150.out.qcow2

diff --git a/tests/qemu-iotests/150.out.raw b/tests/qemu-iotests/150.out.raw

new file mode 100644

index XXXXXXX..XXXXXXX

--- /dev/null

+++ b/tests/qemu-iotests/150.out.raw

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

+QA output created by 150

+

+=== Mapping sparse conversion ===

+

+Offset Length File

+0 0x1000 TEST_DIR/t.IMGFMT

+

+=== Mapping non-sparse conversion ===

+

+Offset Length File

+0 0x100000 TEST_DIR/t.IMGFMT

+*** done

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

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/175

+++ b/tests/qemu-iotests/175

@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15

# the file size. This function hides the resulting difference in the

# stat -c '%b' output.

# Parameter 1: Number of blocks an empty file occupies

-# Parameter 2: Image size in bytes

+# Parameter 2: Minimal number of blocks in an image

+# Parameter 3: Image size in bytes

_filter_blocks()

{

extra_blocks=$1

- img_size=$2

+ min_blocks=$2

+ img_size=$3

- sed -e "s/blocks=$extra_blocks\\(\$\\|[^0-9]\\)/nothing allocated/" \

- -e "s/blocks=$((extra_blocks + img_size / 512))\\(\$\\|[^0-9]\\)/everything allocated/"

+ sed -e "s/blocks=$min_blocks\\(\$\\|[^0-9]\\)/min allocation/" \

+ -e "s/blocks=$((extra_blocks + img_size / 512))\\(\$\\|[^0-9]\\)/max allocation/"

}

# get standard environment, filters and checks

@@ -XXX,XX +XXX,XX @@ size=$((1 * 1024 * 1024))

touch "$TEST_DIR/empty"

extra_blocks=$(stat -c '%b' "$TEST_DIR/empty")

+# We always write the first byte; check how many blocks this filesystem

+# allocates to match empty image alloation.

+printf "\0" > "$TEST_DIR/empty"

+min_blocks=$(stat -c '%b' "$TEST_DIR/empty")

+

echo

echo "== creating image with default preallocation =="

_make_test_img $size | _filter_imgfmt

-stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $size

+stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $min_blocks $size

for mode in off full falloc; do

echo

echo "== creating image with preallocation $mode =="

IMGOPTS=preallocation=$mode _make_test_img $size | _filter_imgfmt

- stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $size

+ stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $min_blocks $size

done

# success, all done

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

index XXXXXXX..XXXXXXX 100644

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

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

@@ -XXX,XX +XXX,XX @@ QA output created by 175

== creating image with default preallocation ==

Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576

-size=1048576, nothing allocated

+size=1048576, min allocation

== creating image with preallocation off ==

Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=off

-size=1048576, nothing allocated

+size=1048576, min allocation

== creating image with preallocation full ==

Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=full

-size=1048576, everything allocated

+size=1048576, max allocation

== creating image with preallocation falloc ==

Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=falloc

-size=1048576, everything allocated

+size=1048576, max allocation

*** done

diff --git a/tests/qemu-iotests/178.out.qcow2 b/tests/qemu-iotests/178.out.qcow2

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/178.out.qcow2

+++ b/tests/qemu-iotests/178.out.qcow2

@@ -XXX,XX +XXX,XX @@ converted image file size in bytes: 196608

== raw input image with data (human) ==

Formatting 'TEST_DIR/t.qcow2', fmt=IMGFMT size=1073741824

-required size: 393216

+required size: 458752

fully allocated size: 1074135040

wrote 512/512 bytes at offset 512

512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

@@ -XXX,XX +XXX,XX @@ converted image file size in bytes: 196608

Formatting 'TEST_DIR/t.qcow2', fmt=IMGFMT size=1073741824

{

- "required": 393216,

+ "required": 458752,

"fully-allocated": 1074135040

}

wrote 512/512 bytes at offset 512

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

index XXXXXXX..XXXXXXX 100644

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

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

@@ -XXX,XX +XXX,XX @@ QA output created by 221

=== Check mapping of unaligned raw image ===

Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65537

-[{ "start": 0, "length": 66048, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]

-[{ "start": 0, "length": 66048, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]

+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},

+{ "start": 4096, "length": 61952, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]

+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},

+{ "start": 4096, "length": 61952, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]

wrote 1/1 bytes at offset 65536

1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

-[{ "start": 0, "length": 65536, "depth": 0, "zero": true, "data": false, "offset": OFFSET},

+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},

+{ "start": 4096, "length": 61440, "depth": 0, "zero": true, "data": false, "offset": OFFSET},

{ "start": 65536, "length": 1, "depth": 0, "zero": false, "data": true, "offset": OFFSET},

{ "start": 65537, "length": 511, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]

-[{ "start": 0, "length": 65536, "depth": 0, "zero": true, "data": false, "offset": OFFSET},

+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},

+{ "start": 4096, "length": 61440, "depth": 0, "zero": true, "data": false, "offset": OFFSET},

{ "start": 65536, "length": 1, "depth": 0, "zero": false, "data": true, "offset": OFFSET},

{ "start": 65537, "length": 511, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]

*** done

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

index XXXXXXX..XXXXXXX 100644

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

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

@@ -XXX,XX +XXX,XX @@ QA output created by 253

=== Check mapping of unaligned raw image ===

Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048575

-[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]

-[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]

+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},

+{ "start": 4096, "length": 1044480, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]

+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},

+{ "start": 4096, "length": 1044480, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]

wrote 65535/65535 bytes at offset 983040

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

-[{ "start": 0, "length": 983040, "depth": 0, "zero": true, "data": false, "offset": OFFSET},

+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},

+{ "start": 4096, "length": 978944, "depth": 0, "zero": true, "data": false, "offset": OFFSET},

{ "start": 983040, "length": 65536, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]

-[{ "start": 0, "length": 983040, "depth": 0, "zero": true, "data": false, "offset": OFFSET},

+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},

+{ "start": 4096, "length": 978944, "depth": 0, "zero": true, "data": false, "offset": OFFSET},

{ "start": 983040, "length": 65536, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]

*** done

--

2.21.0

From: Nir Soffer <nirsof@gmail.com>

Using block_resize we can test allocate_first_block() with file

descriptor opened with O_DIRECT, ensuring that it works for any size

larger than 4096 bytes.

Testing smaller sizes is tricky as the result depends on the filesystem

used for testing. For example on NFS any size will work since O_DIRECT

does not require any alignment.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>

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

Message-id: 20190827010528.8818-3-nsoffer@redhat.com

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

---

tests/qemu-iotests/175 | 28 ++++++++++++++++++++++++++++

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

2 files changed, 36 insertions(+)

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

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/175

+++ b/tests/qemu-iotests/175

@@ -XXX,XX +XXX,XX @@ _filter_blocks()

-e "s/blocks=$((extra_blocks + img_size / 512))\\(\$\\|[^0-9]\\)/max allocation/"

}

+# Resize image using block_resize.

+# Parameter 1: image path

+# Parameter 2: new size

+_block_resize()

+{

+ local path=$1

+ local size=$2

+

+ $QEMU -qmp stdio -nographic -nodefaults \

+ -blockdev file,node-name=file,filename=$path,cache.direct=on \

+ <<EOF

+{'execute': 'qmp_capabilities'}

+{'execute': 'block_resize', 'arguments': {'node-name': 'file', 'size': $size}}

+{'execute': 'quit'}

+EOF

+}

+

# get standard environment, filters and checks

. ./common.rc

. ./common.filter

@@ -XXX,XX +XXX,XX @@ _supported_fmt raw

_supported_proto file

_supported_os Linux

+_default_cache_mode none

+_supported_cache_modes none directsync

+

size=$((1 * 1024 * 1024))

touch "$TEST_DIR/empty"

@@ -XXX,XX +XXX,XX @@ for mode in off full falloc; do

stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $min_blocks $size

done

+for new_size in 4096 1048576; do

+ echo

+ echo "== resize empty image with block_resize =="

+ _make_test_img 0 | _filter_imgfmt

+ _block_resize $TEST_IMG $new_size >/dev/null

+ stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $min_blocks $new_size

+done

+

# success, all done

echo "*** done"

rm -f $seq.full

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

index XXXXXXX..XXXXXXX 100644

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

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

@@ -XXX,XX +XXX,XX @@ size=1048576, max allocation

== creating image with preallocation falloc ==

Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=falloc

size=1048576, max allocation

+

+== resize empty image with block_resize ==

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

+size=4096, min allocation

+

+== resize empty image with block_resize ==

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

+size=1048576, min allocation

*** done

--

2.21.0

fe646693acc changed qemu-img create's output so that it no longer prints

single quotes around parameter values. The subformat and adapter_type

filters in _filter_img_create() have never been adapted to that change.

Fixes: fe646693acc13ac48b98435d14149ab04dc597bc

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

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

Message-id: 20190815153638.4600-2-mreitz@redhat.com

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

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

---

tests/qemu-iotests/059.out | 16 ++++++++--------

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

2 files changed, 10 insertions(+), 10 deletions(-)

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

index XXXXXXX..XXXXXXX 100644

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

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

@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864

qemu-io: can't open device TEST_DIR/t.vmdk: L1 size too big

=== Testing monolithicFlat creation and opening ===

-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648 subformat=monolithicFlat

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

image: TEST_DIR/t.IMGFMT

file format: IMGFMT

virtual size: 2 GiB (2147483648 bytes)

=== Testing monolithicFlat with zeroed_grain ===

qemu-img: TEST_DIR/t.IMGFMT: Flat image can't enable zeroed grain

-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648 subformat=monolithicFlat

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

=== Testing big twoGbMaxExtentFlat ===

-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824000 subformat=twoGbMaxExtentFlat

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

image: TEST_DIR/t.vmdk

file format: vmdk

virtual size: 0.977 TiB (1073741824000 bytes)

@@ -XXX,XX +XXX,XX @@ Format specific information:

qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Invalid extent line: RW 12582912 VMFS "dummy.IMGFMT" 1

=== Testing truncated sparse ===

-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=107374182400 subformat=monolithicSparse

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

qemu-img: Could not open 'TEST_DIR/t.IMGFMT': File truncated, expecting at least 13172736 bytes

=== Converting to streamOptimized from image with small cluster size===

@@ -XXX,XX +XXX,XX @@ wrote 512/512 bytes at offset 10240

512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

=== Testing monolithicFlat with internally generated JSON file name ===

-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 subformat=monolithicFlat

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

qemu-io: can't open: Cannot use relative extent paths with VMDK descriptor file 'json:{"image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug", "inject-error.0.event": "read_aio"}'

=== Testing version 3 ===

@@ -XXX,XX +XXX,XX @@ read 512/512 bytes at offset 64931328

512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

=== Testing 4TB monolithicFlat creation and IO ===

-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4398046511104 subformat=monolithicFlat

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

image: TEST_DIR/t.IMGFMT

file format: IMGFMT

virtual size: 4 TiB (4398046511104 bytes)

@@ -XXX,XX +XXX,XX @@ read 1024/1024 bytes at offset 966367641600

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

=== Testing qemu-img map on extents ===

-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33285996544 subformat=monolithicSparse

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

wrote 1024/1024 bytes at offset 65024

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

wrote 1024/1024 bytes at offset 2147483136

@@ -XXX,XX +XXX,XX @@ Offset Length Mapped to File

0 0x20000 0x3f0000 TEST_DIR/t.vmdk

0x7fff0000 0x20000 0x410000 TEST_DIR/t.vmdk

0x140000000 0x10000 0x430000 TEST_DIR/t.vmdk

-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33285996544 subformat=twoGbMaxExtentSparse

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

wrote 1024/1024 bytes at offset 65024

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

wrote 1024/1024 bytes at offset 2147483136

diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/common.filter

+++ b/tests/qemu-iotests/common.filter

@@ -XXX,XX +XXX,XX @@ _filter_img_create()

-e "s# compat6=\\(on\\|off\\)##g" \

-e "s# static=\\(on\\|off\\)##g" \

-e "s# zeroed_grain=\\(on\\|off\\)##g" \

- -e "s# subformat='[^']*'##g" \

- -e "s# adapter_type='[^']*'##g" \

+ -e "s# subformat=[^ ]*##g" \

+ -e "s# adapter_type=[^ ]*##g" \

-e "s# hwversion=[^ ]*##g" \

-e "s# lazy_refcounts=\\(on\\|off\\)##g" \

-e "s# block_size=[0-9]\\+##g" \

--

2.21.0

This makes iotest 033 pass with e.g. subformat=monolithicFlat. It also

turns a former error in 059 into success.

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

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

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

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

---

block/vmdk.c | 54 ++++++++++++++++++++++++--------------

tests/qemu-iotests/059 | 7 +++--

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

3 files changed, 42 insertions(+), 23 deletions(-)

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

index XXXXXXX..XXXXXXX 100644

--- a/block/vmdk.c

+++ b/block/vmdk.c

@@ -XXX,XX +XXX,XX @@ static const char *next_line(const char *s)

}

static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,

- const char *desc_file_path, QDict *options,

- Error **errp)

+ QDict *options, Error **errp)

{

int ret;

int matches;

@@ -XXX,XX +XXX,XX @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,

const char *p, *np;

int64_t sectors = 0;

int64_t flat_offset;

+ char *desc_file_dir = NULL;

char *extent_path;

BdrvChild *extent_file;

BDRVVmdkState *s = bs->opaque;

@@ -XXX,XX +XXX,XX @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,

continue;

}

- if (!path_is_absolute(fname) && !path_has_protocol(fname) &&

- !desc_file_path[0])

- {

- bdrv_refresh_filename(bs->file->bs);

- error_setg(errp, "Cannot use relative extent paths with VMDK "

- "descriptor file '%s'", bs->file->bs->filename);

- return -EINVAL;

- }

+ if (path_is_absolute(fname)) {

+ extent_path = g_strdup(fname);

+ } else {

+ if (!desc_file_dir) {

+ desc_file_dir = bdrv_dirname(bs->file->bs, errp);

+ if (!desc_file_dir) {

+ bdrv_refresh_filename(bs->file->bs);

+ error_prepend(errp, "Cannot use relative paths with VMDK "

+ "descriptor file '%s': ",

+ bs->file->bs->filename);

+ ret = -EINVAL;

+ goto out;

+ }

+ }

- extent_path = path_combine(desc_file_path, fname);

+ extent_path = g_strconcat(desc_file_dir, fname, NULL);

+ }

ret = snprintf(extent_opt_prefix, 32, "extents.%d", s->num_extents);

assert(ret < 32);

@@ -XXX,XX +XXX,XX @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,

g_free(extent_path);

if (local_err) {

error_propagate(errp, local_err);

- return -EINVAL;

+ ret = -EINVAL;

+ goto out;

}

/* save to extents array */

@@ -XXX,XX +XXX,XX @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,

0, 0, 0, 0, 0, &extent, errp);

if (ret < 0) {

bdrv_unref_child(bs, extent_file);

- return ret;

+ goto out;

}

extent->flat_start_offset = flat_offset << 9;

} else if (!strcmp(type, "SPARSE") || !strcmp(type, "VMFSSPARSE")) {

@@ -XXX,XX +XXX,XX @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,

g_free(buf);

if (ret) {

bdrv_unref_child(bs, extent_file);

- return ret;

+ goto out;

}

extent = &s->extents[s->num_extents - 1];

} else if (!strcmp(type, "SESPARSE")) {

ret = vmdk_open_se_sparse(bs, extent_file, bs->open_flags, errp);

if (ret) {

bdrv_unref_child(bs, extent_file);

- return ret;

+ goto out;

}

extent = &s->extents[s->num_extents - 1];

} else {

error_setg(errp, "Unsupported extent type '%s'", type);

bdrv_unref_child(bs, extent_file);

- return -ENOTSUP;

+ ret = -ENOTSUP;

+ goto out;

}

extent->type = g_strdup(type);

}

- return 0;

+

+ ret = 0;

+ goto out;

invalid:

np = next_line(p);

@@ -XXX,XX +XXX,XX @@ invalid:

np--;

}

error_setg(errp, "Invalid extent line: %.*s", (int)(np - p), p);

- return -EINVAL;

+ ret = -EINVAL;

+

+out:

+ g_free(desc_file_dir);

+ return ret;

}

static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf,

@@ -XXX,XX +XXX,XX @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf,

}

s->create_type = g_strdup(ct);

s->desc_offset = 0;

- ret = vmdk_parse_extents(buf, bs, bs->file->bs->exact_filename, options,

- errp);

+ ret = vmdk_parse_extents(buf, bs, options, errp);

exit:

return ret;

}

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

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/059

+++ b/tests/qemu-iotests/059

@@ -XXX,XX +XXX,XX @@ $QEMU_IMG convert -f qcow2 -O vmdk -o subformat=streamOptimized "$TEST_IMG.qcow2

echo

echo "=== Testing monolithicFlat with internally generated JSON file name ==="

+# Should work, because bdrv_dirname() works fine with blkdebug

IMGOPTS="subformat=monolithicFlat" _make_test_img 64M

-$QEMU_IO -c "open -o driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio" 2>&1 \

- | _filter_testdir | _filter_imgfmt

+$QEMU_IO -c "open -o driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio" \

+ -c info \

+ 2>&1 \

+ | _filter_testdir | _filter_imgfmt | _filter_img_info

_cleanup_test_img

echo

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

index XXXXXXX..XXXXXXX 100644

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

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

@@ -XXX,XX +XXX,XX @@ wrote 512/512 bytes at offset 10240

=== Testing monolithicFlat with internally generated JSON file name ===

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

-qemu-io: can't open: Cannot use relative extent paths with VMDK descriptor file 'json:{"image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug", "inject-error.0.event": "read_aio"}'

+format name: IMGFMT

+cluster size: 0 bytes

+vm state offset: 0 bytes

=== Testing version 3 ===

image: TEST_DIR/iotest-version3.IMGFMT

--

2.21.0

We had a test for a case where relative extent paths did not work, but

unfortunately we just fixed the underlying problem, so it works now.

This patch adds a new test case that still fails.

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

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

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

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

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

---

tests/qemu-iotests/059 | 27 +++++++++++++++++++++++++++

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

2 files changed, 31 insertions(+)

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

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/059

+++ b/tests/qemu-iotests/059

@@ -XXX,XX +XXX,XX @@ $QEMU_IMG convert -f qcow2 -O vmdk -o subformat=streamOptimized "$TEST_IMG.qcow2

echo

echo "=== Testing monolithicFlat with internally generated JSON file name ==="

+

+echo '--- blkdebug ---'

# Should work, because bdrv_dirname() works fine with blkdebug

IMGOPTS="subformat=monolithicFlat" _make_test_img 64M

$QEMU_IO -c "open -o driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio" \

@@ -XXX,XX +XXX,XX @@ $QEMU_IO -c "open -o driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TE

| _filter_testdir | _filter_imgfmt | _filter_img_info

_cleanup_test_img

+echo '--- quorum ---'

+# Should not work, because bdrv_dirname() does not work with quorum

+IMGOPTS="subformat=monolithicFlat" _make_test_img 64M

+cp "$TEST_IMG" "$TEST_IMG.orig"

+

+filename="json:{

+ \"driver\": \"$IMGFMT\",

+ \"file\": {

+ \"driver\": \"quorum\",

+ \"children\": [ {

+ \"driver\": \"file\",

+ \"filename\": \"$TEST_IMG\"

+ }, {

+ \"driver\": \"file\",

+ \"filename\": \"$TEST_IMG.orig\"

+ } ],

+ \"vote-threshold\": 1

+ } }"

+

+filename=$(echo "$filename" | tr '\n' ' ' | sed -e 's/\s\+/ /g')

+$QEMU_IMG info "$filename" 2>&1 \

+ | sed -e "s/'json:[^']*'/\$QUORUM_FILE/g" \

+ | _filter_testdir | _filter_imgfmt | _filter_img_info

+

+

echo

echo "=== Testing version 3 ==="

_use_sample_img iotest-version3.vmdk.bz2

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

index XXXXXXX..XXXXXXX 100644

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

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

@@ -XXX,XX +XXX,XX @@ wrote 512/512 bytes at offset 10240

512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

=== Testing monolithicFlat with internally generated JSON file name ===

+--- blkdebug ---

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

format name: IMGFMT

cluster size: 0 bytes

vm state offset: 0 bytes

+--- quorum ---

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

+qemu-img: Could not open $QUORUM_FILE: Cannot use relative paths with VMDK descriptor file $QUORUM_FILE: Cannot generate a base directory for quorum nodes

=== Testing version 3 ===

image: TEST_DIR/iotest-version3.IMGFMT

--

2.21.0

Compressed writes generally have to write full clusters, not just in

theory but also in practice when it comes to vmdk's streamOptimized

subformat. It currently is just silently broken for writes with

non-zero in-cluster offsets:

$ qemu-img create -f vmdk -o subformat=streamOptimized foo.vmdk 1M

$ qemu-io -c 'write 4k 4k' -c 'read 4k 4k' foo.vmdk

wrote 4096/4096 bytes at offset 4096

4 KiB, 1 ops; 00.01 sec (443.724 KiB/sec and 110.9309 ops/sec)

read failed: Invalid argument

(The technical reason is that vmdk_write_extent() just writes the

incomplete compressed data actually to offset 4k. When reading the

data, vmdk_read_extent() looks at offset 0 and finds the compressed data

size to be 0, because that is what it reads from there. This yields an

error.)

For incomplete writes with zero in-cluster offsets, the error path when

reading the rest of the cluster is a bit different, but the result is

the same:

$ qemu-img create -f vmdk -o subformat=streamOptimized foo.vmdk 1M

$ qemu-io -c 'write 0k 4k' -c 'read 4k 4k' foo.vmdk

wrote 4096/4096 bytes at offset 0

4 KiB, 1 ops; 00.01 sec (362.641 KiB/sec and 90.6603 ops/sec)

read failed: Invalid argument

(Here, vmdk_read_extent() finds the data and then sees that the

uncompressed data is short.)

It is better to reject invalid writes than to make the user believe they

might have succeeded and then fail when trying to read it back.

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

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

Message-id: 20190815153638.4600-5-mreitz@redhat.com

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

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

---

block/vmdk.c | 10 ++++++++++

1 file changed, 10 insertions(+)

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