Re: [Qemu-devel] [PATCH v1 0/7] pci: hotplug handler reworks

[Igor Mammedov]

On Wed, 31 Oct 2018 18:31:30 +0100
David Hildenbrand <address@hidden> wrote:

> On 24.10.18 12:19, David Hildenbrand wrote:
> > This series reworks some pci hotplug handlers (except for s390, that will
> > require more work but is not required for now).
> > 
> > 1. Route all unplug calls via the hotplug handler when called from the
> >    unplug_request handler. This will be required to get multi-stage
> >    hotplug handlers running, but also makes sense on its own (just like we
> >    already did for some CPU/memory hotplug handlers).
> > 
> > 2. Introduce some pre_plug handlers where it makes sense already.
> > 
> > 3. Call the plug/pre_plug handler also for coldplugged devices. Especially
> >    pcihp is special as it overwrites hotplug handlers.
> > 
> > This series will not yet factor out pre_plug/plug/unplug from pci device
> > realize/unrealize functions, this will require more work but this
> > series is also required first to get it running.
> > 
> > David Hildenbrand (7):
> >   pcihp: perform check for bus capability in pre_plug handler
> >   pcihp: overwrite hotplug handler recursively from the start
> >   pcihp: route unplug via the hotplug handler
> >   pci/pcie: route unplug via the hotplug handler
> >   pci/shpc: move hotplug checks to preplug handler
> >   pci/shpc: route unplug via the hotplug handler
> >   spapr_pci: route unplug via the hotplug handler
> > 
> >  hw/acpi/pcihp.c                 | 40 +++++++++++++++++++++++-----
> >  hw/acpi/piix4.c                 | 39 ++++++++++++++-------------
> >  hw/pci-bridge/pci_bridge_dev.c  | 23 +++++++++++++++-
> >  hw/pci-bridge/pcie_pci_bridge.c | 23 +++++++++++++++-
> >  hw/pci/pcie.c                   | 10 ++++++-
> >  hw/pci/pcie_port.c              |  1 +
> >  hw/pci/shpc.c                   | 47 ++++++++++++++++++---------------
> >  hw/ppc/spapr_pci.c              | 33 ++++++++++++++---------
> >  include/hw/acpi/pcihp.h         |  5 ++++
> >  include/hw/pci/pcie.h           |  2 ++
> >  include/hw/pci/shpc.h           |  4 +++
> >  11 files changed, 165 insertions(+), 62 deletions(-)
> >   
> 
> Did some more testing. Can somebody have a look/pick up? Thanks!
Did a quick pass over series, patches overall looks good here is some other nits
that apply to series:
 * make more descriptive commit messages (important for history and for whoever 
comes later to read it)
 * I don't really like a mix of style in callbacks naming
    for ex: there is hotplug and then for unplug you add hot_unplug instead of 
hotunplug
    I'd prefer consistent approach in naming.
    (either use underscores or drop them or maybe drop 'hot' part as it's not 
only for hotplug anymore)