On Tue, Dec 17, 2013 at 4:38 AM, Paolo Bonzini <pbonz...@redhat.com> wrote: > Il 17/12/2013 00:26, Anthony Liguori ha scritto: >> Sharing hot plug code is a good thing. Making hotplug a qdev-level >> concept seems like a bad thing to me. > > Can you explain what you mean?
The question is whether "hotpluggable" as a property applies to all devices or not. But hotplug is strictly a bus level concept. It's a sequence of events that correspond to what happens when you add a new device to a bus after power on. >> The series is a net add of code so I don't think we're winning anything >> by generalizing here. > > Any generalization that's used just once will be a net add of code (and > this code will be reused by SCSI and x86 memory hotplug at least; > perhaps x86 CPU hotplug too). The question is whether there can be code sharing without touching the base class. You could certainly have a HotpluggableBusState and then a HotpluggableDeviceState. Interfaces would be another option too. > Any generalization that requires some boilerplate code will be a net add > of code, too. QEMU being written in C, we unfortunately cannot avoid that. > > So I don't think that lines of code are a good metric. The general concern is about polluting widely used base classes. It's better if we can avoid adding things to DeviceState and Object whenever possible. Regards, Anthony Liguori > Paolo > >> Is there a use-case this enables that isn't possible today? >> >> Regards, >> >> Anthony Liguori >> >>> >>> Patches 8-11 are should be merged as one and are split only for >>> simplifying review (they compile fine but PCI hotplug is broken >>> until the last patch is applyed). >>> >>> git tree for testing: >>> https://github.com/imammedo/qemu/commits/hotplug_dev_inf_v3 >>> >>> tested only ACPI and PCIE hotplug. >>> >>> Hervé Poussineau (1): >>> qom: detect bad reentrance during object_class_foreach >>> >>> Igor Mammedov (9): >>> define hotplug interface >>> qdev: add to BusState "hotplug-handler" link >>> qdev: add "hotpluggable" property to Device >>> hw/acpi: move typeinfo to the file end >>> qdev:pci: refactor PCIDevice to use generic "hotpluggable" property >>> acpi/piix4pm: convert ACPI PCI hotplug to use hotplug-handler API >>> pci/shpc: convert SHPC hotplug to use hotplug-handler API >>> pci/pcie: convert PCIE hotplug to use hotplug-handler API >>> hw/pci: switch to a generic hotplug handling for PCIDevice >>> >>> Paolo Bonzini (1): >>> qom: do not register interface "types" in the type table >>> >>> hw/acpi/piix4.c | 151 >>> ++++++++++++++++++++++------------------- >>> hw/core/Makefile.objs | 1 + >>> hw/core/hotplug.c | 48 +++++++++++++ >>> hw/core/qdev.c | 50 ++++++++++++-- >>> hw/display/cirrus_vga.c | 2 +- >>> hw/display/qxl.c | 2 +- >>> hw/display/vga-pci.c | 2 +- >>> hw/display/vmware_vga.c | 2 +- >>> hw/i386/acpi-build.c | 6 +- >>> hw/ide/piix.c | 4 +- >>> hw/isa/piix4.c | 2 +- >>> hw/pci-bridge/pci_bridge_dev.c | 9 +++ >>> hw/pci-host/piix.c | 6 +- >>> hw/pci/pci.c | 40 +---------- >>> hw/pci/pcie.c | 73 +++++++++++++------- >>> hw/pci/pcie_port.c | 8 +++ >>> hw/pci/shpc.c | 133 +++++++++++++++++++++++------------- >>> hw/usb/hcd-ehci-pci.c | 2 +- >>> hw/usb/hcd-ohci.c | 2 +- >>> hw/usb/hcd-uhci.c | 2 +- >>> hw/usb/hcd-xhci.c | 2 +- >>> include/hw/hotplug.h | 75 ++++++++++++++++++++ >>> include/hw/pci/pci.h | 13 ---- >>> include/hw/pci/pci_bus.h | 2 - >>> include/hw/pci/pcie.h | 5 ++ >>> include/hw/pci/shpc.h | 8 +++ >>> include/hw/qdev-core.h | 8 +++ >>> qom/object.c | 17 ++++- >>> 28 files changed, 455 insertions(+), 220 deletions(-) >>> create mode 100644 hw/core/hotplug.c >>> create mode 100644 include/hw/hotplug.h >>> >>> -- >>> 1.8.3.1 >