| Message ID | c3a68434b32d8f5379a9fa197e89748f54423632.1606853298.git.jag.raman@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Initial support for multi-process Qemu | expand |
Hi On Wed, Dec 2, 2020 at 12:25 AM Jagannathan Raman <jag.raman@oracle.com> wrote: > Associate the file descriptor for a PCIDevice in remote process with > DeviceState object. > > Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com> > Signed-off-by: John G Johnson <john.g.johnson@oracle.com> > Signed-off-by: Jagannathan Raman <jag.raman@oracle.com> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > include/hw/remote/remote-obj.h | 42 +++++++++++ > hw/remote/message.c | 1 + > hw/remote/remote-obj.c | 154 > +++++++++++++++++++++++++++++++++++++++++ > MAINTAINERS | 2 + > hw/remote/meson.build | 1 + > 5 files changed, 200 insertions(+) > create mode 100644 include/hw/remote/remote-obj.h > create mode 100644 hw/remote/remote-obj.c > > diff --git a/include/hw/remote/remote-obj.h > b/include/hw/remote/remote-obj.h > new file mode 100644 > index 0000000..0e95813 > --- /dev/null > +++ b/include/hw/remote/remote-obj.h > @@ -0,0 +1,42 @@ > +/* > + * Copyright © 2020 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. > + * > + */ > + > +#ifndef REMOTE_OBJECT_H > +#define REMOTE_OBJECT_H > + > +#include "io/channel.h" > +#include "qemu/notify.h" > + > +#define TYPE_REMOTE_OBJECT "x-remote-object" > +#define REMOTE_OBJECT(obj) \ > + OBJECT_CHECK(RemoteObject, (obj), TYPE_REMOTE_OBJECT) > +#define REMOTE_OBJECT_GET_CLASS(obj) \ > + OBJECT_GET_CLASS(RemoteObjectClass, (obj), TYPE_REMOTE_OBJECT) > +#define REMOTE_OBJECT_CLASS(klass) \ > + OBJECT_CLASS_CHECK(RemoteObjectClass, (klass), TYPE_REMOTE_OBJECT) > + > +typedef struct { > + ObjectClass parent_class; > + > + unsigned int nr_devs; > + unsigned int max_devs; > +} RemoteObjectClass; > + > +typedef struct { > + /* private */ > + Object parent; > + > + Notifier machine_done; > + > + int fd; > + char *devid; > + QIOChannel *ioc; > +} RemoteObject; > + > +#endif > There is no need for a header if the header isn't actually shared with various .c units. In this series, you can just declare those structs in remote-obj.c diff --git a/hw/remote/message.c b/hw/remote/message.c > index 5d87bf4..1f2edc7 100644 > --- a/hw/remote/message.c > +++ b/hw/remote/message.c > @@ -56,6 +56,7 @@ void coroutine_fn mpqemu_remote_msg_loop_co(void *data) > } > } > qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); > + g_free(com); > Should be squashed with the previous patch > return; > } > diff --git a/hw/remote/remote-obj.c b/hw/remote/remote-obj.c > new file mode 100644 > index 0000000..3b4c0d4 > --- /dev/null > +++ b/hw/remote/remote-obj.c > @@ -0,0 +1,154 @@ > +/* > + * Copyright © 2020 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. > + * > + */ > + > +#include "qemu/osdep.h" > +#include "qemu-common.h" > + > +#include "hw/remote/remote-obj.h" > +#include "qemu/error-report.h" > +#include "qom/object_interfaces.h" > +#include "hw/qdev-core.h" > +#include "io/channel.h" > +#include "hw/qdev-core.h" > +#include "hw/remote/machine.h" > +#include "io/channel-util.h" > +#include "qapi/error.h" > +#include "sysemu/sysemu.h" > +#include "hw/pci/pci.h" > + > +static void remote_object_set_fd(Object *obj, const char *str, Error > **errp) > +{ > + RemoteObject *o = REMOTE_OBJECT(obj); > + > + o->fd = atoi(str); > qemu_strtoi() has better error handling semantics. You may also want to check it's a valid socket fd with fd_is_socket(). Alternatively, you could use qemu_open() which allows to open from QMP fdset ("/dev/fdset/..."). This should be more flexible. +} > + > +static void remote_object_set_devid(Object *obj, const char *str, Error > **errp) > +{ > + RemoteObject *o = REMOTE_OBJECT(obj); > + > + g_free(o->devid); > + > + o->devid = g_strdup(str); > +} > + > +static void property_release_remote_object(Object *obj, const char *name, > + void *opaque) > +{ > + Object *remote_object = OBJECT(opaque); > + > + object_unref(remote_object); > +} > Hmm, an object property, discussed below. + > +static void remote_object_machine_done(Notifier *notifier, void *data) > +{ > + RemoteObject *o = container_of(notifier, RemoteObject, machine_done); > + DeviceState *dev = NULL; > + QIOChannel *ioc = NULL; > + Coroutine *co = NULL; > + RemoteCommDev *comdev = NULL; > + Error *err = NULL; > + > + dev = qdev_find_recursive(sysbus_get_default(), o->devid); > + if (!dev || !object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > + error_report("%s is not a PCI device", o->devid); > + return; > + } > + > + ioc = qio_channel_new_fd(o->fd, &err); > + if (!ioc) { > + error_report_err(err); > + return; > + } > + qio_channel_set_blocking(ioc, false, NULL); > + > + object_property_add(OBJECT(dev), "remote-object", "object", NULL, > NULL, > + property_release_remote_object, (void > *)OBJECT(o)); > In general, we are trying to avoid runtime/dynamic properties and slowly replacing them with class properties. Furthermore, this is not the way QOM handles object properties. You should use object_class_property_add_link(). + /* co-routine should free this. */ > + comdev = g_new0(RemoteCommDev, 1); > + *comdev = (RemoteCommDev) { > + .ioc = ioc, > + .dev = PCI_DEVICE(dev), > + }; > + > + co = qemu_coroutine_create(mpqemu_remote_msg_loop_co, comdev); > + qemu_coroutine_enter(co); > +} > + > +static void remote_object_init(Object *obj) > +{ > + RemoteObjectClass *k = REMOTE_OBJECT_GET_CLASS(obj); > + RemoteObject *o = REMOTE_OBJECT(obj); > + > + if (k->nr_devs >= k->max_devs) { > + error_report("Reached maximum number of devices: %u", > k->max_devs); > + return; > + } > + > + o->ioc = NULL; > + o->fd = -1; > + o->devid = NULL; > + > + k->nr_devs++; > + > + object_property_add_str(obj, "fd", NULL, remote_object_set_fd); > + object_property_set_description(obj, "fd", > + "file descriptor for the object"); > + object_property_add_str(obj, "devid", NULL, remote_object_set_devid); > + object_property_set_description(obj, "devid", > + "id of device to associate"); > Please use class properties + > + o->machine_done.notify = remote_object_machine_done; > + qemu_add_machine_init_done_notifier(&o->machine_done); > +} > + > +static void remote_object_finalize(Object *obj) > +{ > + RemoteObjectClass *k = REMOTE_OBJECT_GET_CLASS(obj); > + RemoteObject *o = REMOTE_OBJECT(obj); > + > + if (o->ioc) { > + qio_channel_shutdown(o->ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL); > + qio_channel_close(o->ioc, NULL); > + } > + > + object_unref(OBJECT(o->ioc)); > + > + k->nr_devs--; > + g_free(o->devid); > +} > + > +static void remote_object_class_init(ObjectClass *klass, void *data) > +{ > + RemoteObjectClass *k = REMOTE_OBJECT_CLASS(klass); > + > + k->max_devs = 1; > Could you explain that limitation, in a code comment and/or commit message? + k->nr_devs = 0; > +} > + > +static const TypeInfo remote_object_info = { > + .name = TYPE_REMOTE_OBJECT, > + .parent = TYPE_OBJECT, > + .instance_size = sizeof(RemoteObject), > + .instance_init = remote_object_init, > + .instance_finalize = remote_object_finalize, > + .class_size = sizeof(RemoteObjectClass), > + .class_init = remote_object_class_init, > + .interfaces = (InterfaceInfo[]) { > + { TYPE_USER_CREATABLE }, > + { } > + } > +}; > + > +static void register_types(void) > +{ > + type_register_static(&remote_object_info); > +} > + > +type_init(register_types); > diff --git a/MAINTAINERS b/MAINTAINERS > index b64e4b8..aedfc27 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -3144,6 +3144,8 @@ F: include/hw/remote/machine.h > F: hw/remote/mpqemu-link.c > F: include/hw/remote/mpqemu-link.h > F: hw/remote/message.c > +F: include/hw/remote/remote-obj.h > +F: hw/remote/remote-obj.c > > Build and test automation > ------------------------- > diff --git a/hw/remote/meson.build b/hw/remote/meson.build > index 9f5c57f..71d0a56 100644 > --- a/hw/remote/meson.build > +++ b/hw/remote/meson.build > @@ -3,5 +3,6 @@ remote_ss = ss.source_set() > remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('machine.c')) > remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: > files('mpqemu-link.c')) > remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('message.c')) > +remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('remote-obj.c')) > > softmmu_ss.add_all(when: 'CONFIG_MULTIPROCESS', if_true: remote_ss) > -- > 1.8.3.1 > >
On Mon, Dec 7, 2020 at 6:03 PM Marc-André Lureau <marcandre.lureau@gmail.com> wrote: > Hi > > On Wed, Dec 2, 2020 at 12:25 AM Jagannathan Raman <jag.raman@oracle.com> > wrote: > >> Associate the file descriptor for a PCIDevice in remote process with >> DeviceState object. >> >> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com> >> Signed-off-by: John G Johnson <john.g.johnson@oracle.com> >> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com> >> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> >> --- >> include/hw/remote/remote-obj.h | 42 +++++++++++ >> hw/remote/message.c | 1 + >> hw/remote/remote-obj.c | 154 >> +++++++++++++++++++++++++++++++++++++++++ >> MAINTAINERS | 2 + >> hw/remote/meson.build | 1 + >> 5 files changed, 200 insertions(+) >> create mode 100644 include/hw/remote/remote-obj.h >> create mode 100644 hw/remote/remote-obj.c >> >> diff --git a/include/hw/remote/remote-obj.h >> b/include/hw/remote/remote-obj.h >> new file mode 100644 >> index 0000000..0e95813 >> --- /dev/null >> +++ b/include/hw/remote/remote-obj.h >> @@ -0,0 +1,42 @@ >> +/* >> + * Copyright © 2020 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. >> + * >> + */ >> + >> +#ifndef REMOTE_OBJECT_H >> +#define REMOTE_OBJECT_H >> + >> +#include "io/channel.h" >> +#include "qemu/notify.h" >> + >> +#define TYPE_REMOTE_OBJECT "x-remote-object" >> +#define REMOTE_OBJECT(obj) \ >> + OBJECT_CHECK(RemoteObject, (obj), TYPE_REMOTE_OBJECT) >> +#define REMOTE_OBJECT_GET_CLASS(obj) \ >> + OBJECT_GET_CLASS(RemoteObjectClass, (obj), TYPE_REMOTE_OBJECT) >> +#define REMOTE_OBJECT_CLASS(klass) \ >> + OBJECT_CLASS_CHECK(RemoteObjectClass, (klass), TYPE_REMOTE_OBJECT) >> + >> +typedef struct { >> + ObjectClass parent_class; >> + >> + unsigned int nr_devs; >> + unsigned int max_devs; >> +} RemoteObjectClass; >> + >> +typedef struct { >> + /* private */ >> + Object parent; >> + >> + Notifier machine_done; >> + >> + int fd; >> + char *devid; >> + QIOChannel *ioc; >> +} RemoteObject; >> + >> +#endif >> > > There is no need for a header if the header isn't actually shared with > various .c units. In this series, you can just declare those structs in > remote-obj.c > > diff --git a/hw/remote/message.c b/hw/remote/message.c >> index 5d87bf4..1f2edc7 100644 >> --- a/hw/remote/message.c >> +++ b/hw/remote/message.c >> @@ -56,6 +56,7 @@ void coroutine_fn mpqemu_remote_msg_loop_co(void *data) >> } >> } >> qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); >> + g_free(com); >> > > > Should be squashed with the previous patch > > >> return; >> } >> diff --git a/hw/remote/remote-obj.c b/hw/remote/remote-obj.c >> new file mode 100644 >> index 0000000..3b4c0d4 >> --- /dev/null >> +++ b/hw/remote/remote-obj.c >> @@ -0,0 +1,154 @@ >> +/* >> + * Copyright © 2020 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. >> + * >> + */ >> + >> +#include "qemu/osdep.h" >> +#include "qemu-common.h" >> + >> +#include "hw/remote/remote-obj.h" >> +#include "qemu/error-report.h" >> +#include "qom/object_interfaces.h" >> +#include "hw/qdev-core.h" >> +#include "io/channel.h" >> +#include "hw/qdev-core.h" >> +#include "hw/remote/machine.h" >> +#include "io/channel-util.h" >> +#include "qapi/error.h" >> +#include "sysemu/sysemu.h" >> +#include "hw/pci/pci.h" >> + >> +static void remote_object_set_fd(Object *obj, const char *str, Error >> **errp) >> +{ >> + RemoteObject *o = REMOTE_OBJECT(obj); >> + >> + o->fd = atoi(str); >> > > qemu_strtoi() has better error handling semantics. You may also want to > check it's a valid socket fd with fd_is_socket(). > > Alternatively, you could use qemu_open() which allows to open from QMP > fdset ("/dev/fdset/..."). This should be more flexible. > A better alternative is qemu_parse_fd(). In some later patch, you use monitor_fd_param(monitor_cur(), ...) for x-pci-proxy-dev "fd" property. That might be the right API, for consistency, use it here too?
diff --git a/include/hw/remote/remote-obj.h b/include/hw/remote/remote-obj.h new file mode 100644 index 0000000..0e95813 --- /dev/null +++ b/include/hw/remote/remote-obj.h @@ -0,0 +1,42 @@ +/* + * Copyright © 2020 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. + * + */ + +#ifndef REMOTE_OBJECT_H +#define REMOTE_OBJECT_H + +#include "io/channel.h" +#include "qemu/notify.h" + +#define TYPE_REMOTE_OBJECT "x-remote-object" +#define REMOTE_OBJECT(obj) \ + OBJECT_CHECK(RemoteObject, (obj), TYPE_REMOTE_OBJECT) +#define REMOTE_OBJECT_GET_CLASS(obj) \ + OBJECT_GET_CLASS(RemoteObjectClass, (obj), TYPE_REMOTE_OBJECT) +#define REMOTE_OBJECT_CLASS(klass) \ + OBJECT_CLASS_CHECK(RemoteObjectClass, (klass), TYPE_REMOTE_OBJECT) + +typedef struct { + ObjectClass parent_class; + + unsigned int nr_devs; + unsigned int max_devs; +} RemoteObjectClass; + +typedef struct { + /* private */ + Object parent; + + Notifier machine_done; + + int fd; + char *devid; + QIOChannel *ioc; +} RemoteObject; + +#endif diff --git a/hw/remote/message.c b/hw/remote/message.c index 5d87bf4..1f2edc7 100644 --- a/hw/remote/message.c +++ b/hw/remote/message.c @@ -56,6 +56,7 @@ void coroutine_fn mpqemu_remote_msg_loop_co(void *data) } } qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); + g_free(com); return; } diff --git a/hw/remote/remote-obj.c b/hw/remote/remote-obj.c new file mode 100644 index 0000000..3b4c0d4 --- /dev/null +++ b/hw/remote/remote-obj.c @@ -0,0 +1,154 @@ +/* + * Copyright © 2020 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. + * + */ + +#include "qemu/osdep.h" +#include "qemu-common.h" + +#include "hw/remote/remote-obj.h" +#include "qemu/error-report.h" +#include "qom/object_interfaces.h" +#include "hw/qdev-core.h" +#include "io/channel.h" +#include "hw/qdev-core.h" +#include "hw/remote/machine.h" +#include "io/channel-util.h" +#include "qapi/error.h" +#include "sysemu/sysemu.h" +#include "hw/pci/pci.h" + +static void remote_object_set_fd(Object *obj, const char *str, Error **errp) +{ + RemoteObject *o = REMOTE_OBJECT(obj); + + o->fd = atoi(str); +} + +static void remote_object_set_devid(Object *obj, const char *str, Error **errp) +{ + RemoteObject *o = REMOTE_OBJECT(obj); + + g_free(o->devid); + + o->devid = g_strdup(str); +} + +static void property_release_remote_object(Object *obj, const char *name, + void *opaque) +{ + Object *remote_object = OBJECT(opaque); + + object_unref(remote_object); +} + +static void remote_object_machine_done(Notifier *notifier, void *data) +{ + RemoteObject *o = container_of(notifier, RemoteObject, machine_done); + DeviceState *dev = NULL; + QIOChannel *ioc = NULL; + Coroutine *co = NULL; + RemoteCommDev *comdev = NULL; + Error *err = NULL; + + dev = qdev_find_recursive(sysbus_get_default(), o->devid); + if (!dev || !object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { + error_report("%s is not a PCI device", o->devid); + return; + } + + ioc = qio_channel_new_fd(o->fd, &err); + if (!ioc) { + error_report_err(err); + return; + } + qio_channel_set_blocking(ioc, false, NULL); + + object_property_add(OBJECT(dev), "remote-object", "object", NULL, NULL, + property_release_remote_object, (void *)OBJECT(o)); + /* co-routine should free this. */ + comdev = g_new0(RemoteCommDev, 1); + *comdev = (RemoteCommDev) { + .ioc = ioc, + .dev = PCI_DEVICE(dev), + }; + + co = qemu_coroutine_create(mpqemu_remote_msg_loop_co, comdev); + qemu_coroutine_enter(co); +} + +static void remote_object_init(Object *obj) +{ + RemoteObjectClass *k = REMOTE_OBJECT_GET_CLASS(obj); + RemoteObject *o = REMOTE_OBJECT(obj); + + if (k->nr_devs >= k->max_devs) { + error_report("Reached maximum number of devices: %u", k->max_devs); + return; + } + + o->ioc = NULL; + o->fd = -1; + o->devid = NULL; + + k->nr_devs++; + + object_property_add_str(obj, "fd", NULL, remote_object_set_fd); + object_property_set_description(obj, "fd", + "file descriptor for the object"); + object_property_add_str(obj, "devid", NULL, remote_object_set_devid); + object_property_set_description(obj, "devid", + "id of device to associate"); + + o->machine_done.notify = remote_object_machine_done; + qemu_add_machine_init_done_notifier(&o->machine_done); +} + +static void remote_object_finalize(Object *obj) +{ + RemoteObjectClass *k = REMOTE_OBJECT_GET_CLASS(obj); + RemoteObject *o = REMOTE_OBJECT(obj); + + if (o->ioc) { + qio_channel_shutdown(o->ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL); + qio_channel_close(o->ioc, NULL); + } + + object_unref(OBJECT(o->ioc)); + + k->nr_devs--; + g_free(o->devid); +} + +static void remote_object_class_init(ObjectClass *klass, void *data) +{ + RemoteObjectClass *k = REMOTE_OBJECT_CLASS(klass); + + k->max_devs = 1; + k->nr_devs = 0; +} + +static const TypeInfo remote_object_info = { + .name = TYPE_REMOTE_OBJECT, + .parent = TYPE_OBJECT, + .instance_size = sizeof(RemoteObject), + .instance_init = remote_object_init, + .instance_finalize = remote_object_finalize, + .class_size = sizeof(RemoteObjectClass), + .class_init = remote_object_class_init, + .interfaces = (InterfaceInfo[]) { + { TYPE_USER_CREATABLE }, + { } + } +}; + +static void register_types(void) +{ + type_register_static(&remote_object_info); +} + +type_init(register_types); diff --git a/MAINTAINERS b/MAINTAINERS index b64e4b8..aedfc27 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3144,6 +3144,8 @@ F: include/hw/remote/machine.h F: hw/remote/mpqemu-link.c F: include/hw/remote/mpqemu-link.h F: hw/remote/message.c +F: include/hw/remote/remote-obj.h +F: hw/remote/remote-obj.c Build and test automation ------------------------- diff --git a/hw/remote/meson.build b/hw/remote/meson.build index 9f5c57f..71d0a56 100644 --- a/hw/remote/meson.build +++ b/hw/remote/meson.build @@ -3,5 +3,6 @@ remote_ss = ss.source_set() remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('machine.c')) remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('mpqemu-link.c')) remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('message.c')) +remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('remote-obj.c')) softmmu_ss.add_all(when: 'CONFIG_MULTIPROCESS', if_true: remote_ss)