Re: [Qemu-devel] [PATCH 1/7] define hotplug interface

[Paolo Bonzini]

Il 06/12/2013 18:03, Igor Mammedov ha scritto:
> Provide generic hotplug interface for devices.
> Intended for replacing hotplug mechanism used by
> PCI/PCIE/SHPC code.
> 
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
> it's scsi-bus like interface, but abstracted from bus altogether
> since all current users care about in hotplug handlers, it's
> hotplug device and hotplugged device and bus only serves
> as a means to get access to hotplug device and it's callbacks.
> ---
>  hw/core/Makefile.objs |    1 +
>  hw/core/hotplug.c     |   25 ++++++++++++++++++++++++
>  include/hw/hotplug.h  |   50 
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 76 insertions(+), 0 deletions(-)
>  create mode 100644 hw/core/hotplug.c
>  create mode 100644 include/hw/hotplug.h
> 
> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
> index 950146c..47f6555 100644
> --- a/hw/core/Makefile.objs
> +++ b/hw/core/Makefile.objs
> @@ -10,4 +10,5 @@ common-obj-$(CONFIG_SOFTMMU) += sysbus.o
>  common-obj-$(CONFIG_SOFTMMU) += null-machine.o
>  common-obj-$(CONFIG_SOFTMMU) += loader.o
>  common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o
> +common-obj-$(CONFIG_SOFTMMU) += hotplug.o
>  
> diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c
> new file mode 100644
> index 0000000..3e84d9c
> --- /dev/null
> +++ b/hw/core/hotplug.c
> @@ -0,0 +1,25 @@
> +/*
> + * Hotplug device interface.
> + *
> + * Copyright (c) 2013 Red Hat Inc.
> + *
> + * Authors:
> + *  Igor Mammedov <address@hidden>,
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +#include "hw/hotplug.h"
> +
> +static const TypeInfo hotplug_device_info = {
> +    .name          = TYPE_HOTPLUG_DEVICE,

Perhaps replace "device" with "handler" throughout?

> +    .parent        = TYPE_INTERFACE,
> +    .class_size = sizeof(HotplugDeviceClass),
> +};
> +
> +static void hotplug_device_register_types(void)
> +{
> +    type_register_static(&hotplug_device_info);
> +}
> +
> +type_init(hotplug_device_register_types)
> diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
> new file mode 100644
> index 0000000..cfa79bb
> --- /dev/null
> +++ b/include/hw/hotplug.h
> @@ -0,0 +1,50 @@
> +/*
> + * Hotplug device interface.
> + *
> + * Copyright (c) 2013 Red Hat Inc.
> + *
> + * Authors:
> + *  Igor Mammedov <address@hidden>,
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +#ifndef HOTPLUG_H
> +#define HOTPLUG_H
> +
> +#include "hw/qdev-core.h"
> +
> +#define TYPE_HOTPLUG_DEVICE "hotplug-device"
> +
> +#define HOTPLUG_DEVICE_CLASS(klass) \
> +     OBJECT_CLASS_CHECK(HotplugDeviceClass, (klass), TYPE_HOTPLUG_DEVICE)
> +#define HOTPLUG_DEVICE_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(HotplugDeviceClass, (obj), TYPE_HOTPLUG_DEVICE)

Missing HOTPLUG_DEVICE macro, see include/hw/stream.h.

> +/**
> + * hotplug_fn:
> + * @hotplug_dev: a device performing hotplug/uplug action
> + * @hotplugged_dev: a device that has been hotplugged
> + * @errp: returns an error if this function fails
> + */
> +typedef void (*hotplug_fn)(DeviceState *hotplug_dev,

First argument should be HotplugDevice (which is a struct containing a
single Object field; again see include/hw/stream.h).

> +                           DeviceState *hotplugged_dev, Error **errp);
> +
> +/**
> + * HotplugDeviceClass:
> + *
> + * Interface to be implemented by a device performing
> + * hardware hotplug/unplug functions.
> + *
> + * @parent: Opaque parent interface.
> + * @hotplug: hotplug callback.
> + * @hot_unplug: hot unplug callback.

Just plug/unplug.  IIRC it is used for cold-plug too.

Otherwise looks great.

Paolo

> + */
> +typedef struct HotplugDeviceClass {
> +    InterfaceClass parent;
> +
> +    hotplug_fn hotplug;
> +    hotplug_fn hot_unplug;
> +} HotplugDeviceClass;
> +
> +#endif
>