Re: [PATCH V3] Rename use_acpi_pci_hotplug to more appropriate use_acpi_hotplug_bridge

[Igor Mammedov]

On Tue, 16 Jun 2020 18:50:13 +0530
Ani Sinha <ani@anisinha.ca> wrote:

> On Tue, Jun 16, 2020 at 6:47 PM Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > On Tue, 16 Jun 2020 12:31:39 +0000
> > Ani Sinha <ani.sinha@nutanix.com> wrote:
> >  
> > > Currently, the option use_acpi_pci_hotplug is being used to control  
> > device  
> > > hotplug capability using ACPI for slots of cold plugged bridges. Hence,  
> > we  
> > > are renaming this option to better reflect what it actually does.
> > >
> > > Signed-off-by: Ani Sinha <ani.sinha@nutanix.com>
> > > ---  
> >
> > what was wrong with v2?  
> 
> 
> i removed the change-id string.
ok, usually we cary on ACK on resping if changes were trivial
and drop them if there was a non trivial change so it could be reviewed again.
Anyways, patch looks good so:

Reviewed-by: Igor Mammedov <imammedo@redhat.com>


> 
> 
> > Also for the future,
> > can you add under --- line change log please?
> >  
> > >  hw/acpi/piix4.c | 21 +++++++++++----------
> > >  1 file changed, 11 insertions(+), 10 deletions(-)  
> 
> 
> It's already here.

I've meant something like:

Signed-off-by: Ani Sinha <ani.sinha@nutanix.com>
---
 vX: - I might be blind and don't see changelog ...

> 
> 
> > >
> > > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > > index 85c199b..7de44bc 100644
> > > --- a/hw/acpi/piix4.c
> > > +++ b/hw/acpi/piix4.c
> > > @@ -77,7 +77,7 @@ typedef struct PIIX4PMState {
> > >      Notifier powerdown_notifier;
> > >
> > >      AcpiPciHpState acpi_pci_hotplug;
> > > -    bool use_acpi_pci_hotplug;
> > > +    bool use_acpi_hotplug_bridge;
> > >
> > >      uint8_t disable_s3;
> > >      uint8_t disable_s4;
> > > @@ -204,16 +204,17 @@ static const VMStateDescription vmstate_pci_status  
> > = {  
> > >      }
> > >  };
> > >
> > > -static bool vmstate_test_use_acpi_pci_hotplug(void *opaque, int  
> > version_id)  
> > > +static bool vmstate_test_use_acpi_hotplug_bridge(void *opaque, int  
> > version_id)  
> > >  {
> > >      PIIX4PMState *s = opaque;
> > > -    return s->use_acpi_pci_hotplug;
> > > +    return s->use_acpi_hotplug_bridge;
> > >  }
> > >
> > > -static bool vmstate_test_no_use_acpi_pci_hotplug(void *opaque, int  
> > version_id)  
> > > +static bool vmstate_test_no_use_acpi_hotplug_bridge(void *opaque,
> > > +                                                    int version_id)
> > >  {
> > >      PIIX4PMState *s = opaque;
> > > -    return !s->use_acpi_pci_hotplug;
> > > +    return !s->use_acpi_hotplug_bridge;
> > >  }
> > >
> > >  static bool vmstate_test_use_memhp(void *opaque)
> > > @@ -290,11 +291,11 @@ static const VMStateDescription vmstate_acpi = {
> > >          VMSTATE_STRUCT_TEST(
> > >  
> > acpi_pci_hotplug.acpi_pcihp_pci_status[ACPI_PCIHP_BSEL_DEFAULT],  
> > >              PIIX4PMState,
> > > -            vmstate_test_no_use_acpi_pci_hotplug,
> > > +            vmstate_test_no_use_acpi_hotplug_bridge,
> > >              2, vmstate_pci_status,
> > >              struct AcpiPciHpPciStatus),
> > >          VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug, PIIX4PMState,
> > > -                            vmstate_test_use_acpi_pci_hotplug),
> > > +                            vmstate_test_use_acpi_hotplug_bridge),
> > >          VMSTATE_END_OF_LIST()
> > >      },
> > >      .subsections = (const VMStateDescription*[]) {
> > > @@ -528,7 +529,7 @@ I2CBus *piix4_pm_init(PCIBus *bus, int devfn,  
> > uint32_t smb_io_base,  
> > >      s->smi_irq = smi_irq;
> > >      s->smm_enabled = smm_enabled;
> > >      if (xen_enabled()) {
> > > -        s->use_acpi_pci_hotplug = false;
> > > +        s->use_acpi_hotplug_bridge = false;
> > >      }
> > >
> > >      qdev_init_nofail(dev);
> > > @@ -593,7 +594,7 @@ static void  
> > piix4_acpi_system_hot_add_init(MemoryRegion *parent,  
> > >      memory_region_add_subregion(parent, GPE_BASE, &s->io_gpe);
> > >
> > >      acpi_pcihp_init(OBJECT(s), &s->acpi_pci_hotplug, bus, parent,
> > > -                    s->use_acpi_pci_hotplug);
> > > +                    s->use_acpi_hotplug_bridge);
> > >
> > >      s->cpu_hotplug_legacy = true;
> > >      object_property_add_bool(OBJECT(s), "cpu-hotplug-legacy",
> > > @@ -631,7 +632,7 @@ static Property piix4_pm_properties[] = {
> > >      DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_DISABLED, PIIX4PMState,  
> > disable_s4, 0),  
> > >      DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2),
> > >      DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support",  
> > PIIX4PMState,  
> > > -                     use_acpi_pci_hotplug, true),
> > > +                     use_acpi_hotplug_bridge, true),
> > >      DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState,
> > >                       acpi_memory_hotplug.is_enabled, true),
> > >      DEFINE_PROP_END_OF_LIST(),  
> >
> >