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.
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);
>>
>> /**
