Re: [Qemu-devel] [PATCH] scsi refcounting fix?

[David Gibson]

On Tue, Apr 24, 2012 at 08:37:22AM +0200, Paolo Bonzini wrote:
> Il 24/04/2012 07:02, David Gibson ha scritto:
> > So the patch below fixes my assertion failure, but again, I can't say
> > I understand this well enough to be confident - it might be leaking
> > scsi reqs instead.  But if this isn't the right fix, I hope one of you
> > can help me find where the real refcounting bug is.
> 
> Thanks for the report!
> 
> The fix seems correct.  However, since refcounting is tricky, I prefer
> to follow existing patterns and make scsi_do_read look like a combination
> of scsi_*_complete + scsi_*_data.  The following does both a ref
> (like in scsi_read_data) and and an unref (like in scsi_flush_complete):
> 
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index a914611..49f5860 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -297,6 +297,13 @@ static void scsi_do_read(void *opaque, int ret)
>          }
>      }
>  
> +    if (r->req.io_canceled) {
> +        return;
> +    }
> +
> +    /* The request is used as the AIO opaque value, so add a ref.  */
> +    scsi_req_ref(&r->req);
> +
>      if (r->req.sg) {
>          dma_acct_start(s->qdev.conf.bs, &r->acct, r->req.sg, BDRV_ACCT_READ);
>          r->req.resid -= r->req.sg->size;
> 
> Can you confirm that this works for you?

This seems to work for me, thanks.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson