Re: [PATCH for-5.1 1/2] block: Require aligned image size to avoid assertion failure

[Kevin Wolf]

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 <kwolf@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.

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?

If so, maybe attaching to a block node with WRITE, but not RESIZE is
what needs to fail when the image size is unaligned?

> Or you use blockdev-create, that seems to work (because of course you
> can set the cache mode on the protocol node when you open it for
> formatting).  But, well, I think there should be a working qemu-img
> create case.
> 
> Also, I’m afraid of breaking existing use cases with this patch (just
> qemu-img create + using the image with cache=none).

I think for raw images, failure on start is better than crashing when
the VM is running. The qcow2 case needs to be fixed, of course.

Either case, I guess patch 2 can already be merged and would solve at
least the immediate bug report.

Kevin

signature.asc
Description: PGP signature