Re: [Qemu-devel] [PATCH RFC] virtio: add virtqueue_fill_partial

[Michael S. Tsirkin]

On Mon, Apr 27, 2015 at 04:58:37PM +0200, Paolo Bonzini wrote:
> On 27/04/2015 16:38, Michael S. Tsirkin wrote:
> > > "If you never leave holes unwritten in the iov, use virtqueue_fill
> > > instead.
> > 
> > Is it just holes? Never data you are unsure about?
> 
> Then "unwritten (or possibly unwritten)"?
> 
> If the data you are unsure is at the end, it's okay.  This is what
> virtio-scsi does when it has a failure (the "!= VIRTIO_SCSI_S_OK" case).
> 
> > >  If the guest is not relying on iov boundaries, it should never
> > > be necessary to use this function."
> > 
> > You never know what guest does though, do you?
> 
> I think that this sentence is wrong, but only because you cannot know
> the format of _all_ request headers/footers and thus it's wrong to make
> such a sweeping generalization.
> 
> Anyhow, I'll describe what I had in mind.  Consider the case I gave
> before, where you have a 1 sector (512-bytes) read.  Consider now two
> guests, both of them passing 2049 bytes for the device-writable area.
> 
> Guest 1 doesn't have anylayout; it placed 2048 bytes in iovec 0 and 1
> byte in iovec 1.  The device thus has to write bytes 0-511 and byte
> 2048.  In this case host_len = 2049, guest_len = 512.
> 
> Guest 2 has anylayout (or uses virtio v1).  It placed 2048 bytes in
> iovec 0 and 1 byte in iovec 1.  The device however "knows" that the read
> is just 512 bytes in length; it writes bytes 0-512 (the last byte is the
> status) and ignores bytes 513-2048.  In this case host_len = guest_len =
> 513.
> 
> However, there could be a legitimate reason for a device to leave some
> bytes uninitialized in the middle, possibly for optimization reasons.
> In this case, you might have to use virtqueue_fill_partial.
> 
> ...
> 
> Oh wait, that's exactly what happens with virtio-scsi!
> 
> sense_len in the response header is usually zero, and in that case we
> don't write the sense[] array.  So, I guess virtio-scsi in the common
> case should return just the offset up to sense_len?
> 
> Paolo

So it appears.

-- 
MST