From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35672) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UUHXP-0005bO-DM for qemu-devel@nongnu.org; Mon, 22 Apr 2013 10:16:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UUHXL-0006wj-Jn for qemu-devel@nongnu.org; Mon, 22 Apr 2013 10:16:11 -0400 Received: from greensocs.com ([87.106.252.221]:39581 helo=s15328186.onlinehome-server.info) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UUHXL-0006uj-BF for qemu-devel@nongnu.org; Mon, 22 Apr 2013 10:16:07 -0400 Message-ID: <5175461E.9040008@greensocs.com> Date: Mon, 22 Apr 2013 16:15:58 +0200 From: =?ISO-8859-1?Q?KONRAD_Fr=E9d=E9ric?= MIME-Version: 1.0 References: <1366077021-28882-1-git-send-email-afaerber@suse.de> <20130418104156.7a5349f0@nial.usersys.redhat.com> <20130418110151.316c17b6@nial.usersys.redhat.com> <5D9ACBBCF6B270468D615C4719A59BE33A469646@szxeml548-mbx.china.huawei.com> <51753ADA.5010504@suse.de> <51754130.9080507@greensocs.com> <517542F2.6080407@suse.de> In-Reply-To: <517542F2.6080407@suse.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?ISO-8859-1?Q?Andreas_F=E4rber?= Cc: Libaiqing , "ehabkost@redhat.com" , Haofeng , "qemu-devel@nongnu.org" , "armbru@redhat.com" , "anthony@codemonkey.ws" , Igor Mammedov , "pbonzini@redhat.com" , "lcapitulino@redhat.com" On 22/04/2013 16:02, Andreas F=E4rber wrote: > Hi Fred, > > Am 22.04.2013 15:54, schrieb KONRAD Fr=E9d=E9ric: >> On 22/04/2013 15:27, Andreas F=E4rber wrote: >>> Am 22.04.2013 13:51, schrieb Libaiqing: >>>> When I use the config below,an error occurs.Is there anything wr= ong? >>>> >>>> Qemu-kvm -enable-kvm -name win7 -M pc-0.15 -m 1024 -smp 2 -boot = c >>>> -device virtio-serial-pci,id=3Dvirtio-serial0,bus=3Dpci.0,addr=3D0x4 >>>> -chardev spicevmc,id=3Dcharchannel0,name=3Dvdagent -device >>>> virtserialport,bus=3Dvirtio-serial0.0,chardev=3Dcharchannel0,id=3Dch= annel0,name=3Dcom.redhat.spice.0 >>>> -drive file=3D/home/img/win7.qed,if=3Dvirtio,index=3D0,format=3Dqed = -monitor >>>> stdio -vga qxl -vnc :1 >>>> >>>> Error output: >>>> -device >>>> virtserialport,bus=3Dvirtio-serial0.0,chardev=3Dcharchannel0,id=3Dch= annel0,name=3Dcom.redhat.spice.0: >>>> Bus 'virtio-serial0.0' is full >>>> -device >>>> virtserialport,bus=3Dvirtio-serial0.0,chardev=3Dcharchannel0,id=3Dch= annel0,name=3Dcom.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 o= nly >>> 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 =3D off >> vectors =3D 2 >> class =3D 0x780 >> indirect_desc =3D on >> event_idx =3D on >> max_ports =3D 31 >> addr =3D 04.0 >> romfile =3D >> rombar =3D 1 >> multifunction =3D off >> command_serr_enable =3D on >> class Class 0780, addr 00:04.0, pci id 1af4:1003 (sub 1af4:00= 03) >> 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 =3D 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 lin= e > 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=3Did0 to creat= e=20 a virtio-serial-bus called id0.0 is that right? Thanks, Fred > >> Sorry for that, >> Fred >> >>>> -----Original Message----- >>>> From: qemu-devel-bounces+libaiqing=3Dhuawei.com@nongnu.org >>>> [mailto:qemu-devel-bounces+libaiqing=3Dhuawei.com@nongnu.org] On Beh= alf >>>> 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=E4rber >>>> Subject: Re: [Qemu-devel] [PATCH] qdev: Fix device_add bus assumptio= ns >>>> >>>> On Thu, 18 Apr 2013 10:41:56 +0200 >>>> Igor Mammedov wrote: >>>> >>>> [...] >>>>>> - if (!bus) { >>>>>> - bus =3D 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 =3D 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->na= me); >>>>> 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, DeviceStat= e >>>>> *dev, >>>>> int indent) qdev_print_props(mon, dev, DEVICE_CLASS(class)->props, >>>>> indent); >>>>> class =3D object_class_get_parent(class); >>>>> } while (class !=3D 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 =3D DEVICE(object_new(driver)); >>>>>> - qdev_set_parent_bus(qdev, bus); >>>>>> + >>>>>> + if (bus) { >>>>>> + qdev_set_parent_bus(qdev, bus); >>>>>> + } >>>>>> id =3D qemu_opts_id(opts); >>>>>> if (id) { >