Re: [RFC PATCH 11/22] qemu-nbd: Use blk_exp_add() to create the export

[Max Reitz]

On 13.08.20 18:29, Kevin Wolf wrote:
> With this change, NBD exports are only created through the BlockExport
> interface any more. This allows us finally to move things from the NBD
> layer to the BlockExport layer if they make sense for other export
> types, too.

I see.

> blk_exp_add() returns only a weak reference, so the explicit
> nbd_export_put() goes away.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  include/block/export.h |  2 ++
>  include/block/nbd.h    |  1 +
>  block/export/export.c  |  2 +-
>  blockdev-nbd.c         |  8 +++++++-
>  qemu-nbd.c             | 28 ++++++++++++++++++++++------
>  5 files changed, 33 insertions(+), 8 deletions(-)

[...]

> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
> index d5b084acc2..8dd127af52 100644
> --- a/blockdev-nbd.c
> +++ b/blockdev-nbd.c

[...]

> @@ -176,7 +182,7 @@ BlockExport *nbd_export_create(BlockExportOptions 
> *exp_args, Error **errp)
>  
>      assert(exp_args->type == BLOCK_EXPORT_TYPE_NBD);
>  
> -    if (!nbd_server) {
> +    if (!nbd_server && !is_qemu_nbd) {

(This begs the question of how difficult it would be to let qemu-nbd use
QMP’s nbd-server-start, but I will not ask it, for I fear the answer.)

>          error_setg(errp, "NBD server not running");
>          return NULL;
>      }
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 48aa8a9d46..d967b8fcb9 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c

[...]

> @@ -1050,9 +1050,27 @@ int main(int argc, char **argv)
>  
>      bs->detect_zeroes = detect_zeroes;
>  
> -    export = nbd_export_new(bs, export_name,
> -                            export_description, bitmap, readonly, shared > 1,
> -                            writethrough, &error_fatal);
> +    nbd_server_is_qemu_nbd(true);
> +
> +    export_opts = g_new(BlockExportOptions, 1);
> +    *export_opts = (BlockExportOptions) {
> +        .type               = BLOCK_EXPORT_TYPE_NBD,
> +        .has_writethrough   = true,
> +        .writethrough       = writethrough,
> +        .u.nbd = {
> +            .device             = g_strdup(bdrv_get_node_name(bs)),
> +            .has_name           = true,
> +            .name               = g_strdup(export_name),
> +            .has_description    = !!export_description,
> +            .description        = g_strdup(export_description),
> +            .has_writable       = true,
> +            .writable           = !readonly,
> +            .has_bitmap         = !!bitmap,
> +            .bitmap             = g_strdup(bitmap),
> +        },
> +    };
> +    blk_exp_add(export_opts, &error_fatal);

Why not use the already-global qmp_block_export_add(), if we don’t need
the return value here?  (Will we require it at some point?)

Max

> +    qapi_free_BlockExportOptions(export_opts);
>  
>      if (device) {
>  #if HAVE_NBD_DEVICE

signature.asc
Description: OpenPGP digital signature