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);