Re: [PATCH] blkif: add indirect descriptors interface to public headers

[David Vrabel <david.vrabel@citrix.com>]

On 12/11/13 14:43, Jan Beulich wrote:
>>>> On 12.11.13 at 15:12, David Vrabel <david.vrabel@citrix.com> wrote:
>> On 12/11/13 13:46, Paul Durrant wrote:
>>>> +    uint64_t       id;           /* private guest value, echoed in resp  */
>>>> +    blkif_sector_t sector_number;/* start sector idx on disk (r/w only)  */
>>>> +    blkif_vdev_t   handle;       /* same as for read/write requests      */
>>>> +    grant_ref_t
>>>> indirect_grefs[BLKIF_MAX_INDIRECT_PAGES_PER_REQUEST];
>>>> +#ifdef __i386__
>>>> +    uint64_t       pad;          /* Make it 64 byte aligned on i386      */
>>>> +#endif
>>
>> This last field looks a bit odd though.  Why is it needed?
> 
> On 64-bit there's a 32 bit hole before "id" and another one at the
> end of the structure. In order for the structure size to be as
> specified in the comment, both these holes need to be accounted
> for.

Ok.

>>>> +};
>>>> +typedef struct blkif_request_indirect blkif_request_indirect_t;
>>>> +
>>>> +struct blkif_request_segment_aligned {
>>>> +    grant_ref_t gref;            /* reference to I/O buffer frame        */
>>>> +    /* @first_sect: first sector in frame to transfer (inclusive).   */
>>>> +    /* @last_sect: last sector in frame to transfer (inclusive).     */
>>>> +    uint8_t     first_sect, last_sect;
>>
>> Missing uint8_t padding here?
> 
> Note that there are _two_ uint8_t-s.

Doh. That's poor style though.   A line per field, please.

David