Re: [Qemu-devel] [PATCH v2 6/6] vmw_pvscsi: Introduce 'x-disable-pcie' backword compatability property

[Paolo Bonzini <pbonzini@redhat.com>]

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.ladkani@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