Re: [PATCH v8] virtio_blk: add discard and write zeroes support

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1271 bytes --]

On Fri, Oct 12, 2018 at 02:06:28PM -0700, Daniel Verkamp wrote:
> +		range[n].flags = cpu_to_le32(flags);
> +		range[n].num_sectors = cpu_to_le32(num_sectors);
> +		range[n].sector = cpu_to_le64(sector);
...
> +/* Discard/write zeroes range for each request. */
> +struct virtio_blk_discard_write_zeroes {
> +	/* discard/write zeroes start sector */
> +	__virtio64 sector;
> +	/* number of discard/write zeroes sectors */
> +	__virtio32 num_sectors;
> +	/* flags for this range */
> +	__virtio32 flags;

cpu_to_le32() is being used on __virtio32 fields instead of cpu_to_virtio32().

From include/uapi/linux/virtio_types.h:

  /*
   * __virtio{16,32,64} have the following meaning:
   * - __u{16,32,64} for virtio devices in legacy mode, accessed in native endian
   * - __le{16,32,64} for standard-compliant virtio devices
   */

From the VIRTIO specification:

  struct virtio_blk_discard_write_zeroes {
         le64 sector;
         le32 num_sectors;
         struct {
                 le32 unmap:1;
                 le32 reserved:31;
         } flags;
  };


Since the VIRTIO spec says these fields are little-endian, I think these
fields should be declared just __u32 and __u64 instead of __virtio32 and
__virtio64.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]