Re: [PATCH 1/2] block: nbd: Fix convert qcow2 compressed to nbd

[Eric Blake <eblake@redhat.com>]

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).

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