[Qemu-devel] [PATCH v2 0/4] vmdk: Support ESX files

[Qemu-devel] [PATCH v2 0/4] vmdk: Support ESX files

[Fam Zheng]

This series add support for VMFS and VMFSSPARSE extents, these types are found
in description file from ESX hosts.

 - VMFS is in monolithiFlat format (raw), but hosted in ESX.

 - VMFSSPARSE is the format we call "vmdk3" with magic bytes "COWD".
 
 There's no reason to open a image of the same type (same magic bytes and same
 header) in two different ways, my judgement is the (old) code need to be
 fixed, as it doesn't follow the latest spec any more, and AFAIK this ESX
 server sparse is its only use case. This series fixes the opening of vmdk3 and
 rename it to vmdk_open_vmfs_sparse which is better representing its main usage
 nowadays.


Fam Zheng (3):
  vmdk: fix L1 and L2 table size in vmdk3 open
  vmdk: support vmfsSparse files
  vmdk: Move l1_size check into vmdk_add_extent()

Paolo Bonzini (1):
  vmdk: support vmfs files

 block/vmdk.c | 52 +++++++++++++++++++++++++++-------------------------
 1 file changed, 27 insertions(+), 25 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v2 1/4] vmdk: fix L1 and L2 table size in vmdk3 open

[Fam Zheng]

VMDK3 header has the field l1dir_size, but vmdk_open_vmdk3 hardcoded the
value. This patch honors the header field.

And the L2 table size is 4096 according to VMDK spec[1], instead of
1 << 9 (512).

[1]:
http://www.vmware.com/support/developer/vddk/vmdk_50_technote.pdf?src=vmdk

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vmdk.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 346bb5c..1392542 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -486,14 +486,14 @@ static int vmdk_open_vmdk3(BlockDriverState *bs,
     if (ret < 0) {
         return ret;
     }
-
-    ret = vmdk_add_extent(bs,
-                             bs->file, false,
-                             le32_to_cpu(header.disk_sectors),
-                             le32_to_cpu(header.l1dir_offset) << 9,
-                             0, 1 << 6, 1 << 9,
-                             le32_to_cpu(header.granularity),
-                             &extent);
+    ret = vmdk_add_extent(bs, file, false,
+                          le32_to_cpu(header.disk_sectors),
+                          le32_to_cpu(header.l1dir_offset) << 9,
+                          0,
+                          le32_to_cpu(header.l1dir_size),
+                          4096,
+                          le32_to_cpu(header.granularity),
+                          &extent);
     if (ret < 0) {
         return ret;
     }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 1/4] vmdk: fix L1 and L2 table size in vmdk3 open

[Paolo Bonzini]

Il 13/08/2013 03:21, Fam Zheng ha scritto:
> VMDK3 header has the field l1dir_size, but vmdk_open_vmdk3 hardcoded the
> value. This patch honors the header field.
> 
> And the L2 table size is 4096 according to VMDK spec[1], instead of
> 1 << 9 (512).

I'm not sure from the VMDK spec that _only_ 4096 is supported for VMDK3
files.  The way I read it, VMDK3 files in hosted products are supposed
to have 2K grain tables (as specified by vmdk_open_vmdk3).

Perhaps we can check if L1size * 64 is enough to cover the whole file,
and if not use 4096?

Paolo

> [1]:
> http://www.vmware.com/support/developer/vddk/vmdk_50_technote.pdf?src=vmdk

Re: [Qemu-devel] [PATCH v2 1/4] vmdk: fix L1 and L2 table size in vmdk3 open

[Fam Zheng]

On Sun, 08/18 17:19, Paolo Bonzini wrote:
> Il 13/08/2013 03:21, Fam Zheng ha scritto:
> > VMDK3 header has the field l1dir_size, but vmdk_open_vmdk3 hardcoded the
> > value. This patch honors the header field.
> > 
> > And the L2 table size is 4096 according to VMDK spec[1], instead of
> > 1 << 9 (512).
> 
> I'm not sure from the VMDK spec that _only_ 4096 is supported for VMDK3
> files.  The way I read it, VMDK3 files in hosted products are supposed
> to have 2K grain tables (as specified by vmdk_open_vmdk3).

I presume "COWD" is only specified in "ESXi Host Sparse Extents"
section, which is also in practice the only known use case to me. There
it says "Grain tables have 4096 entries."

I think you meant 2KB grain table specified in section "Hosted Sparse
Extent Metadata", with 512 entries. If so, it should be for VMDK4 with
"KDMV" magic bytes, so doesn't affect "COWD".

Fam
> 
> Perhaps we can check if L1size * 64 is enough to cover the whole file,
> and if not use 4096?
> 
> Paolo
> 
> > [1]:
> > http://www.vmware.com/support/developer/vddk/vmdk_50_technote.pdf?src=vmdk
> 

Re: [Qemu-devel] [PATCH v2 1/4] vmdk: fix L1 and L2 table size in vmdk3 open

[Paolo Bonzini]

Il 19/08/2013 04:18, Fam Zheng ha scritto:
> On Sun, 08/18 17:19, Paolo Bonzini wrote:
>> Il 13/08/2013 03:21, Fam Zheng ha scritto:
>>> VMDK3 header has the field l1dir_size, but vmdk_open_vmdk3 hardcoded the
>>> value. This patch honors the header field.
>>>
>>> And the L2 table size is 4096 according to VMDK spec[1], instead of
>>> 1 << 9 (512).
>>
>> I'm not sure from the VMDK spec that _only_ 4096 is supported for VMDK3
>> files.  The way I read it, VMDK3 files in hosted products are supposed
>> to have 2K grain tables (as specified by vmdk_open_vmdk3).
> 
> I presume "COWD" is only specified in "ESXi Host Sparse Extents"
> section, which is also in practice the only known use case to me. There
> it says "Grain tables have 4096 entries."
> 
> I think you meant 2KB grain table specified in section "Hosted Sparse
> Extent Metadata", with 512 entries. If so, it should be for VMDK4 with
> "KDMV" magic bytes, so doesn't affect "COWD".

Ok, thanks for explaining.

Paolo

Re: [Qemu-devel] [PATCH v2 1/4] vmdk: fix L1 and L2 table size in vmdk3 open

[Kevin Wolf]

Am 13.08.2013 um 03:21 hat Fam Zheng geschrieben:
> VMDK3 header has the field l1dir_size, but vmdk_open_vmdk3 hardcoded the
> value. This patch honors the header field.
> 
> And the L2 table size is 4096 according to VMDK spec[1], instead of
> 1 << 9 (512).
> 
> [1]:
> http://www.vmware.com/support/developer/vddk/vmdk_50_technote.pdf?src=vmdk
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/vmdk.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 346bb5c..1392542 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -486,14 +486,14 @@ static int vmdk_open_vmdk3(BlockDriverState *bs,
>      if (ret < 0) {
>          return ret;
>      }
> -
> -    ret = vmdk_add_extent(bs,
> -                             bs->file, false,
> -                             le32_to_cpu(header.disk_sectors),
> -                             le32_to_cpu(header.l1dir_offset) << 9,
> -                             0, 1 << 6, 1 << 9,
> -                             le32_to_cpu(header.granularity),
> -                             &extent);
> +    ret = vmdk_add_extent(bs, file, false,
> +                          le32_to_cpu(header.disk_sectors),
> +                          le32_to_cpu(header.l1dir_offset) << 9,
> +                          0,
> +                          le32_to_cpu(header.l1dir_size),
> +                          4096,
> +                          le32_to_cpu(header.granularity),
> +                          &extent);

You'll want to add a sanity check for header.l1dir_dir, or move the
existing check from vmdk_open_vmdk4() to vmdk_add_extent().

Kevin

[Qemu-devel] [PATCH v2 2/4] vmdk: support vmfsSparse files

[Fam Zheng]

VMware ESX hosts use a variant of the VMDK3 format, identified by the
vmfsSparse create type ad the VMFSSPARSE extent type.

It has 16 KB grain tables (L2) and a variable-size grain directory (L1).
In addition, the grain size is always 512, but that is not a problem
because it is included in the header.

The format of the extents is documented in the VMDK spec.  The format
of the descriptor file is not documented precisely, but it can be
found at http://kb.vmware.com/kb/10026353 (Recreating a missing virtual
machine disk (VMDK) descriptor file for delta disks).

With these patches, vmfsSparse files only work if opened through the
descriptor file.  Data files without descriptor files, as far as I
could understand, are not supported by ESX.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>

--
v2: Rebase to patch 01.
    Change le64_to_cpu to le32_to_cpu.
    Rename vmdk_open_vmdk3 to vmdk_open_vmfs_sparse, which represents the
    current usage of this format.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vmdk.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 1392542..2cc9594 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -473,9 +473,9 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent)
     return ret;
 }
 
-static int vmdk_open_vmdk3(BlockDriverState *bs,
-                           BlockDriverState *file,
-                           int flags)
+static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
+                                 BlockDriverState *file,
+                                 int flags)
 {
     int ret;
     uint32_t magic;
@@ -674,7 +674,7 @@ static int vmdk_open_sparse(BlockDriverState *bs,
     magic = be32_to_cpu(magic);
     switch (magic) {
         case VMDK3_MAGIC:
-            return vmdk_open_vmdk3(bs, file, flags);
+            return vmdk_open_vmfs_sparse(bs, file, flags);
             break;
         case VMDK4_MAGIC:
             return vmdk_open_vmdk4(bs, file, flags);
@@ -718,7 +718,8 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
         }
 
         if (sectors <= 0 ||
-            (strcmp(type, "FLAT") && strcmp(type, "SPARSE")) ||
+            (strcmp(type, "FLAT") && strcmp(type, "SPARSE") &&
+             strcmp(type, "VMFSSPARSE")) ||
             (strcmp(access, "RW"))) {
             goto next_line;
         }
@@ -741,8 +742,8 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
                 return ret;
             }
             extent->flat_start_offset = flat_offset << 9;
-        } else if (!strcmp(type, "SPARSE")) {
-            /* SPARSE extent */
+        } else if (!strcmp(type, "SPARSE") || !strcmp(type, "VMFSSPARSE")) {
+            /* SPARSE extent and VMFSSPARSE extent are both "COWD" sparse file*/
             ret = vmdk_open_sparse(bs, extent_file, bs->open_flags);
             if (ret) {
                 bdrv_delete(extent_file);
@@ -789,6 +790,7 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
         goto exit;
     }
     if (strcmp(ct, "monolithicFlat") &&
+        strcmp(ct, "vmfsSparse") &&
         strcmp(ct, "twoGbMaxExtentSparse") &&
         strcmp(ct, "twoGbMaxExtentFlat")) {
         fprintf(stderr,
@@ -1381,7 +1383,6 @@ static int coroutine_fn vmdk_co_write_zeroes(BlockDriverState *bs,
     return ret;
 }
 
-
 static int vmdk_create_extent(const char *filename, int64_t filesize,
                               bool flat, bool compress, bool zeroed_grain)
 {
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 3/4] vmdk: support vmfs files

[Fam Zheng]

From: Paolo Bonzini <pbonzini@redhat.com>

VMware ESX hosts also use different create and extent types for flat
files, respectively "vmfs" and "VMFS".  This is not documented, but it
can be found at http://kb.vmware.com/kb/10002511 (Recreating a missing
virtual machine disk (VMDK) descriptor file).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vmdk.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 2cc9594..b653e2e 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -719,7 +719,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
 
         if (sectors <= 0 ||
             (strcmp(type, "FLAT") && strcmp(type, "SPARSE") &&
-             strcmp(type, "VMFSSPARSE")) ||
+             strcmp(type, "VMFS") && strcmp(type, "VMFSSPARSE")) ||
             (strcmp(access, "RW"))) {
             goto next_line;
         }
@@ -732,7 +732,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
         }
 
         /* save to extents array */
-        if (!strcmp(type, "FLAT")) {
+        if (!strcmp(type, "FLAT") || !strcmp(type, "VMFS")) {
             /* FLAT extent */
             VmdkExtent *extent;
 
@@ -790,6 +790,7 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
         goto exit;
     }
     if (strcmp(ct, "monolithicFlat") &&
+        strcmp(ct, "vmfs") &&
         strcmp(ct, "vmfsSparse") &&
         strcmp(ct, "twoGbMaxExtentSparse") &&
         strcmp(ct, "twoGbMaxExtentFlat")) {
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 4/4] vmdk: Move l1_size check into vmdk_add_extent()

[Fam Zheng]

This header check is common to VMDK3 and VMDK4, so move it into
vmdk_add_extent().

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vmdk.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index b653e2e..63b489d 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -401,6 +401,14 @@ static int vmdk_add_extent(BlockDriverState *bs,
         error_report("invalid granularity, image may be corrupt");
         return -EINVAL;
     }
+    if (l1_size > 512 * 1024 * 1024) {
+        /* Although with big capacity and small l1_entry_sectors, we can get a
+         * big l1_size, we don't want unbounded value to allocate the table.
+         * Limit it to 512M, which is 16PB for default cluster and L2 table
+         * size */
+        error_report("L1 size too big");
+        return -EFBIG;
+    }
 
     s->extents = g_realloc(s->extents,
                               (s->num_extents + 1) * sizeof(VmdkExtent));
@@ -598,14 +606,6 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
     }
     l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1)
                 / l1_entry_sectors;
-    if (l1_size > 512 * 1024 * 1024) {
-        /* although with big capacity and small l1_entry_sectors, we can get a
-         * big l1_size, we don't want unbounded value to allocate the table.
-         * Limit it to 512M, which is 16PB for default cluster and L2 table
-         * size */
-        error_report("L1 size too big");
-        return -EFBIG;
-    }
     if (le32_to_cpu(header.flags) & VMDK4_FLAG_RGD) {
         l1_backup_offset = le64_to_cpu(header.rgd_offset) << 9;
     }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 4/4] vmdk: Move l1_size check into vmdk_add_extent()

[Kevin Wolf]

Am 13.08.2013 um 03:21 hat Fam Zheng geschrieben:
> This header check is common to VMDK3 and VMDK4, so move it into
> vmdk_add_extent().
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>

Aha, so this is the fix for patch 1. If you reorder the patches so that
this one comes first, you don't have broken intermediate patches.

Kevin