Am 09.11.2011 16:42, schrieb Charles Arnold: >>>> On 11/9/2011 at 02:46 AM, in message <4eba4bec.5040...@redhat.com>, Kevin >>>> Wolf > <kw...@redhat.com> wrote: >> Am 08.11.2011 20:25, schrieb Charles Arnold: >>> The Data Offset field in the Dynamic Disk Header is an 8 byte field. >>> Although the specification (2006-10-11) gives an example of initializing >>> only the first 4 bytes, images generated by Microsoft on Windows initialize >>> all 8 bytes. >>> >>> Failure to initialize all 8 bytes results in errors from utilities >>> that check specifically for the complete Data Offset field initialization. >>> >>> Signed-off-by: Charles Arnold <carn...@suse.com> >>> >>> diff --git a/block/vpc.c b/block/vpc.c >>> index 416f489..35ac3fd 100644 >>> --- a/block/vpc.c >>> +++ b/block/vpc.c >>> @@ -585,7 +585,7 @@ static int vpc_create(const char *filename, >> QEMUOptionParameter *options) >>> >>> memcpy(dyndisk_header->magic, "cxsparse", 8); >>> >>> - dyndisk_header->data_offset = be64_to_cpu(0xFFFFFFFF); >>> + dyndisk_header->data_offset = be64_to_cpu(0xFFFFFFFFFFFFFFFFULL); >>> dyndisk_header->table_offset = be64_to_cpu(3 * 512); >>> dyndisk_header->version = be32_to_cpu(0x00010000); >>> dyndisk_header->block_size = be32_to_cpu(block_size); >> >> Can you please add a short comment explaining why we deviate from the >> specification in this point? If someone notices the discrepancy later, >> he shouldn't have to dig up the git commit message. >> >> Kevin > > Using qemu-img to convert a raw image to a vhd image seems to work fine > accept that only 4 bytes of the 8 byte Dynamic Disk Header Data Offset field > are initialized. This seems correct according to the specification but tools > like Citrix's vhd-util fail when querying the image due to the shortened > initialization. Upon further analysis, it was determined that Microsoft also > initializes all 8 bytes when generating a vhd image on windows. Initializing > all 8 bytes seems more logical and may indicate that Microsoft needs to > update the specification because the requirement for this field changed or > that it is simply an oversight that has never been noticed.
Right, I'm not arguing that your change is wrong. What I'm asking for is just something like: - dyndisk_header->data_offset = be64_to_cpu(0xFFFFFFFF); + /* Note: The spec is actually wrong here, it says 0xFFFFFFFF, but MS tools expect all 64 bits to be set */ + dyndisk_header->data_offset = be64_to_cpu(0xFFFFFFFFFFFFFFFFULL); Kevin
