On Wed, Jul 31, 2019 at 11:29:36AM +0200, Damien Hedde wrote:
>
>
> On 7/31/19 8:05 AM, David Gibson wrote:
> > On Mon, Jul 29, 2019 at 04:56:26PM +0200, Damien Hedde wrote:
> >> Deprecate old reset apis and make them use the new one while they
> >> are still used somewhere.
> >>
> >> Signed-off-by: Damien Hedde <damien.he...@greensocs.com>
> >> ---
> >> hw/core/qdev.c | 22 +++-------------------
> >> include/hw/qdev-core.h | 28 ++++++++++++++++++++++------
> >> 2 files changed, 25 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> >> index 559ced070d..e9e5f2d5f9 100644
> >> --- a/hw/core/qdev.c
> >> +++ b/hw/core/qdev.c
> >> @@ -312,25 +312,9 @@ static void device_foreach_reset_child(Object *obj,
> >> void (*func)(Object *))
> >> }
> >> }
> >>
> >> -static int qdev_reset_one(DeviceState *dev, void *opaque)
> >> -{
> >> - device_legacy_reset(dev);
> >> -
> >> - return 0;
> >> -}
> >> -
> >> -static int qbus_reset_one(BusState *bus, void *opaque)
> >> -{
> >> - BusClass *bc = BUS_GET_CLASS(bus);
> >> - if (bc->reset) {
> >> - bc->reset(bus);
> >> - }
> >> - return 0;
> >> -}
> >> -
> >> void qdev_reset_all(DeviceState *dev)
> >> {
> >> - qdev_walk_children(dev, NULL, NULL, qdev_reset_one, qbus_reset_one,
> >> NULL);
> >> + device_reset(dev, false);
> >> }
> >>
> >> void qdev_reset_all_fn(void *opaque)
> >> @@ -340,7 +324,7 @@ void qdev_reset_all_fn(void *opaque)
> >>
> >> void qbus_reset_all(BusState *bus)
> >> {
> >> - qbus_walk_children(bus, NULL, NULL, qdev_reset_one, qbus_reset_one,
> >> NULL);
> >> + bus_reset(bus, false);
> >> }
> >>
> >> void qbus_reset_all_fn(void *opaque)
> >> @@ -922,7 +906,7 @@ static void device_set_realized(Object *obj, bool
> >> value, Error **errp)
> >> }
> >> }
> >> if (dev->hotplugged) {
> >> - device_legacy_reset(dev);
> >> + device_reset(dev, true);
> >
> > So.. is this change in the device_reset() signature really necessary?
> > Even if there are compelling reasons to handle warm reset in the new
> > API, that doesn't been you need to change device_reset() itself from
> > its established meaning of a cold (i.e. as per power cycle) reset.
> > Warm resets are generally called in rather more specific circumstances
> > (often under guest software direction) so it seems likely that users
> > would want to engage with the new reset API directly. Or we could
> > just create a device_warm_reset() wrapper. That would also avoid the
> > bare boolean parameter, which is not great for readability (you have
> > to look up the signature to have any idea what it means).
>
> I've added device_reset_cold/warm wrapper functions to avoid having to
> pass the boolean parameter. it seems I forgot to use them in qdev.c
> I suppose, like you said, we could live with
> + no function with the boolean parameter
> + device_reset doing cold reset
> + device_reset_warm (or device_warm_reset) for the warm version
Ok, good.
I'm afraid the whole series still makes me pretty uncomfortable,
though, since the whole "warm reset" concept still seems way to vague
to me.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
