qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Andreas Färber" <afaerber@suse.de>
To: Charles Arnold <carnold@suse.com>
Cc: Kevin Wolf <kwolf@redhat.com>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] block: Fix vpc initialization of the Dynamic Disk Header
Date: Wed, 09 Nov 2011 17:00:49 +0100	[thread overview]
Message-ID: <4EBAA3B1.707@suse.de> (raw)
In-Reply-To: <4EBA3D0D0200009100076EF0@novprvoes0310.provo.novell.com>

Am 09.11.2011 16:42, schrieb Charles Arnold:
>>>> On 11/9/2011 at 02:46 AM, in message <4EBA4BEC.5040401@redhat.com>, Kevin Wolf
> <kwolf@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 <carnold@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.

Charles, I think Kevin meant inserting a C comment above that line. :)

Maybe: /* Microsoft and Citrix tools expect data_offset to be eight
bytes 0xff although this does not match the spec. */

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

      parent reply	other threads:[~2011-11-09 16:01 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-08 19:25 [Qemu-devel] [PATCH] block: Fix vpc initialization of the Dynamic Disk Header Charles Arnold
2011-11-08 20:16 ` Andreas Färber
2011-11-09  9:46 ` Kevin Wolf
2011-11-09 15:42   ` Charles Arnold
2011-11-09 15:58     ` Kevin Wolf
2011-11-09 16:32       ` Charles Arnold
2011-11-09 16:38         ` Andreas Färber
2011-11-09 16:57         ` Kevin Wolf
2011-11-09 16:00     ` Andreas Färber [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4EBAA3B1.707@suse.de \
    --to=afaerber@suse.de \
    --cc=carnold@suse.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).