Re: [virtio-comment] [PATCH 1/1] ccw: ccw payload description

[Rusty Russell <rusty@au1.ibm.com>]

Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> On Mon, 20 Jan 2014 11:17:17 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
>> On Mon, Jan 20, 2014 at 02:28:00PM +1030, Rusty Russell wrote:
>> > We'll just have to live with the lack of padding, and let's try not to
>> > add any more packed-requiring structures.  If we deprecate
>> > virtio-balloon, the rest will be contained in CCW.
>> > 
>> > Cheers,
>> > Rusty.
>> 
>> For CCW, how about my idea of adding optional padding explicitly?
>> In the compatibility section, we can explicitly specify the
>> the length.
>> Drivers can use offsetof(padding) and avoid packed attribute.
>
> Since Rusty does not like the tables, this looks like a good idea.

Surely making the padding optional makes it more complex, for no real
gain?  It's not that much simpler in practice, is it?

This patch just removes the __attribute__((packed)) where it's useless,
and notes explicitly where it's required:

diff --git a/content.tex b/content.tex
index 803615d..539bf68 100644
--- a/content.tex
+++ b/content.tex
@@ -1992,7 +1992,7 @@ about a queue. It uses the following structure for communicating:
 	struct vq_config_block {
 		be16 index;
 		be16 max_num;
-	} __attribute__ ((packed));
+	};
 \end{lstlisting}
 
 The requested number of buffers for queue index is returned in
@@ -2010,7 +2010,7 @@ structure is
 		be16 num;
 		be64 avail;
 		be64 used;
-	} __attribute__ ((packed));
+	};
 \end{lstlisting}
 
 desc, avail and used contain the guest addresses for the descriptor table,
@@ -2029,7 +2029,7 @@ CCW_CMD_SET_VQ uses the following communication block:
 		be32 align;
 		be16 index;
 		be16 num;
-	} __attribute__ ((packed));
+	};
 \end{lstlisting}
 
 queue contains the guest address for queue index, num the number of buffers
@@ -2071,7 +2071,8 @@ for a total of 8192 feature bits. Feature bits are in
 little-endian byte order.
 
 The CCW commands dealing with features use the following
-communication block:
+communication block.  Note that this command is exactly
+5 bytes long, violating the normal C structure padding rules:
 
 \begin{lstlisting}
 	struct virtio_feature_desc {
@@ -2159,7 +2160,8 @@ two stages:
 
 To communicate the location of the summary and queue indicator bits,
 the driver uses the CCW_CMD_SET_IND_ADAPTER command with the following
-payload:
+payload.  Note that this command is exactly 25 bytes long, violating
+the normal C structure padding rules:
 
 \begin{lstlisting}
 	struct virtio_thinint_area {