Re: [Qemu-devel] [PATCH 0/9] Add full scatter-gather support for SCSI generic devices

[Alex Pyrgiotis <apyrgio@arrikto.com>]

Hi Paolo,

On 12/17/2015 12:31 PM, Paolo Bonzini wrote:
> 
> 
> On 17/12/2015 09:47, Alex Pyrgiotis wrote:
>>>> Which commands have large payloads and are on the data path, for
>>>> scsi-block?  Or is the use case just scsi-generic (e.g. tape devices?)?
>>>>
>>>> (Just trying to understand before I dive into the patches).
>> Sure, no problem. The commands that have large payloads and are on the
>> data path are the classic SCSI READ/WRITE commands. Usually, these
>> commands are implemented with vectored reads/writes, which utilize the
>> controller's scatter-gather list.
>>
>> However, when opening a "scsi-block" device with the default cache
>> policy (cache=writeback), QEMU fallbacks to the "scsi-generic" functions
>> (i.e, SG_IO ioctl requests) for reading/writing data [1]. In this case,
>> the data are copied in a bounce buffer, which is the issue that this
>> patch tackles.
> 
> Right, I forgot about that.  However, falling back to scsi-generic
> effectively means that scsi-block is always O_DIRECT/cache=none.  So why
> not just specify cache=none?

If I understand correctly, what you're saying is that if "scsi-block" is
started with "cache=writeback" and internally uses ioctl()s to bypass
the page cache, why not set "cache=none" beforehand and use
readv()/writev()?

This is a valid suggestion, but this patch does not target only the
"scsi-block" device type. Its purpose is to allow faster read/writes via
ioctl()s, either to a "scsi-block" device or to a "scsi-generic" device.
Note that the latter device type can only use ioctl()s, so it cannot
benefit from the readv()/writev() DMA interface and currently has to use
a bounce buffer.

> We can improve the code to print a warning if you don't.  (It needs some
> care: iscsi never caches, independent of the cache= argument, so we
> don't want to warn for it.  But it can be done).

I wasn't particularly concerned about that issue. I'd may prefer if this
was explicitly addressed in the QEMU doc, under the "cache=" section,
but that's a different discussion.

Thanks,
Alex