Re: [PATCH 04/14] hw/i386/vmport: Introduce vmx-version property

From: "Michael S. Tsirkin" <mst@redhat.com>
To: Liran Alon <liran.alon@oracle.com>
Cc: ehabkost@redhat.com, qemu-devel@nongnu.org,
	Nikita Leshenko <nikita.leshchenko@oracle.com>,
	pbonzini@redhat.com, rth@twiddle.net
Subject: Re: [PATCH 04/14] hw/i386/vmport: Introduce vmx-version property
Date: Tue, 10 Mar 2020 07:44:24 -0400	[thread overview]
Message-ID: <20200310073629-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <10ffd04f-30f9-6333-26ef-169cd1615341@oracle.com>

On Tue, Mar 10, 2020 at 01:28:32PM +0200, Liran Alon wrote:
> 
> On 10/03/2020 13:18, Michael S. Tsirkin wrote:
> > On Tue, Mar 10, 2020 at 01:05:02PM +0200, Liran Alon wrote:
> > > On 10/03/2020 11:32, Michael S. Tsirkin wrote:
> > > > On Tue, Mar 10, 2020 at 01:54:01AM +0200, Liran Alon wrote:
> > > > > Instead of hard-coding the VMX version, make it a VMPORT object property.
> > > > > This would allow user to control it's value via "-global vmport.vmx-version=X".
> > > > > 
> > > > > Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
> > > > > Signed-off-by: Liran Alon <liran.alon@oracle.com>
> > > > More detail on why this is useful?
> > > It's more useful than returning a hard-coded "6" as the vmx-version...
> > 
> > Maybe default should be 6 (a bit of explanation why 6 could be nice).
> The default is indeed defined as 6. As it was before this patch.
> There is not much to explain besides the fact that recent VMware products
> returns 6 here.
> 
> I don't recall any mapping between the returned version here and the
> supported set of VMPort commands. There is a separate mechanism (which we
> implement in another patch) to signal that a command is unsupported /
> failed.
> 
> The term "vmx-version" refers to the version of the Userspace-VMM of VMware
> which is called (confusingly) "vmx".

Short for Virtual Machine eXecutable. Sigh.  People do come up with
names that aren't great. I don't even know whether vmware was there
first and intel shouldn't have shortened virtual machine extensions to
vmx, but in KVM and QEMU it's quite entrenched by now. So let's try to
avoid this in code. If you like how about VMPortExec and
vm-exec-version?  Also lets you use CamelCase consistently and not a mix
of underscores and CamelCase.

> > > We have used it to preserve compatibility for some VMware guests that we run
> > > as-is on top of QEMU/KVM which expects specific vmx-version or else they
> > > fail to run properly.
> > > 
> > > -Liran
> > Any detail on which guest it is?
> I will need to dig in production history to find it... They are usually
> proprietary appliances specially made to run as VMware VMs.
> > Pretending to be a very advanced version has its pitfalls if we
> > then don't behave the way vmware does, right?
> In all those cases, we have taken the version number backwards, not forward.
> > Figuring out the version number is I suspect a bit much to ask of users.
> Most users will indeed not need to touch this. This is for advanced users,
> such as Ravello.
> We usually figured this out by reverse-engineering the failed guest and/or
> examining the original VMware environment it used to run on.
> 
> -Liran

Right if you want this for debugging, prefix property with "x-" so it does
not need to be maintained. Point being, maintaining low level interfaces
has real cost ...

-- 
MST