qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] qemu-img: Allow target be aligned to sector size


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH] qemu-img: Allow target be aligned to sector size
Date: Tue, 7 Sep 2021 16:44:34 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0

07.09.2021 15:48, Hanna Reitz wrote:
On 07.09.21 13:29, Vladimir Sementsov-Ogievskiy wrote:
19.08.2021 13:12, Hanna Reitz wrote:
We cannot write to images opened with O_DIRECT unless we allow them to
be resized so they are aligned to the sector size: Since 9c60a5d1978,
bdrv_node_refresh_perm() ensures that for nodes whose length is not
aligned to the request alignment and where someone has taken a WRITE
permission, the RESIZE permission is taken, too).

Let qemu-img convert pass the BDRV_O_RESIZE flag (which causes
blk_new_open() to take the RESIZE permission) when using cache=none for
the target, so that when writing to it, it can be aligned to the target
sector size.

Without this patch, an error is returned:

$ qemu-img convert -f raw -O raw -t none foo.img /mnt/tmp/foo.img
qemu-img: Could not open '/mnt/tmp/foo.img': Cannot get 'write'
permission without 'resize': Image size is not a multiple of request
alignment

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1994266
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
As I have written in the BZ linked above, I am not sure what behavior we
want.  It can be argued that the current behavior is perfectly OK
because we want the target to have the same size as the source, so if
this cannot be done, we should just print the above error (which I think
explains the problem well enough that users can figure out they need to
resize the source image).

OTOH, it is difficult for me to imagine a case where the user would
prefer the above error to just having qemu-img align the target image's
length.
---
  qemu-img.c | 8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c
index 908fd0cce5..d4b29bf73e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2628,6 +2628,14 @@ static int img_convert(int argc, char **argv)
          goto out;
      }
  +    if (flags & BDRV_O_NOCACHE) {
+        /*
+         * If we open the target with O_DIRECT, it may be necessary to
+         * extend its size to align to the physical sector size.
+         */
+        flags |= BDRV_O_RESIZE;
+    }
+
      if (skip_create) {
          s.target = img_open(tgt_image_opts, out_filename, out_fmt,
                              flags, writethrough, s.quiet, false);


Hmm. Strange. I am experimenting now with master branch without that patch and 
can't reproduce:

[root@kvm master]# dd if=/dev/zero of=a bs=1M count=2
2+0 records in
2+0 records out
2097152 bytes (2.1 MB, 2.0 MiB) copied, 0.00153639 s, 1.4 GB/s
[root@kvm master]# dd if=/dev/zero of=b bs=1M count=1
1+0 records in
1+0 records out
1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.000939314 s, 1.1 GB/s
[root@kvm master]# ls -lthr a b
-rw-r--r--. 1 root root 2.0M Sep  7 14:28 a
-rw-r--r--. 1 root root 1.0M Sep  7 14:28 b
[root@kvm master]# ./build/qemu-img convert -f raw -O raw -t none a b
[root@kvm master]# ls -lthr a b
-rw-r--r--. 1 root root 2.0M Sep  7 14:28 a
-rw-r--r--. 1 root root 2.0M Sep  7 14:28 b
[root@kvm master]# dd if=/dev/zero of=b bs=1 count=2097147
2097147+0 records in
2097147+0 records out
2097147 bytes (2.1 MB, 2.0 MiB) copied, 2.76548 s, 758 kB/s
[root@kvm master]# ls -ltr a b
-rw-r--r--. 1 root root 2097152 Sep  7 14:28 a
-rw-r--r--. 1 root root 2097147 Sep  7 14:29 b
[root@kvm master]# ./build/qemu-img convert -f raw -O raw -t none a b
[root@kvm master]# ls -ltr a b
-rw-r--r--. 1 root root 2097152 Sep  7 14:28 a
-rw-r--r--. 1 root root 2097152 Sep  7 14:29 b

It just works, and do resize target without any additional BDRV_O_RESIZE...

bdrv_getlength() invokes bdrv_nb_sectors(), so we always align sizes to 512 
anyway.  On volumes with a logical sector size of 512, this error cannot be 
reproduced.

You can use loop devices to get volumes with other sector sizes, like so:

$ cd /tmp
$ truncate fs.img -s 128M
$ sudo losetup -f --show --sector-size 4096 fs.img
/dev/loop0
$ sudo mkfs.ext4 /dev/loop0
mke2fs 1.46.4 (18-Aug-2021)
Discarding device blocks: done
Creating filesystem with 32768 4k blocks and 32768 inodes

Allocating group tables: done
Writing inode tables: done
Creating journal (4096 blocks): done
Writing superblocks and filesystem accounting information: done

$ sudo mount /dev/loop0 /mnt/tmp
$ qemu-img create -f raw source.img $((2 * 1048576 + 512))
Formatting 'source.img', fmt=raw size=2097664
$ sudo qemu-img convert -f raw -O raw -t none source.img /mnt/tmp/target.img
qemu-img: Could not open '/mnt/tmp/target.img': Cannot get 'write' permission 
without 'resize': Image size is not a multiple of request alignment



Does it mean, that when logical sector size is 512, qemu-img do resize target 
without any 'resize' permission?

It looks very strange: in both cases we need to resize target. With 512-bytes 
sectors it just works (look it resizes 1M->2M which is a lot larger than 512b). 
And with 4096-bytes sectors it needs additional BDRV_O_RESIZE..

--
Best regards,
Vladimir



reply via email to

[Prev in Thread] Current Thread [Next in Thread]