From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52535) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a8Wb8-00077H-TU for qemu-devel@nongnu.org; Mon, 14 Dec 2015 12:07:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a8Wb4-0005lB-Q0 for qemu-devel@nongnu.org; Mon, 14 Dec 2015 12:07:42 -0500 Received: from mail-wm0-x242.google.com ([2a00:1450:400c:c09::242]:32858) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a8Wb4-0005l1-J7 for qemu-devel@nongnu.org; Mon, 14 Dec 2015 12:07:38 -0500 Received: by wmll124 with SMTP id l124so11023240wml.0 for ; Mon, 14 Dec 2015 09:07:38 -0800 (PST) Sender: Paolo Bonzini References: <1449994112-7054-1-git-send-email-shmulik.ladkani@ravellosystems.com> <1449994112-7054-7-git-send-email-shmulik.ladkani@ravellosystems.com> From: Paolo Bonzini Message-ID: <566EF753.4070605@redhat.com> Date: Mon, 14 Dec 2015 18:07:31 +0100 MIME-Version: 1.0 In-Reply-To: <1449994112-7054-7-git-send-email-shmulik.ladkani@ravellosystems.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 6/6] vmw_pvscsi: Introduce 'x-disable-pcie' backword compatability property List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Shmulik Ladkani , Dmitry Fleytman Cc: Marcel Apfelbaum , idan.brown@ravellosystems.com, qemu-devel@nongnu.org, "Michael S. Tsirkin" 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 > --- > 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