[libvirt] locking down struct size/layout in remote-protocol.x

[Daniel P. Berrange]

On Thu, May 06, 2010 at 04:09:25PM -0600, Eric Blake wrote:
> On 05/06/2010 12:35 PM, Jim Meyering wrote:
> > This week we noticed that a change to struct remote_error
> > was causing trouble (new member addition changed the size of
> > the object being decoded -- bad for the protocol).
> > 
> > In order to ensure that no such changes sneak in again,
> > I'm planning to do the following.
> > 
> >     pdwtags src/libvirt_driver_remote_la-remote_protocol.o
> > 
> > prints, among other things, detailed type info for every struct:
> > 
> >     /* 89 */
> >     struct remote_nonnull_domain {
> >             remote_nonnull_string      name;                 /*     0     8 */
> >             remote_uuid                uuid;                 /*     8    16 */
> >             int                        id;                   /*    24     4 */
> > 
> >             /* size: 32, cachelines: 1, members: 3 */
> >             /* last cacheline: 32 bytes */
> > 
> >             /* BRAIN FART ALERT! 32 != 28 + 0(holes), diff = 4 */
> > 
> >     }; /* size: 32 */
> 
> Ouch.  Architecture sizing plays a role.  On a 32-bit machine, the first
> struct is:
> 
> /* 86 */
> struct remote_nonnull_domain {
>         remote_nonnull_string      name;                 /*     0     4 */
>         remote_uuid                uuid;                 /*     4    16 */
>         int                        id;                   /*    20     4 */
> 
>         /* size: 24, cachelines: 1, members: 3 */
>         /* last cacheline: 24 bytes */
> }; /* size: 24 */
> 
> 
> Are we sure migration between 32-bit and 64-bit hypervisors works?  And
> if it does, then these structs don't quite match what is actually sent
> over the wire.  At any rate,

Yes, the XDR protocol encoding is architecture + wordsize independant. The
struct sizes won't match what is sent on the wire, and the latter is the thing
we actually need to verify.

Perhaps marking all structs with __attribute__((packed))  will make then
architecture invariant enough for checking of the struct to suffice.

Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|