On Mon, Dec 14, 2015 at 06:07:31PM +0100, Paolo Bonzini wrote:
>
>
> On 13/12/2015 09:08, Shmulik Ladkani wrote:
> > Following the previous patch which changed pvscsi to be a pci express
> > device, this patch introduces a boolean property 'x-disable-pcie'.
> >
> > Its default value is false, exposing pvscsi as a pcie device.
> >
> > Setting 'x-disable-pcie' to 'on' preserves the old 'pci device' (non
> > express) behavior. This allows migration to older versions.
> >
> > Signed-off-by: Shmulik Ladkani <shmulik.ladk...@ravellosystems.com>
> > ---
> > hw/scsi/vmw_pvscsi.c | 2 ++
> > include/hw/compat.h | 4 ++++
> > 2 files changed, 6 insertions(+)
> >
> > diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> > index fb53b24..6b4b989 100644
> > --- a/hw/scsi/vmw_pvscsi.c
> > +++ b/hw/scsi/vmw_pvscsi.c
> > @@ -1241,6 +1241,8 @@ static Property pvscsi_properties[] = {
> > DEFINE_PROP_UINT8("use_msg", PVSCSIState, use_msg, 1),
> > DEFINE_PROP_BIT("x-old-pci-configuration", PVSCSIState, compat_flags,
> > PVSCSI_COMPAT_OLD_PCI_CONFIGURATION_BIT, false),
> > + DEFINE_PROP_BIT("x-disable-pcie", PVSCSIState, compat_flags,
> > + PVSCSI_COMPAT_DISABLE_PCIE_BIT, false),
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
> > diff --git a/include/hw/compat.h b/include/hw/compat.h
> > index 66e4aff..bcb36ef 100644
> > --- a/include/hw/compat.h
> > +++ b/include/hw/compat.h
> > @@ -11,6 +11,10 @@
> > .property = "x-old-pci-configuration",\
> > .value = "on",\
> > },{\
> > + .driver = "pvscsi",\
> > + .property = "x-disable-pcie",\
> > + .value = "on",\
> > + },{\
> > .driver = "e1000",\
> > .property = "extra_mac_registers",\
> > .value = "off",\
> >
>
> This series is okay, but the use of "x-" in the compat properties struck
> me as odd.
>
> I see that the "x-" prefix was first introduced for compat properties in
> commit 1811e64 ("hw/virtio: Add PCIe capability to virtio devices",
> 2015-11-10). The rationale for "x-" was either 1) to provide fine
> tuning for debugging-like options; 2) for places where the API is known
> to be somewhat broken and will be fixed later.
>
> Marcel, Michael, what was the justification for using "x-" in commit
> 1811e64? That said, I wouldn't mind using the opportunity of "x-" to
> remove the double negation and rename the property to just "pcie". :)
>
> Paolo
Well qapi/qmp docs say things like:
Any name (command, event, type, field, or enum value) beginning with
"x-" is marked experimental, and may be withdrawn or changed
incompatibly in a future release.
It's thus reasonable to use this for internal properties,
that we don't want users to play with.
BTW, we probably should teach -help to hide these options by default.
--
MST
