qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] cow: make padding in the header explicit


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH] cow: make padding in the header explicit
Date: Fri, 05 Sep 2014 13:01:14 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)

Stefan Hajnoczi <address@hidden> writes:

> On Thu, Sep 04, 2014 at 04:10:14PM +0200, Kevin Wolf wrote:
>> Am 04.09.2014 um 15:51 hat Stefan Hajnoczi geschrieben:
>> > On Thu, Sep 04, 2014 at 06:07:32AM -0600, Eric Blake wrote:
>> > > On 09/04/2014 02:58 AM, Stefan Hajnoczi wrote:
>> > > > On-disk structures should be marked packed so the compiler does not
>> > > > insert padding for field alignment.  Padding should be explicit so
>> > > > on-disk layout is obvious and we don't rely on the 
>> > > > architecture-specific
>> > > > ABI for alignment rules.
>> > > > 
>> > > > The pahole(1) diff shows that the padding is now explicit and offsets
>> > > > are unchanged:
>> > > > 
>> > > >        char backing_file[1024]; /* 8 1024 */
>> > > >        /* --- cacheline 16 boundary (1024 bytes) was 8 bytes ago --- */
>> > > >        int32_t mtime; /* 1032 4 */
>> > > > -
>> > > > -      /* XXX 4 bytes hole, try to pack */
>> > > > -
>> > > > + uint32_t padding; /* 1036 4 */
>> > > >        uint64_t size; /* 1040 8 */
>> > > 
>> > > Was a 32-bit build also inserting this padding, or do we have historical
>> > > differences where 32-bit and 64-bit cow files are actually different,
>> > > and we may need to be prepared to parse files from both sources?
>> > 
>> > Good point.  Let's not merge this patch since it breaks 32-bit hosts.
>> > 
>> > The fact that no one hit problems when exchanging files between 32-bit
>> > and 64-bit machines shows that the cow format is rarely used.
>> > 
>> > At this point we have 2 different formats: one without padding
>> > (i386-style) and one with padding (x86_64-style).  The chance of more
>> > variants is small but who knows, maybe some other host architecture ABI
>> > has yet another alignment rule for uint64_t.
>> > 
>> > I'd like to git rm block/cow.c but I suppose the backwards-compatible
>> > thing to do is to introduce subformats to support both variants.
>> > Opinions?
>> 
>> Can we safely detect which of the subformats we have? But I'm not sure
>> if it's even worth fixing.
>
> I think it would default to the subformat depending on the host
> architecture but allow overriding with -o subformat=i386|x86_64.

Admirable dedication to bug-compatibility, but...

> I'm also not sure if it's worth fixing.  The cow file format is so
> rarely used I wonder if we'd be better off without it.

... good grief, nuke it already :)



reply via email to

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