Re: [Qemu-block] [Qemu-devel] [PULL 13/55] sheepdog: Make sd_prealloc() take a BDS

[Peter Maydell]

On 13 February 2018 at 17:04, Kevin Wolf <address@hidden> wrote:
> From: Max Reitz <address@hidden>
>
> We want to use this function in sd_truncate() later on, so taking a
> filename is not exactly ideal.
>
> Signed-off-by: Max Reitz <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  block/sheepdog.c | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index af125a2c8d..cc1d37b3da 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -1826,10 +1826,10 @@ static int do_sd_create(BDRVSheepdogState *s, 
> uint32_t *vdi_id, int snapshot,
>      return 0;
>  }
>
> -static int sd_prealloc(const char *filename, Error **errp)
> +static int sd_prealloc(BlockDriverState *bs, Error **errp)
>  {
>      BlockBackend *blk = NULL;
> -    BDRVSheepdogState *base = NULL;
> +    BDRVSheepdogState *base = bs->opaque;
>      unsigned long buf_size;
>      uint32_t idx, max_idx;
>      uint32_t object_size;
> @@ -1837,10 +1837,11 @@ static int sd_prealloc(const char *filename, Error 
> **errp)
>      void *buf = NULL;
>      int ret;
>
> -    blk = blk_new_open(filename, NULL, NULL,
> -                       BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
> -    if (blk == NULL) {
> -        ret = -EIO;
> +    blk = blk_new(BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE | 
> BLK_PERM_RESIZE,
> +                  BLK_PERM_ALL);
> +
> +    ret = blk_insert_bs(blk, bs, errp);
> +    if (ret < 0) {
>          goto out_with_err_set;
>      }

Hi. Coverity (CID 1390636) points out that there's now a useless
NULL check of a definitely-not-NULL pointer in the 'out_with_err_set'
code, because this change has removed the code path which could get
there with blk == NULL. The "if (blk)" check could be removed to
make the blk_unref() unconditional, I think.

thanks
-- PMM