| Message ID | 01e8950f954c291acd74c9caf1d2016e898cd80c.1651586203.git.jag.raman@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | vfio-user server in QEMU | expand |
Jagannathan Raman <jag.raman@oracle.com> writes: > Add blocker to prevent hot-unplug of devices > > TYPE_VFIO_USER_SERVER, which is introduced shortly, attaches itself to a > PCIDevice on which it depends. If the attached PCIDevice gets removed > while the server in use, it could cause it crash. To prevent this, > TYPE_VFIO_USER_SERVER adds an unplug blocker for the PCIDevice. Appreciate the explanation :) > > 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/qdev-core.h | 29 +++++++++++++++++++++++++++++ > hw/core/qdev.c | 24 ++++++++++++++++++++++++ > softmmu/qdev-monitor.c | 4 ++++ > 3 files changed, 57 insertions(+) > > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index 92c3d65208..1b9fa25e5c 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -193,6 +193,7 @@ struct DeviceState { > int instance_id_alias; > int alias_required_for_version; > ResettableState reset; > + GSList *unplug_blockers; > }; > > struct DeviceListener { > @@ -419,6 +420,34 @@ void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, > void qdev_machine_creation_done(void); > bool qdev_machine_modified(void); > > +/* Use /** here like we do in the other function comments nearby. In case you're curious: it's GTK-Doc format. It's intended for generating documentation from doc comments. Which we don't do, and perhaps never will. But let's be locally consistent. > + * qdev_add_unplug_blocker: Adds an unplug blocker to a device Recommend imperative mood for function comments: "Add an unplug blocker to a device". More of the same below. [...]
> On May 4, 2022, at 7:13 AM, Markus Armbruster <armbru@redhat.com> wrote: > > Jagannathan Raman <jag.raman@oracle.com> writes: > >> Add blocker to prevent hot-unplug of devices >> >> TYPE_VFIO_USER_SERVER, which is introduced shortly, attaches itself to a >> PCIDevice on which it depends. If the attached PCIDevice gets removed >> while the server in use, it could cause it crash. To prevent this, >> TYPE_VFIO_USER_SERVER adds an unplug blocker for the PCIDevice. > > Appreciate the explanation :) > >> >> 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/qdev-core.h | 29 +++++++++++++++++++++++++++++ >> hw/core/qdev.c | 24 ++++++++++++++++++++++++ >> softmmu/qdev-monitor.c | 4 ++++ >> 3 files changed, 57 insertions(+) >> >> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h >> index 92c3d65208..1b9fa25e5c 100644 >> --- a/include/hw/qdev-core.h >> +++ b/include/hw/qdev-core.h >> @@ -193,6 +193,7 @@ struct DeviceState { >> int instance_id_alias; >> int alias_required_for_version; >> ResettableState reset; >> + GSList *unplug_blockers; >> }; >> >> struct DeviceListener { >> @@ -419,6 +420,34 @@ void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, >> void qdev_machine_creation_done(void); >> bool qdev_machine_modified(void); >> >> +/* > > Use /** here like we do in the other function comments nearby. > > In case you're curious: it's GTK-Doc format. It's intended for > generating documentation from doc comments. Which we don't do, and > perhaps never will. But let's be locally consistent. Sure, will do. > >> + * qdev_add_unplug_blocker: Adds an unplug blocker to a device > > Recommend imperative mood for function comments: "Add an unplug > blocker to a device". OK, got it. Will update the comments. :) -- Jag > > More of the same below. > > [...] >
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 92c3d65208..1b9fa25e5c 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -193,6 +193,7 @@ struct DeviceState { int instance_id_alias; int alias_required_for_version; ResettableState reset; + GSList *unplug_blockers; }; struct DeviceListener { @@ -419,6 +420,34 @@ void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, void qdev_machine_creation_done(void); bool qdev_machine_modified(void); +/* + * qdev_add_unplug_blocker: Adds an unplug blocker to a device + * + * @dev: Device to be blocked from unplug + * @reason: Reason for blocking + */ +void qdev_add_unplug_blocker(DeviceState *dev, Error *reason); + +/* + * qdev_del_unplug_blocker: Removes an unplug blocker from a device + * + * @dev: Device to be unblocked + * @reason: Pointer to the Error used with qdev_add_unplug_blocker. + * Used as a handle to lookup the blocker for deletion. + */ +void qdev_del_unplug_blocker(DeviceState *dev, Error *reason); + +/* + * qdev_unplug_blocked: Confirms if a device is blocked from unplug + * + * @dev: Device to be tested + * @reason: Returns one of the reasons why the device is blocked, + * if any + * + * Returns: true if device is blocked from unplug, false otherwise + */ +bool qdev_unplug_blocked(DeviceState *dev, Error **errp); + /** * GpioPolarity: Polarity of a GPIO line * diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 84f3019440..0806d8fcaa 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -468,6 +468,28 @@ char *qdev_get_dev_path(DeviceState *dev) return NULL; } +void qdev_add_unplug_blocker(DeviceState *dev, Error *reason) +{ + dev->unplug_blockers = g_slist_prepend(dev->unplug_blockers, reason); +} + +void qdev_del_unplug_blocker(DeviceState *dev, Error *reason) +{ + dev->unplug_blockers = g_slist_remove(dev->unplug_blockers, reason); +} + +bool qdev_unplug_blocked(DeviceState *dev, Error **errp) +{ + ERRP_GUARD(); + + if (dev->unplug_blockers) { + error_propagate(errp, error_copy(dev->unplug_blockers->data)); + return true; + } + + return false; +} + static bool device_get_realized(Object *obj, Error **errp) { DeviceState *dev = DEVICE(obj); @@ -704,6 +726,8 @@ static void device_finalize(Object *obj) DeviceState *dev = DEVICE(obj); + g_assert(!dev->unplug_blockers); + QLIST_FOREACH_SAFE(ngl, &dev->gpios, node, next) { QLIST_REMOVE(ngl, node); qemu_free_irqs(ngl->in, ngl->num_in); diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index 12fe60c467..9cfd59d17c 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -898,6 +898,10 @@ void qdev_unplug(DeviceState *dev, Error **errp) HotplugHandlerClass *hdc; Error *local_err = NULL; + if (qdev_unplug_blocked(dev, errp)) { + return; + } + if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) { error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name); return;