[Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Janne Karhunen]

From: Janne Karhunen <janne.karhunen@gmail.com>

Vmdk images have metadata to indicate the vmware virtual
hardware version image was created/tested to run with.
Allow users to specify that version via new 'hwversion'
option.

Signed-off-by: Janne Karhunen <Janne.Karhunen@gmail.com>
---
 block/vmdk.c              | 27 +++++++++++++++++++++++----
 include/block/block_int.h |  2 +-
 qemu-doc.texi             |  3 +++
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 45f9d3c..955c6b3 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1829,8 +1829,8 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
     int64_t total_size = 0, filesize;
     char *adapter_type = NULL;
     char *backing_file = NULL;
+    char *hw_version = NULL;
     char *fmt = NULL;
-    int flags = 0;
     int ret = 0;
     bool flat, split, compress;
     GString *ext_desc_lines;
@@ -1861,7 +1861,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
         "# The Disk Data Base\n"
         "#DDB\n"
         "\n"
-        "ddb.virtualHWVersion = \"%d\"\n"
+        "ddb.virtualHWVersion = \"%s\"\n"
         "ddb.geometry.cylinders = \"%" PRId64 "\"\n"
         "ddb.geometry.heads = \"%" PRIu32 "\"\n"
         "ddb.geometry.sectors = \"63\"\n"
@@ -1878,8 +1878,20 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
                           BDRV_SECTOR_SIZE);
     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);
     if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false)) {
-        flags |= BLOCK_FLAG_COMPAT6;
+        if (strcmp(hw_version, "undefined")) {
+            error_setg(errp,
+                       "compat6 cannot be enabled with hwversion set");
+            ret = -EINVAL;
+            goto exit;
+        }
+        g_free(hw_version);
+        hw_version = g_strdup("6");
+    }
+    if (strcmp(hw_version, "undefined") == 0) {
+        g_free(hw_version);
+        hw_version = g_strdup("4");
     }
     fmt = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
     if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ZEROED_GRAIN, false)) {
@@ -2001,7 +2013,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
                            fmt,
                            parent_desc_line,
                            ext_desc_lines->str,
-                           (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
+                           hw_version,
                            total_size /
                                (int64_t)(63 * number_heads * BDRV_SECTOR_SIZE),
                            number_heads,
@@ -2047,6 +2059,7 @@ exit:
     }
     g_free(adapter_type);
     g_free(backing_file);
+    g_free(hw_version);
     g_free(fmt);
     g_free(desc);
     g_free(path);
@@ -2298,6 +2311,12 @@ static QemuOptsList vmdk_create_opts = {
             .def_value_str = "off"
         },
         {
+            .name = BLOCK_OPT_HWVERSION,
+            .type = QEMU_OPT_STRING,
+            .help = "VMDK hardware version",
+            .def_value_str = "undefined"
+        },
+        {
             .name = BLOCK_OPT_SUBFMT,
             .type = QEMU_OPT_STRING,
             .help =
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 10d8759..931a412 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -38,12 +38,12 @@
 #include "qemu/throttle.h"
 
 #define BLOCK_FLAG_ENCRYPT          1
-#define BLOCK_FLAG_COMPAT6          4
 #define BLOCK_FLAG_LAZY_REFCOUNTS   8
 
 #define BLOCK_OPT_SIZE              "size"
 #define BLOCK_OPT_ENCRYPT           "encryption"
 #define BLOCK_OPT_COMPAT6           "compat6"
+#define BLOCK_OPT_HWVERSION         "hwversion"
 #define BLOCK_OPT_BACKING_FILE      "backing_file"
 #define BLOCK_OPT_BACKING_FMT       "backing_fmt"
 #define BLOCK_OPT_CLUSTER_SIZE      "cluster_size"
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 79141d3..f37fd31 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -693,6 +693,9 @@ Supported options:
 File name of a base image (see @option{create} subcommand).
 @item compat6
 Create a VMDK version 6 image (instead of version 4)
+@item hwversion
+Specify vmdk virtual hardware version. Compat6 flag cannot be enabled
+if hwversion is specified.
 @item subformat
 Specifies which VMDK subformat to use. Valid options are
 @code{monolithicSparse} (default),
-- 
1.9.1

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Janne Karhunen]

Fam,

Any objections to this one?


--
Janne

On Tue, May 3, 2016 at 12:43 PM, Janne Karhunen
<Janne.Karhunen@gmail.com> wrote:
> From: Janne Karhunen <janne.karhunen@gmail.com>
>
> Vmdk images have metadata to indicate the vmware virtual
> hardware version image was created/tested to run with.
> Allow users to specify that version via new 'hwversion'
> option.
>
> Signed-off-by: Janne Karhunen <Janne.Karhunen@gmail.com>
> ---
>  block/vmdk.c              | 27 +++++++++++++++++++++++----
>  include/block/block_int.h |  2 +-
>  qemu-doc.texi             |  3 +++
>  3 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 45f9d3c..955c6b3 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1829,8 +1829,8 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>      int64_t total_size = 0, filesize;
>      char *adapter_type = NULL;
>      char *backing_file = NULL;
> +    char *hw_version = NULL;
>      char *fmt = NULL;
> -    int flags = 0;
>      int ret = 0;
>      bool flat, split, compress;
>      GString *ext_desc_lines;
> @@ -1861,7 +1861,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>          "# The Disk Data Base\n"
>          "#DDB\n"
>          "\n"
> -        "ddb.virtualHWVersion = \"%d\"\n"
> +        "ddb.virtualHWVersion = \"%s\"\n"
>          "ddb.geometry.cylinders = \"%" PRId64 "\"\n"
>          "ddb.geometry.heads = \"%" PRIu32 "\"\n"
>          "ddb.geometry.sectors = \"63\"\n"
> @@ -1878,8 +1878,20 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>                            BDRV_SECTOR_SIZE);
>      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);
>      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false)) {
> -        flags |= BLOCK_FLAG_COMPAT6;
> +        if (strcmp(hw_version, "undefined")) {
> +            error_setg(errp,
> +                       "compat6 cannot be enabled with hwversion set");
> +            ret = -EINVAL;
> +            goto exit;
> +        }
> +        g_free(hw_version);
> +        hw_version = g_strdup("6");
> +    }
> +    if (strcmp(hw_version, "undefined") == 0) {
> +        g_free(hw_version);
> +        hw_version = g_strdup("4");
>      }
>      fmt = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
>      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ZEROED_GRAIN, false)) {
> @@ -2001,7 +2013,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>                             fmt,
>                             parent_desc_line,
>                             ext_desc_lines->str,
> -                           (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
> +                           hw_version,
>                             total_size /
>                                 (int64_t)(63 * number_heads * BDRV_SECTOR_SIZE),
>                             number_heads,
> @@ -2047,6 +2059,7 @@ exit:
>      }
>      g_free(adapter_type);
>      g_free(backing_file);
> +    g_free(hw_version);
>      g_free(fmt);
>      g_free(desc);
>      g_free(path);
> @@ -2298,6 +2311,12 @@ static QemuOptsList vmdk_create_opts = {
>              .def_value_str = "off"
>          },
>          {
> +            .name = BLOCK_OPT_HWVERSION,
> +            .type = QEMU_OPT_STRING,
> +            .help = "VMDK hardware version",
> +            .def_value_str = "undefined"
> +        },
> +        {
>              .name = BLOCK_OPT_SUBFMT,
>              .type = QEMU_OPT_STRING,
>              .help =
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 10d8759..931a412 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -38,12 +38,12 @@
>  #include "qemu/throttle.h"
>
>  #define BLOCK_FLAG_ENCRYPT          1
> -#define BLOCK_FLAG_COMPAT6          4
>  #define BLOCK_FLAG_LAZY_REFCOUNTS   8
>
>  #define BLOCK_OPT_SIZE              "size"
>  #define BLOCK_OPT_ENCRYPT           "encryption"
>  #define BLOCK_OPT_COMPAT6           "compat6"
> +#define BLOCK_OPT_HWVERSION         "hwversion"
>  #define BLOCK_OPT_BACKING_FILE      "backing_file"
>  #define BLOCK_OPT_BACKING_FMT       "backing_fmt"
>  #define BLOCK_OPT_CLUSTER_SIZE      "cluster_size"
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 79141d3..f37fd31 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -693,6 +693,9 @@ Supported options:
>  File name of a base image (see @option{create} subcommand).
>  @item compat6
>  Create a VMDK version 6 image (instead of version 4)
> +@item hwversion
> +Specify vmdk virtual hardware version. Compat6 flag cannot be enabled
> +if hwversion is specified.
>  @item subformat
>  Specifies which VMDK subformat to use. Valid options are
>  @code{monolithicSparse} (default),
> --
> 1.9.1
>

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Fam Zheng]

On Tue, 05/03 02:43, Janne Karhunen wrote:
> From: Janne Karhunen <janne.karhunen@gmail.com>
> 
> Vmdk images have metadata to indicate the vmware virtual
> hardware version image was created/tested to run with.
> Allow users to specify that version via new 'hwversion'
> option.
> 
> Signed-off-by: Janne Karhunen <Janne.Karhunen@gmail.com>
> ---
>  block/vmdk.c              | 27 +++++++++++++++++++++++----
>  include/block/block_int.h |  2 +-
>  qemu-doc.texi             |  3 +++
>  3 files changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 45f9d3c..955c6b3 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1829,8 +1829,8 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>      int64_t total_size = 0, filesize;
>      char *adapter_type = NULL;
>      char *backing_file = NULL;
> +    char *hw_version = NULL;
>      char *fmt = NULL;
> -    int flags = 0;
>      int ret = 0;
>      bool flat, split, compress;
>      GString *ext_desc_lines;
> @@ -1861,7 +1861,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>          "# The Disk Data Base\n"
>          "#DDB\n"
>          "\n"
> -        "ddb.virtualHWVersion = \"%d\"\n"
> +        "ddb.virtualHWVersion = \"%s\"\n"
>          "ddb.geometry.cylinders = \"%" PRId64 "\"\n"
>          "ddb.geometry.heads = \"%" PRIu32 "\"\n"
>          "ddb.geometry.sectors = \"63\"\n"
> @@ -1878,8 +1878,20 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>                            BDRV_SECTOR_SIZE);
>      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);
>      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false)) {
> -        flags |= BLOCK_FLAG_COMPAT6;
> +        if (strcmp(hw_version, "undefined")) {
> +            error_setg(errp,
> +                       "compat6 cannot be enabled with hwversion set");
> +            ret = -EINVAL;
> +            goto exit;
> +        }
> +        g_free(hw_version);
> +        hw_version = g_strdup("6");
> +    }
> +    if (strcmp(hw_version, "undefined") == 0) {
> +        g_free(hw_version);
> +        hw_version = g_strdup("4");
>      }
>      fmt = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
>      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ZEROED_GRAIN, false)) {
> @@ -2001,7 +2013,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>                             fmt,
>                             parent_desc_line,
>                             ext_desc_lines->str,
> -                           (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
> +                           hw_version,
>                             total_size /
>                                 (int64_t)(63 * number_heads * BDRV_SECTOR_SIZE),
>                             number_heads,
> @@ -2047,6 +2059,7 @@ exit:
>      }
>      g_free(adapter_type);
>      g_free(backing_file);
> +    g_free(hw_version);
>      g_free(fmt);
>      g_free(desc);
>      g_free(path);
> @@ -2298,6 +2311,12 @@ static QemuOptsList vmdk_create_opts = {
>              .def_value_str = "off"
>          },
>          {
> +            .name = BLOCK_OPT_HWVERSION,
> +            .type = QEMU_OPT_STRING,
> +            .help = "VMDK hardware version",
> +            .def_value_str = "undefined"
> +        },
> +        {
>              .name = BLOCK_OPT_SUBFMT,
>              .type = QEMU_OPT_STRING,
>              .help =
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 10d8759..931a412 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -38,12 +38,12 @@
>  #include "qemu/throttle.h"
>  
>  #define BLOCK_FLAG_ENCRYPT          1
> -#define BLOCK_FLAG_COMPAT6          4
>  #define BLOCK_FLAG_LAZY_REFCOUNTS   8
>  
>  #define BLOCK_OPT_SIZE              "size"
>  #define BLOCK_OPT_ENCRYPT           "encryption"
>  #define BLOCK_OPT_COMPAT6           "compat6"
> +#define BLOCK_OPT_HWVERSION         "hwversion"
>  #define BLOCK_OPT_BACKING_FILE      "backing_file"
>  #define BLOCK_OPT_BACKING_FMT       "backing_fmt"
>  #define BLOCK_OPT_CLUSTER_SIZE      "cluster_size"
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 79141d3..f37fd31 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -693,6 +693,9 @@ Supported options:
>  File name of a base image (see @option{create} subcommand).
>  @item compat6
>  Create a VMDK version 6 image (instead of version 4)
> +@item hwversion
> +Specify vmdk virtual hardware version. Compat6 flag cannot be enabled
> +if hwversion is specified.
>  @item subformat
>  Specifies which VMDK subformat to use. Valid options are
>  @code{monolithicSparse} (default),
> -- 
> 1.9.1
> 

Reviewed-by: Fam Zheng <famz@redhat.com>

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Fam Zheng]

On Fri, 05/06 08:15, Janne Karhunen wrote:
> Fam,
> 
> Any objections to this one?

Looks good to me. I've left my reviewed-by in direct reply to the patch. Cc'ing
block layer maintainers and qemu-block list.

Fam

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Kevin Wolf]

Am 06.05.2016 um 11:04 hat Fam Zheng geschrieben:
> On Tue, 05/03 02:43, Janne Karhunen wrote:
> > From: Janne Karhunen <janne.karhunen@gmail.com>
> > 
> > Vmdk images have metadata to indicate the vmware virtual
> > hardware version image was created/tested to run with.
> > Allow users to specify that version via new 'hwversion'
> > option.
> > 
> > Signed-off-by: Janne Karhunen <Janne.Karhunen@gmail.com>
> > ---
> >  block/vmdk.c              | 27 +++++++++++++++++++++++----
> >  include/block/block_int.h |  2 +-
> >  qemu-doc.texi             |  3 +++
> >  3 files changed, 27 insertions(+), 5 deletions(-)
> > 
> > diff --git a/block/vmdk.c b/block/vmdk.c
> > index 45f9d3c..955c6b3 100644
> > --- a/block/vmdk.c
> > +++ b/block/vmdk.c
> > @@ -1829,8 +1829,8 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
> >      int64_t total_size = 0, filesize;
> >      char *adapter_type = NULL;
> >      char *backing_file = NULL;
> > +    char *hw_version = NULL;
> >      char *fmt = NULL;
> > -    int flags = 0;
> >      int ret = 0;
> >      bool flat, split, compress;
> >      GString *ext_desc_lines;
> > @@ -1861,7 +1861,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
> >          "# The Disk Data Base\n"
> >          "#DDB\n"
> >          "\n"
> > -        "ddb.virtualHWVersion = \"%d\"\n"
> > +        "ddb.virtualHWVersion = \"%s\"\n"
> >          "ddb.geometry.cylinders = \"%" PRId64 "\"\n"
> >          "ddb.geometry.heads = \"%" PRIu32 "\"\n"
> >          "ddb.geometry.sectors = \"63\"\n"
> > @@ -1878,8 +1878,20 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
> >                            BDRV_SECTOR_SIZE);
> >      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);
> >      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false)) {
> > -        flags |= BLOCK_FLAG_COMPAT6;
> > +        if (strcmp(hw_version, "undefined")) {
> > +            error_setg(errp,
> > +                       "compat6 cannot be enabled with hwversion set");
> > +            ret = -EINVAL;
> > +            goto exit;
> > +        }
> > +        g_free(hw_version);
> > +        hw_version = g_strdup("6");
> > +    }
> > +    if (strcmp(hw_version, "undefined") == 0) {
> > +        g_free(hw_version);
> > +        hw_version = g_strdup("4");
> >      }
> >      fmt = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
> >      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ZEROED_GRAIN, false)) {
> > @@ -2001,7 +2013,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
> >                             fmt,
> >                             parent_desc_line,
> >                             ext_desc_lines->str,
> > -                           (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
> > +                           hw_version,
> >                             total_size /
> >                                 (int64_t)(63 * number_heads * BDRV_SECTOR_SIZE),
> >                             number_heads,
> > @@ -2047,6 +2059,7 @@ exit:
> >      }
> >      g_free(adapter_type);
> >      g_free(backing_file);
> > +    g_free(hw_version);
> >      g_free(fmt);
> >      g_free(desc);
> >      g_free(path);
> > @@ -2298,6 +2311,12 @@ static QemuOptsList vmdk_create_opts = {
> >              .def_value_str = "off"
> >          },
> >          {
> > +            .name = BLOCK_OPT_HWVERSION,
> > +            .type = QEMU_OPT_STRING,
> > +            .help = "VMDK hardware version",
> > +            .def_value_str = "undefined"
> > +        },
> > +        {
> >              .name = BLOCK_OPT_SUBFMT,
> >              .type = QEMU_OPT_STRING,
> >              .help =
> > diff --git a/include/block/block_int.h b/include/block/block_int.h
> > index 10d8759..931a412 100644
> > --- a/include/block/block_int.h
> > +++ b/include/block/block_int.h
> > @@ -38,12 +38,12 @@
> >  #include "qemu/throttle.h"
> >  
> >  #define BLOCK_FLAG_ENCRYPT          1
> > -#define BLOCK_FLAG_COMPAT6          4
> >  #define BLOCK_FLAG_LAZY_REFCOUNTS   8
> >  
> >  #define BLOCK_OPT_SIZE              "size"
> >  #define BLOCK_OPT_ENCRYPT           "encryption"
> >  #define BLOCK_OPT_COMPAT6           "compat6"
> > +#define BLOCK_OPT_HWVERSION         "hwversion"
> >  #define BLOCK_OPT_BACKING_FILE      "backing_file"
> >  #define BLOCK_OPT_BACKING_FMT       "backing_fmt"
> >  #define BLOCK_OPT_CLUSTER_SIZE      "cluster_size"
> > diff --git a/qemu-doc.texi b/qemu-doc.texi
> > index 79141d3..f37fd31 100644
> > --- a/qemu-doc.texi
> > +++ b/qemu-doc.texi
> > @@ -693,6 +693,9 @@ Supported options:
> >  File name of a base image (see @option{create} subcommand).
> >  @item compat6
> >  Create a VMDK version 6 image (instead of version 4)
> > +@item hwversion
> > +Specify vmdk virtual hardware version. Compat6 flag cannot be enabled
> > +if hwversion is specified.
> >  @item subformat
> >  Specifies which VMDK subformat to use. Valid options are
> >  @code{monolithicSparse} (default),
> > -- 
> > 1.9.1
> > 
> 
> Reviewed-by: Fam Zheng <famz@redhat.com>

Thanks, applied to block-next.

Kevin

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Janne Karhunen]

On Fri, May 6, 2016 at 2:05 PM, Kevin Wolf <kwolf@redhat.com> wrote:

>>
>> Reviewed-by: Fam Zheng <famz@redhat.com>
>
> Thanks, applied to block-next.

Great, thanks everyone!


--
Janne

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Janne Karhunen]

On Mon, May 2, 2016 at 5:49 PM, Fam Zheng <famz@redhat.com> wrote:

> Yes.
>
> But you get:

Fair enough, sent another one.

Thanks,


-- 
Janne

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Fam Zheng]

On Mon, 05/02 04:30, Janne Karhunen wrote:
> >>      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false)) {
> >> -        flags |= BLOCK_FLAG_COMPAT6;
> >
> > Please remove BLOCK_FLAG_COMPAT6 from include/block/block_int.h| as well.
> 
> Wasn't it removed?
> 
> -#define BLOCK_FLAG_COMPAT6          4

Yes you are right, it's my oversight. :)

> 
> 
> >> +        if (strcmp(hw_version, "4")) {
> >
> > Doesn't compat6 mean hw_version is 6? I think you want s/4/6/ here.
> 
> Yes and that's the functionality here. So the logic is that if
> BLOCK_OPT_COMPAT6 was defined and hw_version had something other than
> "4" we fail saying can't do both.
> 
> Test case:
> qemu-img convert -f vmdk -O vmdk img.vmdk img.vmdk-output
> ddb.virtualHWVersion = "4" (default)

Yes.

> 
> qemu-img convert -f vmdk -O vmdk -o compat6=on img.vmdk img.vmdk-output
> ddb.virtualHWVersion = "6" (compat6 default)

Yes.

> 
> qemu-img convert -f vmdk -O vmdk -o hwversion=9 img.vmdk img.vmdk-output
> ddb.virtualHWVersion = "9" (the new feature)

Yes.

But you get:

1)
    $ qemu-img create -f vmdk -o compat6=on,hwversion=4 /tmp/a.vmdk 1G; grep -a
    virtualHWVersion /tmp/a.vmdk Formatting '/tmp/a.vmdk', fmt=vmdk size=1073741824 compat6=on hwversion=4
    ddb.virtualHWVersion = "6"

which is wrong.

2) IMO if the documentation says "the options are mutually exclusive" instead
of "specific value combinations are invalid", this:

    $ qemu-img create -f vmdk -o compat6=off,hwversion=9 /tmp/a.vmdk 1G; grep -a virtualHWVersion /tmp/a.vmdk
    Formatting '/tmp/a.vmdk', fmt=vmdk size=1073741824 compat6=off hwversion=9
    ddb.virtualHWVersion = "9"

should better be rejected:

    $ qemu-img create -f vmdk -o compat6=off,hwversion=9 /tmp/a.vmdk 1G; grep -a virtualHWVersion /tmp/a.vmdk
    Formatting '/tmp/a.vmdk', fmt=vmdk size=1073741824 compat6=off hwversion=9
    qemu-img: /tmp/a.vmdk: compat6 and hwversion are mutually exclusive

1) is a bug in your code, and 2) is a documenting problem.

Fam

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Janne Karhunen]

On Fri, Apr 29, 2016 at 12:41 AM, Fam Zheng <famz@redhat.com> wrote:

>> Signed-off-by: Janne Karhunen <Janne.Karhunen@gmail.com>
>
> Sorry for the late reply, I was distracted..

Like wise, was traveling..


>>      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false)) {
>> -        flags |= BLOCK_FLAG_COMPAT6;
>
> Please remove BLOCK_FLAG_COMPAT6 from include/block/block_int.h| as well.

Wasn't it removed?

-#define BLOCK_FLAG_COMPAT6          4


>> +        if (strcmp(hw_version, "4")) {
>
> Doesn't compat6 mean hw_version is 6? I think you want s/4/6/ here.

Yes and that's the functionality here. So the logic is that if
BLOCK_OPT_COMPAT6 was defined and hw_version had something other than
"4" we fail saying can't do both.

Test case:
qemu-img convert -f vmdk -O vmdk img.vmdk img.vmdk-output
ddb.virtualHWVersion = "4" (default)

qemu-img convert -f vmdk -O vmdk -o compat6=on img.vmdk img.vmdk-output
ddb.virtualHWVersion = "6" (compat6 default)

qemu-img convert -f vmdk -O vmdk -o hwversion=9 img.vmdk img.vmdk-output
ddb.virtualHWVersion = "9" (the new feature)


>> +@item hwversion
>> +Specify vmdk virtual hardware version. Option is mutually exclusive
>> +with the compat6 flag.
>
> The code is not really treating them exclusively, it only checks for
> contradiction of values.  So I suggest either fix the document or the code.

It fails instantly if you try to pass both so what do you suggest it
should do (returns -EINVAL)?

qemu-img convert -f vmdk -O vmdk -o compat6=on -o hwversion=9 img.vmdk
img.vmdk.out
qemu-img: img.vmdk-output: error while converting vmdk: compat6 and
hwversion are mutually exclusive


--
Janne

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Fam Zheng]

On Thu, 04/21 12:29, Janne Karhunen wrote:
> From: Janne Karhunen <janne.karhunen@gmail.com>
> 
> Vmdk images have metadata to indicate the vmware virtual
> hardware version image was created/tested to run with.
> Allow users to specify that version via new 'hwversion'
> option.
> 
> Signed-off-by: Janne Karhunen <Janne.Karhunen@gmail.com>

Sorry for the late reply, I was distracted..

> ---
>  block/vmdk.c              | 23 +++++++++++++++++++----
>  include/block/block_int.h |  2 +-
>  qemu-doc.texi             |  3 +++
>  3 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 45f9d3c..4b5cac8 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1829,8 +1829,8 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>      int64_t total_size = 0, filesize;
>      char *adapter_type = NULL;
>      char *backing_file = NULL;
> +    char *hw_version = NULL;
>      char *fmt = NULL;
> -    int flags = 0;
>      int ret = 0;
>      bool flat, split, compress;
>      GString *ext_desc_lines;
> @@ -1861,7 +1861,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>          "# The Disk Data Base\n"
>          "#DDB\n"
>          "\n"
> -        "ddb.virtualHWVersion = \"%d\"\n"
> +        "ddb.virtualHWVersion = \"%s\"\n"
>          "ddb.geometry.cylinders = \"%" PRId64 "\"\n"
>          "ddb.geometry.heads = \"%" PRIu32 "\"\n"
>          "ddb.geometry.sectors = \"63\"\n"
> @@ -1878,8 +1878,16 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>                            BDRV_SECTOR_SIZE);
>      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);
>      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false)) {
> -        flags |= BLOCK_FLAG_COMPAT6;

Please remove BLOCK_FLAG_COMPAT6 from include/block/block_int.h| as well.

> +        if (strcmp(hw_version, "4")) {

Doesn't compat6 mean hw_version is 6? I think you want s/4/6/ here.

> +            error_setg(errp,
> +                       "compat6 and hwversion are mutually exclusive");
> +            ret = -EINVAL;
> +            goto exit;
> +        }
> +        g_free(hw_version);
> +        hw_version = g_strdup("6");
>      }
>      fmt = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
>      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ZEROED_GRAIN, false)) {
> @@ -2001,7 +2009,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>                             fmt,
>                             parent_desc_line,
>                             ext_desc_lines->str,
> -                           (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
> +                           hw_version,
>                             total_size /
>                                 (int64_t)(63 * number_heads * BDRV_SECTOR_SIZE),
>                             number_heads,
> @@ -2047,6 +2055,7 @@ exit:
>      }
>      g_free(adapter_type);
>      g_free(backing_file);
> +    g_free(hw_version);
>      g_free(fmt);
>      g_free(desc);
>      g_free(path);
> @@ -2298,6 +2307,12 @@ static QemuOptsList vmdk_create_opts = {
>              .def_value_str = "off"
>          },
>          {
> +            .name = BLOCK_OPT_HWVERSION,
> +            .type = QEMU_OPT_STRING,
> +            .help = "VMDK hardware version",
> +            .def_value_str = "4"
> +        },
> +        {
>              .name = BLOCK_OPT_SUBFMT,
>              .type = QEMU_OPT_STRING,
>              .help =
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 10d8759..931a412 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -38,12 +38,12 @@
>  #include "qemu/throttle.h"
>  
>  #define BLOCK_FLAG_ENCRYPT          1
> -#define BLOCK_FLAG_COMPAT6          4
>  #define BLOCK_FLAG_LAZY_REFCOUNTS   8
>  
>  #define BLOCK_OPT_SIZE              "size"
>  #define BLOCK_OPT_ENCRYPT           "encryption"
>  #define BLOCK_OPT_COMPAT6           "compat6"
> +#define BLOCK_OPT_HWVERSION         "hwversion"
>  #define BLOCK_OPT_BACKING_FILE      "backing_file"
>  #define BLOCK_OPT_BACKING_FMT       "backing_fmt"
>  #define BLOCK_OPT_CLUSTER_SIZE      "cluster_size"
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 79141d3..d3a5f94 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -693,6 +693,9 @@ Supported options:
>  File name of a base image (see @option{create} subcommand).
>  @item compat6
>  Create a VMDK version 6 image (instead of version 4)
> +@item hwversion
> +Specify vmdk virtual hardware version. Option is mutually exclusive
> +with the compat6 flag.

The code is not really treating them exclusively, it only checks for
contradiction of values.  So I suggest either fix the document or the code.

Fam

>  @item subformat
>  Specifies which VMDK subformat to use. Valid options are
>  @code{monolithicSparse} (default),
> -- 
> 1.9.1
> 
> 

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Fam Zheng]

On Fri, 04/22 08:23, Janne Karhunen wrote:
> On Wed, Apr 20, 2016 at 9:44 PM, Fam Zheng <famz@redhat.com> wrote:
> 
> >> That's certainly doable and kind of makes sense, but I'm not entirely
> >> sure that compat6 flag makes any sense to begin with. Does it?
> >
> > I don't know VMware products well enough to tell, but it has been available
> > since forever, therefore my suggestion is to just keep it which cannot hurt.
> 
> I sent the new patch yesterday with this option still present and it
> works the same as it always has. I also tested it to work on all
> option pairings, so it should be good to go AFAIK. Hope you have some
> time to check that.

Thanks for letting me know! I'll take a look soon. (Patches without correctly
Cc'ing maintainers are very easy to get neglected. Please remember to use
./scripts/get_maintainer.pl (or ./MAINTAINERS) next time.)

Fam

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Janne Karhunen]

On Wed, Apr 20, 2016 at 9:44 PM, Fam Zheng <famz@redhat.com> wrote:

>> That's certainly doable and kind of makes sense, but I'm not entirely
>> sure that compat6 flag makes any sense to begin with. Does it?
>
> I don't know VMware products well enough to tell, but it has been available
> since forever, therefore my suggestion is to just keep it which cannot hurt.

I sent the new patch yesterday with this option still present and it
works the same as it always has. I also tested it to work on all
option pairings, so it should be good to go AFAIK. Hope you have some
time to check that.


-- 
Janne

[Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Janne Karhunen]

From: Janne Karhunen <janne.karhunen@gmail.com>

Vmdk images have metadata to indicate the vmware virtual
hardware version image was created/tested to run with.
Allow users to specify that version via new 'hwversion'
option.

Signed-off-by: Janne Karhunen <Janne.Karhunen@gmail.com>
---
 block/vmdk.c              | 23 +++++++++++++++++++----
 include/block/block_int.h |  2 +-
 qemu-doc.texi             |  3 +++
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 45f9d3c..4b5cac8 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1829,8 +1829,8 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
     int64_t total_size = 0, filesize;
     char *adapter_type = NULL;
     char *backing_file = NULL;
+    char *hw_version = NULL;
     char *fmt = NULL;
-    int flags = 0;
     int ret = 0;
     bool flat, split, compress;
     GString *ext_desc_lines;
@@ -1861,7 +1861,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
         "# The Disk Data Base\n"
         "#DDB\n"
         "\n"
-        "ddb.virtualHWVersion = \"%d\"\n"
+        "ddb.virtualHWVersion = \"%s\"\n"
         "ddb.geometry.cylinders = \"%" PRId64 "\"\n"
         "ddb.geometry.heads = \"%" PRIu32 "\"\n"
         "ddb.geometry.sectors = \"63\"\n"
@@ -1878,8 +1878,16 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
                           BDRV_SECTOR_SIZE);
     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);
     if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false)) {
-        flags |= BLOCK_FLAG_COMPAT6;
+        if (strcmp(hw_version, "4")) {
+            error_setg(errp,
+                       "compat6 and hwversion are mutually exclusive");
+            ret = -EINVAL;
+            goto exit;
+        }
+        g_free(hw_version);
+        hw_version = g_strdup("6");
     }
     fmt = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
     if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ZEROED_GRAIN, false)) {
@@ -2001,7 +2009,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
                            fmt,
                            parent_desc_line,
                            ext_desc_lines->str,
-                           (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
+                           hw_version,
                            total_size /
                                (int64_t)(63 * number_heads * BDRV_SECTOR_SIZE),
                            number_heads,
@@ -2047,6 +2055,7 @@ exit:
     }
     g_free(adapter_type);
     g_free(backing_file);
+    g_free(hw_version);
     g_free(fmt);
     g_free(desc);
     g_free(path);
@@ -2298,6 +2307,12 @@ static QemuOptsList vmdk_create_opts = {
             .def_value_str = "off"
         },
         {
+            .name = BLOCK_OPT_HWVERSION,
+            .type = QEMU_OPT_STRING,
+            .help = "VMDK hardware version",
+            .def_value_str = "4"
+        },
+        {
             .name = BLOCK_OPT_SUBFMT,
             .type = QEMU_OPT_STRING,
             .help =
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 10d8759..931a412 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -38,12 +38,12 @@
 #include "qemu/throttle.h"
 
 #define BLOCK_FLAG_ENCRYPT          1
-#define BLOCK_FLAG_COMPAT6          4
 #define BLOCK_FLAG_LAZY_REFCOUNTS   8
 
 #define BLOCK_OPT_SIZE              "size"
 #define BLOCK_OPT_ENCRYPT           "encryption"
 #define BLOCK_OPT_COMPAT6           "compat6"
+#define BLOCK_OPT_HWVERSION         "hwversion"
 #define BLOCK_OPT_BACKING_FILE      "backing_file"
 #define BLOCK_OPT_BACKING_FMT       "backing_fmt"
 #define BLOCK_OPT_CLUSTER_SIZE      "cluster_size"
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 79141d3..d3a5f94 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -693,6 +693,9 @@ Supported options:
 File name of a base image (see @option{create} subcommand).
 @item compat6
 Create a VMDK version 6 image (instead of version 4)
+@item hwversion
+Specify vmdk virtual hardware version. Option is mutually exclusive
+with the compat6 flag.
 @item subformat
 Specifies which VMDK subformat to use. Valid options are
 @code{monolithicSparse} (default),
-- 
1.9.1

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Fam Zheng]

On Wed, 04/20 20:22, Janne Karhunen wrote:
> On Wed, Apr 20, 2016 at 7:22 PM, Fam Zheng <famz@redhat.com> wrote:
> 
> > For backward compatibility reason, we shouldn't remove a pre-existing option
> > without solid justification and probably a grace period. I suggest adding the
> > hwversion option in addition, and document and check it as exclusive with the
> > compat6 flag.
> 
> That's certainly doable and kind of makes sense, but I'm not entirely
> sure that compat6 flag makes any sense to begin with. Does it?

I don't know VMware products well enough to tell, but it has been available
since forever, therefore my suggestion is to just keep it which cannot hurt.

If we really want to get rid of it, we should start printing a warning message
to indicate using the flag is deprecated and remove it one or a few releases
later. IMO it's not really worth the effort.

Fam

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Janne Karhunen]

On Wed, Apr 20, 2016 at 8:26 PM, Fam Zheng <famz@redhat.com> wrote:

>> In other words, without the patch not a single qemu-img generated vmdk
>> is 'good' for the system I'm using.
>
> Thanks for the explanation, please add this information into the commit message
> in next submission and also fix the option code so I can ack your patch.

Ack, will do. Thanks


-- 
Janne

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Fam Zheng]

On Wed, 04/20 20:02, Janne Karhunen wrote:
> On Wed, Apr 20, 2016 at 7:22 PM, Fam Zheng <famz@redhat.com> wrote:
> 
> >> Replace hardcoded vmdk description file hardware version flag with
> >> a user definable version.
> >
> > Hi Janne,
> >
> > Since this is a new feature, please explain why this change is desired (i.e.
> > the necessity.)
> 
> See here what it controls for vmware:
> 
> https://kb.vmware.com/selfservice/microsites/search.do?languag
> e=en_US&cmd=displayKC&externalId=1003746
> 
> Seems that the vmware cluster I'm using is configured to eat vmdks
> with only certain virtual hardware versions specified as being
> supported, so this patch allows me to define that version. Besides
> that, I've read that to mean that it's a hint from me (as the vmdk
> author) to the cluster that this is the vmware virtual hardware
> version I have tested this vmdk to run with. It's actually relatively
> useful metadata as such.
> 
> In other words, without the patch not a single qemu-img generated vmdk
> is 'good' for the system I'm using.

Thanks for the explanation, please add this information into the commit message
in next submission and also fix the option code so I can ack your patch.

Fam

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Janne Karhunen]

On Wed, Apr 20, 2016 at 7:22 PM, Fam Zheng <famz@redhat.com> wrote:

> For backward compatibility reason, we shouldn't remove a pre-existing option
> without solid justification and probably a grace period. I suggest adding the
> hwversion option in addition, and document and check it as exclusive with the
> compat6 flag.

That's certainly doable and kind of makes sense, but I'm not entirely
sure that compat6 flag makes any sense to begin with. Does it?


--
Janne

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Janne Karhunen]

On Wed, Apr 20, 2016 at 7:22 PM, Fam Zheng <famz@redhat.com> wrote:

>> Replace hardcoded vmdk description file hardware version flag with
>> a user definable version.
>
> Hi Janne,
>
> Since this is a new feature, please explain why this change is desired (i.e.
> the necessity.)

See here what it controls for vmware:

https://kb.vmware.com/selfservice/microsites/search.do?languag
e=en_US&cmd=displayKC&externalId=1003746

Seems that the vmware cluster I'm using is configured to eat vmdks
with only certain virtual hardware versions specified as being
supported, so this patch allows me to define that version. Besides
that, I've read that to mean that it's a hint from me (as the vmdk
author) to the cluster that this is the vmware virtual hardware
version I have tested this vmdk to run with. It's actually relatively
useful metadata as such.

In other words, without the patch not a single qemu-img generated vmdk
is 'good' for the system I'm using.


--
Janne

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Fam Zheng]

On Wed, 04/20 15:27, Janne Karhunen wrote:
> From: Janne Karhunen <janne.karhunen@gmail.com>
> 
> Replace hardcoded vmdk description file hardware version flag with
> a user definable version.

Hi Janne,

Since this is a new feature, please explain why this change is desired (i.e.
the necessity.)

> 
> Signed-off-by: Janne Karhunen <Janne.Karhunen@gmail.com>
> ---
>  block/vmdk.c              | 20 ++++++++++----------
>  include/block/block_int.h |  3 +--
>  qemu-doc.texi             |  4 ++--
>  3 files changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 45f9d3c..1436785 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1829,8 +1829,8 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>      int64_t total_size = 0, filesize;
>      char *adapter_type = NULL;
>      char *backing_file = NULL;
> +    char *hw_version = NULL;
>      char *fmt = NULL;
> -    int flags = 0;
>      int ret = 0;
>      bool flat, split, compress;
>      GString *ext_desc_lines;
> @@ -1861,7 +1861,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>          "# The Disk Data Base\n"
>          "#DDB\n"
>          "\n"
> -        "ddb.virtualHWVersion = \"%d\"\n"
> +        "ddb.virtualHWVersion = \"%s\"\n"
>          "ddb.geometry.cylinders = \"%" PRId64 "\"\n"
>          "ddb.geometry.heads = \"%" PRIu32 "\"\n"
>          "ddb.geometry.sectors = \"63\"\n"
> @@ -1878,9 +1878,8 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>                            BDRV_SECTOR_SIZE);
>      adapter_type = qemu_opt_get_del(opts, BLOCK_OPT_ADAPTER_TYPE);
>      backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
> -    if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false)) {
> -        flags |= BLOCK_FLAG_COMPAT6;
> -    }
> +    hw_version = qemu_opt_get_del(opts, BLOCK_OPT_HWVERSION);
> +
>      fmt = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
>      if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ZEROED_GRAIN, false)) {
>          zeroed_grain = true;
> @@ -2001,7 +2000,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>                             fmt,
>                             parent_desc_line,
>                             ext_desc_lines->str,
> -                           (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
> +                           hw_version,
>                             total_size /
>                                 (int64_t)(63 * number_heads * BDRV_SECTOR_SIZE),
>                             number_heads,
> @@ -2047,6 +2046,7 @@ exit:
>      }
>      g_free(adapter_type);
>      g_free(backing_file);
> +    g_free(hw_version);
>      g_free(fmt);
>      g_free(desc);
>      g_free(path);
> @@ -2292,10 +2292,10 @@ static QemuOptsList vmdk_create_opts = {
>              .help = "File name of a base image"
>          },
>          {
> -            .name = BLOCK_OPT_COMPAT6,
> -            .type = QEMU_OPT_BOOL,
> -            .help = "VMDK version 6 image",
> -            .def_value_str = "off"
> +            .name = BLOCK_OPT_HWVERSION,
> +            .type = QEMU_OPT_STRING,
> +            .help = "VMDK hardware version",
> +            .def_value_str = "4"
>          },

For backward compatibility reason, we shouldn't remove a pre-existing option
without solid justification and probably a grace period. I suggest adding the
hwversion option in addition, and document and check it as exclusive with the
compat6 flag.

Fam

>          {
>              .name = BLOCK_OPT_SUBFMT,
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 10d8759..0852316 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -38,12 +38,11 @@
>  #include "qemu/throttle.h"
>  
>  #define BLOCK_FLAG_ENCRYPT          1
> -#define BLOCK_FLAG_COMPAT6          4
>  #define BLOCK_FLAG_LAZY_REFCOUNTS   8
>  
>  #define BLOCK_OPT_SIZE              "size"
>  #define BLOCK_OPT_ENCRYPT           "encryption"
> -#define BLOCK_OPT_COMPAT6           "compat6"
> +#define BLOCK_OPT_HWVERSION         "hwversion"
>  #define BLOCK_OPT_BACKING_FILE      "backing_file"
>  #define BLOCK_OPT_BACKING_FMT       "backing_fmt"
>  #define BLOCK_OPT_CLUSTER_SIZE      "cluster_size"
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 79141d3..79f4168 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -691,8 +691,8 @@ Supported options:
>  @table @code
>  @item backing_file
>  File name of a base image (see @option{create} subcommand).
> -@item compat6
> -Create a VMDK version 6 image (instead of version 4)
> +@item hwversion
> +Specify vmdk hardware version (default 4)
>  @item subformat
>  Specifies which VMDK subformat to use. Valid options are
>  @code{monolithicSparse} (default),
> -- 
> 1.9.1
> 
> 

[Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Janne Karhunen]

From: Janne Karhunen <janne.karhunen@gmail.com>

Replace hardcoded vmdk description file hardware version flag with
a user definable version.

Signed-off-by: Janne Karhunen <Janne.Karhunen@gmail.com>
---
 block/vmdk.c              | 20 ++++++++++----------
 include/block/block_int.h |  3 +--
 qemu-doc.texi             |  4 ++--
 3 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 45f9d3c..1436785 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1829,8 +1829,8 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
     int64_t total_size = 0, filesize;
     char *adapter_type = NULL;
     char *backing_file = NULL;
+    char *hw_version = NULL;
     char *fmt = NULL;
-    int flags = 0;
     int ret = 0;
     bool flat, split, compress;
     GString *ext_desc_lines;
@@ -1861,7 +1861,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
         "# The Disk Data Base\n"
         "#DDB\n"
         "\n"
-        "ddb.virtualHWVersion = \"%d\"\n"
+        "ddb.virtualHWVersion = \"%s\"\n"
         "ddb.geometry.cylinders = \"%" PRId64 "\"\n"
         "ddb.geometry.heads = \"%" PRIu32 "\"\n"
         "ddb.geometry.sectors = \"63\"\n"
@@ -1878,9 +1878,8 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
                           BDRV_SECTOR_SIZE);
     adapter_type = qemu_opt_get_del(opts, BLOCK_OPT_ADAPTER_TYPE);
     backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
-    if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false)) {
-        flags |= BLOCK_FLAG_COMPAT6;
-    }
+    hw_version = qemu_opt_get_del(opts, BLOCK_OPT_HWVERSION);
+
     fmt = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
     if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ZEROED_GRAIN, false)) {
         zeroed_grain = true;
@@ -2001,7 +2000,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
                            fmt,
                            parent_desc_line,
                            ext_desc_lines->str,
-                           (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
+                           hw_version,
                            total_size /
                                (int64_t)(63 * number_heads * BDRV_SECTOR_SIZE),
                            number_heads,
@@ -2047,6 +2046,7 @@ exit:
     }
     g_free(adapter_type);
     g_free(backing_file);
+    g_free(hw_version);
     g_free(fmt);
     g_free(desc);
     g_free(path);
@@ -2292,10 +2292,10 @@ static QemuOptsList vmdk_create_opts = {
             .help = "File name of a base image"
         },
         {
-            .name = BLOCK_OPT_COMPAT6,
-            .type = QEMU_OPT_BOOL,
-            .help = "VMDK version 6 image",
-            .def_value_str = "off"
+            .name = BLOCK_OPT_HWVERSION,
+            .type = QEMU_OPT_STRING,
+            .help = "VMDK hardware version",
+            .def_value_str = "4"
         },
         {
             .name = BLOCK_OPT_SUBFMT,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 10d8759..0852316 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -38,12 +38,11 @@
 #include "qemu/throttle.h"
 
 #define BLOCK_FLAG_ENCRYPT          1
-#define BLOCK_FLAG_COMPAT6          4
 #define BLOCK_FLAG_LAZY_REFCOUNTS   8
 
 #define BLOCK_OPT_SIZE              "size"
 #define BLOCK_OPT_ENCRYPT           "encryption"
-#define BLOCK_OPT_COMPAT6           "compat6"
+#define BLOCK_OPT_HWVERSION         "hwversion"
 #define BLOCK_OPT_BACKING_FILE      "backing_file"
 #define BLOCK_OPT_BACKING_FMT       "backing_fmt"
 #define BLOCK_OPT_CLUSTER_SIZE      "cluster_size"
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 79141d3..79f4168 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -691,8 +691,8 @@ Supported options:
 @table @code
 @item backing_file
 File name of a base image (see @option{create} subcommand).
-@item compat6
-Create a VMDK version 6 image (instead of version 4)
+@item hwversion
+Specify vmdk hardware version (default 4)
 @item subformat
 Specifies which VMDK subformat to use. Valid options are
 @code{monolithicSparse} (default),
-- 
1.9.1