On Thu, 11 Nov 2021 08:55:24 +0530 (IST)
Ani Sinha <a...@anisinha.ca> wrote:
> On Wed, 10 Nov 2021, Igor Mammedov wrote:
>
> > Mark property as experimental/internal adding 'x-' prefix.
> >
> > Property was introduced in 6.1 and it should have provided
> > ability to turn on native PCIE hotplug on port even when
> > ACPI PCI hotplug is in use is user explicitly sets property
> > on CLI. However that never worked since slot is wired to
> > ACPI hotplug controller.
> > Another non-intended usecase: disable native hotplug on slot
> > when APCI based hotplug is disabled, which works but slot has
> > 'hotplug' property for this taks.
> >
> > It should be relatively safe to rename it to experimental
> > as no users should exist for it and given that the property
> > is broken we don't really want to leave it around for much
> > longer lest users start using it.
> >
> > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
>
> Barring the comment below,
>
> Reviewed-by: Ani Sinha <a...@anisinha.ca>
Thanks!
>
> > ---
> > CC: qemu-sta...@nongnu.org
> > ---
> > hw/i386/pc_q35.c | 2 +-
> > hw/pci/pcie_port.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 797e09500b..fc34b905ee 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -243,7 +243,7 @@ static void pc_q35_init(MachineState *machine)
> > NULL);
> >
> > if (acpi_pcihp) {
> > - object_register_sugar_prop(TYPE_PCIE_SLOT, "native-hotplug",
> > + object_register_sugar_prop(TYPE_PCIE_SLOT, "x-native-hotplug",
> > "false", true);
>
> Let us document the fact that this property is experimental. It was not at
> once obvious to me that an "x-" prefix meant to indicate experimental
> status.
it's common knowledge, but quick grep shows we only documented
x- prefix for qmp commands but not for properties even though
properties were the first to use it. So we probably should
document it somewhere.
I thought we have acceptable property name format documented
but I couldn't find it quickly (that would be a good place
to document it).
Care to post a patch?
>
>
> > }
> >
> > diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
> > index da850e8dde..e95c1e5519 100644
> > --- a/hw/pci/pcie_port.c
> > +++ b/hw/pci/pcie_port.c
> > @@ -148,7 +148,7 @@ static Property pcie_slot_props[] = {
> > DEFINE_PROP_UINT8("chassis", PCIESlot, chassis, 0),
> > DEFINE_PROP_UINT16("slot", PCIESlot, slot, 0),
> > DEFINE_PROP_BOOL("hotplug", PCIESlot, hotplug, true),
> > - DEFINE_PROP_BOOL("native-hotplug", PCIESlot, native_hotplug, true),
> > + DEFINE_PROP_BOOL("x-native-hotplug", PCIESlot, native_hotplug, true),
> > DEFINE_PROP_END_OF_LIST()
> > };
> >
> > --
> > 2.27.0
> >
> >
>
