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

[Marcel Apfelbaum <marcel.a@redhat.com>]

On Mon, 2013-12-09 at 13:40 +0800, Li Guang wrote:
> Hi, Igor
> 
> Igor Mammedov wrote:
> > Provide generic hotplug interface for devices.
> > Intended for replacing hotplug mechanism used by
> > PCI/PCIE/SHPC code.
> >
> > Signed-off-by: Igor Mammedov<imammedo@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<imammedo@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,
> > +    .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<imammedo@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)
> > +
> >    
> 
> Hmm..., this is interface, but device, a bold opinion,
> can have something like TYPE_HOTPLUG_INTERFACE ... ?
Sorry, I did not see this before sending my comments,
I am completely agree with you on this.

Thanks,
Marcel

> 
> > +/**
> > + * hotplug_fn:
> > + * @hotplug_dev: a device performing hotplug/uplug action
> >    
> 
> s/uplug/unplug
> 
> 
> Thanks!
> Li Guang
> 
> > + * @hotplugged_dev: a device that has been hotplugged
> > + * @errp: returns an error if this function fails
> > + */
> > +typedef void (*hotplug_fn)(DeviceState *hotplug_dev,
> > +                           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.
> > + */
> > +typedef struct HotplugDeviceClass {
> > +    InterfaceClass parent;
> > +
> > +    hotplug_fn hotplug;
> > +    hotplug_fn hot_unplug;
> > +} HotplugDeviceClass;
> > +
> > +#endif
> >    
>