| Message ID | 13dba991f1de91711e5c3cad9a332d6e7c5eee7b.1633929457.git.jag.raman@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio-user server in QEMU | expand |
On Mon, Oct 11, 2021 at 01:31:08AM -0400, Jagannathan Raman wrote: > diff --git a/qapi/qom.json b/qapi/qom.json > index 0222bb4506..97de79cc36 100644 > --- a/qapi/qom.json > +++ b/qapi/qom.json > @@ -705,6 +705,20 @@ > { 'struct': 'RemoteObjectProperties', > 'data': { 'fd': 'str', 'devid': 'str' } } > > +## > +# @VfioUserServerProperties: > +# > +# Properties for vfio-user-server objects. > +# > +# @socket: socket to be used by the libvfiouser library > +# > +# @device: the id of the device to be emulated at the server > +# > +# Since: 6.0 6.2 > +## > +{ 'struct': 'VfioUserServerProperties', > + 'data': { 'socket': 'SocketAddress', 'device': 'str' } } > + > ## > # @RngProperties: > # > @@ -830,7 +844,8 @@ > 'tls-creds-psk', > 'tls-creds-x509', > 'tls-cipher-suites', > - 'x-remote-object' > + 'x-remote-object', > + 'vfio-user-server' Should it have the experimental prefix ('x-') or is the QAPI interface stable? I think some things to think about are whether a single process can host multiple device servers, whether hotplug is possible, etc. If the interface is stable then it must be able to accomodate future features (at least ones we can anticipate right now :)). > ] } > > ## > @@ -887,7 +902,8 @@ > 'tls-creds-psk': 'TlsCredsPskProperties', > 'tls-creds-x509': 'TlsCredsX509Properties', > 'tls-cipher-suites': 'TlsCredsProperties', > - 'x-remote-object': 'RemoteObjectProperties' > + 'x-remote-object': 'RemoteObjectProperties', > + 'vfio-user-server': 'VfioUserServerProperties' > } } > > ## > diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c > new file mode 100644 > index 0000000000..c2a300f0ff > --- /dev/null > +++ b/hw/remote/vfio-user-obj.c > @@ -0,0 +1,173 @@ > +/** > + * QEMU vfio-user-server server object > + * > + * Copyright © 2021 Oracle and/or its affiliates. > + * > + * This work is licensed under the terms of the GNU GPL-v2, version 2 or later. > + * > + * See the COPYING file in the top-level directory. > + * > + */ > + > +/** > + * Usage: add options: > + * -machine x-remote > + * -device <PCI-device>,id=<pci-dev-id> > + * -object vfio-user-server,id=<id>,type=unix,path=<socket-path>, I expected socket.type= and socket.path= based on the QAPI schema. Is this command-line example correct? > + * device=<pci-dev-id> > + * > + * Note that vfio-user-server object must be used with x-remote machine only. > + * This server could only support PCI devices for now. > + * > + * type - SocketAddress type - presently "unix" alone is supported. Required > + * option > + * > + * path - named unix socket, it will be created by the server. It is > + * a required option > + * > + * device - id of a device on the server, a required option. PCI devices > + * alone are supported presently. > + */ > + > +#include "qemu/osdep.h" > +#include "qemu-common.h" > + > +#include "qom/object.h" > +#include "qom/object_interfaces.h" > +#include "qemu/error-report.h" > +#include "trace.h" > +#include "sysemu/runstate.h" > +#include "hw/boards.h" > +#include "hw/remote/machine.h" > +#include "qapi/error.h" > +#include "qapi/qapi-visit-sockets.h" > + > +#define TYPE_VFU_OBJECT "vfio-user-server" > +OBJECT_DECLARE_TYPE(VfuObject, VfuObjectClass, VFU_OBJECT) > + > +struct VfuObjectClass { > + ObjectClass parent_class; > + > + unsigned int nr_devs; > + > + /* Maximum number of devices the server could support */ > + unsigned int max_devs; > +}; > + > +struct VfuObject { > + /* private */ > + Object parent; > + > + SocketAddress *socket; > + > + char *device; > +}; > + > +static void vfu_object_set_socket(Object *obj, Visitor *v, const char *name, > + void *opaque, Error **errp) > +{ > + VfuObject *o = VFU_OBJECT(obj); > + > + g_free(o->socket); qapi_free_SocketAddress(o->socket)? > + > + o->socket = NULL; > + > + visit_type_SocketAddress(v, name, &o->socket, errp); > + > + if (o->socket->type != SOCKET_ADDRESS_TYPE_UNIX) { > + error_setg(&error_abort, "vfu: Unsupported socket type - %s", > + o->socket->u.q_unix.path); o->socket must be freed and set it to NULL again, otherwise setting the property appears to fail but the SocketAddress actually retains the invalid value. > + return; > + } > + > + trace_vfu_prop("socket", o->socket->u.q_unix.path); > +} > + > +static void vfu_object_set_device(Object *obj, const char *str, Error **errp) > +{ > + VfuObject *o = VFU_OBJECT(obj); > + > + g_free(o->device); > + > + o->device = g_strdup(str); > + > + trace_vfu_prop("device", str); > +} > + > +static void vfu_object_init(Object *obj) > +{ > + VfuObjectClass *k = VFU_OBJECT_GET_CLASS(obj); > + > + if (!object_dynamic_cast(OBJECT(current_machine), TYPE_REMOTE_MACHINE)) { > + error_setg(&error_abort, "vfu: %s only compatible with %s machine", > + TYPE_VFU_OBJECT, TYPE_REMOTE_MACHINE); > + return; > + } > + > + if (k->nr_devs >= k->max_devs) { > + error_setg(&error_abort, > + "Reached maximum number of vfio-user-server devices: %u", > + k->max_devs); > + return; > + } > + > + k->nr_devs++; > +} > + > +static void vfu_object_finalize(Object *obj) > +{ > + VfuObjectClass *k = VFU_OBJECT_GET_CLASS(obj); > + VfuObject *o = VFU_OBJECT(obj); > + > + k->nr_devs--; > + > + g_free(o->socket); qapi_free_SocketAddress(o->socket)? > + > + g_free(o->device); > + > + if (k->nr_devs == 0) { > + qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); > + } This won't work for all use cases. The user may wish to create/delete vhost-user servers at runtime without terminating the process when there are none left. An boolean option can be added in the future to control this behavior, so it's okay to leave this as is.
> On Oct 27, 2021, at 11:40 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote: > > On Mon, Oct 11, 2021 at 01:31:08AM -0400, Jagannathan Raman wrote: >> diff --git a/qapi/qom.json b/qapi/qom.json >> index 0222bb4506..97de79cc36 100644 >> --- a/qapi/qom.json >> +++ b/qapi/qom.json >> @@ -705,6 +705,20 @@ >> { 'struct': 'RemoteObjectProperties', >> 'data': { 'fd': 'str', 'devid': 'str' } } >> >> +## >> +# @VfioUserServerProperties: >> +# >> +# Properties for vfio-user-server objects. >> +# >> +# @socket: socket to be used by the libvfiouser library >> +# >> +# @device: the id of the device to be emulated at the server >> +# >> +# Since: 6.0 > > 6.2 > >> +## >> +{ 'struct': 'VfioUserServerProperties', >> + 'data': { 'socket': 'SocketAddress', 'device': 'str' } } >> + >> ## >> # @RngProperties: >> # >> @@ -830,7 +844,8 @@ >> 'tls-creds-psk', >> 'tls-creds-x509', >> 'tls-cipher-suites', >> - 'x-remote-object' >> + 'x-remote-object', >> + 'vfio-user-server' > > Should it have the experimental prefix ('x-') or is the QAPI interface > stable? I think some things to think about are whether a single process > can host multiple device servers, whether hotplug is possible, etc. If > the interface is stable then it must be able to accomodate future > features (at least ones we can anticipate right now :)). We did test out hotplug support. We’ll get back to you on the multiple device servers scenario. > >> ] } >> >> ## >> @@ -887,7 +902,8 @@ >> 'tls-creds-psk': 'TlsCredsPskProperties', >> 'tls-creds-x509': 'TlsCredsX509Properties', >> 'tls-cipher-suites': 'TlsCredsProperties', >> - 'x-remote-object': 'RemoteObjectProperties' >> + 'x-remote-object': 'RemoteObjectProperties', >> + 'vfio-user-server': 'VfioUserServerProperties' >> } } >> >> ## >> diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c >> new file mode 100644 >> index 0000000000..c2a300f0ff >> --- /dev/null >> +++ b/hw/remote/vfio-user-obj.c >> @@ -0,0 +1,173 @@ >> +/** >> + * QEMU vfio-user-server server object >> + * >> + * Copyright © 2021 Oracle and/or its affiliates. >> + * >> + * This work is licensed under the terms of the GNU GPL-v2, version 2 or later. >> + * >> + * See the COPYING file in the top-level directory. >> + * >> + */ >> + >> +/** >> + * Usage: add options: >> + * -machine x-remote >> + * -device <PCI-device>,id=<pci-dev-id> >> + * -object vfio-user-server,id=<id>,type=unix,path=<socket-path>, > > I expected socket.type= and socket.path= based on the QAPI schema. Is > this command-line example correct? When I tried the “socket.path” approach, QEMU was not able to parse the arguments. So I had to break it down to a series of individual members. If “socket.path” is the expected way, I’ll see why the parser is not working as expected. > >> + * device=<pci-dev-id> >> + * >> + * Note that vfio-user-server object must be used with x-remote machine only. >> + * This server could only support PCI devices for now. >> + * >> + * type - SocketAddress type - presently "unix" alone is supported. Required >> + * option >> + * >> + * path - named unix socket, it will be created by the server. It is >> + * a required option >> + * >> + * device - id of a device on the server, a required option. PCI devices >> + * alone are supported presently. >> + */ >> + >> +#include "qemu/osdep.h" >> +#include "qemu-common.h" >> + >> +#include "qom/object.h" >> +#include "qom/object_interfaces.h" >> +#include "qemu/error-report.h" >> +#include "trace.h" >> +#include "sysemu/runstate.h" >> +#include "hw/boards.h" >> +#include "hw/remote/machine.h" >> +#include "qapi/error.h" >> +#include "qapi/qapi-visit-sockets.h" >> + >> +#define TYPE_VFU_OBJECT "vfio-user-server" >> +OBJECT_DECLARE_TYPE(VfuObject, VfuObjectClass, VFU_OBJECT) >> + >> +struct VfuObjectClass { >> + ObjectClass parent_class; >> + >> + unsigned int nr_devs; >> + >> + /* Maximum number of devices the server could support */ >> + unsigned int max_devs; >> +}; >> + >> +struct VfuObject { >> + /* private */ >> + Object parent; >> + >> + SocketAddress *socket; >> + >> + char *device; >> +}; >> + >> +static void vfu_object_set_socket(Object *obj, Visitor *v, const char *name, >> + void *opaque, Error **errp) >> +{ >> + VfuObject *o = VFU_OBJECT(obj); >> + >> + g_free(o->socket); > > qapi_free_SocketAddress(o->socket)? OK, will use that. Didn’t know the visitors also define a function for free’ing. Thank you! > >> + >> + o->socket = NULL; >> + >> + visit_type_SocketAddress(v, name, &o->socket, errp); >> + >> + if (o->socket->type != SOCKET_ADDRESS_TYPE_UNIX) { >> + error_setg(&error_abort, "vfu: Unsupported socket type - %s", >> + o->socket->u.q_unix.path); > > o->socket must be freed and set it to NULL again, otherwise setting the > property appears to fail but the SocketAddress actually retains the > invalid value. OK, got it. > >> + return; >> + } >> + >> + trace_vfu_prop("socket", o->socket->u.q_unix.path); >> +} >> + >> +static void vfu_object_set_device(Object *obj, const char *str, Error **errp) >> +{ >> + VfuObject *o = VFU_OBJECT(obj); >> + >> + g_free(o->device); >> + >> + o->device = g_strdup(str); >> + >> + trace_vfu_prop("device", str); >> +} >> + >> +static void vfu_object_init(Object *obj) >> +{ >> + VfuObjectClass *k = VFU_OBJECT_GET_CLASS(obj); >> + >> + if (!object_dynamic_cast(OBJECT(current_machine), TYPE_REMOTE_MACHINE)) { >> + error_setg(&error_abort, "vfu: %s only compatible with %s machine", >> + TYPE_VFU_OBJECT, TYPE_REMOTE_MACHINE); >> + return; >> + } >> + >> + if (k->nr_devs >= k->max_devs) { >> + error_setg(&error_abort, >> + "Reached maximum number of vfio-user-server devices: %u", >> + k->max_devs); >> + return; >> + } >> + >> + k->nr_devs++; >> +} >> + >> +static void vfu_object_finalize(Object *obj) >> +{ >> + VfuObjectClass *k = VFU_OBJECT_GET_CLASS(obj); >> + VfuObject *o = VFU_OBJECT(obj); >> + >> + k->nr_devs--; >> + >> + g_free(o->socket); > > qapi_free_SocketAddress(o->socket)? > >> + >> + g_free(o->device); >> + >> + if (k->nr_devs == 0) { >> + qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); >> + } > > This won't work for all use cases. The user may wish to create/delete > vhost-user servers at runtime without terminating the process when there > are none left. An boolean option can be added in the future to control > this behavior, so it's okay to leave this as is. Thank you, we’ll make a note of this. I’ll add a TODO comment here to ensure we don’t lose this thought. -- Jag
On Fri, Oct 29, 2021 at 02:42:49PM +0000, Jag Raman wrote: > > On Oct 27, 2021, at 11:40 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote: > > On Mon, Oct 11, 2021 at 01:31:08AM -0400, Jagannathan Raman wrote: > >> diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c > >> new file mode 100644 > >> index 0000000000..c2a300f0ff > >> --- /dev/null > >> +++ b/hw/remote/vfio-user-obj.c > >> @@ -0,0 +1,173 @@ > >> +/** > >> + * QEMU vfio-user-server server object > >> + * > >> + * Copyright © 2021 Oracle and/or its affiliates. > >> + * > >> + * This work is licensed under the terms of the GNU GPL-v2, version 2 or later. > >> + * > >> + * See the COPYING file in the top-level directory. > >> + * > >> + */ > >> + > >> +/** > >> + * Usage: add options: > >> + * -machine x-remote > >> + * -device <PCI-device>,id=<pci-dev-id> > >> + * -object vfio-user-server,id=<id>,type=unix,path=<socket-path>, > > > > I expected socket.type= and socket.path= based on the QAPI schema. Is > > this command-line example correct? > > When I tried the “socket.path” approach, QEMU was not able to parse the > arguments. So I had to break it down to a series of individual members. > > If “socket.path” is the expected way, I’ll see why the parser is not working > as expected. CCing Markus regarding QAPI. I'm surprised because the QAPI schema for vfio-user-server objects is: { 'struct': 'VfioUserServerProperties', 'data': { 'socket': 'SocketAddress', 'device': 'str' } } It's not clear to me why the command-line parser flattens the 'socket' field into its 'type' and 'path' sub-fields in your example: -object vfio-user-server,id=<id>,type=unix,path=<socket-path>,device=<pci-dev-id> Maybe because SocketAddress is an enum instead of a struct? Imagine a second SocketAddress field is added to vfio-user-server. How can the parser know which field 'type' and 'path' belong to? I tried it: { 'struct': 'VfioUserServerProperties', 'data': { 'socket': 'SocketAddress', 'socket2': 'SocketAddress', 'device': 'str' } } Now the parser refuses any input I've tried. For example: $ build/qemu-system-x86_64 -object vfio-user-server,id=s,device=asdf,type=unix,path=asdf,type=unix qemu-system-x86_64: -object vfio-user-server,id=s,device=asdf,type=unix,path=asdf,type=unix: Parameter 'type' is missing A similar case happens if the parent struct has 'type' or 'path' fields. They collide with the SocketAddress union fields. I didn't test this though. Questions for Markus: 1. Do you know why the parser behaves like this? 2. Is it good practice to embed SocketAddress into parent structs or should we force it into a struct? Stefan
Stefan Hajnoczi <stefanha@redhat.com> writes: > On Fri, Oct 29, 2021 at 02:42:49PM +0000, Jag Raman wrote: >> > On Oct 27, 2021, at 11:40 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote: >> > On Mon, Oct 11, 2021 at 01:31:08AM -0400, Jagannathan Raman wrote: >> >> diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c >> >> new file mode 100644 >> >> index 0000000000..c2a300f0ff >> >> --- /dev/null >> >> +++ b/hw/remote/vfio-user-obj.c >> >> @@ -0,0 +1,173 @@ >> >> +/** >> >> + * QEMU vfio-user-server server object >> >> + * >> >> + * Copyright © 2021 Oracle and/or its affiliates. >> >> + * >> >> + * This work is licensed under the terms of the GNU GPL-v2, version 2 or later. >> >> + * >> >> + * See the COPYING file in the top-level directory. >> >> + * >> >> + */ >> >> + >> >> +/** >> >> + * Usage: add options: >> >> + * -machine x-remote >> >> + * -device <PCI-device>,id=<pci-dev-id> >> >> + * -object vfio-user-server,id=<id>,type=unix,path=<socket-path>, >> > >> > I expected socket.type= and socket.path= based on the QAPI schema. Is >> > this command-line example correct? >> >> When I tried the “socket.path” approach, QEMU was not able to parse the >> arguments. So I had to break it down to a series of individual members. >> >> If “socket.path” is the expected way, I’ll see why the parser is not working >> as expected. > > CCing Markus regarding QAPI. > > I'm surprised because the QAPI schema for vfio-user-server objects is: > > { 'struct': 'VfioUserServerProperties', > 'data': { 'socket': 'SocketAddress', 'device': 'str' } } > > It's not clear to me why the command-line parser flattens the 'socket' > field into its 'type' and 'path' sub-fields in your example: > > -object vfio-user-server,id=<id>,type=unix,path=<socket-path>,device=<pci-dev-id> > > Maybe because SocketAddress is an enum instead of a struct? > > Imagine a second SocketAddress field is added to vfio-user-server. How > can the parser know which field 'type' and 'path' belong to? I tried it: > > { 'struct': 'VfioUserServerProperties', > 'data': { 'socket': 'SocketAddress', 'socket2': 'SocketAddress', 'device': 'str' } } > > Now the parser refuses any input I've tried. For example: > > $ build/qemu-system-x86_64 -object vfio-user-server,id=s,device=asdf,type=unix,path=asdf,type=unix > qemu-system-x86_64: -object vfio-user-server,id=s,device=asdf,type=unix,path=asdf,type=unix: Parameter 'type' is missing > > A similar case happens if the parent struct has 'type' or 'path' fields. > They collide with the SocketAddress union fields. I didn't test this > though. > > Questions for Markus: > 1. Do you know why the parser behaves like this? Yes: backward compatibility. The straightforward way to do a QAPI-based command line option uses qobject_input_visitor_new_str(), which parses either JSON or dotted keys, and returns the result wrapped in the appropriate QObject visitor. The JSON syntax is derived from the QAPI schema just like for QMP. For the VfioUserServerProperties shown above, it's something like {"socket": {"type": "unix", "path": "dir/socket"}, "device" "mumble"} I did not check my derivation by feeding it to QEMU. Beware of screwups. The dotted keys syntax is derived from the JSON syntax as described in keyval.c. For the JSON above, it should be socket.type=unix,socket.path=dir/socket,device=mumble When we QAPIfy an existing option instead of adding a new QAPI-based one, we have an additional problem: the dotted keys syntax has to match the old syntax (the JSON syntax is all new, so no problem there). The old syntax almost always has its quirks. Ideally, we'd somehow get from quirky old to boring new in an orderly manner. Sadly, we still don't have good solutions for that. To make progress, we commonly combine JSON new with quirky old. qemu-system-FOO -object works that way. object_option_parse() parses either JSON or QemuOpts. It wraps the former in a QObject visitor, and the latter in an opts visitor. QemuOpts is flat by design[*], so the opts visitor parses flat QemuOpts from a (possibly non-flat) QAPI type. How exactly it flattens, and how it handles clashes I don't remember. Sadly, this means that we get quirky old even for new object types. Questions? > 2. Is it good practice to embed SocketAddress into parent structs or > should we force it into a struct? I'm not sure I got your question. An example might help. [*] You can play games with dotted keys to simulate nesting, but the opts visitor predates all that.
Am 04.11.2021 um 13:13 hat Markus Armbruster geschrieben: > The old syntax almost always has its quirks. Ideally, we'd somehow get > from quirky old to boring new in an orderly manner. Sadly, we still > don't have good solutions for that. To make progress, we commonly > combine JSON new with quirky old. > > qemu-system-FOO -object works that way. object_option_parse() parses > either JSON or QemuOpts. It wraps the former in a QObject visitor, and > the latter in an opts visitor. > > QemuOpts is flat by design[*], so the opts visitor parses flat QemuOpts > from a (possibly non-flat) QAPI type. How exactly it flattens, and how > it handles clashes I don't remember. > > Sadly, this means that we get quirky old even for new object types. For -object in the system emulator (the tools all use the keyval visitor, so there it would work as expected), the only reason that we need to keep the quirky old code path around is the list handling in memory-backend.host-nodes. The main difficulty there is that the old QemuOpts based code path allows specifying the option twice and both of them would effectively be combined. Do we have any idea how to replicate this in a keyval parser based world? If not, do we want to use the remaining time until 6.2 to deprecate this? The nasty part is that the only syntax that works both now and in the future is JSON. We can't easily accept the new keyval syntax while still using the QemuOpts based code. Kevin
On Thu, Nov 04, 2021 at 01:13:02PM +0100, Markus Armbruster wrote: > Stefan Hajnoczi <stefanha@redhat.com> writes: > > > On Fri, Oct 29, 2021 at 02:42:49PM +0000, Jag Raman wrote: > >> > On Oct 27, 2021, at 11:40 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote: > >> > On Mon, Oct 11, 2021 at 01:31:08AM -0400, Jagannathan Raman wrote: > >> >> diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c > >> >> new file mode 100644 > >> >> index 0000000000..c2a300f0ff > >> >> --- /dev/null > >> >> +++ b/hw/remote/vfio-user-obj.c > >> >> @@ -0,0 +1,173 @@ > >> >> +/** > >> >> + * QEMU vfio-user-server server object > >> >> + * > >> >> + * Copyright © 2021 Oracle and/or its affiliates. > >> >> + * > >> >> + * This work is licensed under the terms of the GNU GPL-v2, version 2 or later. > >> >> + * > >> >> + * See the COPYING file in the top-level directory. > >> >> + * > >> >> + */ > >> >> + > >> >> +/** > >> >> + * Usage: add options: > >> >> + * -machine x-remote > >> >> + * -device <PCI-device>,id=<pci-dev-id> > >> >> + * -object vfio-user-server,id=<id>,type=unix,path=<socket-path>, > >> > > >> > I expected socket.type= and socket.path= based on the QAPI schema. Is > >> > this command-line example correct? > >> > >> When I tried the “socket.path” approach, QEMU was not able to parse the > >> arguments. So I had to break it down to a series of individual members. > >> > >> If “socket.path” is the expected way, I’ll see why the parser is not working > >> as expected. > > > > CCing Markus regarding QAPI. > > > > I'm surprised because the QAPI schema for vfio-user-server objects is: > > > > { 'struct': 'VfioUserServerProperties', > > 'data': { 'socket': 'SocketAddress', 'device': 'str' } } > > > > It's not clear to me why the command-line parser flattens the 'socket' > > field into its 'type' and 'path' sub-fields in your example: > > > > -object vfio-user-server,id=<id>,type=unix,path=<socket-path>,device=<pci-dev-id> > > > > Maybe because SocketAddress is an enum instead of a struct? > > > > Imagine a second SocketAddress field is added to vfio-user-server. How > > can the parser know which field 'type' and 'path' belong to? I tried it: > > > > { 'struct': 'VfioUserServerProperties', > > 'data': { 'socket': 'SocketAddress', 'socket2': 'SocketAddress', 'device': 'str' } } > > > > Now the parser refuses any input I've tried. For example: > > > > $ build/qemu-system-x86_64 -object vfio-user-server,id=s,device=asdf,type=unix,path=asdf,type=unix > > qemu-system-x86_64: -object vfio-user-server,id=s,device=asdf,type=unix,path=asdf,type=unix: Parameter 'type' is missing > > > > A similar case happens if the parent struct has 'type' or 'path' fields. > > They collide with the SocketAddress union fields. I didn't test this > > though. > > > > Questions for Markus: > > 1. Do you know why the parser behaves like this? > > Yes: backward compatibility. > > The straightforward way to do a QAPI-based command line option uses > qobject_input_visitor_new_str(), which parses either JSON or dotted > keys, and returns the result wrapped in the appropriate QObject visitor. > > The JSON syntax is derived from the QAPI schema just like for QMP. For > the VfioUserServerProperties shown above, it's something like > > {"socket": {"type": "unix", "path": "dir/socket"}, "device" "mumble"} > > I did not check my derivation by feeding it to QEMU. Beware of > screwups. > > The dotted keys syntax is derived from the JSON syntax as described in > keyval.c. For the JSON above, it should be > > socket.type=unix,socket.path=dir/socket,device=mumble > > When we QAPIfy an existing option instead of adding a new QAPI-based > one, we have an additional problem: the dotted keys syntax has to match > the old syntax (the JSON syntax is all new, so no problem there). > > The old syntax almost always has its quirks. Ideally, we'd somehow get > from quirky old to boring new in an orderly manner. Sadly, we still > don't have good solutions for that. To make progress, we commonly > combine JSON new with quirky old. > > qemu-system-FOO -object works that way. object_option_parse() parses > either JSON or QemuOpts. It wraps the former in a QObject visitor, and > the latter in an opts visitor. > > QemuOpts is flat by design[*], so the opts visitor parses flat QemuOpts > from a (possibly non-flat) QAPI type. How exactly it flattens, and how > it handles clashes I don't remember. > > Sadly, this means that we get quirky old even for new object types. > > Questions? > > > 2. Is it good practice to embed SocketAddress into parent structs or > > should we force it into a struct? > > I'm not sure I got your question. An example might help. I think Question 2 isn't relevant anymore. Thanks for the explanation! > [*] You can play games with dotted keys to simulate nesting, but the > opts visitor predates all that. >
Kevin Wolf <kwolf@redhat.com> writes: > Am 04.11.2021 um 13:13 hat Markus Armbruster geschrieben: >> The old syntax almost always has its quirks. Ideally, we'd somehow get >> from quirky old to boring new in an orderly manner. Sadly, we still >> don't have good solutions for that. To make progress, we commonly >> combine JSON new with quirky old. >> >> qemu-system-FOO -object works that way. object_option_parse() parses >> either JSON or QemuOpts. It wraps the former in a QObject visitor, and >> the latter in an opts visitor. >> >> QemuOpts is flat by design[*], so the opts visitor parses flat QemuOpts >> from a (possibly non-flat) QAPI type. How exactly it flattens, and how >> it handles clashes I don't remember. >> >> Sadly, this means that we get quirky old even for new object types. > > For -object in the system emulator (the tools all use the keyval > visitor, so there it would work as expected), the only reason that we > need to keep the quirky old code path around is the list handling in > memory-backend.host-nodes. > > The main difficulty there is that the old QemuOpts based code path > allows specifying the option twice and both of them would effectively be > combined. Do we have any idea how to replicate this in a keyval parser > based world? I can see just two clean solutions, but both involve upending a lot of code. We can fuse keyval parser and visitor to get a schema-directed parser. We can change the abstract keyval syntax to permit repeated keys. This means replacing QDict in in the abstract syntax tree, with fallout in the visitor. Even if we find a practical solution, I don't like the combination of "you may give the same parameter multiple times, and the last one wins" and "for a list-valued parameter, the values of repeated parameters are collected into a list". Each makes sense on its own. The combination not so much. Inheriting "last one wins" from QemuOpts may have been a mistake. The keyval way of doing lists (inherited from the block layer's usage of dotted keys? I don't remember) requires the user to count, which isn't exactly nice, either. > If not, do we want to use the remaining time until 6.2 to deprecate > this? The nasty part is that the only syntax that works both now and in > the future is JSON. We can't easily accept the new keyval syntax while > still using the QemuOpts based code. What exactly do you propose to deprecate?
Am 05.11.2021 um 11:08 hat Markus Armbruster geschrieben: > Kevin Wolf <kwolf@redhat.com> writes: > > > Am 04.11.2021 um 13:13 hat Markus Armbruster geschrieben: > >> The old syntax almost always has its quirks. Ideally, we'd somehow get > >> from quirky old to boring new in an orderly manner. Sadly, we still > >> don't have good solutions for that. To make progress, we commonly > >> combine JSON new with quirky old. > >> > >> qemu-system-FOO -object works that way. object_option_parse() parses > >> either JSON or QemuOpts. It wraps the former in a QObject visitor, and > >> the latter in an opts visitor. > >> > >> QemuOpts is flat by design[*], so the opts visitor parses flat QemuOpts > >> from a (possibly non-flat) QAPI type. How exactly it flattens, and how > >> it handles clashes I don't remember. > >> > >> Sadly, this means that we get quirky old even for new object types. > > > > For -object in the system emulator (the tools all use the keyval > > visitor, so there it would work as expected), the only reason that we > > need to keep the quirky old code path around is the list handling in > > memory-backend.host-nodes. > > > > The main difficulty there is that the old QemuOpts based code path > > allows specifying the option twice and both of them would effectively be > > combined. Do we have any idea how to replicate this in a keyval parser > > based world? > > I can see just two clean solutions, but both involve upending a lot of > code. > > We can fuse keyval parser and visitor to get a schema-directed parser. > > We can change the abstract keyval syntax to permit repeated keys. This > means replacing QDict in in the abstract syntax tree, with fallout in > the visitor. > > Even if we find a practical solution, I don't like the combination of > "you may give the same parameter multiple times, and the last one wins" > and "for a list-valued parameter, the values of repeated parameters are > collected into a list". Each makes sense on its own. The combination > not so much. Inheriting "last one wins" from QemuOpts may have been a > mistake. > > The keyval way of doing lists (inherited from the block layer's usage of > dotted keys? I don't remember) requires the user to count, which isn't > exactly nice, either. Yes. If we didn't have to maintain compatibility (or actually as soon as we degrade non-JSON option lists to HMP-level support), I would introduce [] and {} syntax for lists and dicts, even if that means that use of these characters in strings doesn't work any more or only in a limited way. I think this would be the best compromise for usability. Anyway, this doesn't help us with the compatibility problem we're discussing here. > > If not, do we want to use the remaining time until 6.2 to deprecate > > this? The nasty part is that the only syntax that works both now and in > > the future is JSON. We can't easily accept the new keyval syntax while > > still using the QemuOpts based code. > > What exactly do you propose to deprecate? We can deprecate on two different levels. I think it's useful to do both: 1. Broad deprecation: Stable non-JSON interfaces are degraded to a HMP-like compatibility promise. Obviously, this can only be done for options that support JSON. Peter Maydell also wants to do this only after a big user (read: libvirt) has implemented and is using JSON, basically as a proof that the alternative is working. So this can certainly be done for -object. I believe libvirt also uses JSON for -device now, so this should be fine now, too. Possibly -drive (in favour of -blockdev), though I'm not completely sure if we have gotten rid of the final users of -drive. (CCing Peter Krempa for details.) This degradation of the compatibility promise doesn't tell users what exactly is going to change, which is why doing the second one, too, might be nice. 2. Narrow deprecation: We can just deprecate the non-JSON form, or certain aspects of it, of memory-backend.host-nodes. This is the specific things that stops us from switching -object to keyval. a. Deprecate the whole option. If you want to use it and need a stable interface, you have to use JSON. We'll just switch the non-JSON form on a flag day. Before it, you need to use QemuOpts + OptsVisitor syntax for the list; after it, you need to use keyval syntax. b. Deprecate only repeating the option. memory-backend is changed to first try visiting a list, and if that fails, it visits a string and goes through a string visitor locally to keep supporting the integer range syntax. c. Deprecate all list values, but keep supporting a single integer value by using an alternate between list and int. Picking one of these four options is enough to convert -object to keyval. I would suggest doing both 1. and one of the options in 2. Kevin
On Fri, Nov 05, 2021 at 14:19:34 +0100, Kevin Wolf wrote: > Am 05.11.2021 um 11:08 hat Markus Armbruster geschrieben: > > Kevin Wolf <kwolf@redhat.com> writes: > > > > > Am 04.11.2021 um 13:13 hat Markus Armbruster geschrieben: [...] > We can deprecate on two different levels. I think it's useful to do > both: > > 1. Broad deprecation: Stable non-JSON interfaces are degraded to > a HMP-like compatibility promise. Obviously, this can only be done > for options that support JSON. Peter Maydell also wants to do this > only after a big user (read: libvirt) has implemented and is > using JSON, basically as a proof that the alternative is working. > > So this can certainly be done for -object. I believe libvirt also > uses JSON for -device now, so this should be fine now, too. Possibly > -drive (in favour of -blockdev), though I'm not completely sure if we > have gotten rid of the final users of -drive. (CCing Peter Krempa for > details.) Libvirt uses JSON exclusively with -object (starting from qemu-6.0) and with -device (starting from qemu-6.2). We could also easily do -netdev if JSON support will be added for the option as internally we already create the JSON object (so that the code is identical for QMP hotplug) and just convert it back to the commandline syntax. Libvirt also uses JSON with -blockdev since -blockdev support was introduced (IIRC qemu-4.2 added all the necessary bits for us). If -blockdev is used libvirt does not use -drive at all (except for SD cards for ARM boards), but that is not tested very well.
Kevin Wolf <kwolf@redhat.com> writes: > Am 05.11.2021 um 11:08 hat Markus Armbruster geschrieben: >> Kevin Wolf <kwolf@redhat.com> writes: >> >> > Am 04.11.2021 um 13:13 hat Markus Armbruster geschrieben: >> >> The old syntax almost always has its quirks. Ideally, we'd somehow get >> >> from quirky old to boring new in an orderly manner. Sadly, we still >> >> don't have good solutions for that. To make progress, we commonly >> >> combine JSON new with quirky old. >> >> >> >> qemu-system-FOO -object works that way. object_option_parse() parses >> >> either JSON or QemuOpts. It wraps the former in a QObject visitor, and >> >> the latter in an opts visitor. >> >> >> >> QemuOpts is flat by design[*], so the opts visitor parses flat QemuOpts >> >> from a (possibly non-flat) QAPI type. How exactly it flattens, and how >> >> it handles clashes I don't remember. >> >> >> >> Sadly, this means that we get quirky old even for new object types. >> > >> > For -object in the system emulator (the tools all use the keyval >> > visitor, so there it would work as expected), the only reason that we >> > need to keep the quirky old code path around is the list handling in >> > memory-backend.host-nodes. >> > >> > The main difficulty there is that the old QemuOpts based code path >> > allows specifying the option twice and both of them would effectively be >> > combined. Do we have any idea how to replicate this in a keyval parser >> > based world? >> >> I can see just two clean solutions, but both involve upending a lot of >> code. >> >> We can fuse keyval parser and visitor to get a schema-directed parser. >> >> We can change the abstract keyval syntax to permit repeated keys. This >> means replacing QDict in in the abstract syntax tree, with fallout in >> the visitor. >> >> Even if we find a practical solution, I don't like the combination of >> "you may give the same parameter multiple times, and the last one wins" >> and "for a list-valued parameter, the values of repeated parameters are >> collected into a list". Each makes sense on its own. The combination >> not so much. Inheriting "last one wins" from QemuOpts may have been a >> mistake. >> >> The keyval way of doing lists (inherited from the block layer's usage of >> dotted keys? I don't remember) requires the user to count, which isn't >> exactly nice, either. > > Yes. If we didn't have to maintain compatibility (or actually as soon as > we degrade non-JSON option lists to HMP-level support), I would > introduce [] and {} syntax for lists and dicts, even if that means that > use of these characters in strings doesn't work any more or only in a > limited way. I think this would be the best compromise for usability. > > Anyway, this doesn't help us with the compatibility problem we're > discussing here. > >> > If not, do we want to use the remaining time until 6.2 to deprecate >> > this? The nasty part is that the only syntax that works both now and in >> > the future is JSON. We can't easily accept the new keyval syntax while >> > still using the QemuOpts based code. >> >> What exactly do you propose to deprecate? > > We can deprecate on two different levels. I think it's useful to do > both: > > 1. Broad deprecation: Stable non-JSON interfaces are degraded to > a HMP-like compatibility promise. Calling it "deprecation" might be confusing. HMP isn't deprecated, it's merely not a stable interface. That's kind of like "deprecated when you need stable", but saying "not a stable interface" is clearer. When I write "deprecate" below, I mean something like "go use something else (no conditions)". When I mean "use something else when you need stable", I write "degrade" (short for "degrade to an HMP-like compatibility promise"). > Obviously, this can only be done > for options that support JSON. We can also degrade or even deprecate sugar options in favor of the real ones. Case by case, I guess. > Peter Maydell also wants to do this > only after a big user (read: libvirt) has implemented and is > using JSON, basically as a proof that the alternative is working. > > So this can certainly be done for -object. I believe libvirt also > uses JSON for -device now, so this should be fine now, too. The non-sugar options supporting JSON are -audiodev, -blockdev, -compat, -display (partially), -machine (I think), -object. -netdev is QAPIfied, but still uses QemuOpts. Too late for 6.2, I'm afraid. > Possibly > -drive (in favour of -blockdev), though I'm not completely sure if we > have gotten rid of the final users of -drive. (CCing Peter Krempa for > details.) The problem with deprecating -drive is configuring onboard block devices. We need a stable interface for that, and it must be usable together with -blockdev. We provided such an interface (machine properties) for some onboard block devices starting with commit ebc29e1bea "pc: Support firmware configuration with -blockdev". Many more remain, I believe. > This degradation of the compatibility promise doesn't tell users what > exactly is going to change, which is why doing the second one, too, > might be nice. > > 2. Narrow deprecation: We can just deprecate the non-JSON form, or > certain aspects of it, of memory-backend.host-nodes. This is the > specific things that stops us from switching -object to keyval. > > a. Deprecate the whole option. If you want to use it and need a > stable interface, you have to use JSON. We'll just switch the > non-JSON form on a flag day. Before it, you need to use QemuOpts + > OptsVisitor syntax for the list; after it, you need to use keyval > syntax. I parse "the whole option" as "-object with dotted keys argument". Correct? > b. Deprecate only repeating the option. memory-backend is changed to > first try visiting a list, and if that fails, it visits a string > and goes through a string visitor locally to keep supporting the > integer range syntax. Possible problem: integer range syntax must not leak into the JSON form. > c. Deprecate all list values, but keep supporting a single integer > value by using an alternate between list and int. Single int should also not leak into JSON. > Picking one of these four options is enough to convert -object to > keyval. I would suggest doing both 1. and one of the options in 2. I'm grateful for your analysis.
Am 06.11.2021 um 07:34 hat Markus Armbruster geschrieben: > Kevin Wolf <kwolf@redhat.com> writes: > > > Am 05.11.2021 um 11:08 hat Markus Armbruster geschrieben: > >> Kevin Wolf <kwolf@redhat.com> writes: > >> > >> > Am 04.11.2021 um 13:13 hat Markus Armbruster geschrieben: > >> >> The old syntax almost always has its quirks. Ideally, we'd somehow get > >> >> from quirky old to boring new in an orderly manner. Sadly, we still > >> >> don't have good solutions for that. To make progress, we commonly > >> >> combine JSON new with quirky old. > >> >> > >> >> qemu-system-FOO -object works that way. object_option_parse() parses > >> >> either JSON or QemuOpts. It wraps the former in a QObject visitor, and > >> >> the latter in an opts visitor. > >> >> > >> >> QemuOpts is flat by design[*], so the opts visitor parses flat QemuOpts > >> >> from a (possibly non-flat) QAPI type. How exactly it flattens, and how > >> >> it handles clashes I don't remember. > >> >> > >> >> Sadly, this means that we get quirky old even for new object types. > >> > > >> > For -object in the system emulator (the tools all use the keyval > >> > visitor, so there it would work as expected), the only reason that we > >> > need to keep the quirky old code path around is the list handling in > >> > memory-backend.host-nodes. > >> > > >> > The main difficulty there is that the old QemuOpts based code path > >> > allows specifying the option twice and both of them would effectively be > >> > combined. Do we have any idea how to replicate this in a keyval parser > >> > based world? > >> > >> I can see just two clean solutions, but both involve upending a lot of > >> code. > >> > >> We can fuse keyval parser and visitor to get a schema-directed parser. > >> > >> We can change the abstract keyval syntax to permit repeated keys. This > >> means replacing QDict in in the abstract syntax tree, with fallout in > >> the visitor. > >> > >> Even if we find a practical solution, I don't like the combination of > >> "you may give the same parameter multiple times, and the last one wins" > >> and "for a list-valued parameter, the values of repeated parameters are > >> collected into a list". Each makes sense on its own. The combination > >> not so much. Inheriting "last one wins" from QemuOpts may have been a > >> mistake. > >> > >> The keyval way of doing lists (inherited from the block layer's usage of > >> dotted keys? I don't remember) requires the user to count, which isn't > >> exactly nice, either. > > > > Yes. If we didn't have to maintain compatibility (or actually as soon as > > we degrade non-JSON option lists to HMP-level support), I would > > introduce [] and {} syntax for lists and dicts, even if that means that > > use of these characters in strings doesn't work any more or only in a > > limited way. I think this would be the best compromise for usability. > > > > Anyway, this doesn't help us with the compatibility problem we're > > discussing here. > > > >> > If not, do we want to use the remaining time until 6.2 to deprecate > >> > this? The nasty part is that the only syntax that works both now and in > >> > the future is JSON. We can't easily accept the new keyval syntax while > >> > still using the QemuOpts based code. > >> > >> What exactly do you propose to deprecate? > > > > We can deprecate on two different levels. I think it's useful to do > > both: > > > > 1. Broad deprecation: Stable non-JSON interfaces are degraded to > > a HMP-like compatibility promise. > > Calling it "deprecation" might be confusing. HMP isn't deprecated, it's > merely not a stable interface. That's kind of like "deprecated when you > need stable", but saying "not a stable interface" is clearer. > > When I write "deprecate" below, I mean something like "go use something > else (no conditions)". When I mean "use something else when you need > stable", I write "degrade" (short for "degrade to an HMP-like > compatibility promise"). > > > Obviously, this can only be done > > for options that support JSON. > > We can also degrade or even deprecate sugar options in favor of the real > ones. Case by case, I guess. Right. And essentially, the non-JSON form would be considered a sugar option, even if the option string is the same. > > Peter Maydell also wants to do this > > only after a big user (read: libvirt) has implemented and is > > using JSON, basically as a proof that the alternative is working. > > > > So this can certainly be done for -object. I believe libvirt also > > uses JSON for -device now, so this should be fine now, too. > > The non-sugar options supporting JSON are -audiodev, -blockdev, -compat, > -display (partially), -machine (I think), -object. > > -netdev is QAPIfied, but still uses QemuOpts. Too late for 6.2, I'm > afraid. Ok. Not sure about the libvirt status for some of these, but -object and -device are the ones that I know are going to be in the way in the future, so degrading their non-JSON form would already be helpful. > > Possibly > > -drive (in favour of -blockdev), though I'm not completely sure if we > > have gotten rid of the final users of -drive. (CCing Peter Krempa for > > details.) > > The problem with deprecating -drive is configuring onboard block > devices. We need a stable interface for that, and it must be usable > together with -blockdev. > > We provided such an interface (machine properties) for some onboard > block devices starting with commit ebc29e1bea "pc: Support firmware > configuration with -blockdev". Many more remain, I believe. So maybe we need to define a form of -drive (or a new option) that would stay stable and can just take a block node and create a DriveInfo for it. Anyway, not for 6.2, so let's ignore this for now. > > This degradation of the compatibility promise doesn't tell users what > > exactly is going to change, which is why doing the second one, too, > > might be nice. > > > > 2. Narrow deprecation: We can just deprecate the non-JSON form, or > > certain aspects of it, of memory-backend.host-nodes. This is the > > specific things that stops us from switching -object to keyval. > > > > a. Deprecate the whole option. If you want to use it and need a > > stable interface, you have to use JSON. We'll just switch the > > non-JSON form on a flag day. Before it, you need to use QemuOpts + > > OptsVisitor syntax for the list; after it, you need to use keyval > > syntax. > > I parse "the whole option" as "-object with dotted keys argument". > Correct? No, degrading non-JSON -object (it's still QemuOpts, so "dotted keys" aren't even supported) is already option 1. This one is specifically "memory-backend.host-nodes on the CLI". > > b. Deprecate only repeating the option. memory-backend is changed to > > first try visiting a list, and if that fails, it visits a string > > and goes through a string visitor locally to keep supporting the > > integer range syntax. > > Possible problem: integer range syntax must not leak into the JSON form. > > > c. Deprecate all list values, but keep supporting a single integer > > value by using an alternate between list and int. > > Single int should also not leak into JSON. Honestly, I don't care about them leaking into JSON and QMP, and I don't think you should either. If we insist on a perfectly idiomatic QAPI schema as if we were writing the objects today, we won't have made any progress even in 10 years. Many QOM objects have properties that are a mess and it will show in the schema. Strings that are parsed, alternates to provide different syntax for the same thing, etc. I don't like any of this, but we're not designing new interfaces here, but describing existing ones. I do understand and agree that you want to keep the core infrastructure reasonably clean from hacks, because it affects everything and we're touching it a lot. But if an individual property in some QOM object is in the way, we should just hack around it. Spending a lot of thought on how to get it cleaned up would have a much higher cost than maintaining a small hack. Kevin
On Mon, Nov 08, 2021 at 13:05:10 +0100, Kevin Wolf wrote: > Am 06.11.2021 um 07:34 hat Markus Armbruster geschrieben: > > Kevin Wolf <kwolf@redhat.com> writes: > > > Am 05.11.2021 um 11:08 hat Markus Armbruster geschrieben: > > >> Kevin Wolf <kwolf@redhat.com> writes: > > >> > Am 04.11.2021 um 13:13 hat Markus Armbruster geschrieben: [...] > > > Peter Maydell also wants to do this > > > only after a big user (read: libvirt) has implemented and is > > > using JSON, basically as a proof that the alternative is working. > > > > > > So this can certainly be done for -object. I believe libvirt also > > > uses JSON for -device now, so this should be fine now, too. > > > > The non-sugar options supporting JSON are -audiodev, -blockdev, -compat, > > -display (partially), -machine (I think), -object. > > > > -netdev is QAPIfied, but still uses QemuOpts. Too late for 6.2, I'm > > afraid. > > Ok. Not sure about the libvirt status for some of these, but -object and Interresting. We don't do JSON for -audiodev, -compat, -display or -machine. -audiodev and -compat are recent enough so I suppose those accepted JSON always. Converting them will be trivial. For -display and -machine we'll need a witness to switch to the new syntax but I think I can convert them in libvirt if it helps qemu to have a more consistent message. > -device are the ones that I know are going to be in the way in the > future, so degrading their non-JSON form would already be helpful.
diff --git a/qapi/qom.json b/qapi/qom.json index 0222bb4506..97de79cc36 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -705,6 +705,20 @@ { 'struct': 'RemoteObjectProperties', 'data': { 'fd': 'str', 'devid': 'str' } } +## +# @VfioUserServerProperties: +# +# Properties for vfio-user-server objects. +# +# @socket: socket to be used by the libvfiouser library +# +# @device: the id of the device to be emulated at the server +# +# Since: 6.0 +## +{ 'struct': 'VfioUserServerProperties', + 'data': { 'socket': 'SocketAddress', 'device': 'str' } } + ## # @RngProperties: # @@ -830,7 +844,8 @@ 'tls-creds-psk', 'tls-creds-x509', 'tls-cipher-suites', - 'x-remote-object' + 'x-remote-object', + 'vfio-user-server' ] } ## @@ -887,7 +902,8 @@ 'tls-creds-psk': 'TlsCredsPskProperties', 'tls-creds-x509': 'TlsCredsX509Properties', 'tls-cipher-suites': 'TlsCredsProperties', - 'x-remote-object': 'RemoteObjectProperties' + 'x-remote-object': 'RemoteObjectProperties', + 'vfio-user-server': 'VfioUserServerProperties' } } ## diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c new file mode 100644 index 0000000000..c2a300f0ff --- /dev/null +++ b/hw/remote/vfio-user-obj.c @@ -0,0 +1,173 @@ +/** + * QEMU vfio-user-server server object + * + * Copyright © 2021 Oracle and/or its affiliates. + * + * This work is licensed under the terms of the GNU GPL-v2, version 2 or later. + * + * See the COPYING file in the top-level directory. + * + */ + +/** + * Usage: add options: + * -machine x-remote + * -device <PCI-device>,id=<pci-dev-id> + * -object vfio-user-server,id=<id>,type=unix,path=<socket-path>, + * device=<pci-dev-id> + * + * Note that vfio-user-server object must be used with x-remote machine only. + * This server could only support PCI devices for now. + * + * type - SocketAddress type - presently "unix" alone is supported. Required + * option + * + * path - named unix socket, it will be created by the server. It is + * a required option + * + * device - id of a device on the server, a required option. PCI devices + * alone are supported presently. + */ + +#include "qemu/osdep.h" +#include "qemu-common.h" + +#include "qom/object.h" +#include "qom/object_interfaces.h" +#include "qemu/error-report.h" +#include "trace.h" +#include "sysemu/runstate.h" +#include "hw/boards.h" +#include "hw/remote/machine.h" +#include "qapi/error.h" +#include "qapi/qapi-visit-sockets.h" + +#define TYPE_VFU_OBJECT "vfio-user-server" +OBJECT_DECLARE_TYPE(VfuObject, VfuObjectClass, VFU_OBJECT) + +struct VfuObjectClass { + ObjectClass parent_class; + + unsigned int nr_devs; + + /* Maximum number of devices the server could support */ + unsigned int max_devs; +}; + +struct VfuObject { + /* private */ + Object parent; + + SocketAddress *socket; + + char *device; +}; + +static void vfu_object_set_socket(Object *obj, Visitor *v, const char *name, + void *opaque, Error **errp) +{ + VfuObject *o = VFU_OBJECT(obj); + + g_free(o->socket); + + o->socket = NULL; + + visit_type_SocketAddress(v, name, &o->socket, errp); + + if (o->socket->type != SOCKET_ADDRESS_TYPE_UNIX) { + error_setg(&error_abort, "vfu: Unsupported socket type - %s", + o->socket->u.q_unix.path); + return; + } + + trace_vfu_prop("socket", o->socket->u.q_unix.path); +} + +static void vfu_object_set_device(Object *obj, const char *str, Error **errp) +{ + VfuObject *o = VFU_OBJECT(obj); + + g_free(o->device); + + o->device = g_strdup(str); + + trace_vfu_prop("device", str); +} + +static void vfu_object_init(Object *obj) +{ + VfuObjectClass *k = VFU_OBJECT_GET_CLASS(obj); + + if (!object_dynamic_cast(OBJECT(current_machine), TYPE_REMOTE_MACHINE)) { + error_setg(&error_abort, "vfu: %s only compatible with %s machine", + TYPE_VFU_OBJECT, TYPE_REMOTE_MACHINE); + return; + } + + if (k->nr_devs >= k->max_devs) { + error_setg(&error_abort, + "Reached maximum number of vfio-user-server devices: %u", + k->max_devs); + return; + } + + k->nr_devs++; +} + +static void vfu_object_finalize(Object *obj) +{ + VfuObjectClass *k = VFU_OBJECT_GET_CLASS(obj); + VfuObject *o = VFU_OBJECT(obj); + + k->nr_devs--; + + g_free(o->socket); + + g_free(o->device); + + if (k->nr_devs == 0) { + qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); + } +} + +static void vfu_object_class_init(ObjectClass *klass, void *data) +{ + VfuObjectClass *k = VFU_OBJECT_CLASS(klass); + + /* Limiting maximum number of devices to 1 until IOMMU support is added */ + k->max_devs = 1; + k->nr_devs = 0; + + object_class_property_add(klass, "socket", "SocketAddress", NULL, + vfu_object_set_socket, NULL, NULL); + object_class_property_set_description(klass, "socket", + "SocketAddress " + "(ex: type=unix,path=/tmp/sock). " + "Only UNIX is presently supported"); + object_class_property_add_str(klass, "device", NULL, + vfu_object_set_device); + object_class_property_set_description(klass, "device", + "device ID - only PCI devices " + "are presently supported"); +} + +static const TypeInfo vfu_object_info = { + .name = TYPE_VFU_OBJECT, + .parent = TYPE_OBJECT, + .instance_size = sizeof(VfuObject), + .instance_init = vfu_object_init, + .instance_finalize = vfu_object_finalize, + .class_size = sizeof(VfuObjectClass), + .class_init = vfu_object_class_init, + .interfaces = (InterfaceInfo[]) { + { TYPE_USER_CREATABLE }, + { } + } +}; + +static void vfu_register_types(void) +{ + type_register_static(&vfu_object_info); +} + +type_init(vfu_register_types); diff --git a/MAINTAINERS b/MAINTAINERS index 661f91a160..79ff8331dc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3421,6 +3421,7 @@ F: include/hw/remote/proxy-memory-listener.h F: hw/remote/iohub.c F: include/hw/remote/iohub.h F: subprojects/libvfio-user +F: hw/remote/vfio-user-obj.c EBPF: M: Jason Wang <jasowang@redhat.com> diff --git a/hw/remote/meson.build b/hw/remote/meson.build index dfea6b533b..534ac5df79 100644 --- a/hw/remote/meson.build +++ b/hw/remote/meson.build @@ -6,6 +6,7 @@ remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('message.c')) remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('remote-obj.c')) remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('proxy.c')) remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('iohub.c')) +remote_ss.add(when: 'CONFIG_VFIO_USER_SERVER', if_true: files('vfio-user-obj.c')) remote_ss.add(when: 'CONFIG_VFIO_USER_SERVER', if_true: vfiouser) diff --git a/hw/remote/trace-events b/hw/remote/trace-events index 0b23974f90..7da12f0d96 100644 --- a/hw/remote/trace-events +++ b/hw/remote/trace-events @@ -2,3 +2,6 @@ mpqemu_send_io_error(int cmd, int size, int nfds) "send command %d size %d, %d file descriptors to remote process" mpqemu_recv_io_error(int cmd, int size, int nfds) "failed to receive %d size %d, %d file descriptors to remote process" + +# vfio-user-obj.c +vfu_prop(const char *prop, const char *val) "vfu: setting %s as %s"