[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block: Fix vpc initialization of the Dynamic Di
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH] block: Fix vpc initialization of the Dynamic Disk Header |
Date: |
Wed, 09 Nov 2011 16:58:05 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:7.0) Gecko/20110927 Thunderbird/7.0 |
Am 09.11.2011 16:42, schrieb Charles Arnold:
>>>> On 11/9/2011 at 02:46 AM, in message <address@hidden>, Kevin Wolf
> <address@hidden> 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 <address@hidden>
>>>
>>> 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