On Thu, Feb 17, 2022 at 02:48:50AM -0500, Jagannathan Raman wrote:
> Add blocker to prevent hot-unplug of devices
>
> Signed-off-by: Elena Ufimtseva <elena.ufimts...@oracle.com>
> Signed-off-by: John G Johnson <john.g.john...@oracle.com>
> Signed-off-by: Jagannathan Raman <jag.ra...@oracle.com>
> ---
> include/hw/qdev-core.h | 35 +++++++++++++++++++++++++++++++++++
> softmmu/qdev-monitor.c | 26 ++++++++++++++++++++++++++
> 2 files changed, 61 insertions(+)
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 92c3d65208..4b1d77f44a 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,40 @@ void qdev_simple_device_unplug_cb(HotplugHandler
> *hotplug_dev,
> void qdev_machine_creation_done(void);
> bool qdev_machine_modified(void);
>
> +/**
> + * Device Unplug blocker: prevents a device from being unplugged. It could
^^^^^^^^^^^^^^^^^^^^^This looks strange. gtkdoc will probably treat it as the doc comment for qdev_add_unplug_blocker(), which is actually defined below. I suggest not trying to define a new section in the documentation and instead just focussing on doc comments for qdev_add_unplug_block() and other functions. The gtkdoc way of defining sections is covered here but it's almost never used in QEMU: https://developer-old.gnome.org/gtk-doc-manual/stable/documenting_sections.html.en > + * be used to indicate that another object depends on the device. > + * > + * 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); Does the caller have to call qdev_del_unplug_blocker() later? An assert(!dev->unplug_blockers) would be nice when DeviceState is destroyed. That way leaks will be caught. > + > +/** > + * 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/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c > index 01f3834db5..69d9cf3f25 100644 > --- a/softmmu/qdev-monitor.c > +++ b/softmmu/qdev-monitor.c > @@ -945,10 +945,36 @@ void qmp_device_del(const char *id, Error **errp) > return; > } > > + if (qdev_unplug_blocked(dev, errp)) { > + return; > + } > + > qdev_unplug(dev, errp); > } > } > > +void qdev_add_unplug_blocker(DeviceState *dev, Error *reason) These functions belong in hw/core/qdev.c because they are part of the DeviceState API, not qdev monitor commands? > +{ > + 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; > +} > + > void hmp_device_add(Monitor *mon, const QDict *qdict) > { > Error *err = NULL; > -- > 2.20.1 >
signature.asc
Description: PGP signature
