[Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions

[Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions

[Andreas Färber]

Drop an unreachable fallback bus assignment to SysBus.

If no ,bus= is specified, only search busses recursively for bus type if
the DeviceClass has a bus_type specified. Handle resulting NULL cases.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 qdev-monitor.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/qdev-monitor.c b/qdev-monitor.c
index 9a78ccf..73d7946 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -18,6 +18,7 @@
  */
 
 #include "hw/qdev.h"
+#include "hw/sysbus.h"
 #include "monitor/monitor.h"
 #include "monitor/qdev.h"
 #include "qmp-commands.h"
@@ -415,7 +416,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
     DeviceClass *k;
     const char *driver, *path, *id;
     DeviceState *qdev;
-    BusState *bus;
+    BusState *bus = NULL;
 
     driver = qemu_opt_get(opts, "driver");
     if (!driver) {
@@ -453,7 +454,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
                           driver, object_get_typename(OBJECT(bus)));
             return NULL;
         }
-    } else {
+    } else if (k->bus_type != NULL) {
         bus = qbus_find_recursive(sysbus_get_default(), NULL, k->bus_type);
         if (!bus) {
             qerror_report(QERR_NO_BUS_FOR_DEVICE,
@@ -461,18 +462,17 @@ DeviceState *qdev_device_add(QemuOpts *opts)
             return NULL;
         }
     }
-    if (qdev_hotplug && !bus->allow_hotplug) {
+    if (qdev_hotplug && bus && !bus->allow_hotplug) {
         qerror_report(QERR_BUS_NO_HOTPLUG, bus->name);
         return NULL;
     }
 
-    if (!bus) {
-        bus = sysbus_get_default();
-    }
-
     /* create device, set properties */
     qdev = DEVICE(object_new(driver));
-    qdev_set_parent_bus(qdev, bus);
+
+    if (bus) {
+        qdev_set_parent_bus(qdev, bus);
+    }
 
     id = qemu_opts_id(opts);
     if (id) {
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions

[Igor Mammedov]

On Tue, 16 Apr 2013 03:50:21 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Drop an unreachable fallback bus assignment to SysBus.
> 
> If no ,bus= is specified, only search busses recursively for bus type if
> the DeviceClass has a bus_type specified. Handle resulting NULL cases.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  qdev-monitor.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 9a78ccf..73d7946 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -18,6 +18,7 @@
>   */
>  
>  #include "hw/qdev.h"
> +#include "hw/sysbus.h"
>  #include "monitor/monitor.h"
>  #include "monitor/qdev.h"
>  #include "qmp-commands.h"
> @@ -415,7 +416,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>      DeviceClass *k;
>      const char *driver, *path, *id;
>      DeviceState *qdev;
> -    BusState *bus;
> +    BusState *bus = NULL;
>  
>      driver = qemu_opt_get(opts, "driver");
>      if (!driver) {
> @@ -453,7 +454,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>                            driver, object_get_typename(OBJECT(bus)));
>              return NULL;
>          }
> -    } else {
> +    } else if (k->bus_type != NULL) {
>          bus = qbus_find_recursive(sysbus_get_default(), NULL, k->bus_type);
>          if (!bus) {
>              qerror_report(QERR_NO_BUS_FOR_DEVICE,
> @@ -461,18 +462,17 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>              return NULL;
>          }
>      }
> -    if (qdev_hotplug && !bus->allow_hotplug) {
> +    if (qdev_hotplug && bus && !bus->allow_hotplug) {
>          qerror_report(QERR_BUS_NO_HOTPLUG, bus->name);
>          return NULL;
>      }
>  
> -    if (!bus) {
> -        bus = sysbus_get_default();
> -    }
> -
I've checked all direct childs of TYPE_DEVICE and they all set k->bus_type,
with only one exception of TYPE_CPU. So it should be safe to remove fallback
from qdev_device_add POV.
However  TYPE_CPU breaks assumption that device always has parent_bus set to not
NULL in qdev_unplug() and qdev_print()

It would be better to add something like this:
// untested

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 4eb0134..45009ba 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -207,7 +207,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
 {
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
 
-    if (!dev->parent_bus->allow_hotplug) {
+    if (dev->parent_bus && !dev->parent_bus->allow_hotplug) {
         error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
         return;
     }
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 9a78ccf..2476e4e 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -557,7 +557,9 @@ static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
         qdev_print_props(mon, dev, DEVICE_CLASS(class)->props, indent);
         class = object_class_get_parent(class);
     } while (class != object_class_by_name(TYPE_DEVICE));
-    bus_print_dev(dev->parent_bus, mon, dev, indent);
+    if (dev->parent_bus) {
+        bus_print_dev(dev->parent_bus, mon, dev, indent);
+    }
     QLIST_FOREACH(child, &dev->child_bus, sibling) {
         qbus_print(mon, child, indent);
     }

>      /* create device, set properties */
>      qdev = DEVICE(object_new(driver));
> -    qdev_set_parent_bus(qdev, bus);
> +
> +    if (bus) {
> +        qdev_set_parent_bus(qdev, bus);
> +    }
>  
>      id = qemu_opts_id(opts);
>      if (id) {

Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions

[Igor Mammedov]

On Thu, 18 Apr 2013 10:41:56 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

[...]
> > -    if (!bus) {
> > -        bus = sysbus_get_default();
> > -    }
> > -
> I've checked all direct childs of TYPE_DEVICE and they all set k->bus_type,
> with only one exception of TYPE_CPU. So it should be safe to remove fallback
> from qdev_device_add POV.
> However  TYPE_CPU breaks assumption that device always has parent_bus set
> to not NULL in qdev_unplug() and qdev_print()
Err, qdev_print() is safe since it's called on bus children only, so it has
parent_bus.

> 
> It would be better to add something like this:
> // untested
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 4eb0134..45009ba 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -207,7 +207,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
>  {
>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
>  
> -    if (!dev->parent_bus->allow_hotplug) {
> +    if (dev->parent_bus && !dev->parent_bus->allow_hotplug) {
>          error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
>          return;
>      }
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 9a78ccf..2476e4e 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -557,7 +557,9 @@ static void qdev_print(Monitor *mon, DeviceState *dev,
> int indent) qdev_print_props(mon, dev, DEVICE_CLASS(class)->props, indent);
>          class = object_class_get_parent(class);
>      } while (class != object_class_by_name(TYPE_DEVICE));
> -    bus_print_dev(dev->parent_bus, mon, dev, indent);
> +    if (dev->parent_bus) {
> +        bus_print_dev(dev->parent_bus, mon, dev, indent);
> +    }
>      QLIST_FOREACH(child, &dev->child_bus, sibling) {
>          qbus_print(mon, child, indent);
>      }
> 
> >      /* create device, set properties */
> >      qdev = DEVICE(object_new(driver));
> > -    qdev_set_parent_bus(qdev, bus);
> > +
> > +    if (bus) {
> > +        qdev_set_parent_bus(qdev, bus);
> > +    }
> >  
> >      id = qemu_opts_id(opts);
> >      if (id) {
> 
> 

Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions

[Libaiqing]

Hi Igor,

  When I use the config below,an error occurs.Is there anything wrong?

  Qemu-kvm -enable-kvm -name win7 -M pc-0.15 -m 1024 -smp 2 -boot c  -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x4 -chardev spicevmc,id=charchannel0,name=vdagent -device virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0  -drive file=/home/img/win7.qed,if=virtio,index=0,format=qed  -monitor stdio   -vga qxl  -vnc :1

Error output:
    -device virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0: Bus 'virtio-serial0.0' is full
    -device virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0: Bus 'virtio-serial0.0' not found

Any feedback are appliciated.
-----Original Message-----
From: qemu-devel-bounces+libaiqing=huawei.com@nongnu.org [mailto:qemu-devel-bounces+libaiqing=huawei.com@nongnu.org] On Behalf Of Igor Mammedov
Sent: Thursday, April 18, 2013 5:02 PM
To: Igor Mammedov
Cc: ehabkost@redhat.com; qemu-devel@nongnu.org; armbru@redhat.com; anthony@codemonkey.ws; pbonzini@redhat.com; lcapitulino@redhat.com; Andreas Färber
Subject: Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions

On Thu, 18 Apr 2013 10:41:56 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

[...]
> > -    if (!bus) {
> > -        bus = sysbus_get_default();
> > -    }
> > -
> I've checked all direct childs of TYPE_DEVICE and they all set k->bus_type,
> with only one exception of TYPE_CPU. So it should be safe to remove fallback
> from qdev_device_add POV.
> However  TYPE_CPU breaks assumption that device always has parent_bus set
> to not NULL in qdev_unplug() and qdev_print()
Err, qdev_print() is safe since it's called on bus children only, so it has
parent_bus.

> 
> It would be better to add something like this:
> // untested
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 4eb0134..45009ba 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -207,7 +207,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
>  {
>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
>  
> -    if (!dev->parent_bus->allow_hotplug) {
> +    if (dev->parent_bus && !dev->parent_bus->allow_hotplug) {
>          error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
>          return;
>      }
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 9a78ccf..2476e4e 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -557,7 +557,9 @@ static void qdev_print(Monitor *mon, DeviceState *dev,
> int indent) qdev_print_props(mon, dev, DEVICE_CLASS(class)->props, indent);
>          class = object_class_get_parent(class);
>      } while (class != object_class_by_name(TYPE_DEVICE));
> -    bus_print_dev(dev->parent_bus, mon, dev, indent);
> +    if (dev->parent_bus) {
> +        bus_print_dev(dev->parent_bus, mon, dev, indent);
> +    }
>      QLIST_FOREACH(child, &dev->child_bus, sibling) {
>          qbus_print(mon, child, indent);
>      }
> 
> >      /* create device, set properties */
> >      qdev = DEVICE(object_new(driver));
> > -    qdev_set_parent_bus(qdev, bus);
> > +
> > +    if (bus) {
> > +        qdev_set_parent_bus(qdev, bus);
> > +    }
> >  
> >      id = qemu_opts_id(opts);
> >      if (id) {
> 
> 

Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions

[Andreas Färber]

Hi,

Am 22.04.2013 13:51, schrieb Libaiqing:
>   When I use the config below,an error occurs.Is there anything wrong?
> 
>   Qemu-kvm -enable-kvm -name win7 -M pc-0.15 -m 1024 -smp 2 -boot c  -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x4 -chardev spicevmc,id=charchannel0,name=vdagent -device virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0  -drive file=/home/img/win7.qed,if=virtio,index=0,format=qed  -monitor stdio   -vga qxl  -vnc :1
> 
> Error output:
>     -device virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0: Bus 'virtio-serial0.0' is full
>     -device virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0: Bus 'virtio-serial0.0' not found
> 
> Any feedback are appliciated.

This does not sound related to this patch at all...

Instead it sounds as if the virtio refactorings had some effect not only
on virtio-net but also on virtio-serial. Fred?

Andreas

> -----Original Message-----
> From: qemu-devel-bounces+libaiqing=huawei.com@nongnu.org [mailto:qemu-devel-bounces+libaiqing=huawei.com@nongnu.org] On Behalf Of Igor Mammedov
> Sent: Thursday, April 18, 2013 5:02 PM
> To: Igor Mammedov
> Cc: ehabkost@redhat.com; qemu-devel@nongnu.org; armbru@redhat.com; anthony@codemonkey.ws; pbonzini@redhat.com; lcapitulino@redhat.com; Andreas Färber
> Subject: Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions
> 
> On Thu, 18 Apr 2013 10:41:56 +0200
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> [...]
>>> -    if (!bus) {
>>> -        bus = sysbus_get_default();
>>> -    }
>>> -
>> I've checked all direct childs of TYPE_DEVICE and they all set k->bus_type,
>> with only one exception of TYPE_CPU. So it should be safe to remove fallback
>> from qdev_device_add POV.
>> However  TYPE_CPU breaks assumption that device always has parent_bus set
>> to not NULL in qdev_unplug() and qdev_print()
> Err, qdev_print() is safe since it's called on bus children only, so it has
> parent_bus.
> 
>>
>> It would be better to add something like this:
>> // untested
>>
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index 4eb0134..45009ba 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -207,7 +207,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
>>  {
>>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
>>  
>> -    if (!dev->parent_bus->allow_hotplug) {
>> +    if (dev->parent_bus && !dev->parent_bus->allow_hotplug) {
>>          error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
>>          return;
>>      }
>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>> index 9a78ccf..2476e4e 100644
>> --- a/qdev-monitor.c
>> +++ b/qdev-monitor.c
>> @@ -557,7 +557,9 @@ static void qdev_print(Monitor *mon, DeviceState *dev,
>> int indent) qdev_print_props(mon, dev, DEVICE_CLASS(class)->props, indent);
>>          class = object_class_get_parent(class);
>>      } while (class != object_class_by_name(TYPE_DEVICE));
>> -    bus_print_dev(dev->parent_bus, mon, dev, indent);
>> +    if (dev->parent_bus) {
>> +        bus_print_dev(dev->parent_bus, mon, dev, indent);
>> +    }
>>      QLIST_FOREACH(child, &dev->child_bus, sibling) {
>>          qbus_print(mon, child, indent);
>>      }
>>
>>>      /* create device, set properties */
>>>      qdev = DEVICE(object_new(driver));
>>> -    qdev_set_parent_bus(qdev, bus);
>>> +
>>> +    if (bus) {
>>> +        qdev_set_parent_bus(qdev, bus);
>>> +    }
>>>  
>>>      id = qemu_opts_id(opts);
>>>      if (id) {
>>
>>
> 
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions

[KONRAD Frédéric]

On 22/04/2013 15:27, Andreas Färber wrote:
> Hi,
>
> Am 22.04.2013 13:51, schrieb Libaiqing:
>>    When I use the config below,an error occurs.Is there anything wrong?
>>
>>    Qemu-kvm -enable-kvm -name win7 -M pc-0.15 -m 1024 -smp 2 -boot c  -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x4 -chardev spicevmc,id=charchannel0,name=vdagent -device virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0  -drive file=/home/img/win7.qed,if=virtio,index=0,format=qed  -monitor stdio   -vga qxl  -vnc :1
>>
>> Error output:
>>      -device virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0: Bus 'virtio-serial0.0' is full
>>      -device virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0: Bus 'virtio-serial0.0' not found
>>
>> Any feedback are appliciated.
> This does not sound related to this patch at all...
>
> Instead it sounds as if the virtio refactorings had some effect not only
> on virtio-net but also on virtio-serial. Fred?
>
> Andreas
Hi,

Yes, sounds like the same issue as virtio-net:

     bus: pci.0
       type PCI
       dev: virtio-serial-pci, id "virtio-serial0"
         ioeventfd = off
         vectors = 2
         class = 0x780
         indirect_desc = on
         event_idx = on
         max_ports = 31
         addr = 04.0
         romfile = <null>
         rombar = 1
         multifunction = off
         command_serr_enable = on
         class Class 0780, addr 00:04.0, pci id 1af4:1003 (sub 1af4:0003)
         bar 0: i/o at 0xc040 [0xc05f]
         bar 1: mem at 0xfebf1000 [0xfebf1fff]
         bus: virtio-serial0.0
           type virtio-pci-bus
           dev: virtio-serial-device, id ""
             max_ports = 31
             bus: virtio-serial-bus.0
               type virtio-serial-bus

The autogenerated bus name "deviceid.n" (virtio-serial0.0) became a 
virtio-bus...

virtio-serial-bus.0 is the right bus to connect virtserialport.

Any idea how to fix that?

Sorry for that,
Fred

>
>> -----Original Message-----
>> From: qemu-devel-bounces+libaiqing=huawei.com@nongnu.org [mailto:qemu-devel-bounces+libaiqing=huawei.com@nongnu.org] On Behalf Of Igor Mammedov
>> Sent: Thursday, April 18, 2013 5:02 PM
>> To: Igor Mammedov
>> Cc: ehabkost@redhat.com; qemu-devel@nongnu.org; armbru@redhat.com; anthony@codemonkey.ws; pbonzini@redhat.com; lcapitulino@redhat.com; Andreas Färber
>> Subject: Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions
>>
>> On Thu, 18 Apr 2013 10:41:56 +0200
>> Igor Mammedov <imammedo@redhat.com> wrote:
>>
>> [...]
>>>> -    if (!bus) {
>>>> -        bus = sysbus_get_default();
>>>> -    }
>>>> -
>>> I've checked all direct childs of TYPE_DEVICE and they all set k->bus_type,
>>> with only one exception of TYPE_CPU. So it should be safe to remove fallback
>>> from qdev_device_add POV.
>>> However  TYPE_CPU breaks assumption that device always has parent_bus set
>>> to not NULL in qdev_unplug() and qdev_print()
>> Err, qdev_print() is safe since it's called on bus children only, so it has
>> parent_bus.
>>
>>> It would be better to add something like this:
>>> // untested
>>>
>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>> index 4eb0134..45009ba 100644
>>> --- a/hw/core/qdev.c
>>> +++ b/hw/core/qdev.c
>>> @@ -207,7 +207,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
>>>   {
>>>       DeviceClass *dc = DEVICE_GET_CLASS(dev);
>>>   
>>> -    if (!dev->parent_bus->allow_hotplug) {
>>> +    if (dev->parent_bus && !dev->parent_bus->allow_hotplug) {
>>>           error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
>>>           return;
>>>       }
>>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>>> index 9a78ccf..2476e4e 100644
>>> --- a/qdev-monitor.c
>>> +++ b/qdev-monitor.c
>>> @@ -557,7 +557,9 @@ static void qdev_print(Monitor *mon, DeviceState *dev,
>>> int indent) qdev_print_props(mon, dev, DEVICE_CLASS(class)->props, indent);
>>>           class = object_class_get_parent(class);
>>>       } while (class != object_class_by_name(TYPE_DEVICE));
>>> -    bus_print_dev(dev->parent_bus, mon, dev, indent);
>>> +    if (dev->parent_bus) {
>>> +        bus_print_dev(dev->parent_bus, mon, dev, indent);
>>> +    }
>>>       QLIST_FOREACH(child, &dev->child_bus, sibling) {
>>>           qbus_print(mon, child, indent);
>>>       }
>>>
>>>>       /* create device, set properties */
>>>>       qdev = DEVICE(object_new(driver));
>>>> -    qdev_set_parent_bus(qdev, bus);
>>>> +
>>>> +    if (bus) {
>>>> +        qdev_set_parent_bus(qdev, bus);
>>>> +    }
>>>>   
>>>>       id = qemu_opts_id(opts);
>>>>       if (id) {
>>>
>>
>

Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions

[Andreas Färber]

Hi Fred,

Am 22.04.2013 15:54, schrieb KONRAD Frédéric:
> On 22/04/2013 15:27, Andreas Färber wrote:
>> Am 22.04.2013 13:51, schrieb Libaiqing:
>>>    When I use the config below,an error occurs.Is there anything wrong?
>>>
>>>    Qemu-kvm -enable-kvm -name win7 -M pc-0.15 -m 1024 -smp 2 -boot c 
>>> -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x4
>>> -chardev spicevmc,id=charchannel0,name=vdagent -device
>>> virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0 
>>> -drive file=/home/img/win7.qed,if=virtio,index=0,format=qed  -monitor
>>> stdio   -vga qxl  -vnc :1
>>>
>>> Error output:
>>>      -device
>>> virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0:
>>> Bus 'virtio-serial0.0' is full
>>>      -device
>>> virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0:
>>> Bus 'virtio-serial0.0' not found
>>>
>>> Any feedback are appliciated.
>> This does not sound related to this patch at all...
>>
>> Instead it sounds as if the virtio refactorings had some effect not only
>> on virtio-net but also on virtio-serial. Fred?
> 
> Yes, sounds like the same issue as virtio-net:
> 
>     bus: pci.0
>       type PCI
>       dev: virtio-serial-pci, id "virtio-serial0"
>         ioeventfd = off
>         vectors = 2
>         class = 0x780
>         indirect_desc = on
>         event_idx = on
>         max_ports = 31
>         addr = 04.0
>         romfile = <null>
>         rombar = 1
>         multifunction = off
>         command_serr_enable = on
>         class Class 0780, addr 00:04.0, pci id 1af4:1003 (sub 1af4:0003)
>         bar 0: i/o at 0xc040 [0xc05f]
>         bar 1: mem at 0xfebf1000 [0xfebf1fff]
>         bus: virtio-serial0.0
>           type virtio-pci-bus
>           dev: virtio-serial-device, id ""
>             max_ports = 31
>             bus: virtio-serial-bus.0
>               type virtio-serial-bus
> 
> The autogenerated bus name "deviceid.n" (virtio-serial0.0) became a
> virtio-bus...
> 
> virtio-serial-bus.0 is the right bus to connect virtserialport.
> 
> Any idea how to fix that?

The only idea I can come up with right now is to overwrite the bus name
on realize/qdev-init of the containing (virtio-serial-pci) device.

Whether we want that is another question... :) It would fix command line
backwards compatibility but would be inconsistent then; I guess the
former is more important here.

Regards,
Andreas

> 
> Sorry for that,
> Fred
> 
>>
>>> -----Original Message-----
>>> From: qemu-devel-bounces+libaiqing=huawei.com@nongnu.org
>>> [mailto:qemu-devel-bounces+libaiqing=huawei.com@nongnu.org] On Behalf
>>> Of Igor Mammedov
>>> Sent: Thursday, April 18, 2013 5:02 PM
>>> To: Igor Mammedov
>>> Cc: ehabkost@redhat.com; qemu-devel@nongnu.org; armbru@redhat.com;
>>> anthony@codemonkey.ws; pbonzini@redhat.com; lcapitulino@redhat.com;
>>> Andreas Färber
>>> Subject: Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions
>>>
>>> On Thu, 18 Apr 2013 10:41:56 +0200
>>> Igor Mammedov <imammedo@redhat.com> wrote:
>>>
>>> [...]
>>>>> -    if (!bus) {
>>>>> -        bus = sysbus_get_default();
>>>>> -    }
>>>>> -
>>>> I've checked all direct childs of TYPE_DEVICE and they all set
>>>> k->bus_type,
>>>> with only one exception of TYPE_CPU. So it should be safe to remove
>>>> fallback
>>>> from qdev_device_add POV.
>>>> However  TYPE_CPU breaks assumption that device always has
>>>> parent_bus set
>>>> to not NULL in qdev_unplug() and qdev_print()
>>> Err, qdev_print() is safe since it's called on bus children only, so
>>> it has
>>> parent_bus.
>>>
>>>> It would be better to add something like this:
>>>> // untested
>>>>
>>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>>> index 4eb0134..45009ba 100644
>>>> --- a/hw/core/qdev.c
>>>> +++ b/hw/core/qdev.c
>>>> @@ -207,7 +207,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
>>>>   {
>>>>       DeviceClass *dc = DEVICE_GET_CLASS(dev);
>>>>   -    if (!dev->parent_bus->allow_hotplug) {
>>>> +    if (dev->parent_bus && !dev->parent_bus->allow_hotplug) {
>>>>           error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
>>>>           return;
>>>>       }
>>>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>>>> index 9a78ccf..2476e4e 100644
>>>> --- a/qdev-monitor.c
>>>> +++ b/qdev-monitor.c
>>>> @@ -557,7 +557,9 @@ static void qdev_print(Monitor *mon, DeviceState
>>>> *dev,
>>>> int indent) qdev_print_props(mon, dev, DEVICE_CLASS(class)->props,
>>>> indent);
>>>>           class = object_class_get_parent(class);
>>>>       } while (class != object_class_by_name(TYPE_DEVICE));
>>>> -    bus_print_dev(dev->parent_bus, mon, dev, indent);
>>>> +    if (dev->parent_bus) {
>>>> +        bus_print_dev(dev->parent_bus, mon, dev, indent);
>>>> +    }
>>>>       QLIST_FOREACH(child, &dev->child_bus, sibling) {
>>>>           qbus_print(mon, child, indent);
>>>>       }
>>>>
>>>>>       /* create device, set properties */
>>>>>       qdev = DEVICE(object_new(driver));
>>>>> -    qdev_set_parent_bus(qdev, bus);
>>>>> +
>>>>> +    if (bus) {
>>>>> +        qdev_set_parent_bus(qdev, bus);
>>>>> +    }
>>>>>         id = qemu_opts_id(opts);
>>>>>       if (id) {
>>>>
>>>
>>
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions

[KONRAD Frédéric]

On 22/04/2013 16:02, Andreas Färber wrote:
> Hi Fred,
>
> Am 22.04.2013 15:54, schrieb KONRAD Frédéric:
>> On 22/04/2013 15:27, Andreas Färber wrote:
>>> Am 22.04.2013 13:51, schrieb Libaiqing:
>>>>     When I use the config below,an error occurs.Is there anything wrong?
>>>>
>>>>     Qemu-kvm -enable-kvm -name win7 -M pc-0.15 -m 1024 -smp 2 -boot c
>>>> -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x4
>>>> -chardev spicevmc,id=charchannel0,name=vdagent -device
>>>> virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0
>>>> -drive file=/home/img/win7.qed,if=virtio,index=0,format=qed  -monitor
>>>> stdio   -vga qxl  -vnc :1
>>>>
>>>> Error output:
>>>>       -device
>>>> virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0:
>>>> Bus 'virtio-serial0.0' is full
>>>>       -device
>>>> virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0:
>>>> Bus 'virtio-serial0.0' not found
>>>>
>>>> Any feedback are appliciated.
>>> This does not sound related to this patch at all...
>>>
>>> Instead it sounds as if the virtio refactorings had some effect not only
>>> on virtio-net but also on virtio-serial. Fred?
>> Yes, sounds like the same issue as virtio-net:
>>
>>      bus: pci.0
>>        type PCI
>>        dev: virtio-serial-pci, id "virtio-serial0"
>>          ioeventfd = off
>>          vectors = 2
>>          class = 0x780
>>          indirect_desc = on
>>          event_idx = on
>>          max_ports = 31
>>          addr = 04.0
>>          romfile = <null>
>>          rombar = 1
>>          multifunction = off
>>          command_serr_enable = on
>>          class Class 0780, addr 00:04.0, pci id 1af4:1003 (sub 1af4:0003)
>>          bar 0: i/o at 0xc040 [0xc05f]
>>          bar 1: mem at 0xfebf1000 [0xfebf1fff]
>>          bus: virtio-serial0.0
>>            type virtio-pci-bus
>>            dev: virtio-serial-device, id ""
>>              max_ports = 31
>>              bus: virtio-serial-bus.0
>>                type virtio-serial-bus
>>
>> The autogenerated bus name "deviceid.n" (virtio-serial0.0) became a
>> virtio-bus...
>>
>> virtio-serial-bus.0 is the right bus to connect virtserialport.
>>
>> Any idea how to fix that?
> The only idea I can come up with right now is to overwrite the bus name
> on realize/qdev-init of the containing (virtio-serial-pci) device.
>
> Whether we want that is another question... :) It would fix command line
> backwards compatibility but would be inconsistent then; I guess the
> former is more important here.
>
> Regards,
> Andreas
I'm not sure that only overwriting the bus name will fix the issue.

virtio-serial-device's bus still won't have the right name?

Here with the command line, we expect virtio-serial-pci,id=id0 to create 
a virtio-serial-bus called id0.0 is that right?

Thanks,
Fred

>
>> Sorry for that,
>> Fred
>>
>>>> -----Original Message-----
>>>> From: qemu-devel-bounces+libaiqing=huawei.com@nongnu.org
>>>> [mailto:qemu-devel-bounces+libaiqing=huawei.com@nongnu.org] On Behalf
>>>> Of Igor Mammedov
>>>> Sent: Thursday, April 18, 2013 5:02 PM
>>>> To: Igor Mammedov
>>>> Cc: ehabkost@redhat.com; qemu-devel@nongnu.org; armbru@redhat.com;
>>>> anthony@codemonkey.ws; pbonzini@redhat.com; lcapitulino@redhat.com;
>>>> Andreas Färber
>>>> Subject: Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions
>>>>
>>>> On Thu, 18 Apr 2013 10:41:56 +0200
>>>> Igor Mammedov <imammedo@redhat.com> wrote:
>>>>
>>>> [...]
>>>>>> -    if (!bus) {
>>>>>> -        bus = sysbus_get_default();
>>>>>> -    }
>>>>>> -
>>>>> I've checked all direct childs of TYPE_DEVICE and they all set
>>>>> k->bus_type,
>>>>> with only one exception of TYPE_CPU. So it should be safe to remove
>>>>> fallback
>>>>> from qdev_device_add POV.
>>>>> However  TYPE_CPU breaks assumption that device always has
>>>>> parent_bus set
>>>>> to not NULL in qdev_unplug() and qdev_print()
>>>> Err, qdev_print() is safe since it's called on bus children only, so
>>>> it has
>>>> parent_bus.
>>>>
>>>>> It would be better to add something like this:
>>>>> // untested
>>>>>
>>>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>>>> index 4eb0134..45009ba 100644
>>>>> --- a/hw/core/qdev.c
>>>>> +++ b/hw/core/qdev.c
>>>>> @@ -207,7 +207,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
>>>>>    {
>>>>>        DeviceClass *dc = DEVICE_GET_CLASS(dev);
>>>>>    -    if (!dev->parent_bus->allow_hotplug) {
>>>>> +    if (dev->parent_bus && !dev->parent_bus->allow_hotplug) {
>>>>>            error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
>>>>>            return;
>>>>>        }
>>>>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>>>>> index 9a78ccf..2476e4e 100644
>>>>> --- a/qdev-monitor.c
>>>>> +++ b/qdev-monitor.c
>>>>> @@ -557,7 +557,9 @@ static void qdev_print(Monitor *mon, DeviceState
>>>>> *dev,
>>>>> int indent) qdev_print_props(mon, dev, DEVICE_CLASS(class)->props,
>>>>> indent);
>>>>>            class = object_class_get_parent(class);
>>>>>        } while (class != object_class_by_name(TYPE_DEVICE));
>>>>> -    bus_print_dev(dev->parent_bus, mon, dev, indent);
>>>>> +    if (dev->parent_bus) {
>>>>> +        bus_print_dev(dev->parent_bus, mon, dev, indent);
>>>>> +    }
>>>>>        QLIST_FOREACH(child, &dev->child_bus, sibling) {
>>>>>            qbus_print(mon, child, indent);
>>>>>        }
>>>>>
>>>>>>        /* create device, set properties */
>>>>>>        qdev = DEVICE(object_new(driver));
>>>>>> -    qdev_set_parent_bus(qdev, bus);
>>>>>> +
>>>>>> +    if (bus) {
>>>>>> +        qdev_set_parent_bus(qdev, bus);
>>>>>> +    }
>>>>>>          id = qemu_opts_id(opts);
>>>>>>        if (id) {
>

Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions

[Andreas Färber]

Am 22.04.2013 16:15, schrieb KONRAD Frédéric:
> On 22/04/2013 16:02, Andreas Färber wrote:
>> Hi Fred,
>>
>> Am 22.04.2013 15:54, schrieb KONRAD Frédéric:
>>> On 22/04/2013 15:27, Andreas Färber wrote:
>>>> Am 22.04.2013 13:51, schrieb Libaiqing:
>>>>>     When I use the config below,an error occurs.Is there anything
>>>>> wrong?
>>>>>
>>>>>     Qemu-kvm -enable-kvm -name win7 -M pc-0.15 -m 1024 -smp 2 -boot c
>>>>> -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x4
>>>>> -chardev spicevmc,id=charchannel0,name=vdagent -device
>>>>> virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0
>>>>>
>>>>> -drive file=/home/img/win7.qed,if=virtio,index=0,format=qed  -monitor
>>>>> stdio   -vga qxl  -vnc :1
>>>>>
>>>>> Error output:
>>>>>       -device
>>>>> virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0:
>>>>>
>>>>> Bus 'virtio-serial0.0' is full
>>>>>       -device
>>>>> virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0:
>>>>>
>>>>> Bus 'virtio-serial0.0' not found
>>>>>
>>>>> Any feedback are appliciated.
>>>> This does not sound related to this patch at all...
>>>>
>>>> Instead it sounds as if the virtio refactorings had some effect not
>>>> only
>>>> on virtio-net but also on virtio-serial. Fred?
>>> Yes, sounds like the same issue as virtio-net:
>>>
>>>      bus: pci.0
>>>        type PCI
>>>        dev: virtio-serial-pci, id "virtio-serial0"
>>>          ioeventfd = off
>>>          vectors = 2
>>>          class = 0x780
>>>          indirect_desc = on
>>>          event_idx = on
>>>          max_ports = 31
>>>          addr = 04.0
>>>          romfile = <null>
>>>          rombar = 1
>>>          multifunction = off
>>>          command_serr_enable = on
>>>          class Class 0780, addr 00:04.0, pci id 1af4:1003 (sub
>>> 1af4:0003)
>>>          bar 0: i/o at 0xc040 [0xc05f]
>>>          bar 1: mem at 0xfebf1000 [0xfebf1fff]
>>>          bus: virtio-serial0.0
>>>            type virtio-pci-bus
>>>            dev: virtio-serial-device, id ""
>>>              max_ports = 31
>>>              bus: virtio-serial-bus.0
>>>                type virtio-serial-bus
>>>
>>> The autogenerated bus name "deviceid.n" (virtio-serial0.0) became a
>>> virtio-bus...
>>>
>>> virtio-serial-bus.0 is the right bus to connect virtserialport.
>>>
>>> Any idea how to fix that?
>> The only idea I can come up with right now is to overwrite the bus name
>> on realize/qdev-init of the containing (virtio-serial-pci) device.
>>
>> Whether we want that is another question... :) It would fix command line
>> backwards compatibility but would be inconsistent then; I guess the
>> former is more important here.
>>
>> Regards,
>> Andreas
> I'm not sure that only overwriting the bus name will fix the issue.
> 
> virtio-serial-device's bus still won't have the right name?

I was talking of virtio-serial-pci overwriting virtio-serial-device's
bus with its own id.0 after it's been created by virtio-serial-device
with NULL argument! Assuming it gets created in instance_init and can't
just access its parent's id in its own realize function to have it
correct from the start.

Andreas

> Here with the command line, we expect virtio-serial-pci,id=id0 to create
> a virtio-serial-bus called id0.0 is that right?
> 
> Thanks,
> Fred

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions

[KONRAD Frédéric]

On 22/04/2013 16:22, Andreas Färber wrote:
> Am 22.04.2013 16:15, schrieb KONRAD Frédéric:
>> On 22/04/2013 16:02, Andreas Färber wrote:
>>> Hi Fred,
>>>
>>> Am 22.04.2013 15:54, schrieb KONRAD Frédéric:
>>>> On 22/04/2013 15:27, Andreas Färber wrote:
>>>>> Am 22.04.2013 13:51, schrieb Libaiqing:
>>>>>>      When I use the config below,an error occurs.Is there anything
>>>>>> wrong?
>>>>>>
>>>>>>      Qemu-kvm -enable-kvm -name win7 -M pc-0.15 -m 1024 -smp 2 -boot c
>>>>>> -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x4
>>>>>> -chardev spicevmc,id=charchannel0,name=vdagent -device
>>>>>> virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0
>>>>>>
>>>>>> -drive file=/home/img/win7.qed,if=virtio,index=0,format=qed  -monitor
>>>>>> stdio   -vga qxl  -vnc :1
>>>>>>
>>>>>> Error output:
>>>>>>        -device
>>>>>> virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0:
>>>>>>
>>>>>> Bus 'virtio-serial0.0' is full
>>>>>>        -device
>>>>>> virtserialport,bus=virtio-serial0.0,chardev=charchannel0,id=channel0,name=com.redhat.spice.0:
>>>>>>
>>>>>> Bus 'virtio-serial0.0' not found
>>>>>>
>>>>>> Any feedback are appliciated.
>>>>> This does not sound related to this patch at all...
>>>>>
>>>>> Instead it sounds as if the virtio refactorings had some effect not
>>>>> only
>>>>> on virtio-net but also on virtio-serial. Fred?
>>>> Yes, sounds like the same issue as virtio-net:
>>>>
>>>>       bus: pci.0
>>>>         type PCI
>>>>         dev: virtio-serial-pci, id "virtio-serial0"
>>>>           ioeventfd = off
>>>>           vectors = 2
>>>>           class = 0x780
>>>>           indirect_desc = on
>>>>           event_idx = on
>>>>           max_ports = 31
>>>>           addr = 04.0
>>>>           romfile = <null>
>>>>           rombar = 1
>>>>           multifunction = off
>>>>           command_serr_enable = on
>>>>           class Class 0780, addr 00:04.0, pci id 1af4:1003 (sub
>>>> 1af4:0003)
>>>>           bar 0: i/o at 0xc040 [0xc05f]
>>>>           bar 1: mem at 0xfebf1000 [0xfebf1fff]
>>>>           bus: virtio-serial0.0
>>>>             type virtio-pci-bus
>>>>             dev: virtio-serial-device, id ""
>>>>               max_ports = 31
>>>>               bus: virtio-serial-bus.0
>>>>                 type virtio-serial-bus
>>>>
>>>> The autogenerated bus name "deviceid.n" (virtio-serial0.0) became a
>>>> virtio-bus...
>>>>
>>>> virtio-serial-bus.0 is the right bus to connect virtserialport.
>>>>
>>>> Any idea how to fix that?
>>> The only idea I can come up with right now is to overwrite the bus name
>>> on realize/qdev-init of the containing (virtio-serial-pci) device.
>>>
>>> Whether we want that is another question... :) It would fix command line
>>> backwards compatibility but would be inconsistent then; I guess the
>>> former is more important here.
>>>
>>> Regards,
>>> Andreas
>> I'm not sure that only overwriting the bus name will fix the issue.
>>
>> virtio-serial-device's bus still won't have the right name?
> I was talking of virtio-serial-pci overwriting virtio-serial-device's
> bus with its own id.0 after it's been created by virtio-serial-device
> with NULL argument! Assuming it gets created in instance_init and can't
> just access its parent's id in its own realize function to have it
> correct from the start.
>
> Andreas

Ok, I'll take a look.

Thanks for help :),
Fred
>
>> Here with the command line, we expect virtio-serial-pci,id=id0 to create
>> a virtio-serial-bus called id0.0 is that right?
>>
>> Thanks,
>> Fred

Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions

[Andreas Färber]

Am 18.04.2013 10:41, schrieb Igor Mammedov:
> On Tue, 16 Apr 2013 03:50:21 +0200
> Andreas Färber <afaerber@suse.de> wrote:
> 
>> Drop an unreachable fallback bus assignment to SysBus.
>>
>> If no ,bus= is specified, only search busses recursively for bus type if
>> the DeviceClass has a bus_type specified. Handle resulting NULL cases.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>>  qdev-monitor.c | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>> index 9a78ccf..73d7946 100644
>> --- a/qdev-monitor.c
>> +++ b/qdev-monitor.c
>> @@ -18,6 +18,7 @@
>>   */
>>  
>>  #include "hw/qdev.h"
>> +#include "hw/sysbus.h"
>>  #include "monitor/monitor.h"
>>  #include "monitor/qdev.h"
>>  #include "qmp-commands.h"
>> @@ -415,7 +416,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>>      DeviceClass *k;
>>      const char *driver, *path, *id;
>>      DeviceState *qdev;
>> -    BusState *bus;
>> +    BusState *bus = NULL;
>>  
>>      driver = qemu_opt_get(opts, "driver");
>>      if (!driver) {
>> @@ -453,7 +454,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>>                            driver, object_get_typename(OBJECT(bus)));
>>              return NULL;
>>          }
>> -    } else {
>> +    } else if (k->bus_type != NULL) {
>>          bus = qbus_find_recursive(sysbus_get_default(), NULL, k->bus_type);
>>          if (!bus) {
>>              qerror_report(QERR_NO_BUS_FOR_DEVICE,
>> @@ -461,18 +462,17 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>>              return NULL;
>>          }
>>      }
>> -    if (qdev_hotplug && !bus->allow_hotplug) {
>> +    if (qdev_hotplug && bus && !bus->allow_hotplug) {
>>          qerror_report(QERR_BUS_NO_HOTPLUG, bus->name);
>>          return NULL;
>>      }
>>  
>> -    if (!bus) {
>> -        bus = sysbus_get_default();
>> -    }
>> -
> I've checked all direct childs of TYPE_DEVICE and they all set k->bus_type,
> with only one exception of TYPE_CPU.

> So it should be safe to remove fallback
> from qdev_device_add POV.

Sorry, don't understand what exactly you are suggesting?

> However  TYPE_CPU breaks assumption that device always has parent_bus set to not
> NULL in qdev_unplug() and qdev_print()
> 
> It would be better to add something like this:
> // untested
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 4eb0134..45009ba 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -207,7 +207,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
>  {
>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
>  
> -    if (!dev->parent_bus->allow_hotplug) {
> +    if (dev->parent_bus && !dev->parent_bus->allow_hotplug) {
>          error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
>          return;
>      }

This part looks good, can you resend as a proper patch? Even if we can't
test the no-parent_bus unplug path ATM, it shouldn't hurt to fix
assumptions about it.

Andreas

> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 9a78ccf..2476e4e 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -557,7 +557,9 @@ static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
>          qdev_print_props(mon, dev, DEVICE_CLASS(class)->props, indent);
>          class = object_class_get_parent(class);
>      } while (class != object_class_by_name(TYPE_DEVICE));
> -    bus_print_dev(dev->parent_bus, mon, dev, indent);
> +    if (dev->parent_bus) {
> +        bus_print_dev(dev->parent_bus, mon, dev, indent);
> +    }
>      QLIST_FOREACH(child, &dev->child_bus, sibling) {
>          qbus_print(mon, child, indent);
>      }
> 
>>      /* create device, set properties */
>>      qdev = DEVICE(object_new(driver));
>> -    qdev_set_parent_bus(qdev, bus);
>> +
>> +    if (bus) {
>> +        qdev_set_parent_bus(qdev, bus);
>> +    }
>>  
>>      id = qemu_opts_id(opts);
>>      if (id) {
> 
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions

[Anthony Liguori]

Applied.  Thanks.

Regards,

Anthony Liguori