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

2013-08-19 Thread 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

2013-08-19 Thread 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 f...@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



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

2013-08-18 Thread 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

2013-08-18 Thread 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
 



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

2013-08-12 Thread 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 f...@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