[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] block: nbd: Fix convert qcow2 compressed to nbd
From: |
Nir Soffer |
Subject: |
Re: [PATCH 1/2] block: nbd: Fix convert qcow2 compressed to nbd |
Date: |
Mon, 27 Jul 2020 18:12:15 +0300 |
On Mon, Jul 27, 2020 at 5:04 PM Eric Blake <eblake@redhat.com> wrote:
>
> On 7/26/20 10:25 AM, Nir Soffer wrote:
> > When converting to qcow2 compressed format, the last step is a special
> > zero length compressed write, ending in call to bdrv_co_truncate(). This
> > call always fail for the nbd driver since it does not implement
>
> fails
>
> > bdrv_co_truncate().
>
> Arguably, qemu-img should be taught to ignore the failure, since it is
> not unique to the nbd driver. But I can live with your approach here.
>
> >
> > For block devices, which have the same limits, the call succeeds since
> > file driver implements bdrv_co_truncate(). If the caller asked to
> > truncate to the same or smaller size with exact=false, the truncate
> > succeeds. Implement the same logic for nbd.
> >
> > Example failing without this change:
> >
>
> >
> > Fixes: https://bugzilla.redhat.com/1860627
> > Signed-off-by: Nir Soffer <nsoffer@redhat.com>
> > ---
> > block/nbd.c | 27 +++++++++++++++++++++++++++
> > 1 file changed, 27 insertions(+)
> >
> > diff --git a/block/nbd.c b/block/nbd.c
> > index 65a4f56924..2154113af3 100644
> > --- a/block/nbd.c
> > +++ b/block/nbd.c
> > @@ -1966,6 +1966,30 @@ static void nbd_close(BlockDriverState *bs)
> > nbd_clear_bdrvstate(s);
> > }
> >
> > +/*
> > + * NBD cannot truncate, but if the caller ask to truncate to the same
> > size, or
>
> asks
>
> > + * to a smaller size with extact=false, there is not reason to fail the
>
> exact, no
>
> > + * operation.
> > + */
> > +static int coroutine_fn nbd_co_truncate(BlockDriverState *bs, int64_t
> > offset,
> > + bool exact, PreallocMode prealloc,
> > + BdrvRequestFlags flags, Error
> > **errp)
> > +{
> > + BDRVNBDState *s = bs->opaque;
> > +
> > + if (offset != s->info.size && exact) {
> > + error_setg(errp, "Cannot resize NBD nodes");
> > + return -ENOTSUP;
> > + }
> > +
> > + if (offset > s->info.size) {
> > + error_setg(errp, "Cannot grow NBD nodes");
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
>
> Looks reasonable. As Max said, I wonder if we want to reject particular
> preallocation modes (looking at block/file-posix.c:raw_co_truncate), in
> the case where the image was resized down and then back up (since
> s->info.size is constant, but the BDS size is not if inexact resize
> succeeds).
Do we want to fail if someone specifies -o preallocation={falloc,full}?
I see we convert DRV_REQ_MAY_UNMAP to NBD_CMD_FLAG_NO_HOLE
so using -o preallocation=falloc,full should be correct. But the last
request zero
length write request does not do anything, so failing does not look useful.
> As you have a bugzilla entry, I think this is safe for -rc2; I'll be
> touching up the typos and queuing it through my NBD tree later today.
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3226
> Virtualization: qemu.org | libvirt.org
>
[PATCH 1/2] block: nbd: Fix convert qcow2 compressed to nbd, Nir Soffer, 2020/07/26