Re: [Qemu-devel] [PATCH 00/11 v3] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API

[Anthony Liguori]

On Tue, Dec 17, 2013 at 4:38 AM, Paolo Bonzini <address@hidden> 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
>