Re: [Qemu-devel] [PATCH 3/5] scsi-block: extract scsi_block_is_passthrough

[Fam Zheng <famz@redhat.com>]

On Mon, 07/28 17:08, Paolo Bonzini wrote:
> This will be used for both scsi_block_new_request and the scsi-block
> implementation of parse_cdb.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/scsi/scsi-disk.c | 38 ++++++++++++++++++++++++++------------
>  1 file changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index d47ecd6..81b7276 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -2501,12 +2501,8 @@ static int scsi_block_initfn(SCSIDevice *dev)
>      return scsi_initfn(&s->qdev);
>  }
>  
> -static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
> -                                           uint32_t lun, uint8_t *buf,
> -                                           void *hba_private)
> +static bool scsi_block_is_passthrough(SCSIDiskState *s, uint8_t *buf)
>  {
> -    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
> -
>      switch (buf[0]) {
>      case READ_6:
>      case READ_10:
> @@ -2523,9 +2519,9 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
>      case WRITE_VERIFY_12:
>      case WRITE_VERIFY_16:
>          /* If we are not using O_DIRECT, we might read stale data from the
> -	 * host cache if writes were made using other commands than these
> -	 * ones (such as WRITE SAME or EXTENDED COPY, etc.).  So, without
> -	 * O_DIRECT everything must go through SG_IO.
> +         * host cache if writes were made using other commands than these
> +         * ones (such as WRITE SAME or EXTENDED COPY, etc.).  So, without
> +         * O_DIRECT everything must go through SG_IO.
>           */
>          if (!(bdrv_get_flags(s->qdev.conf.bs) & BDRV_O_NOCACHE)) {
>              break;
> @@ -2542,13 +2538,31 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
>           * just make scsi-block operate the same as scsi-generic for them.
>           */
>          if (s->qdev.type != TYPE_ROM) {
> -            return scsi_req_alloc(&scsi_disk_dma_reqops, &s->qdev, tag, lun,
> -                                  hba_private);
> +            return false;
>          }
> +        break;
> +
> +    default:
> +        break;

Out of curiosity, why add this default branch?

Fam

>      }
>  
> -    return scsi_req_alloc(&scsi_generic_req_ops, &s->qdev, tag, lun,
> -                          hba_private);
> +    return true;
> +}
> +
> +
> +static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
> +                                           uint32_t lun, uint8_t *buf,
> +                                           void *hba_private)
> +{
> +    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
> +
> +    if (scsi_block_is_passthrough(s, buf)) {
> +        return scsi_req_alloc(&scsi_generic_req_ops, &s->qdev, tag, lun,
> +                              hba_private);
> +    } else {
> +        return scsi_req_alloc(&scsi_disk_dma_reqops, &s->qdev, tag, lun,
> +                              hba_private);
> +    }
>  }
>  #endif
>  
> -- 
> 1.9.3
> 
> 
>