From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52780) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UUGuP-0007Eg-BI for qemu-devel@nongnu.org; Mon, 22 Apr 2013 09:35:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UUGuN-0008QX-Hu for qemu-devel@nongnu.org; Mon, 22 Apr 2013 09:35:53 -0400 Received: from cantor2.suse.de ([195.135.220.15]:57892 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UUGuN-0008Q9-6f for qemu-devel@nongnu.org; Mon, 22 Apr 2013 09:35:51 -0400 Message-ID: <51753CB5.80607@suse.de> Date: Mon, 22 Apr 2013 15:35:49 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1366077021-28882-1-git-send-email-afaerber@suse.de> <20130418104156.7a5349f0@nial.usersys.redhat.com> In-Reply-To: <20130418104156.7a5349f0@nial.usersys.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 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: Igor Mammedov Cc: ehabkost@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, lcapitulino@redhat.com, anthony@codemonkey.ws, pbonzini@redhat.com Am 18.04.2013 10:41, schrieb Igor Mammedov: > On Tue, 16 Apr 2013 03:50:21 +0200 > Andreas F=E4rber wrote: >=20 >> Drop an unreachable fallback bus assignment to SysBus. >> >> If no ,bus=3D is specified, only search busses recursively for bus typ= e if >> the DeviceClass has a bus_type specified. Handle resulting NULL cases. >> >> Signed-off-by: Andreas F=E4rber >> --- >> 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 @@ >> */ >> =20 >> #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 =3D NULL; >> =20 >> driver =3D 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 !=3D NULL) { >> bus =3D qbus_find_recursive(sysbus_get_default(), NULL, k->bu= s_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; >> } >> =20 >> - if (!bus) { >> - bus =3D sysbus_get_default(); >> - } >> - > I've checked all direct childs of TYPE_DEVICE and they all set k->bus_t= ype, > 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 s= et to not > NULL in qdev_unplug() and qdev_print() >=20 > It would be better to add something like this: > // untested >=20 > 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); > =20 > - 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 *d= ev, 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); > } >=20 >> /* create device, set properties */ >> qdev =3D DEVICE(object_new(driver)); >> - qdev_set_parent_bus(qdev, bus); >> + >> + if (bus) { >> + qdev_set_parent_bus(qdev, bus); >> + } >> =20 >> id =3D qemu_opts_id(opts); >> if (id) { >=20 >=20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg