From: Kevin Wolf <kwolf@redhat.com>
To: Max Reitz <mreitz@redhat.com>
Cc: nsoffer@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org
Subject: Re: [PATCH for-5.1 1/2] block: Require aligned image size to avoid assertion failure
Date: Tue, 14 Jul 2020 13:08:37 +0200 [thread overview]
Message-ID: <20200714110837.GB5120@linux.fritz.box> (raw)
In-Reply-To: <b91329db-303f-41ab-5638-3ec7543b55ac@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 3496 bytes --]
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 <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.
>
> 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.
But we have unaligned images in practice, so this is pure theory anyway.
> > 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.
Kevin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2020-07-14 11:09 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-10 14:21 [PATCH for-5.1 0/2] qemu-img convert: Fix abort with unaligned image size Kevin Wolf
2020-07-10 14:21 ` [PATCH for-5.1 1/2] block: Require aligned image size to avoid assertion failure Kevin Wolf
2020-07-10 14:37 ` Eric Blake
2020-07-13 11:19 ` Max Reitz
2020-07-13 11:52 ` Max Reitz
2020-07-13 14:29 ` Kevin Wolf
2020-07-14 9:56 ` Max Reitz
2020-07-14 11:08 ` Kevin Wolf [this message]
2020-07-14 16:22 ` Max Reitz
2020-07-15 9:20 ` Kevin Wolf
2020-07-13 16:33 ` Nir Soffer
2020-07-13 16:56 ` Kevin Wolf
2020-07-15 13:22 ` Nir Soffer
2020-07-15 13:42 ` Kevin Wolf
2020-07-15 14:03 ` Nir Soffer
2020-07-15 14:03 ` Daniel P. Berrangé
2020-07-10 14:21 ` [PATCH for-5.1 2/2] file-posix: Allow byte-aligned O_DIRECT with NFS Kevin Wolf
2020-07-10 14:39 ` Eric Blake
2020-07-13 16:29 ` Nir Soffer
2020-07-10 14:43 ` [PATCH for-5.1 0/2] qemu-img convert: Fix abort with unaligned image size no-reply
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=20200714110837.GB5120@linux.fritz.box \
--to=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=nsoffer@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/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).