Am 14.07.2020 um 18:22 hat Max Reitz geschrieben: > On 14.07.20 13:08, Kevin Wolf wrote: > > Am 14.07.2020 um 11:56 hat Max Reitz geschrieben: > >> On 13.07.20 16:29, Kevin Wolf wrote: > >>> Am 13.07.2020 um 13:19 hat Max Reitz geschrieben: > >>>> On 10.07.20 16:21, Kevin Wolf wrote: > >>>>> Unaligned requests will automatically be aligned to bl.request_alignment > >>>>> and we don't want to extend requests to access space beyond the end of > >>>>> the image, so it's required that the image size is aligned. > >>>>> > >>>>> With write requests, this could cause assertion failures like this if > >>>>> RESIZE permissions weren't requested: > >>>>> > >>>>> qemu-img: block/io.c:1910: bdrv_co_write_req_prepare: Assertion > >>>>> `end_sector <= bs->total_sectors || child->perm & BLK_PERM_RESIZE' > >>>>> failed. > >>>>> > >>>>> This was e.g. triggered by qemu-img converting to a target image with 4k > >>>>> request alignment when the image was only aligned to 512 bytes, but not > >>>>> to 4k. > >>>>> > >>>>> Signed-off-by: Kevin Wolf <kw...@redhat.com> > >>>>> --- > >>>>> block.c | 10 ++++++++++ > >>>>> 1 file changed, 10 insertions(+) > >>>> > >>>> (I think we had some proposal like this before, but I can’t find it, > >>>> unfortunately...) > >>>> > >>>> I can’t see how with this patch you could create qcow2 images and then > >>>> use them with direct I/O, because AFAICS, qemu-img create doesn’t allow > >>>> specifying caching options, so AFAIU you’re stuck with: > >>>> > >>>> $ ./qemu-img create -f qcow2 /mnt/tmp/foo.qcow2 1M > >>>> Formatting '/mnt/tmp/foo.qcow2', fmt=qcow2 cluster_size=65536 > >>>> compression_type=zlib size=1048576 lazy_refcounts=off refcount_bits=16 > >>>> > >>>> $ sudo ./qemu-io -t none /mnt/tmp/foo.qcow2 > >>>> qemu-io: can't open device /mnt/tmp/foo.qcow2: Image size is not a > >>>> multiple of request alignment > >>>> > >>>> (/mnt/tmp is a filesystem on a “losetup -b 4096” device.) > >>> > >>> Hm, that looks like some regrettable collateral damage... > >>> > >>> Well, you could argue that we should be writing full L1 tables with zero > >>> padding instead of just the used part. I thought we had fixed this long > >>> ago. But looks like we haven't. > >> > >> That would help for the standard case. It wouldn’t when the cluster > >> size is smaller than the request alignment, which, while maybe not > >> important, would still be a shame. > > > > I don't think it would be unreasonable to require a cluster size that is > > a multiple of the logical block size of your host storage if you want to > > use O_DIRECT. > > True. > > > But we have unaligned images in practice, so this is pure theory anyway. > > Hm. Maybe it would help to just adjust the error message to instruct > the user to resize the image to fit the request alignment? (e.g. “is > not a multiple of the request alignment %u (try resizing the image to > %llu bytes)”)
This would require management tools to automatically do this or we would break any users that don't manually invoke QEMU. I don't think this is a realistic option, especially since "management tools" must probably include all those one-off shell scripts that people use. > >>> But we should still avoid crashing in other cases, so what is the > >>> difference between both? Is it just that qcow2 has the RESIZE permission > >>> anyway so it doesn't matter? > >> > >> I assume so. > >> > >>> If so, maybe attaching to a block node with WRITE, but not RESIZE is > >>> what needs to fail when the image size is unaligned? > >> > >> That sounds reasonable. > >> > >> The obvious question is what happens when the RESIZE capability is > >> removed. Dropping capabilities may never fail – I suppose we could > >> force-keep the RESIZE capability for such nodes? > > > > It's not nice, but I think we already have this kind of behaviour for > > unlocking failures. So yes, that sounds like an option. > > > >> Or we could immediately align such files to the block size once they > >> are opened (with the RESIZE capability). > > > > Automatically resizing the image file is obviously harmless for qcow2 > > images, but it would be a guest-visible change for raw images. It might > > be better to avoid this. > > Well, it seems to be what already happens if the guest device has taken > the RESIZE capability (i.e., whenever there’s no failing assertion). > The only difference that appears to me is just that it happens only when > writing to the end of the image instead of unconditionally when opening it. I would have considered this as part of the bug rather than a desirable future behaviour. blk_check_byte_request() tries to catch any request going past EOF, it just doesn't know anything about request_alignment. Kevin
signature.asc
Description: PGP signature
