Patchwork [3/4] qdev: allow both pre- and post-order vists in qdev walking functions

login
register
mail settings
Submitter Paolo Bonzini
Date Dec. 6, 2013, 4:54 p.m.
Message ID <1386348867-25038-4-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/298103/
State New
Headers show

Comments

Paolo Bonzini - Dec. 6, 2013, 4:54 p.m.
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 <pbonzini@redhat.com>
---
 hw/core/qdev.c         |   45 +++++++++++++++++++++++++++++++++------------
 include/hw/qdev-core.h |   13 +++++++++----
 2 files changed, 42 insertions(+), 16 deletions(-)
Bandan Das - Dec. 6, 2013, 11:27 p.m.
Paolo Bonzini <pbonzini@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 <pbonzini@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.

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);
>  
>  /**
Paolo Bonzini - Dec. 9, 2013, 9:50 a.m.
Il 07/12/2013 00:27, Bandan Das ha scritto:
> Paolo Bonzini <pbonzini@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 <pbonzini@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);
>>  
>>  /**
Bandan Das - Dec. 9, 2013, 5:56 p.m.
Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 07/12/2013 00:27, Bandan Das ha scritto:
>> Paolo Bonzini <pbonzini@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 <pbonzini@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);
>>>  
>>>  /**
Paolo Bonzini - Dec. 9, 2013, 6:04 p.m.
Il 09/12/2013 18:56, Bandan Das ha scritto:
>> > 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)

Yes, but it is a bit less flexible.

> Or there's a case where we would like to traverse pre for parent and post
> for children's buses (or something similar)..

Probably not, but there may be a case where you want both pre and post
(i.e. before and after).  For example a "display tree" functionality
where the post-order callbacks simply decrement the current indentation.

Paolo
Paolo Bonzini - Dec. 9, 2013, 6:05 p.m.
Il 09/12/2013 18:56, Bandan Das ha scritto:
>> > 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)

Yes, but it is a bit less flexible.

If we want to remove the flexibility, I'd rather have two separate
functions for pre- and post-order, not a new "bool" parameter.

> Or there's a case where we would like to traverse pre for parent and post
> for children's buses (or something similar)..

Probably not, but there may be a case where you want both pre and post
(i.e. before and after).  For example a "display tree" functionality
where the post-order callbacks simply decrement the current indentation.

Paolo
Bandan Das - Dec. 9, 2013, 6:15 p.m.
Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 09/12/2013 18:56, Bandan Das ha scritto:
>>> > 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)
>
> Yes, but it is a bit less flexible.

Well, my motivation was that very soon we will have an infinite number of
arguments to the walk functions (oh wait! we already have) :)

>> Or there's a case where we would like to traverse pre for parent and post
>> for children's buses (or something similar)..
>
> Probably not, but there may be a case where you want both pre and post
> (i.e. before and after).  For example a "display tree" functionality
> where the post-order callbacks simply decrement the current indentation.

Oh so there is such a case. Ok, got it now, thanks!

> Paolo

Patch

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