Paolo Bonzini <pbonz...@redhat.com> writes:
> Il 07/12/2013 00:27, Bandan Das ha scritto:
>> Paolo Bonzini <pbonz...@redhat.com> writes:
>>
>>> Resetting should be done in post-order, not pre-order. However,
>>> qdev_walk_children and qbus_walk_children do not allow this. Fix
>>> it by adding two extra arguments to the functions.
>>> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
>>> ---
>>> hw/core/qdev.c | 45 +++++++++++++++++++++++++++++++++------------
>>> include/hw/qdev-core.h | 13 +++++++++----
>>> 2 files changed, 42 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>> index 758de9f..1c114b7 100644
>>> --- a/hw/core/qdev.c
>>> +++ b/hw/core/qdev.c
>>> @@ -240,12 +240,12 @@ static int qbus_reset_one(BusState *bus, void *opaque)
>>>
>>> void qdev_reset_all(DeviceState *dev)
>>> {
>>> - qdev_walk_children(dev, qdev_reset_one, qbus_reset_one, NULL);
>>> + qdev_walk_children(dev, qdev_reset_one, qbus_reset_one, NULL, NULL,
>>> NULL);
>>> }
>>>
>>> void qbus_reset_all(BusState *bus)
>>> {
>>> - qbus_walk_children(bus, qdev_reset_one, qbus_reset_one, NULL);
>>> + qbus_walk_children(bus, qdev_reset_one, qbus_reset_one, NULL, NULL,
>>> NULL);
>>> }
>>>
>>> void qbus_reset_all_fn(void *opaque)
>>> @@ -343,49 +343,70 @@ BusState *qdev_get_child_bus(DeviceState *dev, const
>>> char *name)
>>> return NULL;
>>> }
>>>
>>> -int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn,
>>> - qbus_walkerfn *busfn, void *opaque)
>>> +int qbus_walk_children(BusState *bus,
>>> + qdev_walkerfn *pre_devfn, qbus_walkerfn *pre_busfn,
>>> + qdev_walkerfn *post_devfn, qbus_walkerfn
>>> *post_busfn,
>>> + void *opaque)
>>> {
>>
>> It's either pre or post right ? I also thought that the traversal
>> applies to the whole hierarchy not just buses or devices individually..
>> Why not just have a single parameter that says pre or post, not much
>> difference but atleast one parameter less.
>
> This is a generic walk function.
> For reset you want post-order, but in other cases pre-order may make
> more sense, for example realize.
Sorry, not sure if I get it. What I meant was will this work ?
int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn,
- qbus_walkerfn *busfn, void *opaque)
+ qbus_walkerfn *busfn, bool ispostorder, void *opaque)
{
BusChild *kid;
int err;
- if (busfn) {
+ if (!ispostorder && busfn) {
err = busfn(bus, opaque);
if (err) {
return err;
@@ -351,17 +351,24 @@ int qbus_walk_children(BusState *bus, qdev_walkerfn
*devfn,
}
QTAILQ_FOREACH(kid, &bus->children, sibling) {
- err = qdev_walk_children(kid->child, devfn, busfn, opaque);
+ err = qdev_walk_children(kid->child, devfn, busfn, ispostorder,
opaque);
if (err < 0) {
return err;
}
}
+ if (ispostorder && busfn) {
+ err = busfn(bus, opaque);
+ if (err) {
+ return err;
+ }
+ }
+
return 0;
}
Or there's a case where we would like to traverse pre for parent and post
for children's buses (or something similar)..
> Paolo
>
>> Bandan
>>
>>> BusChild *kid;
>>> int err;
>>>
>>> - if (busfn) {
>>> - err = busfn(bus, opaque);
>>> + if (pre_busfn) {
>>> + err = pre_busfn(bus, opaque);
>>> if (err) {
>>> return err;
>>> }
>>> }
>>>
>>> QTAILQ_FOREACH(kid, &bus->children, sibling) {
>>> - err = qdev_walk_children(kid->child, devfn, busfn, opaque);
>>> + err = qdev_walk_children(kid->child,
>>> + pre_devfn, pre_busfn,
>>> + post_devfn, post_busfn, opaque);
>>> if (err < 0) {
>>> return err;
>>> }
>>> }
>>>
>>> + if (post_busfn) {
>>> + err = post_busfn(bus, opaque);
>>> + if (err) {
>>> + return err;
>>> + }
>>> + }
>>> +
>>> return 0;
>>> }
>>>
>>> -int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn,
>>> - qbus_walkerfn *busfn, void *opaque)
>>> +int qdev_walk_children(DeviceState *dev,
>>> + qdev_walkerfn *pre_devfn, qbus_walkerfn *pre_busfn,
>>> + qdev_walkerfn *post_devfn, qbus_walkerfn
>>> *post_busfn,
>>> + void *opaque)
>>> {
>>> BusState *bus;
>>> int err;
>>>
>>> - if (devfn) {
>>> - err = devfn(dev, opaque);
>>> + if (pre_devfn) {
>>> + err = pre_devfn(dev, opaque);
>>> if (err) {
>>> return err;
>>> }
>>> }
>>>
>>> QLIST_FOREACH(bus, &dev->child_bus, sibling) {
>>> - err = qbus_walk_children(bus, devfn, busfn, opaque);
>>> + err = qbus_walk_children(bus, pre_devfn, pre_busfn,
>>> + post_devfn, post_busfn, opaque);
>>> if (err < 0) {
>>> return err;
>>> }
>>> }
>>>
>>> + if (post_devfn) {
>>> + err = post_devfn(dev, opaque);
>>> + if (err) {
>>> + return err;
>>> + }
>>> + }
>>> +
>>> return 0;
>>> }
>>>
>>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>>> index d840f06..21ea2c6 100644
>>> --- a/include/hw/qdev-core.h
>>> +++ b/include/hw/qdev-core.h
>>> @@ -274,10 +274,15 @@ BusState *qbus_create(const char *typename,
>>> DeviceState *parent, const char *nam
>>> /* Returns > 0 if either devfn or busfn skip walk somewhere in cursion,
>>> * < 0 if either devfn or busfn terminate walk somewhere in
>>> cursion,
>>> * 0 otherwise. */
>>> -int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn,
>>> - qbus_walkerfn *busfn, void *opaque);
>>> -int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn,
>>> - qbus_walkerfn *busfn, void *opaque);
>>> +int qbus_walk_children(BusState *bus,
>>> + qdev_walkerfn *pre_devfn, qbus_walkerfn *pre_busfn,
>>> + qdev_walkerfn *post_devfn, qbus_walkerfn
>>> *post_busfn,
>>> + void *opaque);
>>> +int qdev_walk_children(DeviceState *dev,
>>> + qdev_walkerfn *pre_devfn, qbus_walkerfn *pre_busfn,
>>> + qdev_walkerfn *post_devfn, qbus_walkerfn
>>> *post_busfn,
>>> + void *opaque);
>>> +
>>> void qdev_reset_all(DeviceState *dev);
>>>
>>> /**
