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
