On Mon, Dec 01, 2014 at 09:16:41AM +0100, David Hildenbrand wrote:

quoted

quoted

Based on patch by Cornelia Huck.

Note: for consistency, and to avoid sparse errors,
      convert all fields, even those no longer in use
      for virtio v1.0.

Signed-off-by: Cornelia Huck <redacted>
Signed-off-by: Michael S. Tsirkin <redacted>

...

quoted

-static unsigned int features[] = {
+static unsigned int features_legacy[] = {
 	VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
 	VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, VIRTIO_BLK_F_SCSI,
 	VIRTIO_BLK_F_WCE, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
 	VIRTIO_BLK_F_MQ,
+}
+;
+static unsigned int features[] = {
+	VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
+	VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
+	VIRTIO_BLK_F_TOPOLOGY,
+	VIRTIO_BLK_F_MQ,
+	VIRTIO_F_VERSION_1,

We can fit this into less lines, like done for features_legacy.

Wrt packing code more tightly, I did it like this to
make it easier to compare the arrays.
Each flag is on the same line in original and new array.

This just looks inconsistent to me.

1. features_legacy is tightly packed
2. half of features is tightly packed

So either all tightly packed or put every item on a single line. At least
that's what I would do :)

quoted

I was asking myself if we could do the conversion of the statical values
somehow upfront, to reduce the patch size and avoid cpu_to_virtio.* at those
places.

Otherwise looks good to me.