Re: [PATCH 11/11] block/export: convert vhost-user-blk server to block export API

[Markus Armbruster]

Stefan Hajnoczi <stefanha@redhat.com> writes:

> Use the new QAPI block exports API instead of defining our own QOM
> objects.
>
> This is a large change because the lifecycle of VuBlockDev needs to
> follow BlockExportDriver. QOM properties are replaced by QAPI options
> objects.
>
> VuBlockDev is renamed VuBlkExport and contains a BlockExport field.
> Several fields can be dropped since BlockExport already has equivalents.
>
> The file names and meson build integration will be adjusted in a future
> patch. libvhost-user should probably be built as a static library that
> is linked into QEMU instead of as a .c file that results in duplicate
> compilation.
>
> The new command-line syntax is:
>
>   $ qemu-storage-daemon \
>       --blockdev file,node-name=drive0,filename=test.img \
>       --export 
> vhost-user-blk,node-name=drive0,id=export0,unix-socket=/tmp/vhost-user-blk.sock
>
> Note that unix-socket is optional because we may wish to accept chardevs
> too in the future.

It's optional in the QAPI schema, but the code cosunming the --export
appears to require it.

There is no need to make it optional now just in case: Changing an
option parameter from mandatory to optional is backward-compatible.

> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  qapi/block-export.json               |  19 +-
>  block/export/vhost-user-blk-server.h |  23 +-
>  block/export/export.c                |   8 +-
>  block/export/vhost-user-blk-server.c | 461 ++++++++-------------------
>  block/export/meson.build             |   1 +
>  block/meson.build                    |   1 -
>  6 files changed, 156 insertions(+), 357 deletions(-)
>
> diff --git a/qapi/block-export.json b/qapi/block-export.json
> index ace0d66e17..840dcbe833 100644
> --- a/qapi/block-export.json
> +++ b/qapi/block-export.json
> @@ -84,6 +84,19 @@
>    'data': { '*name': 'str', '*description': 'str',
>              '*bitmap': 'str' } }
>  
> +##
> +# @BlockExportOptionsVhostUserBlk:
> +#
> +# A vhost-user-blk block export.
> +#
> +# @unix-socket: Path where the vhost-user UNIX domain socket will be created.
> +# @logical-block-size: Logical block size in bytes.
> +#
> +# Since: 5.2
> +##
> +{ 'struct': 'BlockExportOptionsVhostUserBlk',
> +  'data': { '*unix-socket': 'str', '*logical-block-size': 'size' } }

This is where we make @unix-socket optional.

Default behavior is not documented.

> +
>  ##
>  # @NbdServerAddOptions:
>  #
> @@ -180,11 +193,12 @@
>  # An enumeration of block export types
>  #
>  # @nbd: NBD export
> +# @vhost-user-blk: vhost-user-blk export (since 5.2)
>  #
>  # Since: 4.2
>  ##
>  { 'enum': 'BlockExportType',
> -  'data': [ 'nbd' ] }
> +  'data': [ 'nbd', 'vhost-user-blk' ] }
>  
>  ##
>  # @BlockExportOptions:
> @@ -213,7 +227,8 @@
>              '*writethrough': 'bool' },
>    'discriminator': 'type',
>    'data': {
> -      'nbd': 'BlockExportOptionsNbd'
> +      'nbd': 'BlockExportOptionsNbd',
> +      'vhost-user-blk': 'BlockExportOptionsVhostUserBlk'
>     } }
>  
>  ##
[...]
> diff --git a/block/export/vhost-user-blk-server.c 
> b/block/export/vhost-user-blk-server.c
> index 44d3c45fa2..9908b3287e 100644
> --- a/block/export/vhost-user-blk-server.c
> +++ b/block/export/vhost-user-blk-server.c
[...]
> -static char *vu_get_unix_socket(Object *obj, Error **errp)
> +static int vu_blk_exp_create(BlockExport *exp, BlockExportOptions *opts,
> +                             Error **errp)
>  {
> -    VuBlockDev *vus = VHOST_USER_BLK_SERVER(obj);
> -    return g_strdup(vus->addr->u.q_unix.path);
> -}
> -
> -static bool vu_get_block_writable(Object *obj, Error **errp)
> -{
> -    VuBlockDev *vus = VHOST_USER_BLK_SERVER(obj);
> -    return vus->writable;
> -}
> -
> -static void vu_set_block_writable(Object *obj, bool value, Error **errp)
> -{
> -    VuBlockDev *vus = VHOST_USER_BLK_SERVER(obj);
> -
> -    if (!vu_prop_modifiable(vus, errp)) {
> -            return;
> -    }
> -
> -    vus->writable = value;
> -}
> -
> -static void vu_get_blk_size(Object *obj, Visitor *v, const char *name,
> -                            void *opaque, Error **errp)
> -{
> -    VuBlockDev *vus = VHOST_USER_BLK_SERVER(obj);
> -    uint32_t value = vus->blk_size;
> -
> -    visit_type_uint32(v, name, &value, errp);
> -}
> -
> -static void vu_set_blk_size(Object *obj, Visitor *v, const char *name,
> -                            void *opaque, Error **errp)
> -{
> -    VuBlockDev *vus = VHOST_USER_BLK_SERVER(obj);
> -
> +    VuBlkExport *vexp = container_of(exp, VuBlkExport, export);
> +    BlockExportOptionsVhostUserBlk *vu_opts = &opts->u.vhost_user_blk;
> +    SocketAddress addr = {
> +        .type = SOCKET_ADDRESS_TYPE_UNIX,
> +        .u.q_unix.path = vu_opts->has_unix_socket ?
> +                         vu_opts->unix_socket :
> +                         NULL,
> +    };
>      Error *local_err = NULL;
> -    uint32_t value;
> +    uint64_t logical_block_size;
>  
> -    if (!vu_prop_modifiable(vus, errp)) {
> -            return;
> +    if (!vu_opts->has_unix_socket) {
> +        error_setg(errp, "Missing unix-socket path to listen on");
> +        return -EINVAL;
>      }

This is where we require @unix-socket.

>  
> -    visit_type_uint32(v, name, &value, &local_err);
> -    if (local_err) {
> -        goto out;
> -    }
> +    vexp->writable = opts->writable;
> +    vexp->blkcfg.wce = 0;
>  
> -    check_block_size(object_get_typename(obj), name, value, &local_err);
> +    if (vu_opts->has_logical_block_size) {
> +        logical_block_size = vu_opts->logical_block_size;
> +    } else {
> +        logical_block_size = BDRV_SECTOR_SIZE;
> +    }
> +    check_block_size(exp->id, "logical-block-size", logical_block_size,
> +                     &local_err);
>      if (local_err) {
> -        goto out;
> +        error_propagate(errp, local_err);
> +        return -EINVAL;
> +    }
> +    vexp->blk_size = logical_block_size;
> +    blk_set_guest_block_size(exp->blk, logical_block_size);
> +    vu_blk_initialize_config(blk_bs(exp->blk), &vexp->blkcfg,
> +                               logical_block_size);
> +
> +    blk_set_allow_aio_context_change(exp->blk, true);
> +    blk_add_aio_context_notifier(exp->blk, blk_aio_attached, blk_aio_detach,
> +                                 vexp);
> +
> +    if (!vhost_user_server_start(&vexp->vu_server, &addr, exp->ctx,
> +                                 VHOST_USER_BLK_MAX_QUEUES, &vu_blk_iface,
> +                                 errp)) {
> +        blk_remove_aio_context_notifier(exp->blk, blk_aio_attached,
> +                                        blk_aio_detach, vexp);
> +        return -EADDRNOTAVAIL;
>      }
>  
> -    vus->blk_size = value;
> -
> -out:
> -    error_propagate(errp, local_err);
> -}
> -
> -static void vhost_user_blk_server_instance_finalize(Object *obj)
> -{
> -    VuBlockDev *vub = VHOST_USER_BLK_SERVER(obj);
> -
> -    vhost_user_blk_server_stop(vub);
> -
> -    /*
> -     * Unlike object_property_add_str, object_class_property_add_str
> -     * doesn't have a release method. Thus manual memory freeing is
> -     * needed.
> -     */
> -    free_socket_addr(vub->addr);
> -    g_free(vub->node_name);
> -}
> -
> -static void vhost_user_blk_server_complete(UserCreatable *obj, Error **errp)
> -{
> -    VuBlockDev *vub = VHOST_USER_BLK_SERVER(obj);
> -
> -    vhost_user_blk_server_start(vub, errp);
> +    return 0;
>  }
[...]