RE: [PATCH] vmdk: allow specification of tools version

2021-09-08 Thread Weissschuh, Thomas [ext]
> -Original Message-
> From: Eric Blake 
> Sent: Wednesday, September 8, 2021 7:56 PM
> To: Weissschuh, Thomas [ext] 
> Cc: Fam Zheng ; Kevin Wolf ; Hanna Reitz
> ; Markus Armbruster ; qemu-
> bl...@nongnu.org; tho...@t-8ch.de; qemu-devel@nongnu.org
> Subject: Re: [PATCH] vmdk: allow specification of tools version
> 
> On Wed, Sep 08, 2021 at 07:42:50PM +0200, Thomas Weißschuh wrote:
> > VMDK files support an attribute that represents the version of the
> > guest tools that are installed on the disk.
> > This attribute is used by vSphere before a machine has been started to
> > determine if the VM has the guest tools installed.
> > This is important when configuring "Operating system customizations"
> > in vSphere, as it checks for the presence of the guest tools before
> > allowing those customizations.
> > Thus when the VM has not yet booted normally it would be impossible to
> > customize it, therefore preventing a customized first-boot.
> >
> > The attribute should not hurt on disks that do not have the guest
> > tools installed and indeed the VMware tools also unconditionally add
> > this attribute.
> > (Defaulting to the value "2147483647", as is done in this patch)
> >
> > Signed-off-by: Thomas Weißschuh 
> > ---
> >  block/vmdk.c | 24 
> >  qapi/block-core.json |  2 ++
> >  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> UI review:
> 
> > +++ b/qapi/block-core.json
> > @@ -4597,6 +4597,7 @@
> >  # @adapter-type: The adapter type used to fill in the descriptor.
> Default: ide.
> >  # @hwversion: Hardware version. The meaningful options are "4" or "6".
> >  # Default: "4".
> > +# @toolsversion: VMware guest tools version.
> 
> Missing a '(since 6.2)' blurb, and a description of its default value.

I'll add that to v2, which I'll send as soon as some more review comments came 
in.

> >  # @zeroed-grain: Whether to enable zeroed-grain feature for sparse
> subformats.
> >  #Default: false.
> >  #
> > @@ -4610,6 +4611,7 @@
> >  '*backing-file':'str',
> >  '*adapter-type':'BlockdevVmdkAdapterType',
> >  '*hwversion':   'str',
> > +'*toolsversion':'str',
> 
> Is it an arbitrary string, or must a valid value always be parseable as a
> numeric value?  If the latter, then make the QMP representation numeric
> rather than string.

In the vmdk itself it is an arbitrary string but the actual values seem to be 
always numeric.
But I do have some very faint recollection of seeing normal version numbers 
(a.b.c) there at some point, but can't confirm this at the moment.

Thomas


Re: [PATCH] vmdk: allow specification of tools version

2021-09-08 Thread Eric Blake
On Wed, Sep 08, 2021 at 07:42:50PM +0200, Thomas Weißschuh wrote:
> VMDK files support an attribute that represents the version of the guest
> tools that are installed on the disk.
> This attribute is used by vSphere before a machine has been started to
> determine if the VM has the guest tools installed.
> This is important when configuring "Operating system customizations" in
> vSphere, as it checks for the presence of the guest tools before
> allowing those customizations.
> Thus when the VM has not yet booted normally it would be impossible to
> customize it, therefore preventing a customized first-boot.
> 
> The attribute should not hurt on disks that do not have the guest tools
> installed and indeed the VMware tools also unconditionally add this
> attribute.
> (Defaulting to the value "2147483647", as is done in this patch)
> 
> Signed-off-by: Thomas Weißschuh 
> ---
>  block/vmdk.c | 24 
>  qapi/block-core.json |  2 ++
>  2 files changed, 22 insertions(+), 4 deletions(-)

UI review:

> +++ b/qapi/block-core.json
> @@ -4597,6 +4597,7 @@
>  # @adapter-type: The adapter type used to fill in the descriptor. Default: 
> ide.
>  # @hwversion: Hardware version. The meaningful options are "4" or "6".
>  # Default: "4".
> +# @toolsversion: VMware guest tools version.

Missing a '(since 6.2)' blurb, and a description of its default value.

>  # @zeroed-grain: Whether to enable zeroed-grain feature for sparse 
> subformats.
>  #Default: false.
>  #
> @@ -4610,6 +4611,7 @@
>  '*backing-file':'str',
>  '*adapter-type':'BlockdevVmdkAdapterType',
>  '*hwversion':   'str',
> +'*toolsversion':'str',

Is it an arbitrary string, or must a valid value always be parseable
as a numeric value?  If the latter, then make the QMP representation
numeric rather than string.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org