On Wed, Sep 30, 2020 at 04:33:18PM +0200, Markus Armbruster wrote: > Stefan Hajnoczi <stefa...@redhat.com> writes: > > > On Wed, Sep 30, 2020 at 07:28:58AM +0200, Markus Armbruster wrote: > >> Stefan Hajnoczi <stefa...@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. > >> > > >> > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com> > >> > --- > >> > v2: > >> > * Replace str unix-socket with SocketAddress addr to match NBD and > >> > support file descriptor passing > >> > * Make addr mandatory [Markus] > >> > * Update vhost-user-blk-test.c to use --export syntax > >> > --- > >> > qapi/block-export.json | 21 +- > >> > block/export/vhost-user-blk-server.h | 23 +- > >> > block/export/export.c | 8 +- > >> > block/export/vhost-user-blk-server.c | 452 +++++++-------------------- > >> > tests/qtest/vhost-user-blk-test.c | 2 +- > >> > util/vhost-user-server.c | 10 +- > >> > block/export/meson.build | 1 + > >> > block/meson.build | 1 - > >> > 8 files changed, 158 insertions(+), 360 deletions(-) > >> > > >> > diff --git a/qapi/block-export.json b/qapi/block-export.json > >> > index ace0d66e17..2e44625bb1 100644 > >> > --- a/qapi/block-export.json > >> > +++ b/qapi/block-export.json > >> > @@ -84,6 +84,21 @@ > >> > 'data': { '*name': 'str', '*description': 'str', > >> > '*bitmap': 'str' } } > >> > > >> > +## > >> > +# @BlockExportOptionsVhostUserBlk: > >> > +# > >> > +# A vhost-user-blk block export. > >> > +# > >> > +# @addr: The vhost-user socket on which to listen. Both 'unix' and 'fd' > >> > +# SocketAddress types are supported. Passed fds must be UNIX > >> > domain > >> > +# sockets. > >> > >> "addr.type must be 'unix' or 'fd'" is not visible in introspection. > >> Awkward. Practical problem only if other addresses ever become > >> available here. Is that possible? > > > > addr.type=fd itself has the same problem, because it is a file > > descriptor without type information. Therefore the QMP client cannot > > introspect which types of file descriptors can be passed. > > Yes, but if introspection could tell us which which values of addr.type > are valid, then a client should figure out the address families, as > follows. Any valid value other than 'fd' corresponds to an address > family. The set of values valid for addr.type therefore corresponds to > a set of address families. The address families in that set are all > valid with 'fd', aren't they?
Yes, in this case the address families in addr.type are the ones also supported by addr.type=fd. > > Two ideas: > > > > 1. Introduce per-address family fd types (SocketAddrFdTcpInet, > > SocketAddrFdTcpInet6, SocketAddrFdUnix, etc) to express specific > > address families in the QAPI schema. > > > > Then use per-command unions to express the address families supported > > by specific commands. For example, > > BlockExportOptionsVhostUserBlkSocketAddr would only allow > > SocketAddrUnix and SocketAddrFdUnix. That way new address families > > can be supported in the future and introspection reports. > > Awkward. These types would have to differ structurally, or else they > are indistinguishable in introspection. > > > 2. Use a side-channel (query-features, I think we discussed something > > like this a while back) to report features that cannot be > > introspected. > > We implemented this in the form of QAPI feature flags, visible in > introspection. You could do something like > > 'addr': { 'type': 'SocketAddress', > 'features': [ 'unix' ] } That is nice. > > > I think the added complexity for achieving full introspection is not > > worth it. It becomes harder to define new QAPI commands, increases the > > chance of errors, and is more tedious to program for clients/servers. > > Hence my question: is it possible that address families other than unix > become available here? > > When that happens, we have an introspection problem of the sort we > common solve with a feature flag. > > > Accepting any SocketAddr seems reasonable to me since vhost-user > > requires an address family that has file descriptor passing. Very few > > address families support this feature and we don't expect to add new > > ones often. > > Your answer appears to be "yes in theory, quite unlikely in practice". > Correct? Yes. Stefan
signature.asc
Description: PGP signature