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 <kw...@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, either way:
Reviewed-by: Max Reitz <mre...@redhat.com>
> + !(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: OpenPGP digital signature
