The qdev_unplug() function contains a g_assert(hotplug_ctrl) statement,
so QEMU crashes when the user tries to device_add + device_del a device
that does not have a corresponding hotplug controller. This could be
provoked for a couple of devices in the past (see commit 4c93950659487c7ad
or 84ebd3e8c7d4fe955 for example). So devices clearly need a hotplug
controller when they are suitable for device_add.
The code in qdev_device_add() already checks whether the bus has a proper
hotplug controller, but for devices that do not have a corresponding bus,
there is no appropriate check available. In that case we should check
whether the machine itself provides a suitable hotplug controller and
refuse to plug the device if none is available.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
This is the follow-up patch from my earlier try "hw/core/qdev: Do not
allow hot-plugging without hotplug controller" ... AFAICS the function
qdev_device_add() is now the right spot to do the check.
hw/core/qdev.c | 28 ++++++++++++++++++++--------
include/hw/qdev-core.h | 1 +
qdev-monitor.c | 9 +++++++++
3 files changed, 30 insertions(+), 8 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 606ab53..a953ec9 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -253,19 +253,31 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
dev->alias_required_for_version = required_for_version;
}
+HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev)
+{
+ MachineState *machine;
+ MachineClass *mc;
+ Object *m_obj = qdev_get_machine();
+
+ if (object_dynamic_cast(m_obj, TYPE_MACHINE)) {
+ machine = MACHINE(m_obj);
+ mc = MACHINE_GET_CLASS(machine);
+ if (mc->get_hotplug_handler) {
+ return mc->get_hotplug_handler(machine, dev);
+ }
+ }
+
+ return NULL;
+}
+
HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
{
- HotplugHandler *hotplug_ctrl = NULL;
+ HotplugHandler *hotplug_ctrl;
if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
hotplug_ctrl = dev->parent_bus->hotplug_handler;
- } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {
- MachineState *machine = MACHINE(qdev_get_machine());
- MachineClass *mc = MACHINE_GET_CLASS(machine);
-
- if (mc->get_hotplug_handler) {
- hotplug_ctrl = mc->get_hotplug_handler(machine, dev);
- }
+ } else {
+ hotplug_ctrl = qdev_get_machine_hotplug_handler(dev);
}
return hotplug_ctrl;
}
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 0891461..5aa536d 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -285,6 +285,7 @@ DeviceState *qdev_try_create(BusState *bus, const char *name);
void qdev_init_nofail(DeviceState *dev);
void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
int required_for_version);
+HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev);
HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev);
void qdev_unplug(DeviceState *dev, Error **errp);
void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 8fd6df9..2891dde 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -626,6 +626,15 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
return NULL;
}
+ /* In case we don't have a bus, there must be a machine hotplug handler */
+ if (qdev_hotplug && !bus && !qdev_get_machine_hotplug_handler(dev)) {
+ error_setg(errp, "Device '%s' can not be hotplugged on this machine",
+ driver);
+ object_unparent(OBJECT(dev));
+ object_unref(OBJECT(dev));
+ return NULL;
+ }
+
dev->opts = opts;
object_property_set_bool(OBJECT(dev), true, "realized", &err);
if (err != NULL) {
--
1.8.3.1
On Tue, Oct 03, 2017 at 06:46:02PM +0200, Thomas Huth wrote: > The qdev_unplug() function contains a g_assert(hotplug_ctrl) statement, > so QEMU crashes when the user tries to device_add + device_del a device > that does not have a corresponding hotplug controller. This could be > provoked for a couple of devices in the past (see commit 4c93950659487c7ad > or 84ebd3e8c7d4fe955 for example). So devices clearly need a hotplug > controller when they are suitable for device_add. > The code in qdev_device_add() already checks whether the bus has a proper > hotplug controller, but for devices that do not have a corresponding bus, > there is no appropriate check available. In that case we should check > whether the machine itself provides a suitable hotplug controller and > refuse to plug the device if none is available. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > This is the follow-up patch from my earlier try "hw/core/qdev: Do not > allow hot-plugging without hotplug controller" ... AFAICS the function > qdev_device_add() is now the right spot to do the check. > > hw/core/qdev.c | 28 ++++++++++++++++++++-------- > include/hw/qdev-core.h | 1 + > qdev-monitor.c | 9 +++++++++ > 3 files changed, 30 insertions(+), 8 deletions(-) > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 606ab53..a953ec9 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -253,19 +253,31 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, > dev->alias_required_for_version = required_for_version; > } > > +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev) > +{ > + MachineState *machine; > + MachineClass *mc; > + Object *m_obj = qdev_get_machine(); > + > + if (object_dynamic_cast(m_obj, TYPE_MACHINE)) { > + machine = MACHINE(m_obj); > + mc = MACHINE_GET_CLASS(machine); > + if (mc->get_hotplug_handler) { > + return mc->get_hotplug_handler(machine, dev); > + } > + } > + > + return NULL; > +} > + > HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev) > { > - HotplugHandler *hotplug_ctrl = NULL; > + HotplugHandler *hotplug_ctrl; > > if (dev->parent_bus && dev->parent_bus->hotplug_handler) { > hotplug_ctrl = dev->parent_bus->hotplug_handler; > - } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) { > - MachineState *machine = MACHINE(qdev_get_machine()); > - MachineClass *mc = MACHINE_GET_CLASS(machine); > - > - if (mc->get_hotplug_handler) { > - hotplug_ctrl = mc->get_hotplug_handler(machine, dev); > - } > + } else { > + hotplug_ctrl = qdev_get_machine_hotplug_handler(dev); > } > return hotplug_ctrl; > } > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index 0891461..5aa536d 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -285,6 +285,7 @@ DeviceState *qdev_try_create(BusState *bus, const char *name); > void qdev_init_nofail(DeviceState *dev); > void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, > int required_for_version); > +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev); > HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev); > void qdev_unplug(DeviceState *dev, Error **errp); > void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, > diff --git a/qdev-monitor.c b/qdev-monitor.c > index 8fd6df9..2891dde 100644 > --- a/qdev-monitor.c > +++ b/qdev-monitor.c > @@ -626,6 +626,15 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) > return NULL; > } > > + /* In case we don't have a bus, there must be a machine hotplug handler */ > + if (qdev_hotplug && !bus && !qdev_get_machine_hotplug_handler(dev)) { > + error_setg(errp, "Device '%s' can not be hotplugged on this machine", > + driver); > + object_unparent(OBJECT(dev)); Isn't it better to check qdev_get_machine_hotplug_handler() earlier (before the qdev_set_parent_bus() and qdev_set_id() lines), so object_unparent() isn't necessary? (We probably don't need to call object_unparent() here, already, because bus is NULL. But moving the check before the "if (bus) qdev_set_parent_bus()" statement would make this more obvious). I would prefer to eventually make MachineClass::get_hotplug_handler() get a typename or DeviceClass* argument instead of DeviceState*, so we don't even create the device object. But I don't think it's a requirement for this bug fix. > + object_unref(OBJECT(dev)); > + return NULL; > + } > + > dev->opts = opts; > object_property_set_bool(OBJECT(dev), true, "realized", &err); > if (err != NULL) { > -- > 1.8.3.1 > -- Eduardo
On Tue, 3 Oct 2017 15:49:46 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Tue, Oct 03, 2017 at 06:46:02PM +0200, Thomas Huth wrote: > > The qdev_unplug() function contains a g_assert(hotplug_ctrl) statement, > > so QEMU crashes when the user tries to device_add + device_del a device > > that does not have a corresponding hotplug controller. This could be > > provoked for a couple of devices in the past (see commit 4c93950659487c7ad > > or 84ebd3e8c7d4fe955 for example). So devices clearly need a hotplug > > controller when they are suitable for device_add. > > The code in qdev_device_add() already checks whether the bus has a proper > > hotplug controller, but for devices that do not have a corresponding bus, > > there is no appropriate check available. In that case we should check > > whether the machine itself provides a suitable hotplug controller and > > refuse to plug the device if none is available. > > > > Signed-off-by: Thomas Huth <thuth@redhat.com> > > --- > > This is the follow-up patch from my earlier try "hw/core/qdev: Do not > > allow hot-plugging without hotplug controller" ... AFAICS the function > > qdev_device_add() is now the right spot to do the check. > > > > hw/core/qdev.c | 28 ++++++++++++++++++++-------- > > include/hw/qdev-core.h | 1 + > > qdev-monitor.c | 9 +++++++++ > > 3 files changed, 30 insertions(+), 8 deletions(-) > > > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > > index 606ab53..a953ec9 100644 > > --- a/hw/core/qdev.c > > +++ b/hw/core/qdev.c > > @@ -253,19 +253,31 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, > > dev->alias_required_for_version = required_for_version; > > } > > > > +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev) > > +{ > > + MachineState *machine; > > + MachineClass *mc; > > + Object *m_obj = qdev_get_machine(); > > + > > + if (object_dynamic_cast(m_obj, TYPE_MACHINE)) { > > + machine = MACHINE(m_obj); > > + mc = MACHINE_GET_CLASS(machine); > > + if (mc->get_hotplug_handler) { > > + return mc->get_hotplug_handler(machine, dev); > > + } > > + } > > + > > + return NULL; > > +} > > + > > HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev) > > { > > - HotplugHandler *hotplug_ctrl = NULL; > > + HotplugHandler *hotplug_ctrl; > > > > if (dev->parent_bus && dev->parent_bus->hotplug_handler) { > > hotplug_ctrl = dev->parent_bus->hotplug_handler; > > - } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) { > > - MachineState *machine = MACHINE(qdev_get_machine()); > > - MachineClass *mc = MACHINE_GET_CLASS(machine); > > - > > - if (mc->get_hotplug_handler) { > > - hotplug_ctrl = mc->get_hotplug_handler(machine, dev); > > - } > > + } else { > > + hotplug_ctrl = qdev_get_machine_hotplug_handler(dev); > > } > > return hotplug_ctrl; > > } > > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > > index 0891461..5aa536d 100644 > > --- a/include/hw/qdev-core.h > > +++ b/include/hw/qdev-core.h > > @@ -285,6 +285,7 @@ DeviceState *qdev_try_create(BusState *bus, const char *name); > > void qdev_init_nofail(DeviceState *dev); > > void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, > > int required_for_version); > > +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev); > > HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev); > > void qdev_unplug(DeviceState *dev, Error **errp); > > void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, > > diff --git a/qdev-monitor.c b/qdev-monitor.c > > index 8fd6df9..2891dde 100644 > > --- a/qdev-monitor.c > > +++ b/qdev-monitor.c > > @@ -626,6 +626,15 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) > > return NULL; > > } > > > > + /* In case we don't have a bus, there must be a machine hotplug handler */ > > + if (qdev_hotplug && !bus && !qdev_get_machine_hotplug_handler(dev)) { > > + error_setg(errp, "Device '%s' can not be hotplugged on this machine", > > + driver); > > + object_unparent(OBJECT(dev)); > > Isn't it better to check qdev_get_machine_hotplug_handler() > earlier (before the qdev_set_parent_bus() and qdev_set_id() > lines), so object_unparent() isn't necessary? > > (We probably don't need to call object_unparent() here, already, > because bus is NULL. But moving the check before the "if (bus) > qdev_set_parent_bus()" statement would make this more obvious). it might be bus or bus-less device, so making check before qdev_set_parent_bus() should be simpler. > I would prefer to eventually make > MachineClass::get_hotplug_handler() get a typename or > DeviceClass* argument instead of DeviceState*, so we don't even > create the device object. But I don't think it's a requirement > for this bug fix. choice of hotplug handler might theoretically depend on plugged device instance (over-engineered? as far as I recall none does it so far) > > > > + object_unref(OBJECT(dev)); > > + return NULL; > > + } wrt error exit path, I'd rework error path in qdev_device_add() in separate patch first to look like it is in device_set_realized() and then just jump to appropriate label from here. > > + > > dev->opts = opts; > > object_property_set_bool(OBJECT(dev), true, "realized", &err); > > if (err != NULL) { > > -- > > 1.8.3.1 > > >
On 05.10.2017 10:36, Igor Mammedov wrote: > On Tue, 3 Oct 2017 15:49:46 -0300 > Eduardo Habkost <ehabkost@redhat.com> wrote: > >> On Tue, Oct 03, 2017 at 06:46:02PM +0200, Thomas Huth wrote: >>> The qdev_unplug() function contains a g_assert(hotplug_ctrl) statement, >>> so QEMU crashes when the user tries to device_add + device_del a device >>> that does not have a corresponding hotplug controller. This could be >>> provoked for a couple of devices in the past (see commit 4c93950659487c7ad >>> or 84ebd3e8c7d4fe955 for example). So devices clearly need a hotplug >>> controller when they are suitable for device_add. >>> The code in qdev_device_add() already checks whether the bus has a proper >>> hotplug controller, but for devices that do not have a corresponding bus, >>> there is no appropriate check available. In that case we should check >>> whether the machine itself provides a suitable hotplug controller and >>> refuse to plug the device if none is available. >>> >>> Signed-off-by: Thomas Huth <thuth@redhat.com> >>> --- >>> This is the follow-up patch from my earlier try "hw/core/qdev: Do not >>> allow hot-plugging without hotplug controller" ... AFAICS the function >>> qdev_device_add() is now the right spot to do the check. >>> >>> hw/core/qdev.c | 28 ++++++++++++++++++++-------- >>> include/hw/qdev-core.h | 1 + >>> qdev-monitor.c | 9 +++++++++ >>> 3 files changed, 30 insertions(+), 8 deletions(-) >>> >>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >>> index 606ab53..a953ec9 100644 >>> --- a/hw/core/qdev.c >>> +++ b/hw/core/qdev.c >>> @@ -253,19 +253,31 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, >>> dev->alias_required_for_version = required_for_version; >>> } >>> >>> +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev) >>> +{ >>> + MachineState *machine; >>> + MachineClass *mc; >>> + Object *m_obj = qdev_get_machine(); >>> + >>> + if (object_dynamic_cast(m_obj, TYPE_MACHINE)) { >>> + machine = MACHINE(m_obj); >>> + mc = MACHINE_GET_CLASS(machine); >>> + if (mc->get_hotplug_handler) { >>> + return mc->get_hotplug_handler(machine, dev); >>> + } >>> + } >>> + >>> + return NULL; >>> +} >>> + >>> HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev) >>> { >>> - HotplugHandler *hotplug_ctrl = NULL; >>> + HotplugHandler *hotplug_ctrl; >>> >>> if (dev->parent_bus && dev->parent_bus->hotplug_handler) { >>> hotplug_ctrl = dev->parent_bus->hotplug_handler; >>> - } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) { >>> - MachineState *machine = MACHINE(qdev_get_machine()); >>> - MachineClass *mc = MACHINE_GET_CLASS(machine); >>> - >>> - if (mc->get_hotplug_handler) { >>> - hotplug_ctrl = mc->get_hotplug_handler(machine, dev); >>> - } >>> + } else { >>> + hotplug_ctrl = qdev_get_machine_hotplug_handler(dev); >>> } >>> return hotplug_ctrl; >>> } >>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h >>> index 0891461..5aa536d 100644 >>> --- a/include/hw/qdev-core.h >>> +++ b/include/hw/qdev-core.h >>> @@ -285,6 +285,7 @@ DeviceState *qdev_try_create(BusState *bus, const char *name); >>> void qdev_init_nofail(DeviceState *dev); >>> void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, >>> int required_for_version); >>> +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev); >>> HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev); >>> void qdev_unplug(DeviceState *dev, Error **errp); >>> void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, >>> diff --git a/qdev-monitor.c b/qdev-monitor.c >>> index 8fd6df9..2891dde 100644 >>> --- a/qdev-monitor.c >>> +++ b/qdev-monitor.c >>> @@ -626,6 +626,15 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) >>> return NULL; >>> } >>> >>> + /* In case we don't have a bus, there must be a machine hotplug handler */ >>> + if (qdev_hotplug && !bus && !qdev_get_machine_hotplug_handler(dev)) { >>> + error_setg(errp, "Device '%s' can not be hotplugged on this machine", >>> + driver); >>> + object_unparent(OBJECT(dev)); >> >> Isn't it better to check qdev_get_machine_hotplug_handler() >> earlier (before the qdev_set_parent_bus() and qdev_set_id() >> lines), so object_unparent() isn't necessary? >> >> (We probably don't need to call object_unparent() here, already, >> because bus is NULL. But moving the check before the "if (bus) >> qdev_set_parent_bus()" statement would make this more obvious). > it might be bus or bus-less device, so making check before > qdev_set_parent_bus() should be simpler. The check for devices that have a bus is already done earlier in this function ("if (qdev_hotplug && bus && !qbus_is_hotpluggable(bus))") ... but yes, I'll move the new check for bus-less devices a little bit earlier so that the unparent is not necessary anymore. >> I would prefer to eventually make >> MachineClass::get_hotplug_handler() get a typename or >> DeviceClass* argument instead of DeviceState*, so we don't even >> create the device object. But I don't think it's a requirement >> for this bug fix. > choice of hotplug handler might theoretically depend on plugged > device instance (over-engineered? as far as I recall none does it so far) Yes, currently we might be able to do it with the typename only ... but that seems to be a rather big rework right now, and we might indeed need a real device instance later again, so I'd prefer to rather not do that rework right now. >>> + object_unref(OBJECT(dev)); >>> + return NULL; >>> + } > wrt error exit path, I'd rework error path in qdev_device_add() in separate patch first > to look like it is in device_set_realized() and then just jump to appropriate label > from here. Ok, I can have a try whether that looks better. Thomas
On Thu, 5 Oct 2017 11:00:29 +0200 Thomas Huth <thuth@redhat.com> wrote: > On 05.10.2017 10:36, Igor Mammedov wrote: > > On Tue, 3 Oct 2017 15:49:46 -0300 > > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > >> On Tue, Oct 03, 2017 at 06:46:02PM +0200, Thomas Huth wrote: > >>> The qdev_unplug() function contains a g_assert(hotplug_ctrl) statement, > >>> so QEMU crashes when the user tries to device_add + device_del a device > >>> that does not have a corresponding hotplug controller. This could be > >>> provoked for a couple of devices in the past (see commit 4c93950659487c7ad > >>> or 84ebd3e8c7d4fe955 for example). So devices clearly need a hotplug > >>> controller when they are suitable for device_add. > >>> The code in qdev_device_add() already checks whether the bus has a proper > >>> hotplug controller, but for devices that do not have a corresponding bus, > >>> there is no appropriate check available. In that case we should check > >>> whether the machine itself provides a suitable hotplug controller and > >>> refuse to plug the device if none is available. > >>> > >>> Signed-off-by: Thomas Huth <thuth@redhat.com> > >>> --- > >>> This is the follow-up patch from my earlier try "hw/core/qdev: Do not > >>> allow hot-plugging without hotplug controller" ... AFAICS the function > >>> qdev_device_add() is now the right spot to do the check. > >>> > >>> hw/core/qdev.c | 28 ++++++++++++++++++++-------- > >>> include/hw/qdev-core.h | 1 + > >>> qdev-monitor.c | 9 +++++++++ > >>> 3 files changed, 30 insertions(+), 8 deletions(-) > >>> > >>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c > >>> index 606ab53..a953ec9 100644 > >>> --- a/hw/core/qdev.c > >>> +++ b/hw/core/qdev.c > >>> @@ -253,19 +253,31 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, > >>> dev->alias_required_for_version = required_for_version; > >>> } > >>> > >>> +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev) > >>> +{ > >>> + MachineState *machine; > >>> + MachineClass *mc; > >>> + Object *m_obj = qdev_get_machine(); > >>> + > >>> + if (object_dynamic_cast(m_obj, TYPE_MACHINE)) { > >>> + machine = MACHINE(m_obj); > >>> + mc = MACHINE_GET_CLASS(machine); > >>> + if (mc->get_hotplug_handler) { > >>> + return mc->get_hotplug_handler(machine, dev); > >>> + } > >>> + } > >>> + > >>> + return NULL; > >>> +} > >>> + > >>> HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev) > >>> { > >>> - HotplugHandler *hotplug_ctrl = NULL; > >>> + HotplugHandler *hotplug_ctrl; > >>> > >>> if (dev->parent_bus && dev->parent_bus->hotplug_handler) { > >>> hotplug_ctrl = dev->parent_bus->hotplug_handler; > >>> - } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) { > >>> - MachineState *machine = MACHINE(qdev_get_machine()); > >>> - MachineClass *mc = MACHINE_GET_CLASS(machine); > >>> - > >>> - if (mc->get_hotplug_handler) { > >>> - hotplug_ctrl = mc->get_hotplug_handler(machine, dev); > >>> - } > >>> + } else { > >>> + hotplug_ctrl = qdev_get_machine_hotplug_handler(dev); > >>> } > >>> return hotplug_ctrl; > >>> } > >>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > >>> index 0891461..5aa536d 100644 > >>> --- a/include/hw/qdev-core.h > >>> +++ b/include/hw/qdev-core.h > >>> @@ -285,6 +285,7 @@ DeviceState *qdev_try_create(BusState *bus, const char *name); > >>> void qdev_init_nofail(DeviceState *dev); > >>> void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, > >>> int required_for_version); > >>> +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev); > >>> HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev); > >>> void qdev_unplug(DeviceState *dev, Error **errp); > >>> void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, > >>> diff --git a/qdev-monitor.c b/qdev-monitor.c > >>> index 8fd6df9..2891dde 100644 > >>> --- a/qdev-monitor.c > >>> +++ b/qdev-monitor.c > >>> @@ -626,6 +626,15 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) > >>> return NULL; > >>> } > >>> > >>> + /* In case we don't have a bus, there must be a machine hotplug handler */ > >>> + if (qdev_hotplug && !bus && !qdev_get_machine_hotplug_handler(dev)) { > >>> + error_setg(errp, "Device '%s' can not be hotplugged on this machine", > >>> + driver); > >>> + object_unparent(OBJECT(dev)); > >> > >> Isn't it better to check qdev_get_machine_hotplug_handler() > >> earlier (before the qdev_set_parent_bus() and qdev_set_id() > >> lines), so object_unparent() isn't necessary? > >> > >> (We probably don't need to call object_unparent() here, already, > >> because bus is NULL. But moving the check before the "if (bus) > >> qdev_set_parent_bus()" statement would make this more obvious). > > it might be bus or bus-less device, so making check before > > qdev_set_parent_bus() should be simpler. > > The check for devices that have a bus is already done earlier in this > function ("if (qdev_hotplug && bus && !qbus_is_hotpluggable(bus))") ... > but yes, I'll move the new check for bus-less devices a little bit > earlier so that the unparent is not necessary anymore. > > >> I would prefer to eventually make > >> MachineClass::get_hotplug_handler() get a typename or > >> DeviceClass* argument instead of DeviceState*, so we don't even > >> create the device object. But I don't think it's a requirement > >> for this bug fix. > > choice of hotplug handler might theoretically depend on plugged > > device instance (over-engineered? as far as I recall none does it so far) > > Yes, currently we might be able to do it with the typename only ... but > that seems to be a rather big rework right now, and we might indeed need > a real device instance later again, so I'd prefer to rather not do that > rework right now. it was just remark why it uses 'dev' and a call for an action. > > >>> + object_unref(OBJECT(dev)); > >>> + return NULL; > >>> + } > > wrt error exit path, I'd rework error path in qdev_device_add() in separate patch first > > to look like it is in device_set_realized() and then just jump to appropriate label > > from here. > > Ok, I can have a try whether that looks better. the function already has couple error exit point that duplicate cleanup steps, so it probably would read better with cleanup > > Thomas >
On Thu, 5 Oct 2017 11:12:41 +0200 Igor Mammedov <imammedo@redhat.com> wrote: > On Thu, 5 Oct 2017 11:00:29 +0200 > Thomas Huth <thuth@redhat.com> wrote: [...] > > Yes, currently we might be able to do it with the typename only ... but > > that seems to be a rather big rework right now, and we might indeed need > > a real device instance later again, so I'd prefer to rather not do that > > rework right now. > it was just remark why it uses 'dev' and a call for an action. forgot the most important word s/a call/NOT a call/ [...]
On Tue, 3 Oct 2017 18:46:02 +0200 Thomas Huth <thuth@redhat.com> wrote: > The qdev_unplug() function contains a g_assert(hotplug_ctrl) statement, > so QEMU crashes when the user tries to device_add + device_del a device > that does not have a corresponding hotplug controller. This could be > provoked for a couple of devices in the past (see commit 4c93950659487c7ad > or 84ebd3e8c7d4fe955 for example). So devices clearly need a hotplug > controller when they are suitable for device_add. > The code in qdev_device_add() already checks whether the bus has a proper > hotplug controller, but for devices that do not have a corresponding bus, > there is no appropriate check available. In that case we should check > whether the machine itself provides a suitable hotplug controller and > refuse to plug the device if none is available. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > This is the follow-up patch from my earlier try "hw/core/qdev: Do not > allow hot-plugging without hotplug controller" ... AFAICS the function > qdev_device_add() is now the right spot to do the check. > > hw/core/qdev.c | 28 ++++++++++++++++++++-------- > include/hw/qdev-core.h | 1 + > qdev-monitor.c | 9 +++++++++ > 3 files changed, 30 insertions(+), 8 deletions(-) > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 606ab53..a953ec9 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -253,19 +253,31 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, > dev->alias_required_for_version = required_for_version; > } > > +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev) > +{ > + MachineState *machine; > + MachineClass *mc; > + Object *m_obj = qdev_get_machine(); > + > + if (object_dynamic_cast(m_obj, TYPE_MACHINE)) { > + machine = MACHINE(m_obj); > + mc = MACHINE_GET_CLASS(machine); > + if (mc->get_hotplug_handler) { > + return mc->get_hotplug_handler(machine, dev); > + } > + } > + > + return NULL; > +} > + > HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev) > { > - HotplugHandler *hotplug_ctrl = NULL; > + HotplugHandler *hotplug_ctrl; > > if (dev->parent_bus && dev->parent_bus->hotplug_handler) { > hotplug_ctrl = dev->parent_bus->hotplug_handler; > - } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) { > - MachineState *machine = MACHINE(qdev_get_machine()); > - MachineClass *mc = MACHINE_GET_CLASS(machine); > - > - if (mc->get_hotplug_handler) { > - hotplug_ctrl = mc->get_hotplug_handler(machine, dev); > - } > + } else { > + hotplug_ctrl = qdev_get_machine_hotplug_handler(dev); > } > return hotplug_ctrl; > } > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index 0891461..5aa536d 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -285,6 +285,7 @@ DeviceState *qdev_try_create(BusState *bus, const char *name); > void qdev_init_nofail(DeviceState *dev); > void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, > int required_for_version); > +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev); > HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev); > void qdev_unplug(DeviceState *dev, Error **errp); > void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, > diff --git a/qdev-monitor.c b/qdev-monitor.c > index 8fd6df9..2891dde 100644 > --- a/qdev-monitor.c > +++ b/qdev-monitor.c > @@ -626,6 +626,15 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) > return NULL; > } > > + /* In case we don't have a bus, there must be a machine hotplug handler */ > + if (qdev_hotplug && !bus && !qdev_get_machine_hotplug_handler(dev)) { current machine hotplug handler serves both cold and hot-plug so in reality it's just 'plug' handler. Is there a point -device/device_add devices on board that doesn't have 'hotplug' handler that would wire device up properly? Do we have such devices? > + error_setg(errp, "Device '%s' can not be hotplugged on this machine", > + driver); > + object_unparent(OBJECT(dev)); > + object_unref(OBJECT(dev)); > + return NULL; > + } > + > dev->opts = opts; > object_property_set_bool(OBJECT(dev), true, "realized", &err); > if (err != NULL) {
On 04.10.2017 13:36, Igor Mammedov wrote: > On Tue, 3 Oct 2017 18:46:02 +0200 > Thomas Huth <thuth@redhat.com> wrote: > >> The qdev_unplug() function contains a g_assert(hotplug_ctrl) statement, >> so QEMU crashes when the user tries to device_add + device_del a device >> that does not have a corresponding hotplug controller. This could be >> provoked for a couple of devices in the past (see commit 4c93950659487c7ad >> or 84ebd3e8c7d4fe955 for example). So devices clearly need a hotplug >> controller when they are suitable for device_add. >> The code in qdev_device_add() already checks whether the bus has a proper >> hotplug controller, but for devices that do not have a corresponding bus, >> there is no appropriate check available. In that case we should check >> whether the machine itself provides a suitable hotplug controller and >> refuse to plug the device if none is available. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> This is the follow-up patch from my earlier try "hw/core/qdev: Do not >> allow hot-plugging without hotplug controller" ... AFAICS the function >> qdev_device_add() is now the right spot to do the check. >> >> hw/core/qdev.c | 28 ++++++++++++++++++++-------- >> include/hw/qdev-core.h | 1 + >> qdev-monitor.c | 9 +++++++++ >> 3 files changed, 30 insertions(+), 8 deletions(-) >> >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >> index 606ab53..a953ec9 100644 >> --- a/hw/core/qdev.c >> +++ b/hw/core/qdev.c >> @@ -253,19 +253,31 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, >> dev->alias_required_for_version = required_for_version; >> } >> >> +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev) >> +{ >> + MachineState *machine; >> + MachineClass *mc; >> + Object *m_obj = qdev_get_machine(); >> + >> + if (object_dynamic_cast(m_obj, TYPE_MACHINE)) { >> + machine = MACHINE(m_obj); >> + mc = MACHINE_GET_CLASS(machine); >> + if (mc->get_hotplug_handler) { >> + return mc->get_hotplug_handler(machine, dev); >> + } >> + } >> + >> + return NULL; >> +} >> + >> HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev) >> { >> - HotplugHandler *hotplug_ctrl = NULL; >> + HotplugHandler *hotplug_ctrl; >> >> if (dev->parent_bus && dev->parent_bus->hotplug_handler) { >> hotplug_ctrl = dev->parent_bus->hotplug_handler; >> - } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) { >> - MachineState *machine = MACHINE(qdev_get_machine()); >> - MachineClass *mc = MACHINE_GET_CLASS(machine); >> - >> - if (mc->get_hotplug_handler) { >> - hotplug_ctrl = mc->get_hotplug_handler(machine, dev); >> - } >> + } else { >> + hotplug_ctrl = qdev_get_machine_hotplug_handler(dev); >> } >> return hotplug_ctrl; >> } >> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h >> index 0891461..5aa536d 100644 >> --- a/include/hw/qdev-core.h >> +++ b/include/hw/qdev-core.h >> @@ -285,6 +285,7 @@ DeviceState *qdev_try_create(BusState *bus, const char *name); >> void qdev_init_nofail(DeviceState *dev); >> void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, >> int required_for_version); >> +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev); >> HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev); >> void qdev_unplug(DeviceState *dev, Error **errp); >> void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, >> diff --git a/qdev-monitor.c b/qdev-monitor.c >> index 8fd6df9..2891dde 100644 >> --- a/qdev-monitor.c >> +++ b/qdev-monitor.c >> @@ -626,6 +626,15 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) >> return NULL; >> } >> >> + /* In case we don't have a bus, there must be a machine hotplug handler */ >> + if (qdev_hotplug && !bus && !qdev_get_machine_hotplug_handler(dev)) { > current machine hotplug handler serves both cold and hot-plug so in reality it's > just 'plug' handler. > > Is there a point -device/device_add devices on board that doesn't have 'hotplug' > handler that would wire device up properly? Sorry, I did not get that question ... do you mean whether there's any code that uses qdev_device_add() to add a device without hotplug controller? I don't think so. It's currently only used by qmp_device_add() for the QMP/HMP command, in vl.c for -device and in the USB code for xen-usb host device. So this function currently really only makes sense for devices that have a hotplug controller. Thomas
On Wed, Oct 04, 2017 at 09:29:54PM +0200, Thomas Huth wrote: > On 04.10.2017 13:36, Igor Mammedov wrote: > > On Tue, 3 Oct 2017 18:46:02 +0200 > > Thomas Huth <thuth@redhat.com> wrote: > > > >> The qdev_unplug() function contains a g_assert(hotplug_ctrl) statement, > >> so QEMU crashes when the user tries to device_add + device_del a device > >> that does not have a corresponding hotplug controller. This could be > >> provoked for a couple of devices in the past (see commit 4c93950659487c7ad > >> or 84ebd3e8c7d4fe955 for example). So devices clearly need a hotplug > >> controller when they are suitable for device_add. > >> The code in qdev_device_add() already checks whether the bus has a proper > >> hotplug controller, but for devices that do not have a corresponding bus, > >> there is no appropriate check available. In that case we should check > >> whether the machine itself provides a suitable hotplug controller and > >> refuse to plug the device if none is available. > >> > >> Signed-off-by: Thomas Huth <thuth@redhat.com> > >> --- > >> This is the follow-up patch from my earlier try "hw/core/qdev: Do not > >> allow hot-plugging without hotplug controller" ... AFAICS the function > >> qdev_device_add() is now the right spot to do the check. > >> > >> hw/core/qdev.c | 28 ++++++++++++++++++++-------- > >> include/hw/qdev-core.h | 1 + > >> qdev-monitor.c | 9 +++++++++ > >> 3 files changed, 30 insertions(+), 8 deletions(-) > >> > >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c > >> index 606ab53..a953ec9 100644 > >> --- a/hw/core/qdev.c > >> +++ b/hw/core/qdev.c > >> @@ -253,19 +253,31 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, > >> dev->alias_required_for_version = required_for_version; > >> } > >> > >> +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev) > >> +{ > >> + MachineState *machine; > >> + MachineClass *mc; > >> + Object *m_obj = qdev_get_machine(); > >> + > >> + if (object_dynamic_cast(m_obj, TYPE_MACHINE)) { > >> + machine = MACHINE(m_obj); > >> + mc = MACHINE_GET_CLASS(machine); > >> + if (mc->get_hotplug_handler) { > >> + return mc->get_hotplug_handler(machine, dev); > >> + } > >> + } > >> + > >> + return NULL; > >> +} > >> + > >> HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev) > >> { > >> - HotplugHandler *hotplug_ctrl = NULL; > >> + HotplugHandler *hotplug_ctrl; > >> > >> if (dev->parent_bus && dev->parent_bus->hotplug_handler) { > >> hotplug_ctrl = dev->parent_bus->hotplug_handler; > >> - } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) { > >> - MachineState *machine = MACHINE(qdev_get_machine()); > >> - MachineClass *mc = MACHINE_GET_CLASS(machine); > >> - > >> - if (mc->get_hotplug_handler) { > >> - hotplug_ctrl = mc->get_hotplug_handler(machine, dev); > >> - } > >> + } else { > >> + hotplug_ctrl = qdev_get_machine_hotplug_handler(dev); > >> } > >> return hotplug_ctrl; > >> } > >> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > >> index 0891461..5aa536d 100644 > >> --- a/include/hw/qdev-core.h > >> +++ b/include/hw/qdev-core.h > >> @@ -285,6 +285,7 @@ DeviceState *qdev_try_create(BusState *bus, const char *name); > >> void qdev_init_nofail(DeviceState *dev); > >> void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, > >> int required_for_version); > >> +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev); > >> HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev); > >> void qdev_unplug(DeviceState *dev, Error **errp); > >> void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, > >> diff --git a/qdev-monitor.c b/qdev-monitor.c > >> index 8fd6df9..2891dde 100644 > >> --- a/qdev-monitor.c > >> +++ b/qdev-monitor.c > >> @@ -626,6 +626,15 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) > >> return NULL; > >> } > >> > >> + /* In case we don't have a bus, there must be a machine hotplug handler */ > >> + if (qdev_hotplug && !bus && !qdev_get_machine_hotplug_handler(dev)) { > > current machine hotplug handler serves both cold and hot-plug so in reality it's > > just 'plug' handler. > > > > Is there a point -device/device_add devices on board that doesn't have 'hotplug' > > handler that would wire device up properly? > > Sorry, I did not get that question ... do you mean whether there's any > code that uses qdev_device_add() to add a device without hotplug > controller? I don't think so. It's currently only used by > qmp_device_add() for the QMP/HMP command, in vl.c for -device and in the > USB code for xen-usb host device. So this function currently really only > makes sense for devices that have a hotplug controller. I assume you are talking only about hotpluggable devices. With -device, qdev_device_add() is also used for devices that don't have a hotplug controller, but this is supposed to be true only for non-hotpluggable devices. -- Eduardo
On 04.10.2017 23:21, Eduardo Habkost wrote: > On Wed, Oct 04, 2017 at 09:29:54PM +0200, Thomas Huth wrote: >> On 04.10.2017 13:36, Igor Mammedov wrote: >>> On Tue, 3 Oct 2017 18:46:02 +0200 >>> Thomas Huth <thuth@redhat.com> wrote: >>> >>>> The qdev_unplug() function contains a g_assert(hotplug_ctrl) statement, >>>> so QEMU crashes when the user tries to device_add + device_del a device >>>> that does not have a corresponding hotplug controller. This could be >>>> provoked for a couple of devices in the past (see commit 4c93950659487c7ad >>>> or 84ebd3e8c7d4fe955 for example). So devices clearly need a hotplug >>>> controller when they are suitable for device_add. >>>> The code in qdev_device_add() already checks whether the bus has a proper >>>> hotplug controller, but for devices that do not have a corresponding bus, >>>> there is no appropriate check available. In that case we should check >>>> whether the machine itself provides a suitable hotplug controller and >>>> refuse to plug the device if none is available. >>>> >>>> Signed-off-by: Thomas Huth <thuth@redhat.com> >>>> --- >>>> This is the follow-up patch from my earlier try "hw/core/qdev: Do not >>>> allow hot-plugging without hotplug controller" ... AFAICS the function >>>> qdev_device_add() is now the right spot to do the check. >>>> >>>> hw/core/qdev.c | 28 ++++++++++++++++++++-------- >>>> include/hw/qdev-core.h | 1 + >>>> qdev-monitor.c | 9 +++++++++ >>>> 3 files changed, 30 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >>>> index 606ab53..a953ec9 100644 >>>> --- a/hw/core/qdev.c >>>> +++ b/hw/core/qdev.c >>>> @@ -253,19 +253,31 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, >>>> dev->alias_required_for_version = required_for_version; >>>> } >>>> >>>> +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev) >>>> +{ >>>> + MachineState *machine; >>>> + MachineClass *mc; >>>> + Object *m_obj = qdev_get_machine(); >>>> + >>>> + if (object_dynamic_cast(m_obj, TYPE_MACHINE)) { >>>> + machine = MACHINE(m_obj); >>>> + mc = MACHINE_GET_CLASS(machine); >>>> + if (mc->get_hotplug_handler) { >>>> + return mc->get_hotplug_handler(machine, dev); >>>> + } >>>> + } >>>> + >>>> + return NULL; >>>> +} >>>> + >>>> HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev) >>>> { >>>> - HotplugHandler *hotplug_ctrl = NULL; >>>> + HotplugHandler *hotplug_ctrl; >>>> >>>> if (dev->parent_bus && dev->parent_bus->hotplug_handler) { >>>> hotplug_ctrl = dev->parent_bus->hotplug_handler; >>>> - } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) { >>>> - MachineState *machine = MACHINE(qdev_get_machine()); >>>> - MachineClass *mc = MACHINE_GET_CLASS(machine); >>>> - >>>> - if (mc->get_hotplug_handler) { >>>> - hotplug_ctrl = mc->get_hotplug_handler(machine, dev); >>>> - } >>>> + } else { >>>> + hotplug_ctrl = qdev_get_machine_hotplug_handler(dev); >>>> } >>>> return hotplug_ctrl; >>>> } >>>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h >>>> index 0891461..5aa536d 100644 >>>> --- a/include/hw/qdev-core.h >>>> +++ b/include/hw/qdev-core.h >>>> @@ -285,6 +285,7 @@ DeviceState *qdev_try_create(BusState *bus, const char *name); >>>> void qdev_init_nofail(DeviceState *dev); >>>> void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, >>>> int required_for_version); >>>> +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev); >>>> HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev); >>>> void qdev_unplug(DeviceState *dev, Error **errp); >>>> void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, >>>> diff --git a/qdev-monitor.c b/qdev-monitor.c >>>> index 8fd6df9..2891dde 100644 >>>> --- a/qdev-monitor.c >>>> +++ b/qdev-monitor.c >>>> @@ -626,6 +626,15 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) >>>> return NULL; >>>> } >>>> >>>> + /* In case we don't have a bus, there must be a machine hotplug handler */ >>>> + if (qdev_hotplug && !bus && !qdev_get_machine_hotplug_handler(dev)) { >>> current machine hotplug handler serves both cold and hot-plug so in reality it's >>> just 'plug' handler. >>> >>> Is there a point -device/device_add devices on board that doesn't have 'hotplug' >>> handler that would wire device up properly? >> >> Sorry, I did not get that question ... do you mean whether there's any >> code that uses qdev_device_add() to add a device without hotplug >> controller? I don't think so. It's currently only used by >> qmp_device_add() for the QMP/HMP command, in vl.c for -device and in the >> USB code for xen-usb host device. So this function currently really only >> makes sense for devices that have a hotplug controller. > > I assume you are talking only about hotpluggable devices. With > -device, qdev_device_add() is also used for devices that don't > have a hotplug controller, but this is supposed to be true only > for non-hotpluggable devices. Right, the cold-pluggable devices should be fine because of the "qdev_hotplug" check, forgot to mention that, sorry. Thomas
© 2016 - 2024 Red Hat, Inc.