Re: [PATCH] virtio-blk: document data[] size constraints

[Stefan Hajnoczi <stefanha@redhat.com>]

On Wed, Jan 30, 2019 at 12:01:01PM -0500, Michael S. Tsirkin wrote:
> On Wed, Jan 30, 2019 at 04:57:18PM +0800, Stefan Hajnoczi wrote:
> > The struct virtio_blk_req->data[] field is a multiple of 512 bytes long
> > for read and write requests.  Flush requests don't use data[] at all.
> > 
> > The new discard and write zeroes requests being introduced in VIRTIO 1.1
> > put struct virtio_blk_discard_write_zeroes elements into data[], so it
> > must be a multiple of the struct size.
> > 
> > The uint8_t data[][512] pseudo-code makes it look like discard and write
> > zeroes requests must pad to 512 bytes.  This wastes memory since struct
> > virtio_blk_discard_write_data is only 16 bytes long.
> > 
> > Furthermore, all known implementations wishing to take advantage of this
> > upcoming VIRTIO 1.1 feature do not use 512-byte padding (Linux
> > virtio_blk.ko, QEMU virtio-blk device emulation, the SPDK virtio-blk
> > driver, and the SPDK vhost-user-blk device backend).
> > 
> > This patch documents the data[] size constraints clearly in the driver
> > normative section.  This is clearer than the current pseudo-code.
> > 
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Changpeng Liu <changpeng.liu@intel.com>
> > Cc: Stefano Garzarella <sgarzare@redhat.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> Yes I don't see how we can require 512 bytes there.
> Looks good, thanks!
> As a reminder next step is github issue, sending Fixes: tag and
> asking for a vote.
> I also noticed a couple of minor defects listed below.
> Would you mind taking a look and handling them?

Thanks, I have sent v2 and created the GitHub issue.

Stefan

Attachment: signature.asc
Description: PGP signature