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