qemu-devel
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]