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

["KONRAD Frédéric" <fred.konrad@greensocs.com>]

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) {
>>>
>>
>