Re: [Qemu-devel] [PATCH 5/7] scsi-disk: add need_fua_emulation to SCSIDiskClass

[Fam Zheng]

On Mon, 05/23 14:54, Paolo Bonzini wrote:
> scsi-block will be able to do FUA just by passing the request through
> to the LUN (which is also more efficient); there is no need to emulate
> it like we do for scsi-disk.

Even for scsi-disk, shall we just use the block layer FUA fallback already by
passing BDRV_REQ_FUA in blk_aio_pwritev and simply the code in this series?

> 
> Add a new method to distinguish this.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  hw/scsi/scsi-disk.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 4b5db59..f9870e9 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -69,6 +69,7 @@ typedef struct SCSIDiskClass {
>      SCSIDeviceClass parent_class;
>      DMAIOFunc       *dma_readv;
>      DMAIOFunc       *dma_writev;
> +    bool            (*need_fua_emulation)(SCSICommand *cmd);
>  } SCSIDiskClass;
>  
>  typedef struct SCSIDiskReq {
> @@ -78,6 +79,7 @@ typedef struct SCSIDiskReq {
>      uint32_t sector_count;
>      uint32_t buflen;
>      bool started;
> +    bool need_fua_emulation;
>      struct iovec iov;
>      QEMUIOVector qiov;
>      BlockAcctCookie acct;
> @@ -239,7 +241,7 @@ static void scsi_write_do_fua(SCSIDiskReq *r)
>          goto done;
>      }
>  
> -    if (scsi_is_cmd_fua(&r->req.cmd)) {
> +    if (r->need_fua_emulation) {

Should this evaluate a call to r->need_fua_emulation() instead of the function
pointer itself?

>          block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct, 0,
>                           BLOCK_ACCT_FLUSH);
>          r->req.aiocb = blk_aio_flush(s->qdev.conf.blk, scsi_aio_complete, r);
> @@ -416,7 +418,7 @@ static void scsi_read_data(SCSIRequest *req)
>  
>      first = !r->started;
>      r->started = true;
> -    if (first && scsi_is_cmd_fua(&r->req.cmd)) {
> +    if (first && r->need_fua_emulation) {
>          block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct, 0,
>                           BLOCK_ACCT_FLUSH);
>          r->req.aiocb = blk_aio_flush(s->qdev.conf.blk, scsi_do_read_cb, r);
> @@ -2157,6 +2159,7 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, 
> uint8_t *buf)
>  {
>      SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
>      SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
> +    SCSIDiskClass *sdc = (SCSIDiskClass *) object_get_class(OBJECT(s));
>      uint32_t len;
>      uint8_t command;
>  
> @@ -2215,6 +2218,7 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, 
> uint8_t *buf)
>          scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
>          return 0;
>      }
> +    r->need_fua_emulation = sdc->need_fua_emulation(&r->req.cmd);
>      if (r->sector_count == 0) {
>          scsi_req_complete(&r->req, GOOD);
>      }
> @@ -2704,6 +2708,7 @@ static void scsi_disk_base_class_initfn(ObjectClass 
> *klass, void *data)
>      dc->reset = scsi_disk_reset;
>      sdc->dma_readv = scsi_dma_readv;
>      sdc->dma_writev = scsi_dma_writev;
> +    sdc->need_fua_emulation = scsi_is_cmd_fua;
>  }
>  
>  static const TypeInfo scsi_disk_base_info = {
> -- 
> 2.5.5
> 
> 
>