From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43009) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vp4nm-0007xH-Tb for qemu-devel@nongnu.org; Fri, 06 Dec 2013 18:27:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vp4ng-00032V-TW for qemu-devel@nongnu.org; Fri, 06 Dec 2013 18:27:18 -0500 Received: from mx1.redhat.com ([209.132.183.28]:21968) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vp4ng-00032I-Kk for qemu-devel@nongnu.org; Fri, 06 Dec 2013 18:27:12 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rB6NRAPl013352 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 6 Dec 2013 18:27:10 -0500 From: Bandan Das References: <1386348867-25038-1-git-send-email-pbonzini@redhat.com> <1386348867-25038-4-git-send-email-pbonzini@redhat.com> Date: Fri, 06 Dec 2013 18:27:08 -0500 In-Reply-To: <1386348867-25038-4-git-send-email-pbonzini@redhat.com> (Paolo Bonzini's message of "Fri, 6 Dec 2013 17:54:26 +0100") Message-ID: MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 3/4] qdev: allow both pre- and post-order vists in qdev walking functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, mst@redhat.com Paolo Bonzini 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 > --- > 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); > > /**