Re: [Qemu-block] [PATCH v2 5/6] nbd: Merge nbd_export_bitmap into nbd_export_new

[Vladimir Sementsov-Ogievskiy]

10.01.2019 10:13, Eric Blake 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.
> 
> We also no longer need logic for setting a different bitmap
> name compared to the bitmap being exported.
> 
> Signed-off-by: Eric Blake <address@hidden>
> ---
>   include/block/nbd.h |  9 ++---
>   blockdev-nbd.c      | 11 +-----
>   nbd/server.c        | 87 +++++++++++++++++++++------------------------
>   qemu-nbd.c          |  4 +--
>   4 files changed, 46 insertions(+), 65 deletions(-)
> 
> diff --git a/include/block/nbd.h b/include/block/nbd.h
> index 2f9a2aeb73c..1971b557896 100644
> --- a/include/block/nbd.h
> +++ b/include/block/nbd.h
> @@ -296,9 +296,9 @@ typedef struct NBDClient NBDClient;
> 
>   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);
>   void nbd_export_close(NBDExport *exp);
>   void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error 
> **errp);
>   void nbd_export_get(NBDExport *exp);
> @@ -319,9 +319,6 @@ void nbd_client_put(NBDClient *client);
>   void nbd_server_start(SocketAddress *addr, const char *tls_creds,
>                         Error **errp);
> 
> -void nbd_export_bitmap(NBDExport *exp, const char *bitmap,
> -                       const char *bitmap_export_name, Error **errp);
> -
>   /* nbd_read
>    * Reads @size bytes from @ioc. Returns 0 on success.
>    */
> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
> index 003ba7d7180..0df6307be2d 100644
> --- a/blockdev-nbd.c
> +++ b/blockdev-nbd.c
> @@ -175,7 +175,7 @@ void qmp_nbd_server_add(const char *device, bool 
> has_name, const char *name,
>           writable = false;
>       }
> 
> -    exp = nbd_export_new(bs, 0, -1, name, NULL,
> +    exp = nbd_export_new(bs, 0, -1, name, NULL, bitmap,

bitmap is guaranteed to be NULL if has_bitmap is false? Or should we do 
has_bitmap ? bitmap : NULL?

if yes or with fixed:
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>


-- 
Best regards,
Vladimir