[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-5.1 v2 1/2] block: Require aligned image size to avoid as
From: |
Kevin Wolf |
Subject: |
Re: [PATCH for-5.1 v2 1/2] block: Require aligned image size to avoid assertion failure |
Date: |
Fri, 17 Jul 2020 13:32:43 +0200 |
Am 17.07.2020 um 13:02 hat Max Reitz geschrieben:
> On 16.07.20 16:26, Kevin Wolf wrote:
> > Unaligned requests will automatically be aligned to bl.request_alignment
> > and we can't extend write requests to access space beyond the end of the
> > image without resizing the image, so if we have the WRITE permission,
> > but not the RESIZE one, it's required that the image size is aligned.
> >
> > Failing to meet this requirement 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.
> >
> > Turn this into a graceful error in bdrv_check_perm() so that WRITE
> > without RESIZE can only be taken if the image size is aligned. If a user
> > holds both permissions and drops only RESIZE, the function will return
> > an error, but bdrv_child_try_set_perm() will ignore the failure silently
> > if permissions are only requested to be relaxed and just keep both
> > permissions while returning success.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> > block.c | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/block.c b/block.c
> > index 35a372df57..6371928edb 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -2025,6 +2025,22 @@ static int bdrv_check_perm(BlockDriverState *bs,
> > BlockReopenQueue *q,
> > return -EPERM;
> > }
> >
> > + /*
> > + * Unaligned requests will automatically be aligned to
> > bl.request_alignment
> > + * and without RESIZE we can't extend requests to write to space
> > beyond the
> > + * end of the image, so it's required that the image size is aligned.
> > + */
> > + if ((cumulative_perms & BLK_PERM_WRITE) &&
>
> What about WRITE_UNCHANGED? I think this would only matter with nodes
> that can have backing files (i.e., qcow2 in practice) because
> WRITE_UNCHANGED is only used by COR and block jobs doing something with
> a backing chain, so it shouldn’t matter in practice, but, well.
So basically just replacing the line with this?
if ((cumulative_perms & (BLK_PERM_WRITE | BDRV_PERM_WRITE_UNCHANGED)) &&
I can do that while applying if it is what you mean.
> So, either way:
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
Thanks!
Kevin
> > + !(cumulative_perms & BLK_PERM_RESIZE))
> > + {
> > + if ((bs->total_sectors * BDRV_SECTOR_SIZE) %
> > bs->bl.request_alignment) {
> > + error_setg(errp, "Cannot get 'write' permission without
> > 'resize': "
> > + "Image size is not a multiple of request "
> > + "alignment");
> > + return -EPERM;
> > + }
> > + }
> > +
> > /* Check this node */
> > if (!drv) {
> > return 0;
> >
>
>
signature.asc
Description: PGP signature