On Fri, 06 Dec 2013 18:26:37 +0100
Paolo Bonzini <pbonz...@redhat.com> wrote:
> 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 <imamm...@redhat.com>
> > ---
> > 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 <imamm...@redhat.com>,
> > + *
> > + * 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?
Marcel and Li suggested s/device/interface/ what do you think about it?
>
> > + .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 <imamm...@redhat.com>,
> > + *
> > + * 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.
Since there were no users for it, I've dropped it. But I can sure add it.
>
> > +/**
> > + * 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
> >
>
