Am 18.04.2013 10:41, schrieb Igor Mammedov:
> On Tue, 16 Apr 2013 03:50:21 +0200
> Andreas Färber <afaer...@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 <afaer...@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
