Re: [Qemu-block] [Qemu-devel] [PATCH 7/7] scsi-block: always use SG_IO

[Eric Blake]

On 05/23/2016 06:54 AM, Paolo Bonzini wrote:
> Using pread/pwrite or io_submit has the advantage of eliminating the
> bounce buffer, but drops the SCSI status.  This keeps the guest from
> seeing unit attention codes, as well as statuses such as RESERVATION
> CONFLICT.  Because we know scsi-block operates on an SBC device we can
> still use the DMA helpers with SG_IO; just remember to patch the CDBs
> if the transfer is split into multiple segments.
> 
> This means that scsi-block will always use the thread-pool unfortunately,
> instead of respecting aio=native.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  hw/scsi/scsi-disk.c | 214 
> +++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 196 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index f823781..c0eefc0 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -83,6 +83,7 @@ typedef struct SCSIDiskReq {
>      struct iovec iov;
>      QEMUIOVector qiov;
>      BlockAcctCookie acct;
> +    unsigned char *status;
>  } SCSIDiskReq;
>  
>  #define SCSI_DISK_F_REMOVABLE             0
> @@ -190,6 +191,15 @@ static bool scsi_disk_req_check_error(SCSIDiskReq *r, 
> int ret, bool acct_failed)
>          return scsi_handle_rw_error(r, -ret, acct_failed);
>      }
>  
> +    if (r->status && *r->status) {
> +        if (acct_failed) {
> +            SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);

Another use of DO_UPCAST(); would container_of() be better?


> +
> +static BlockAIOCB *scsi_block_do_sgio(SCSIBlockReq *req,
> +                                      int64_t offset, QEMUIOVector *iov,
> +                                      int direction,
> +                                      BlockCompletionFunc *cb, void *opaque)
> +{
> +    sg_io_hdr_t *io_header = &req->io_header;
> +    SCSIDiskReq *r = &req->req;
> +    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
> +    int nb_logical_blocks;
> +    uint64_t lba;
> +    BlockAIOCB *aiocb;
> +
> +    /* This is not supported yet.  It can only happen if the guest does
> +     * reads and writes that are not aligned to one logical sectors

s/one // ?

> +     * _and_ cover multiple MemoryRegions.
> +     */
> +    assert(offset % s->qdev.blocksize == 0);
> +    assert(iov->size % s->qdev.blocksize == 0);

Is asserting for something an ill-behaved guest can do the right behavior?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature