Re: [Qemu-devel] [RFC PATCH 2/4] block: vhdx header for the QEMU support of VHDX images

[Stefan Hajnoczi]

On Tue, Feb 19, 2013 at 08:11:48AM -0500, Jeff Cody wrote:
> On Tue, Feb 19, 2013 at 10:02:51AM +0100, Stefan Hajnoczi wrote:
> > On Mon, Feb 18, 2013 at 06:03:30PM -0500, Jeff Cody wrote:
> > > +/* Individual region table entry.  There may be a maximum of 2047 of 
> > > these
> > > + *
> > > + *  There are two known region table properties.  Both are required.
> > > + *  BAT (block allocation table):  2DC27766F62342009D64115E9BFD4A08
> > > + *  Metadata:                      8B7CA20647904B9AB8FE575F050F886E
> > > + */
> > > +typedef struct vhdx_region_table_entry {
> > > +    ms_guid     guid;                   /* 128-bit unique identifier */
> > > +    uint64_t    file_offset;            /* offset of the object in the 
> > > file.
> > > +                                           Must be multiple of 1MB */
> > > +    uint32_t    length;                 /* length, in bytes, of the 
> > > object */
> > > +    union vhdx_rt_bitfield {
> > > +        struct {
> > > +        uint32_t    required:1;        /* 1 if this region must be 
> > > recognized
> > > +                                          in order to load the file */
> > > +        uint32_t    reserved:31;
> > > +        } bits;
> > > +        uint32_t data;
> > > +    } bitfield;
> > 
> > Bitfield in a file format structure, yikes.  Endianness, layout, etc.
> > Better to use uint32_t flags with a VHDX_RT_FLAG_REQUIRED bitmask
> > constant?
> 
> Yeah, pretty ugly - it is also how it is present in the VHDX spec,
> which is why I left the structure definition the same.  The endianness
> of it has to be dealt with either way during the parsing and writing,
> so I didn't see any compelling reason to abstract the struct away from
> a bitfield.

In the VHDX spec they can assume Visual C++ compiler layout.

We need to be careful when loading/saving this struct to disk - QEMU
should be able to open VHDX image files on big-endian ppc hosts.

Much clearer to use bitmasks IMO.  That way we guarantee to get layout
correct.  Using bitfields means your code or future modifications might
miss out a conversion and break some host platform (depending on
compiler and endianness).

Stefan