[v1] Fixes around device realization

[PATCH 22/24] qdev: Assert devices are plugged into a bus that can take them

Posted by Markus Armbruster 5 years, 6 months ago

This would have caught some of the bugs I just fixed.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/core/qdev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 9e5538aeae..0df995eb94 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -97,6 +97,11 @@ static void bus_add_child(BusState *bus, DeviceState *child)
 void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
 {
     BusState *old_parent_bus = dev->parent_bus;
+    DeviceClass *dc = DEVICE_GET_CLASS(dev);
+
+    assert(dc->bus_type
+           ? bus && object_dynamic_cast(OBJECT(bus), dc->bus_type)
+           : !bus);
 
     if (old_parent_bus) {
         trace_qdev_update_parent_bus(dev, object_get_typename(OBJECT(dev)),
-- 
2.21.1

Re: [PATCH 22/24] qdev: Assert devices are plugged into a bus that can take them

Posted by Markus Armbruster 5 years, 6 months ago

Markus Armbruster <armbru@redhat.com> writes:

> This would have caught some of the bugs I just fixed.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/core/qdev.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 9e5538aeae..0df995eb94 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -97,6 +97,11 @@ static void bus_add_child(BusState *bus, DeviceState *child)
>  void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
>  {
>      BusState *old_parent_bus = dev->parent_bus;
> +    DeviceClass *dc = DEVICE_GET_CLASS(dev);
> +
> +    assert(dc->bus_type
> +           ? bus && object_dynamic_cast(OBJECT(bus), dc->bus_type)
> +           : !bus);
>  
>      if (old_parent_bus) {
>          trace_qdev_update_parent_bus(dev, object_get_typename(OBJECT(dev)),

Actually, !bus crashes below in bus_add_child().  Simpler assertion:

       assert(dc->bus_type && object_dynamic_cast(OBJECT(bus), dc->bus_type));

Re: [PATCH 22/24] qdev: Assert devices are plugged into a bus that can take them

Posted by Mark Cave-Ayland 5 years, 5 months ago

On 18/05/2020 06:04, Markus Armbruster wrote:

> This would have caught some of the bugs I just fixed.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/core/qdev.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 9e5538aeae..0df995eb94 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -97,6 +97,11 @@ static void bus_add_child(BusState *bus, DeviceState *child)
>  void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
>  {
>      BusState *old_parent_bus = dev->parent_bus;
> +    DeviceClass *dc = DEVICE_GET_CLASS(dev);
> +
> +    assert(dc->bus_type
> +           ? bus && object_dynamic_cast(OBJECT(bus), dc->bus_type)
> +           : !bus);
>  
>      if (old_parent_bus) {
>          trace_qdev_update_parent_bus(dev, object_get_typename(OBJECT(dev)),

Works for me. If you've managed to fix up a large number of bad cases, let's not
allow people to go on making the same mistakes.

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.