| Message ID | 1391609340-11023-1-git-send-email-afaerber@suse.de |
|---|---|
| State | New |
| Headers | show |
Il 05/02/2014 15:09, Andreas Färber ha scritto: > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 82a9123..14c8765 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -131,21 +131,27 @@ DeviceState *qdev_create(BusState *bus, const char *name) > DeviceState *qdev_try_create(BusState *bus, const char *type) > { > DeviceState *dev; > + ObjectClass *oc; > + DeviceClass *dc; > > - if (object_class_by_name(type) == NULL) { > + oc = object_class_by_name(type); > + if (oc == NULL) { > return NULL; > } > + dc = DEVICE_CLASS(oc); > dev = DEVICE(object_new(type)); > if (!dev) { > return NULL; > } > > - if (!bus) { > + if (!bus && dc->bus_type && strcmp(dc->bus_type, "System") == 0) { Should you check instead if dev is-a TYPE_SYSBUS_DEVICE? Does this also leave the nand device out of info qtree, or is it still dumped? Paolo > bus = sysbus_get_default(); > } > > - qdev_set_parent_bus(dev, bus); > - object_unref(OBJECT(dev)); > + if (bus != NULL) { > + qdev_set_parent_bus(dev, bus); > + object_unref(OBJECT(dev)); > + } > return dev; > } > >
Am 05.02.2014 17:38, schrieb Paolo Bonzini: > Il 05/02/2014 15:09, Andreas Färber ha scritto: >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >> index 82a9123..14c8765 100644 >> --- a/hw/core/qdev.c >> +++ b/hw/core/qdev.c >> @@ -131,21 +131,27 @@ DeviceState *qdev_create(BusState *bus, const >> char *name) >> DeviceState *qdev_try_create(BusState *bus, const char *type) >> { >> DeviceState *dev; >> + ObjectClass *oc; >> + DeviceClass *dc; >> >> - if (object_class_by_name(type) == NULL) { >> + oc = object_class_by_name(type); >> + if (oc == NULL) { >> return NULL; >> } >> + dc = DEVICE_CLASS(oc); >> dev = DEVICE(object_new(type)); >> if (!dev) { >> return NULL; >> } >> >> - if (!bus) { >> + if (!bus && dc->bus_type && strcmp(dc->bus_type, "System") == 0) { > > Should you check instead if dev is-a TYPE_SYSBUS_DEVICE? Hmm, that would catch TYPE_SYS_BUS_DEVICE-derived types as well, which might in theory override bus_type. The ugly thing here is the cyclic dependency betwen qdev and sysbus, therefore no TYPE_SYSTEM_BUS. > Does this also leave the nand device out of info qtree, or is it still > dumped? As I pointed out several times already, devices that are not on a bus are generally *not* printed by info qtree. It depends on the old qbus concept. Therefore info qtree is not of much use anymore and should be dropped in favor of the qom-list and qom-get operations, which get us all devices and all properties. I had once attached a qom-tree for that and need to put that into a proper patch. Best before the next question pops up. :) Andreas
Am 05.02.2014 17:46, schrieb Andreas Färber: > Am 05.02.2014 17:38, schrieb Paolo Bonzini: >> Il 05/02/2014 15:09, Andreas Färber ha scritto: >>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >>> index 82a9123..14c8765 100644 >>> --- a/hw/core/qdev.c >>> +++ b/hw/core/qdev.c >>> @@ -131,21 +131,27 @@ DeviceState *qdev_create(BusState *bus, const >>> char *name) >>> DeviceState *qdev_try_create(BusState *bus, const char *type) >>> { >>> DeviceState *dev; >>> + ObjectClass *oc; >>> + DeviceClass *dc; >>> >>> - if (object_class_by_name(type) == NULL) { >>> + oc = object_class_by_name(type); >>> + if (oc == NULL) { >>> return NULL; >>> } >>> + dc = DEVICE_CLASS(oc); >>> dev = DEVICE(object_new(type)); >>> if (!dev) { >>> return NULL; >>> } >>> >>> - if (!bus) { >>> + if (!bus && dc->bus_type && strcmp(dc->bus_type, "System") == 0) { >> >> Should you check instead if dev is-a TYPE_SYSBUS_DEVICE? > > Hmm, that would catch TYPE_SYS_BUS_DEVICE-derived types as well, which > might in theory override bus_type. > > The ugly thing here is the cyclic dependency betwen qdev and sysbus, > therefore no TYPE_SYSTEM_BUS. BTW this patch became optional due to the one-line nand fix (which also does not show the device in legacy info qtree), but I am still in favor of merging this or a variation thereof to avoid future troubles. Andreas > >> Does this also leave the nand device out of info qtree, or is it still >> dumped? > > As I pointed out several times already, devices that are not on a bus > are generally *not* printed by info qtree. It depends on the old qbus > concept. Therefore info qtree is not of much use anymore and should be > dropped in favor of the qom-list and qom-get operations, which get us > all devices and all properties. I had once attached a qom-tree for that > and need to put that into a proper patch. Best before the next question > pops up. :) > > Andreas >
On Thu, Feb 6, 2014 at 2:46 AM, Andreas Färber <afaerber@suse.de> wrote: > Am 05.02.2014 17:38, schrieb Paolo Bonzini: >> Il 05/02/2014 15:09, Andreas Färber ha scritto: >>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >>> index 82a9123..14c8765 100644 >>> --- a/hw/core/qdev.c >>> +++ b/hw/core/qdev.c >>> @@ -131,21 +131,27 @@ DeviceState *qdev_create(BusState *bus, const >>> char *name) >>> DeviceState *qdev_try_create(BusState *bus, const char *type) >>> { >>> DeviceState *dev; >>> + ObjectClass *oc; >>> + DeviceClass *dc; >>> >>> - if (object_class_by_name(type) == NULL) { >>> + oc = object_class_by_name(type); >>> + if (oc == NULL) { >>> return NULL; >>> } >>> + dc = DEVICE_CLASS(oc); >>> dev = DEVICE(object_new(type)); >>> if (!dev) { >>> return NULL; >>> } >>> >>> - if (!bus) { >>> + if (!bus && dc->bus_type && strcmp(dc->bus_type, "System") == 0) { >> >> Should you check instead if dev is-a TYPE_SYSBUS_DEVICE? > > Hmm, that would catch TYPE_SYS_BUS_DEVICE-derived types as well, which > might in theory override bus_type. > > The ugly thing here is the cyclic dependency betwen qdev and sysbus, > therefore no TYPE_SYSTEM_BUS. > >> Does this also leave the nand device out of info qtree, or is it still >> dumped? > 2c: Leaving it out would be a good thing. No information is better than wrong information - NAND devices are not on sysbus. Regards, Peter > As I pointed out several times already, devices that are not on a bus > are generally *not* printed by info qtree. It depends on the old qbus > concept. Therefore info qtree is not of much use anymore and should be > dropped in favor of the qom-list and qom-get operations, which get us > all devices and all properties. I had once attached a qom-tree for that > and need to put that into a proper patch. Best before the next question > pops up. :) > > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >
diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 82a9123..14c8765 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -131,21 +131,27 @@ DeviceState *qdev_create(BusState *bus, const char *name) DeviceState *qdev_try_create(BusState *bus, const char *type) { DeviceState *dev; + ObjectClass *oc; + DeviceClass *dc; - if (object_class_by_name(type) == NULL) { + oc = object_class_by_name(type); + if (oc == NULL) { return NULL; } + dc = DEVICE_CLASS(oc); dev = DEVICE(object_new(type)); if (!dev) { return NULL; } - if (!bus) { + if (!bus && dc->bus_type && strcmp(dc->bus_type, "System") == 0) { bus = sysbus_get_default(); } - qdev_set_parent_bus(dev, bus); - object_unref(OBJECT(dev)); + if (bus != NULL) { + qdev_set_parent_bus(dev, bus); + object_unref(OBJECT(dev)); + } return dev; }
Commit 7426aa72c36c908a7d0eae3e38568bb0a70de479 (nand: Don't inherit from Sysbus) made NAND a device rather than SysBus device. This led to all boards using the device (akita, borzoi, spitz, terrier, tosa, axis-dev88) crashing on "info qtree". The difference to the bus-less ARMCPU is that the device was still being created by qdev_create() rather than object_new(), which makes assumptions about a NULL BusState being the SysBus. Fix that. A longer-term solution will be to stop using qdev_create(). Reported-by: Markus Armbruster <armbru@redhat.com> Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com> Signed-off-by: Andreas Färber <afaerber@suse.de> --- hw/core/qdev.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)