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
