Re: [RFC PATCH-for-9.0 v2 09/19] hw/block/xen_blkif: Align structs with QEMU_ALIGNED() instead of #pragma

[Anthony PERARD]

On Tue, Nov 14, 2023 at 03:38:05PM +0100, Philippe Mathieu-Daudé wrote:
> Except imported source files, QEMU code base uses
> the QEMU_ALIGNED() macro to align its structures.

This patch only convert the alignment, but discard pack. We need both or
the struct is changed.

> ---
>  hw/block/xen_blkif.h | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/block/xen_blkif.h b/hw/block/xen_blkif.h
> index 99733529c1..c1d154d502 100644
> --- a/hw/block/xen_blkif.h
> +++ b/hw/block/xen_blkif.h
> @@ -18,7 +18,6 @@ struct blkif_common_response {
>  };
>  
>  /* i386 protocol version */
> -#pragma pack(push, 4)
>  struct blkif_x86_32_request {
>      uint8_t        operation;        /* BLKIF_OP_???                         
> */
>      uint8_t        nr_segments;      /* number of segments                   
> */
> @@ -26,7 +25,7 @@ struct blkif_x86_32_request {
>      uint64_t       id;               /* private guest value, echoed in resp  
> */
>      blkif_sector_t sector_number;    /* start sector idx on disk (r/w only)  
> */
>      struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
> -};
> +} QEMU_ALIGNED(4);

E.g. for this one, I've compare the output of
`pahole --class_name=blkif_x86_32_request build/qemu-system-i386`:

--- before
+++ after
@@ -1,11 +1,15 @@
 struct blkif_x86_32_request {
        uint8_t                    operation;            /*     0     1 */
        uint8_t                    nr_segments;          /*     1     1 */
        uint16_t                   handle;               /*     2     2 */
-       uint64_t                   id;                   /*     4     8 */
-       uint64_t                   sector_number;        /*    12     8 */
-       struct blkif_request_segment seg[11];            /*    20    88 */

-       /* size: 108, cachelines: 2, members: 6 */
-       /* last cacheline: 44 bytes */
-} __attribute__((__packed__));
+       /* XXX 4 bytes hole, try to pack */
+
+       uint64_t                   id;                   /*     8     8 */
+       uint64_t                   sector_number;        /*    16     8 */
+       struct blkif_request_segment seg[11];            /*    24    88 */
+
+       /* size: 112, cachelines: 2, members: 6 */
+       /* sum members: 108, holes: 1, sum holes: 4 */
+       /* last cacheline: 48 bytes */
+} __attribute__((__aligned__(8)));

Thanks,

-- 
Anthony PERARD