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

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



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:

There's a way out.

> /* 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,

Not a problem.  We don't send pointers over the wire.
The types are designed to be used portably.

>> And here's a sample of its output:
>>
>>     verify (sizeof (struct remote_nonnull_domain) == 32);
>
> we'd have to make this output conditional on sizeof(void*) to be
> portable to both 32- and 64- bit machines.

Right.  There are a couple options:

- maintain two sets of sizeof-verifying "verify" directives,
  one for 32-bit, the other for 64-bit.  With this, the verification
  is performed at compile time and requires no special tools, in general.
  However, in the unusual event that we update remote_protocol.x and
  need to regenerate the directives, we'll have to run pdwtags on both
  i686 and x86_64.

- maintain a textual representation of these structs and their members
  (including type names, but not sizes) and make pdwtags a requirement
  for running "make syntax-check", which would perform the verification.
  I.e., keep a copy of the output of pdwtags, but without the comments.


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