Akihiko's v4 patch was doing too many things at once to

my taste, so I split it to follow dumbly each steps.

https://lore.kernel.org/qemu-devel/20250104-bus-v4-1-244cf1c6e2aa@daynix.com/

Akihiko Odaki (6):

hw/qdev: Pass bus argument to qdev_hotplug_allowed()

hw/qdev: Factor qdev_hotunplug_allowed() out

hw/qdev: Introduce qdev_hotplug_unplug_allowed_common()

hw/qdev: Check DevClass::hotpluggable in hotplug_unplug_allowed_common

hw/qdev: Check qbus_is_hotpluggable in hotplug_unplug_allowed_common

hw/qdev: Check machine_hotplug_handler in

hotplug_unplug_allowed_common

include/hw/qdev-core.h | 3 ++-

hw/core/qdev-hotplug.c | 42 +++++++++++++++++++++++++++++++++++++++++-

system/qdev-monitor.c | 37 +++++--------------------------------

3 files changed, 48 insertions(+), 34 deletions(-)

--

2.47.1

From: Akihiko Odaki <akihiko.odaki@daynix.com>

In preparation of checking the parent bus is hot(un)pluggable

in a few commits, pass a 'bus' argument to qdev_hotplug_allowed().

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

---

include/hw/qdev-core.h | 2 +-

hw/core/qdev-hotplug.c | 2 +-

system/qdev-monitor.c | 2 +-

3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h

index XXXXXXX..XXXXXXX 100644

--- a/include/hw/qdev-core.h

+++ b/include/hw/qdev-core.h

@@ -XXX,XX +XXX,XX @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,

int required_for_version);

HotplugHandler *qdev_get_bus_hotplug_handler(DeviceState *dev);

HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev);

-bool qdev_hotplug_allowed(DeviceState *dev, Error **errp);

+bool qdev_hotplug_allowed(DeviceState *dev, BusState *bus, Error **errp);

/**

* qdev_get_hotplug_handler() - Get handler responsible for device wiring

diff --git a/hw/core/qdev-hotplug.c b/hw/core/qdev-hotplug.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/core/qdev-hotplug.c

+++ b/hw/core/qdev-hotplug.c

@@ -XXX,XX +XXX,XX @@ HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev)

return NULL;

}

-bool qdev_hotplug_allowed(DeviceState *dev, Error **errp)

+bool qdev_hotplug_allowed(DeviceState *dev, BusState *bus, Error **errp)

{

MachineState *machine;

MachineClass *mc;

diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c

index XXXXXXX..XXXXXXX 100644

--- a/system/qdev-monitor.c

+++ b/system/qdev-monitor.c

@@ -XXX,XX +XXX,XX @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,

/* Check whether the hotplug is allowed by the machine */

if (phase_check(PHASE_MACHINE_READY)) {

- if (!qdev_hotplug_allowed(dev, errp)) {

+ if (!qdev_hotplug_allowed(dev, bus, errp)) {

goto err_del_dev;

}

--

2.47.1

From: Akihiko Odaki <akihiko.odaki@daynix.com>

Factor qdev_hotunplug_allowed() out of qdev_unplug().

Start checking the device is not blocked.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

---

include/hw/qdev-core.h | 1 +

hw/core/qdev-hotplug.c | 5 +++++

system/qdev-monitor.c | 2 +-

3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h

index XXXXXXX..XXXXXXX 100644

--- a/include/hw/qdev-core.h

+++ b/include/hw/qdev-core.h

@@ -XXX,XX +XXX,XX @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,

HotplugHandler *qdev_get_bus_hotplug_handler(DeviceState *dev);

HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev);

bool qdev_hotplug_allowed(DeviceState *dev, BusState *bus, Error **errp);

+bool qdev_hotunplug_allowed(DeviceState *dev, Error **errp);

/**

* qdev_get_hotplug_handler() - Get handler responsible for device wiring

diff --git a/hw/core/qdev-hotplug.c b/hw/core/qdev-hotplug.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/core/qdev-hotplug.c

+++ b/hw/core/qdev-hotplug.c

@@ -XXX,XX +XXX,XX @@ bool qdev_hotplug_allowed(DeviceState *dev, BusState *bus, Error **errp)

return true;

}

+bool qdev_hotunplug_allowed(DeviceState *dev, Error **errp)

+{

+ return !qdev_unplug_blocked(dev, errp);

+}

+

HotplugHandler *qdev_get_bus_hotplug_handler(DeviceState *dev)

{

if (dev->parent_bus) {

diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c

index XXXXXXX..XXXXXXX 100644

--- a/system/qdev-monitor.c

+++ b/system/qdev-monitor.c

@@ -XXX,XX +XXX,XX @@ void qdev_unplug(DeviceState *dev, Error **errp)

HotplugHandlerClass *hdc;

Error *local_err = NULL;

- if (qdev_unplug_blocked(dev, errp)) {

+ if (!qdev_hotunplug_allowed(dev, errp)) {

return;

}

--

2.47.1

From: Akihiko Odaki <akihiko.odaki@daynix.com>

Introduce qdev_hotplug_unplug_allowed_common() to hold

common code between checking hot-plug/unplug is allowed.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

---

hw/core/qdev-hotplug.c | 13 ++++++++++++-

1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev-hotplug.c b/hw/core/qdev-hotplug.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/core/qdev-hotplug.c

+++ b/hw/core/qdev-hotplug.c

@@ -XXX,XX +XXX,XX @@ HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev)

return NULL;

}

+static bool qdev_hotplug_unplug_allowed_common(DeviceState *dev, BusState *bus,

+ Error **errp)

+{

+ return true;

+}

+

bool qdev_hotplug_allowed(DeviceState *dev, BusState *bus, Error **errp)

{

MachineState *machine;

MachineClass *mc;

Object *m_obj = qdev_get_machine();

+ if (!qdev_hotplug_unplug_allowed_common(dev, bus, errp)) {

+ return false;

+ }

+

if (object_dynamic_cast(m_obj, TYPE_MACHINE)) {

machine = MACHINE(m_obj);

mc = MACHINE_GET_CLASS(machine);

@@ -XXX,XX +XXX,XX @@ bool qdev_hotplug_allowed(DeviceState *dev, BusState *bus, Error **errp)

bool qdev_hotunplug_allowed(DeviceState *dev, Error **errp)

{

- return !qdev_unplug_blocked(dev, errp);

+ return !qdev_unplug_blocked(dev, errp) &&

+ qdev_hotplug_unplug_allowed_common(dev, dev->parent_bus, errp);

}

HotplugHandler *qdev_get_bus_hotplug_handler(DeviceState *dev)

--

2.47.1

From: Akihiko Odaki <akihiko.odaki@daynix.com>

Check the same code once in the common helper.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

---

hw/core/qdev-hotplug.c | 8 ++++++++

system/qdev-monitor.c | 11 -----------

2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/hw/core/qdev-hotplug.c b/hw/core/qdev-hotplug.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/core/qdev-hotplug.c

+++ b/hw/core/qdev-hotplug.c

@@ -XXX,XX +XXX,XX @@ static bool qdev_hotplug_unplug_allowed_common(DeviceState *dev, BusState *bus,

return false;

}

+ if (bus) {

+ if (!qbus_is_hotpluggable(bus)) {

+ error_setg(errp, "Bus '%s' does not support hotplugging",

+ bus->name);

+ return false;

+ }

+ }

+

return true;

}

diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c

index XXXXXXX..XXXXXXX 100644

--- a/system/qdev-monitor.c

+++ b/system/qdev-monitor.c

@@ -XXX,XX +XXX,XX @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,

return NULL;

}

- if (phase_check(PHASE_MACHINE_READY) && bus && !qbus_is_hotpluggable(bus)) {

- error_setg(errp, "Bus '%s' does not support hotplugging", bus->name);

- return NULL;

- }

-

if (migration_is_running()) {

error_setg(errp, "device_add not allowed while migrating");

return NULL;

@@ -XXX,XX +XXX,XX @@ void qdev_unplug(DeviceState *dev, Error **errp)

return;

}

- if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) {

- error_setg(errp, "Bus '%s' does not support hotplugging",

- dev->parent_bus->name);

- return;

- }

-

if (migration_is_running() && !dev->allow_unplug_during_migration) {

error_setg(errp, "device_del not allowed while migrating");

return;

--

2.47.1

From: Akihiko Odaki <akihiko.odaki@daynix.com>

Commit 03fcbd9dc508 ("qdev: Check for the availability of a hotplug

controller before adding a device") says:

> 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.

> 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, here is no appropriate check available yet. 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.

However, it forgot to add the corresponding check to qdev_unplug().

Check the machine hotplug handler once in the common

qdev_hotplug_unplug_allowed_common() helper so both hotplug

and hot-unplug path are covered.

Fixes: 7716b8ca74 ("qdev: HotplugHandler: Add support for unplugging BUS-less devices")

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

---

hw/core/qdev-hotplug.c | 7 +++++++

system/qdev-monitor.c | 14 +++-----------

2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/hw/core/qdev-hotplug.c b/hw/core/qdev-hotplug.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/core/qdev-hotplug.c

+++ b/hw/core/qdev-hotplug.c

@@ -XXX,XX +XXX,XX @@ static bool qdev_hotplug_unplug_allowed_common(DeviceState *dev, BusState *bus,

bus->name);

return false;

}

+ } else {

+ if (!qdev_get_machine_hotplug_handler(dev)) {

+ /* No bus, no machine hotplug handler --> device is not hotpluggable */

+ error_setg(errp, "Device '%s' can not be hotplugged on this machine",

+ object_get_typename(OBJECT(dev)));

+ return false;

+ }

}

return true;

diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c

index XXXXXXX..XXXXXXX 100644

--- a/system/qdev-monitor.c

+++ b/system/qdev-monitor.c

@@ -XXX,XX +XXX,XX @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,

dev = qdev_new(driver);

/* Check whether the hotplug is allowed by the machine */

- if (phase_check(PHASE_MACHINE_READY)) {

- if (!qdev_hotplug_allowed(dev, bus, errp)) {

- goto err_del_dev;

- }

-

- if (!bus && !qdev_get_machine_hotplug_handler(dev)) {

- /* No bus, no machine hotplug handler --> device is not hotpluggable */

- error_setg(errp, "Device '%s' can not be hotplugged on this machine",

- driver);

- goto err_del_dev;

- }

+ if (phase_check(PHASE_MACHINE_READY) &&

+ !qdev_hotplug_allowed(dev, bus, errp)) {

+ goto err_del_dev;

}

/*

--

2.47.1