From: Hanna Reitz <hreitz@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, qemu-devel@nongnu.org
Subject: Re: [PATCH] qemu-img: Allow target be aligned to sector size
Date: Tue, 7 Sep 2021 16:00:56 +0200 [thread overview]
Message-ID: <d2ca08c1-6602-47c5-8977-79f31df5964e@redhat.com> (raw)
In-Reply-To: <fdca4c29-434d-e589-fd00-2388962666ed@virtuozzo.com>
On 07.09.21 15:44, Vladimir Sementsov-Ogievskiy wrote:
> 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?
Well, it creates the target with the size of the source, and the size of
the source will always be reported to be aligned to 512. If we didn’t
have bdrv_nb_sectors() but used a byte granularity for a BDS’s length
internally, then you could see this error with 512-byte-sectored
volumes, too.
So from qemu’s point of view, it doesn’t need a `resize` permission
because it doesn’t need resize the target, because it’s always created
with a 512-byte-aligned size anyway.
Hanna
> 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..
next prev parent reply other threads:[~2021-09-07 14:11 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-19 10:12 [PATCH] qemu-img: Allow target be aligned to sector size Hanna Reitz
2021-08-19 14:31 ` Jose R. Ziviani
2021-08-19 15:14 ` Hanna Reitz
2021-08-19 18:58 ` Jose R. Ziviani
2021-09-07 9:58 ` Hanna Reitz
2021-09-07 11:29 ` Vladimir Sementsov-Ogievskiy
2021-09-07 12:48 ` Hanna Reitz
2021-09-07 13:44 ` Vladimir Sementsov-Ogievskiy
2021-09-07 14:00 ` Hanna Reitz [this message]
2021-09-07 14:18 ` Vladimir Sementsov-Ogievskiy
2021-09-14 9:24 ` Hanna Reitz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d2ca08c1-6602-47c5-8977-79f31df5964e@redhat.com \
--to=hreitz@redhat.com \
--cc=kwolf@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=vsementsov@virtuozzo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).