[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 2/4] block: vhdx header for the QEMU support
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [RFC PATCH 2/4] block: vhdx header for the QEMU support of VHDX images |
Date: |
Tue, 19 Feb 2013 09:05:27 -0500 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, Feb 19, 2013 at 02:55:23PM +0100, Stefan Hajnoczi wrote:
> 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
Fair enough. You are right, it will likely be too easy to goof and
miss a proper bitfield translation. I'll convert over to bitmasks -
the other upside of that is I will struggle less to keep lines under
80 char long, without a union and struct to wade through :)
Thanks,
Jeff
[Qemu-devel] [RFC PATCH 4/4] block: add vhdx to Makefile.obj for compile, Jeff Cody, 2013/02/18
[Qemu-devel] [RFC PATCH 3/4] block: VHDX block driver support, Jeff Cody, 2013/02/18