[v2,21/35] scsi-disk: bump SCSIRequest reference count until aio completion runs

In some cases a request may be canceled before the completion
callback runs.  Keep a reference to the request between starting
an AIO operation, and let scsi_*_complete remove it.

Since scsi_handle_rw_error returns whether something else has to
be done for the request by the caller, it makes sense to transfer
ownership of the ref to scsi_handle_rw_error when it returns 1;
scsi_dma_restart_bh will then free the reference after restarting
the operation.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
        v1->v2: Add "return" after calling scsi_write_complete with
        ENOMEDIUM.  Bump refcount before testing data direction.

 hw/scsi-disk.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

On 10/17/2011 05:37 PM, Paolo Bonzini wrote:
> In some cases a request may be canceled before the completion
> callback runs.  Keep a reference to the request between starting
> an AIO operation, and let scsi_*_complete remove it.
>
> Since scsi_handle_rw_error returns whether something else has to
> be done for the request by the caller, it makes sense to transfer
> ownership of the ref to scsi_handle_rw_error when it returns 1;
> scsi_dma_restart_bh will then free the reference after restarting
> the operation.
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>          v1->v2: Add "return" after calling scsi_write_complete with
>          ENOMEDIUM.  Bump refcount before testing data direction.
>
>   hw/scsi-disk.c |   16 ++++++++++++++++
>   1 files changed, 16 insertions(+), 0 deletions(-)
>
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index e28c39d..702e6ca 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -139,6 +139,7 @@ static void scsi_read_complete(void * opaque, int ret)
>
>       if (ret) {
>           if (scsi_handle_rw_error(r, -ret, SCSI_REQ_STATUS_RETRY_READ)) {
> +            /* Leave in ref for scsi_dma_restart_bh.  */
>               return;
>           }
>       }
> @@ -149,6 +150,7 @@ static void scsi_read_complete(void * opaque, int ret)
>       r->sector += n;
>       r->sector_count -= n;
>       scsi_req_data(&r->req, r->qiov.size);
> +    scsi_req_unref(&r->req);
>   }
>
>   static void scsi_flush_complete(void * opaque, int ret)
> @@ -163,11 +165,13 @@ static void scsi_flush_complete(void * opaque, int ret)
>
>       if (ret<  0) {
>           if (scsi_handle_rw_error(r, -ret, SCSI_REQ_STATUS_RETRY_FLUSH)) {
> +            /* Leave in ref for scsi_dma_restart_bh.  */
>               return;
>           }
>       }
>
>       scsi_req_complete(&r->req, GOOD);
> +    scsi_req_unref(&r->req);
>   }
>
>   /* Read more data from scsi device into buffer.  */
> @@ -193,6 +197,8 @@ static void scsi_read_data(SCSIRequest *req)
>       /* No data transfer may already be in progress */
>       assert(r->req.aiocb == NULL);
>
> +    /* Save a ref for scsi_read_complete, in case r is canceled.  */
> +    scsi_req_ref(&r->req);
>       if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
>           DPRINTF("Data transfer direction invalid\n");
>           scsi_read_complete(r, -EINVAL);
> @@ -201,7 +207,9 @@ static void scsi_read_data(SCSIRequest *req)
>
>       if (s->tray_open) {
>           scsi_read_complete(r, -ENOMEDIUM);
> +        return;
>       }
> +
>       n = scsi_init_iovec(r);
>       bdrv_acct_start(s->bs,&r->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
>       r->req.aiocb = bdrv_aio_readv(s->bs, r->sector,&r->qiov, n,
> @@ -279,6 +287,7 @@ static void scsi_write_complete(void * opaque, int ret)
>           DPRINTF("Write complete tag=0x%x more=%d\n", r->req.tag, r->qiov.size);
>           scsi_req_data(&r->req, r->qiov.size);
>       }
> +    scsi_req_unref(&r->req);
>   }
>
>   static void scsi_write_data(SCSIRequest *req)
> @@ -290,6 +299,8 @@ static void scsi_write_data(SCSIRequest *req)
>       /* No data transfer may already be in progress */
>       assert(r->req.aiocb == NULL);
>
> +    /* Save a ref for scsi_write_complete, in case r is canceled.  */
> +    scsi_req_ref(&r->req);
>       if (r->req.cmd.mode != SCSI_XFER_TO_DEV) {
>           DPRINTF("Data transfer direction invalid\n");
>           scsi_write_complete(r, -EINVAL);
> @@ -300,6 +311,7 @@ static void scsi_write_data(SCSIRequest *req)
>       if (n) {
>           if (s->tray_open) {
>               scsi_write_complete(r, -ENOMEDIUM);
> +            return;
>           }
>           bdrv_acct_start(s->bs,&r->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_WRITE);
>           r->req.aiocb = bdrv_aio_writev(s->bs, r->sector,&r->qiov, n,
> @@ -344,6 +356,8 @@ static void scsi_dma_restart_bh(void *opaque)
>                       scsi_req_complete(&r->req, GOOD);
>                   }
>               }
> +            /* This reference was left in by scsi_handle_rw_error.  */
> +            scsi_req_unref(&r->req);
>           }
>       }
>   }
> @@ -1345,6 +1359,8 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
>           r->iov.iov_len = rc;
>           break;
>       case SYNCHRONIZE_CACHE:
> +        /* Save a ref for scsi_flush_complete, in case r is canceled.  */
> +        scsi_req_ref(&r->req);
>           bdrv_acct_start(s->bs,&r->acct, 0, BDRV_ACCT_FLUSH);
>           r->req.aiocb = bdrv_aio_flush(s->bs, scsi_flush_complete, r);
>           if (r->req.aiocb == NULL) {

This needs to be redone after dropping 20/35.

Paolo