| Message ID | 20200922160401.294055-1-stefanha@redhat.com |
|---|---|
| Headers | show |
| Series | block/export: convert vhost-user-blk-server to block exports API | expand |
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; > } [...]
On Wed, Sep 23, 2020 at 03:42:30PM +0200, Markus Armbruster wrote: > 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. Good point, it should be mandatory. Stefan
This patch series converts Coiby Xu's vhost-user-blk-server from a QOM object to the block exports API. The block exports API provides a standard QMP and command-line interface for managing block exports (NBD, FUSE, vhost-user-blk, etc). A fair amount of init/shutdown code is removed because the block exports API already takes care of that functionality. Most of the patches are vhost-user-blk-server cleanups. The syntax for launching qemu-storage-daemon is: $ qemu-storage-daemon \ --blockdev file,node-name=drive0,filename=test.img \ --export vhost-user-blk,node-name=drive0,id=export0,writable=on,unix-socket=/tmp/vhost-user-blk.sock QEMU can connect to the vhost-user-blk export like this: $ qemu-system-x86_64 \ -M accel=kvm,memory-backend=mem \ -m 1G \ -object memory-backend-memfd,size=1G,id=mem \ -cpu host \ -chardev socket,path=/tmp/vhost-user-blk.sock,id=char0 \ -device vhost-user-blk-pci,chardev=char0 Based-on: 20200918080912.321299-1-coiby.xu@gmail.com ("[PATCH v10 0/7] vhost-user block device backend implementation") Based-on: 20200907182011.521007-1-kwolf@redhat.com ("[PATCH 00/29] block/export: Add infrastructure and QAPI for block exports") Stefan Hajnoczi (11): block/export: shorten serial string to fit util/vhost-user-server: s/fileds/fields/ typo fix util/vhost-user-server: drop unnecessary QOM cast util/vhost-user-server: drop unnecessary watch deletion block/export: consolidate request structs into VuBlockReq util/vhost-user-server: drop unused DevicePanicNotifier util/vhost-user-server: fix memory leak in vu_message_read() util/vhost-user-server: check EOF when reading payload util/vhost-user-server: rework vu_client_trip() coroutine lifecycle block/export: report flush errors block/export: convert vhost-user-blk server to block export API qapi/block-export.json | 19 +- block/export/vhost-user-blk-server.h | 23 +- util/vhost-user-server.h | 32 +- block/export/export.c | 8 +- block/export/vhost-user-blk-server.c | 534 ++++++++------------------- util/vhost-user-server.c | 322 ++++++++-------- block/export/meson.build | 1 + block/meson.build | 1 - 8 files changed, 360 insertions(+), 580 deletions(-) -- 2.26.2