[PATCH] vmdk: allow specification of tools version

Thomas Weißschuh posted 1 patch 2 years, 7 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210908174250.12946-1-thomas.weissschuh.ext@zeiss.com
There is a newer version of this series
block/vmdk.c         | 24 ++++++++++++++++++++----
qapi/block-core.json |  2 ++
2 files changed, 22 insertions(+), 4 deletions(-)
[PATCH] vmdk: allow specification of tools version
Posted by Thomas Weißschuh 2 years, 7 months ago
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 <thomas.weissschuh.ext@zeiss.com>
---
 block/vmdk.c         | 24 ++++++++++++++++++++----
 qapi/block-core.json |  2 ++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 4499f136bd..93ef6426b0 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -60,6 +60,7 @@
 #define VMDK_ZEROED  (-3)
 
 #define BLOCK_OPT_ZEROED_GRAIN "zeroed_grain"
+#define BLOCK_OPT_TOOLSVERSION "toolsversion"
 
 typedef struct {
     uint32_t version;
@@ -2344,6 +2345,7 @@ static int coroutine_fn vmdk_co_do_create(int64_t size,
                                           BlockdevVmdkAdapterType adapter_type,
                                           const char *backing_file,
                                           const char *hw_version,
+                                          const char *toolsversion,
                                           bool compat6,
                                           bool zeroed_grain,
                                           vmdk_create_extent_fn extent_fn,
@@ -2384,7 +2386,8 @@ static int coroutine_fn vmdk_co_do_create(int64_t size,
         "ddb.geometry.cylinders = \"%" PRId64 "\"\n"
         "ddb.geometry.heads = \"%" PRIu32 "\"\n"
         "ddb.geometry.sectors = \"63\"\n"
-        "ddb.adapterType = \"%s\"\n";
+        "ddb.adapterType = \"%s\"\n"
+        "ddb.toolsVersion = \"%s\"\n";
 
     ext_desc_lines = g_string_new(NULL);
 
@@ -2401,6 +2404,9 @@ static int coroutine_fn vmdk_co_do_create(int64_t size,
     if (!hw_version) {
         hw_version = "4";
     }
+    if (!toolsversion) {
+        toolsversion = "2147483647";
+    }
 
     if (adapter_type != BLOCKDEV_VMDK_ADAPTER_TYPE_IDE) {
         /* that's the number of heads with which vmware operates when
@@ -2525,7 +2531,8 @@ static int coroutine_fn vmdk_co_do_create(int64_t size,
                            size /
                                (int64_t)(63 * number_heads * BDRV_SECTOR_SIZE),
                            number_heads,
-                           BlockdevVmdkAdapterType_str(adapter_type));
+                           BlockdevVmdkAdapterType_str(adapter_type),
+                           toolsversion);
     desc_len = strlen(desc);
     /* the descriptor offset = 0x200 */
     if (!split && !flat) {
@@ -2617,6 +2624,7 @@ static int coroutine_fn vmdk_co_create_opts(BlockDriver *drv,
     BlockdevVmdkAdapterType adapter_type_enum;
     char *backing_file = NULL;
     char *hw_version = NULL;
+    char *toolsversion = NULL;
     char *fmt = NULL;
     BlockdevVmdkSubformat subformat;
     int ret = 0;
@@ -2649,6 +2657,7 @@ static int coroutine_fn vmdk_co_create_opts(BlockDriver *drv,
     adapter_type = qemu_opt_get_del(opts, BLOCK_OPT_ADAPTER_TYPE);
     backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
     hw_version = qemu_opt_get_del(opts, BLOCK_OPT_HWVERSION);
+    toolsversion = qemu_opt_get_del(opts, BLOCK_OPT_TOOLSVERSION);
     compat6 = qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false);
     if (strcmp(hw_version, "undefined") == 0) {
         g_free(hw_version);
@@ -2692,14 +2701,15 @@ static int coroutine_fn vmdk_co_create_opts(BlockDriver *drv,
         .opts = opts,
     };
     ret = vmdk_co_do_create(total_size, subformat, adapter_type_enum,
-                            backing_file, hw_version, compat6, zeroed_grain,
-                            vmdk_co_create_opts_cb, &data, errp);
+                            backing_file, hw_version, toolsversion, compat6,
+                            zeroed_grain, vmdk_co_create_opts_cb, &data, errp);
 
 exit:
     g_free(backing_fmt);
     g_free(adapter_type);
     g_free(backing_file);
     g_free(hw_version);
+    g_free(toolsversion);
     g_free(fmt);
     g_free(desc);
     g_free(path);
@@ -2782,6 +2792,7 @@ static int coroutine_fn vmdk_co_create(BlockdevCreateOptions *create_options,
                             opts->adapter_type,
                             opts->backing_file,
                             opts->hwversion,
+                            opts->toolsversion,
                             false,
                             opts->zeroed_grain,
                             vmdk_co_create_cb,
@@ -3031,6 +3042,11 @@ static QemuOptsList vmdk_create_opts = {
             .help = "VMDK hardware version",
             .def_value_str = "undefined"
         },
+        {
+            .name = BLOCK_OPT_TOOLSVERSION,
+            .type = QEMU_OPT_STRING,
+            .help = "VMware guest tools version",
+        },
         {
             .name = BLOCK_OPT_SUBFMT,
             .type = QEMU_OPT_STRING,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index c8ce1d9d5d..b530d4a333 100644
--- a/qapi/block-core.json
+++ 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.
 # @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',
             '*zeroed-grain':    'bool' } }
 
 
-- 
2.17.1


Re: [PATCH] vmdk: allow specification of tools version
Posted by Eric Blake 2 years, 7 months ago
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 <thomas.weissschuh.ext@zeiss.com>
> ---
>  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


RE: [PATCH] vmdk: allow specification of tools version
Posted by Weissschuh, Thomas [ext] 2 years, 7 months ago
> -----Original Message-----
> From: Eric Blake <eblake@redhat.com>
> Sent: Wednesday, September 8, 2021 7:56 PM
> To: Weissschuh, Thomas [ext] <thomas.weissschuh.ext@zeiss.com>
> Cc: Fam Zheng <fam@euphon.net>; Kevin Wolf <kwolf@redhat.com>; Hanna Reitz
> <hreitz@redhat.com>; Markus Armbruster <armbru@redhat.com>; qemu-
> block@nongnu.org; thomas@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 <thomas.weissschuh.ext@zeiss.com>
> > ---
> >  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