On Fri, May 27, 2022 at 11:34 PM Kevin Wolf <kw...@redhat.com> wrote:
>
> Am 23.05.2022 um 10:46 hat Xie Yongji geschrieben:
> > This implements a VDUSE block backends based on
> > the libvduse library. We can use it to export the BDSs
> > for both VM and container (host) usage.
> >
> > The new command-line syntax is:
> >
> > $ qemu-storage-daemon \
> > --blockdev file,node-name=drive0,filename=test.img \
> > --export vduse-blk,node-name=drive0,id=vduse-export0,writable=on
> >
> > After the qemu-storage-daemon started, we need to use
> > the "vdpa" command to attach the device to vDPA bus:
> >
> > $ vdpa dev add name vduse-export0 mgmtdev vduse
> >
> > Also the device must be removed via the "vdpa" command
> > before we stop the qemu-storage-daemon.
> >
> > Signed-off-by: Xie Yongji <xieyon...@bytedance.com>
> > Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
> > ---
> > MAINTAINERS | 4 +-
> > block/export/export.c | 6 +
> > block/export/meson.build | 5 +
> > block/export/vduse-blk.c | 307 ++++++++++++++++++++++++++++++++++
> > block/export/vduse-blk.h | 20 +++
> > meson.build | 13 ++
> > meson_options.txt | 2 +
> > qapi/block-export.json | 28 +++-
> > scripts/meson-buildoptions.sh | 4 +
> > 9 files changed, 385 insertions(+), 4 deletions(-)
> > create mode 100644 block/export/vduse-blk.c
> > create mode 100644 block/export/vduse-blk.h
>
> > diff --git a/qapi/block-export.json b/qapi/block-export.json
> > index 0685cb8b9a..e4bd4de363 100644
> > --- a/qapi/block-export.json
> > +++ b/qapi/block-export.json
> > @@ -177,6 +177,23 @@
> > '*allow-other': 'FuseExportAllowOther' },
> > 'if': 'CONFIG_FUSE' }
> >
> > +##
> > +# @BlockExportOptionsVduseBlk:
> > +#
> > +# A vduse-blk block export.
> > +#
> > +# @num-queues: the number of virtqueues. Defaults to 1.
> > +# @queue-size: the size of virtqueue. Defaults to 256.
> > +# @logical-block-size: Logical block size in bytes. Range [512, PAGE_SIZE]
> > +# and must be power of 2. Defaults to 512 bytes.
> > +#
> > +# Since: 7.1
> > +##
> > +{ 'struct': 'BlockExportOptionsVduseBlk',
> > + 'data': { '*num-queues': 'uint16',
> > + '*queue-size': 'uint16',
> > + '*logical-block-size': 'size'} }
> > +
> > ##
> > # @NbdServerAddOptions:
> > #
> > @@ -280,6 +297,7 @@
> > # @nbd: NBD export
> > # @vhost-user-blk: vhost-user-blk export (since 5.2)
> > # @fuse: FUSE export (since: 6.0)
> > +# @vduse-blk: vduse-blk export (since 7.1)
> > #
> > # Since: 4.2
> > ##
> > @@ -287,7 +305,8 @@
> > 'data': [ 'nbd',
> > { 'name': 'vhost-user-blk',
> > 'if': 'CONFIG_VHOST_USER_BLK_SERVER' },
> > - { 'name': 'fuse', 'if': 'CONFIG_FUSE' } ] }
> > + { 'name': 'fuse', 'if': 'CONFIG_FUSE' },
> > + { 'name': 'vduse-blk', 'if': 'CONFIG_VDUSE_BLK_EXPORT' } ] }
> >
> > ##
> > # @BlockExportOptions:
> > @@ -295,7 +314,8 @@
> > # Describes a block export, i.e. how single node should be exported on an
> > # external interface.
> > #
> > -# @id: A unique identifier for the block export (across all export types)
> > +# @id: A unique identifier for the block export (across the host for
> > vduse-blk
> > +# export type or across all export types for other types)
>
> I find this sentence a bit confusing, but more importantly, it shows
> that you are using one value for two different purposes: The ID to
> identfy the export within QEMU (must be distinct from any other exports
> in the same QEMU process, but can overlap with names used by other
> processes), and the VDUSE name to uniquely identify it on the host (must
> be distinct from other VDUSE devices on the same host, but can overlap
> with other export types like NBD in the same process).
>
> We can fix this on top, but I would suggest having a separate option for
> the VDUSE device name, like BlockExportOptionsNbdBase contains a 'name'
> option for the export name that is different from the export ID in QEMU.
>
Yes, we discussed whether we need a separate option before. I think I
can send a patch to do it.
Thanks,
Yongji
