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

[Hanna Reitz <hreitz@redhat.com>]

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