On Mon, 20 Jan 2014 13:36:39 +0200 "Michael S. Tsirkin" <m...@redhat.com> wrote:
> On Tue, Jan 14, 2014 at 05:55:54PM +0100, Igor Mammedov wrote: > > make qdev_unplug()/device_set_realized() to call hotplug handler's > > plug/unplug methods if available and remove not needed anymore > > hot(un)plug handling from PCIDevice. > > > > In case if hotplug handler is not available, revert to the legacy > > hotplug method. > > When isn't it available? > For buses other than PCI? For example scsi virtio usb buses. Paolo said he would convert scsi bus to this interface once series is in. Eventually all hotluggable buses will/should be converted to new interface so that we could get rid of legacy implementation. > > > Signed-off-by: Igor Mammedov <imamm...@redhat.com> > > --- > > v2: > > * fix test-qdev-global-props build failure during "make check" > > --- > > hw/core/qdev.c | 17 +++++++++++++---- > > hw/pci/pci.c | 29 +---------------------------- > > include/hw/pci/pci.h | 10 ---------- > > include/hw/pci/pci_bus.h | 2 -- > > tests/Makefile | 2 +- > > 5 files changed, 15 insertions(+), 45 deletions(-) > > > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > > index d8b83f1..3486e5d 100644 > > --- a/hw/core/qdev.c > > +++ b/hw/core/qdev.c > > @@ -213,7 +213,6 @@ void qdev_unplug(DeviceState *dev, Error **errp) > > error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name); > > return; > > } > > - assert(dc->unplug != NULL); > > > > if (!dc->hotpluggable) { > > error_set(errp, QERR_DEVICE_NO_HOTPLUG, > > @@ -223,9 +222,13 @@ void qdev_unplug(DeviceState *dev, Error **errp) > > > > qdev_hot_removed = true; > > > > - if (dc->unplug(dev) < 0) { > > - error_set(errp, QERR_UNDEFINED_ERROR); > > - return; > > + if (dev->parent_bus && dev->parent_bus->hotplug_handler) { > > + hotplug_handler_unplug(dev->parent_bus->hotplug_handler, dev, > > errp); > > + } else { > > + assert(dc->unplug != NULL); > > + if (dc->unplug(dev) < 0) { /* legacy handler */ > > + error_set(errp, QERR_UNDEFINED_ERROR); > > + } > > } > > } > > > > @@ -720,6 +723,12 @@ static void device_set_realized(Object *obj, bool > > value, Error **err) > > dc->realize(dev, &local_err); > > } > > > > + if (dev->parent_bus && dev->parent_bus->hotplug_handler && > > + local_err == NULL) { > > + hotplug_handler_plug(dev->parent_bus->hotplug_handler, > > + dev, &local_err); > > + } > > + > > if (qdev_get_vmsd(dev) && local_err == NULL) { > > vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), > > dev, > > dev->instance_id_alias, > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > > index b8770ef..7e36f29 100644 > > --- a/hw/pci/pci.c > > +++ b/hw/pci/pci.c > > @@ -35,6 +35,7 @@ > > #include "hw/pci/msi.h" > > #include "hw/pci/msix.h" > > #include "exec/address-spaces.h" > > +#include "hw/hotplug.h" > > > > //#define DEBUG_PCI > > #ifdef DEBUG_PCI > > @@ -346,13 +347,6 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, > > pci_map_irq_fn map_irq, > > bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0])); > > } > > > > -void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState > > *qdev) > > -{ > > - bus->qbus.allow_hotplug = 1; > > - bus->hotplug = hotplug; > > - bus->hotplug_qdev = qdev; > > -} > > - > > PCIBus *pci_register_bus(DeviceState *parent, const char *name, > > pci_set_irq_fn set_irq, pci_map_irq_fn map_irq, > > void *irq_opaque, > > @@ -1750,29 +1744,9 @@ static int pci_qdev_init(DeviceState *qdev) > > } > > pci_add_option_rom(pci_dev, is_default_rom); > > > > - if (bus->hotplug) { > > - /* Let buses differentiate between hotplug and when device is > > - * enabled during qemu machine creation. */ > > - rc = bus->hotplug(bus->hotplug_qdev, pci_dev, > > - qdev->hotplugged ? PCI_HOTPLUG_ENABLED: > > - PCI_COLDPLUG_ENABLED); > > - if (rc != 0) { > > - int r = pci_unregister_device(&pci_dev->qdev); > > - assert(!r); > > - return rc; > > - } > > - } > > return 0; > > } > > > > -static int pci_unplug_device(DeviceState *qdev) > > -{ > > - PCIDevice *dev = PCI_DEVICE(qdev); > > - > > - return dev->bus->hotplug(dev->bus->hotplug_qdev, dev, > > - PCI_HOTPLUG_DISABLED); > > -} > > - > > PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool > > multifunction, > > const char *name) > > { > > @@ -2243,7 +2217,6 @@ static void pci_device_class_init(ObjectClass *klass, > > void *data) > > { > > DeviceClass *k = DEVICE_CLASS(klass); > > k->init = pci_qdev_init; > > - k->unplug = pci_unplug_device; > > k->exit = pci_unregister_device; > > k->bus_type = TYPE_PCI_BUS; > > k->props = pci_props; > > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > > index 03d4bee..c709d62 100644 > > --- a/include/hw/pci/pci.h > > +++ b/include/hw/pci/pci.h > > @@ -327,15 +327,6 @@ typedef void (*pci_set_irq_fn)(void *opaque, int > > irq_num, int level); > > typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num); > > typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin); > > > > -typedef enum { > > - PCI_HOTPLUG_DISABLED, > > - PCI_HOTPLUG_ENABLED, > > - PCI_COLDPLUG_ENABLED, > > -} PCIHotplugState; > > - > > -typedef int (*pci_hotplug_fn)(DeviceState *qdev, PCIDevice *pci_dev, > > - PCIHotplugState state); > > - > > #define TYPE_PCI_BUS "PCI" > > #define PCI_BUS(obj) OBJECT_CHECK(PCIBus, (obj), TYPE_PCI_BUS) > > #define TYPE_PCIE_BUS "PCIE" > > @@ -354,7 +345,6 @@ PCIBus *pci_bus_new(DeviceState *parent, const char > > *name, > > void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn > > map_irq, > > void *irq_opaque, int nirq); > > int pci_bus_get_irq_level(PCIBus *bus, int irq_num); > > -void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState > > *dev); > > /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */ > > int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin); > > PCIBus *pci_register_bus(DeviceState *parent, const char *name, > > diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h > > index 9df1788..fabaeee 100644 > > --- a/include/hw/pci/pci_bus.h > > +++ b/include/hw/pci/pci_bus.h > > @@ -16,8 +16,6 @@ struct PCIBus { > > pci_set_irq_fn set_irq; > > pci_map_irq_fn map_irq; > > pci_route_irq_fn route_intx_to_irq; > > - pci_hotplug_fn hotplug; > > - DeviceState *hotplug_qdev; > > void *irq_opaque; > > PCIDevice *devices[PCI_SLOT_MAX * PCI_FUNC_MAX]; > > PCIDevice *parent_dev; > > diff --git a/tests/Makefile b/tests/Makefile > > index 0aaf657..0bbdecb 100644 > > --- a/tests/Makefile > > +++ b/tests/Makefile > > @@ -163,7 +163,7 @@ tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o > > xbzrle.o page_cache.o libqemuuti > > tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o > > tests/test-int128$(EXESUF): tests/test-int128.o > > tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \ > > - hw/core/qdev.o hw/core/qdev-properties.o \ > > + hw/core/qdev.o hw/core/qdev-properties.o hw/core/hotplug.o\ > > hw/core/irq.o \ > > $(qom-core-obj) \ > > $(test-qapi-obj-y) \ > > -- > > 1.7.1 >