Re: [Qemu-devel] [PULL 19/20] nbd: Merge nbd_export_bitmap into nbd_export_new

[Peter Maydell]

On Mon, 14 Jan 2019 at 16:47, Eric Blake <address@hidden> wrote:
>
> We only have one caller that wants to export a bitmap name,
> which it does right after creation of the export. But there is
> still a brief window of time where an NBD client could see the
> export but not the dirty bitmap, which a robust client would
> have to interpret as meaning the entire image should be treated
> as dirty.  Better is to eliminate the window entirely, by
> inlining nbd_export_bitmap() into nbd_export_new(), and refusing
> to create the bitmap in the first place if the requested bitmap
> can't be located.


>  NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size,
>                            const char *name, const char *description,
> -                          uint16_t nbdflags, void (*close)(NBDExport *),
> -                          bool writethrough, BlockBackend *on_eject_blk,
> -                          Error **errp)
> +                          const char *bitmap, uint16_t nbdflags,
> +                          void (*close)(NBDExport *), bool writethrough,
> +                          BlockBackend *on_eject_blk, Error **errp)
>  {
>      AioContext *ctx;
>      BlockBackend *blk;
> @@ -1507,6 +1507,43 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t 
> dev_offset, off_t size,
>      }
>      exp->size -= exp->size % BDRV_SECTOR_SIZE;
>
> +    if (bitmap) {
> +        BdrvDirtyBitmap *bm = NULL;
> +        BlockDriverState *bs = blk_bs(blk);

lgtm.com points out that this local variable 'bs' shadows
the 'bs' argument to the function. Is this intentional?
I think that the two can't ever be different, in which case
you could just delete the variable declaration here, but
I'm not an expert on the block layer APIs.

thanks
-- PMM