Hello Hanna, On Thu, Aug 19, 2021 at 12:12:00PM +0200, 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 <hre...@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.
This is timely convenient because I'm currently investigating an issue detected by a libvirt-tck test: https://gitlab.com/libvirt/libvirt-tck/-/blob/master/scripts/qemu/200-qcow2-single-backing-file.t It fails with the same message: qemu-system-x86_64: -device virtio-blk-pci,bus=pci.0,addr=0x3,drive=libvirt-1-format,id=virtio-disk1,write-cache=on: Cannot get 'write' permission without 'resize': Image size is not a multiple of request alignment Libvirt-tck basically wants to make sure that libvirt won't pass a 'backing' argument if we force a QCOW2 image to be interpreted as a RAW image. But, the actual size of a (not preallocated) QCOW2 is usually unaligned so we ended up failing at that requirement. I crafted a reproducer (tck-main is a QCOW2 image): $ ./qemu-system-x86_64 \ -machine pc-i440fx-6.0,accel=kvm -m 1024 -display none -no-user-config -nodefaults \ -kernel vmlinuz -initrd initrd \ -blockdev driver=file,filename=/var/cache/libvirt-tck/storage-fs/tck/tck-main,node-name=a,cache.direct=on \ -blockdev node-name=name,driver=raw,file=a \ -device virtio-blk-pci,drive=name And if I remove 'cache.direct=on' OR if I remove the device virtio-blk-pci I don't hit the failure. In order to fix the libvirt-tck test case, I think that creating a preallocated QCOW2 image is the best approach, considering their test case goal. But, if you don't mind, could you explain why cache.direct=on doesn't set resize permission with virtio-blk-pci? Thank you! > --- > 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); > -- > 2.31.1 > >
signature.asc
Description: Digital signature