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

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3074 bytes --]

On 26.07.20 17:25, 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
> bdrv_co_truncate().
> 
> 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:
> 
> In one shell starts qemu-nbd:
> 
> $ truncate -s 1g test.tar
> $ qemu-nbd --socket=/tmp/nbd.sock --persistent --format=raw --offset 1536 test.tar
> 
> In another shell convert an image to qcow2 compressed via NBD:
> 
> $ echo "disk data" > disk.raw
> $ truncate -s 1g disk.raw
> $ qemu-img convert -f raw -O qcow2 -c disk1.raw nbd+unix:///?socket=/tmp/nbd.sock; echo $?
> 1
> 
> qemu-img failed, but the conversion was successful:
> 
> $ qemu-img info nbd+unix:///?socket=/tmp/nbd.sock
> image: nbd+unix://?socket=/tmp/nbd.sock
> file format: qcow2
> virtual size: 1 GiB (1073741824 bytes)
> ...
> 
> $ qemu-img check nbd+unix:///?socket=/tmp/nbd.sock
> No errors were found on the image.
> 1/16384 = 0.01% allocated, 100.00% fragmented, 100.00% compressed clusters
> Image end offset: 393216
> 
> $ qemu-img compare disk.raw nbd+unix:///?socket=/tmp/nbd.sock
> Images are identical.
> 
> 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
> + * to a smaller size with extact=false, there is not reason to fail the
> + * 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;
> +}

Can we also check @prealloc, like iscsi.c does it?

(Yes, preallocation only makes sense for growing, but you can for
example truncate a 2 GB NBD node to 1 GB, and then grow it to 1.5 GB,
and for the latter operation it might make sense to give a preallocation
parameter.  (Except block_resize doesn’t allow that, but, well.))

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]