| Message ID | 1324510451.31148.140661014597833@webmail.messagingengine.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Dec 21, 2011 at 11:34 PM, B Gordon <bbgordonn@fastmail.fm> wrote: > VMDK: footer must take precedence over header when present > > In e.g. streamOptimized VMDKs from VSphere 4 with this flag set the > header l1_table is bogus and only the footer l1_table can be used to > correctly read extents. > > Also reverts recent change to VMDK4Header so order of rgd_ and > gd_offset matches the VMDK spec. Hi Fam, In commit bb45ded93115ad4303471c9a492579dc36716547 you moved the rgd_offset field after gd_offset. The VMDK spec has the rgd_offset field before gd_offset. Could you please review B.B. Gordon's patch which reverts this change and implements the header/footer precedence described in the "Header and Footer" section for Stream-Optimized Compressed Sparse Extents? If you still have the .vmdk files you tested against it would be interesting to verify that applying this new patch doesn't break them. http://patchwork.ozlabs.org/patch/132758/ Thanks, Stefan
On Thu, Dec 22, 2011 at 7:34 AM, B Gordon <bbgordonn@fastmail.fm> wrote: > VMDK: footer must take precedence over header when present > > In e.g. streamOptimized VMDKs from VSphere 4 with this flag set the > header l1_table is bogus and only the footer l1_table can be used to > correctly read extents. > > Also reverts recent change to VMDK4Header so order of rgd_ and > gd_offset matches the VMDK spec. > > https://bugs.launchpad.net/qemu/+bug/907063 > > Signed-off-by: B.B. Gordon <bbgordonn@fastmail.fm> > > diff --git a/block/vmdk.c b/block/vmdk.c > index 5623ac1..77ff9e1 100644 > --- a/block/vmdk.c > +++ b/block/vmdk.c > @@ -35,6 +35,7 @@ > #define VMDK4_FLAG_RGD (1 << 1) > #define VMDK4_FLAG_COMPRESS (1 << 16) > #define VMDK4_FLAG_MARKER (1 << 17) > +#define VMDK4_HEADER_AT_END 0xffffffffffffffffULL > > typedef struct { > uint32_t version; > @@ -57,8 +58,8 @@ typedef struct { > int64_t desc_offset; > int64_t desc_size; > int32_t num_gtes_per_gte; > - int64_t gd_offset; > int64_t rgd_offset; > + int64_t gd_offset; > int64_t grain_offset; > char filler[1]; > char check_bytes[4]; > @@ -443,11 +445,27 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, > VMDK4Header header; > VmdkExtent *extent; > int64_t l1_backup_offset = 0; > + struct stat sb; Please use `bdrv_getlength' to get file sizes, rather than using `stat' which is system dependent. > > ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header)); > if (ret < 0) { > return ret; > } > + if (le64_to_cpu(header.gd_offset) == VMDK4_HEADER_AT_END) { > + uint64_t filesize; > + if (stat(bs->filename, &sb) == -1) { > + fprintf(stderr, "Unable to get file size.\n"); > + return -1; > + } > + filesize = (((uint64_t)sb.st_size + (512 - 1)) & > ~(uint64_t)(512 - 1)); > + ret = bdrv_pread(bs->file, > + (filesize - 1024 + sizeof(magic)), > + &header, > + sizeof(header)); > + if (ret != sizeof(header)) { > + return ret; if ret is positive and less than sizeof(header), the return value is meaningless. Also makes problem when ret == 0. > + } > + } > if (header.capacity == 0 && header.desc_offset) { > return vmdk_open_desc_file(bs, flags, header.desc_offset << 9); > } >
diff --git a/block/vmdk.c b/block/vmdk.c index 5623ac1..77ff9e1 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -35,6 +35,7 @@ #define VMDK4_FLAG_RGD (1 << 1) #define VMDK4_FLAG_COMPRESS (1 << 16) #define VMDK4_FLAG_MARKER (1 << 17) +#define VMDK4_HEADER_AT_END 0xffffffffffffffffULL typedef struct { uint32_t version; @@ -57,8 +58,8 @@ typedef struct { int64_t desc_offset; int64_t desc_size; int32_t num_gtes_per_gte; - int64_t gd_offset; int64_t rgd_offset; + int64_t gd_offset; int64_t grain_offset; char filler[1]; char check_bytes[4]; @@ -443,11 +445,27 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, VMDK4Header header; VmdkExtent *extent; int64_t l1_backup_offset = 0; + struct stat sb; ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header)); if (ret < 0) { return ret; } + if (le64_to_cpu(header.gd_offset) == VMDK4_HEADER_AT_END) { + uint64_t filesize; + if (stat(bs->filename, &sb) == -1) { + fprintf(stderr, "Unable to get file size.\n"); + return -1; + } + filesize = (((uint64_t)sb.st_size + (512 - 1)) & ~(uint64_t)(512 - 1)); + ret = bdrv_pread(bs->file, + (filesize - 1024 + sizeof(magic)), + &header, + sizeof(header)); + if (ret != sizeof(header)) { + return ret; + } + } if (header.capacity == 0 && header.desc_offset) { return vmdk_open_desc_file(bs, flags, header.desc_offset << 9);
VMDK: footer must take precedence over header when present In e.g. streamOptimized VMDKs from VSphere 4 with this flag set the header l1_table is bogus and only the footer l1_table can be used to correctly read extents. Also reverts recent change to VMDK4Header so order of rgd_ and gd_offset matches the VMDK spec. https://bugs.launchpad.net/qemu/+bug/907063 Signed-off-by: B.B. Gordon <bbgordonn@fastmail.fm> }