[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 05/33] Switch to new api in qdev/bus
From: |
Damien Hedde |
Subject: |
Re: [Qemu-devel] [PATCH v3 05/33] Switch to new api in qdev/bus |
Date: |
Wed, 31 Jul 2019 11:29:36 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.1 |
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 <address@hidden>
>> ---
>> 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
Damien