Re: [Qemu-devel] [PATCH 1/2] qdev: make reset propagation overrideable by subclasses

[Andreas Färber]

Am 11.01.2013 15:17, schrieb Anthony Liguori:
> Signed-off-by: Anthony Liguori <address@hidden>
> ---
>  hw/qdev-core.h | 16 +++++++++++++++-
>  hw/qdev.c      | 11 ++++++++++-
>  2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/qdev-core.h b/hw/qdev-core.h
> index 853bd08..f40fd15 100644
> --- a/hw/qdev-core.h
> +++ b/hw/qdev-core.h
> @@ -36,9 +36,23 @@ typedef struct DeviceClass {
>      Property *props;
>      int no_user;
>  
> -    /* callbacks */
> +    /**
> +     * reset:
> +     *
> +     * Cold reset of a device. This function must be implemented by a 
> device's
> +     * that never have children busses.
> +     */
>      void (*reset)(DeviceState *dev);
>  
> +    /**
> +     * reset_all:
> +     *
> +     * Cold reset of a device with children.  The default implementation of 
> this
> +     * method will invoke DeviceClass::reset and then recursively call
> +     * qbus_reset_all() on each child in an arbitrary order.
> +     */
> +    void (*reset_all)(DeviceState *dev);

This documentation is bogus, it should go into DeviceClass as @reset and
@reset_all since these are struct fields, not standalone functions.

This also happens to conflict with the realize/unrealize callbacks being
added, v3 adding DeviceClass documentation on Eduardo's request. Can you
please apply mine first?

No objections to adding such a second reset callback. Maybe add a
DeviceReset typedef to facilitate overriding these in subclasses?

Regards,
Andreas

> +
>      /* device state */
>      const struct VMStateDescription *vmsd;
>  
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 1b68d02..e02b5be 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -223,11 +223,17 @@ static int qbus_reset_one(BusState *bus, void *opaque)
>      return 0;
>  }
>  
> -void qdev_reset_all(DeviceState *dev)
> +static void qdev_reset_children(DeviceState *dev)
>  {
>      qdev_walk_children(dev, qdev_reset_one, qbus_reset_one, NULL);
>  }
>  
> +void qdev_reset_all(DeviceState *dev)
> +{
> +    DeviceClass *dc = DEVICE_GET_CLASS(dev);
> +    dc->reset_all(dev);
> +}
> +
>  void qbus_reset_all(BusState *bus)
>  {
>      qbus_walk_children(bus, qdev_reset_one, qbus_reset_one, NULL);
> @@ -714,7 +720,10 @@ static void device_unparent(Object *obj)
>  
>  static void device_class_init(ObjectClass *class, void *data)
>  {
> +    DeviceClass *dc = DEVICE_CLASS(class);
> +
>      class->unparent = device_unparent;
> +    dc->reset_all = qdev_reset_children;
>  }
>  
>  void device_reset(DeviceState *dev)

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg