On Sun, Jan 27, 2019 at 12:57:20PM +0000, Stefan Hajnoczi wrote: > On Fri, Jan 25, 2019 at 02:17:01PM -0500, Michael S. Tsirkin wrote: > > On Fri, Jan 25, 2019 at 03:12:45PM +0000, Stefan Hajnoczi wrote: > > > Based on the Linux guest driver code and the lack of more evidence in > > > the spec, I'm pretty sure data[] doesn't need to be padded to 512 bytes > > > for discard/write zero requests. > > > > OK. Must devices support such padding? > > I see no reason to require padding. Host APIs and physical storage > controllers do not require it.
I'm not talking about requiring padding I'm talking about supporting padding on device side. One reason would be ease of extending the spec for guest. E.g. imagine adding more fields to the command. With suppport for padding guest simply adds fields to its structure, and the unused ones are ignored by device. Without support for padding you need two versions of the structure. Another reason would be compatibility. For better or worse the spec does make it look like 512 padding is present. This patch was merged very recently so I agree it's not too late to clarify it that it's not required, but it seems a bit too much to disallow that completely. Let's assume for a minute a device turns up that already implemented the 512 byte assumption? If padding is allowed then we can write a driver that works with both devices. > > > > Looking at the QEMU sources, it seems like it can deal with both and > > > > always sets the status right behind the last byte: > > > > > > > > req->in = (void *)in_iov[in_num - 1].iov_base > > > > + in_iov[in_num - 1].iov_len > > > > - sizeof(struct virtio_blk_inhdr); > > > > > > > > Anyway, I think the virtio spec should be clearer here to avoid bad > > > > implementations in the future, so maybe Changpeng or Michael could > > > > update the spec here a little bit? > > > > > > Yep, good point. VIRTIO 1.1 is available for public comments, so I've > > > CCed the list. > > > > > > Stefan > > > > Thanks! > > Care creating a github issue? And maybe proposing a spec patch. > > Okay. I will do this on Wednesday 30th of January. > > Stefan
