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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list