On 7/31/19 11:29 AM, 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).
If the boolean is not meaningful, we can use an enum...
> 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
>
> Damien
>
