From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39965) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XKRFv-0006oO-1S for qemu-devel@nongnu.org; Thu, 21 Aug 2014 08:14:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XKRFm-0003dd-0s for qemu-devel@nongnu.org; Thu, 21 Aug 2014 08:14:14 -0400 Received: from mail-wi0-x236.google.com ([2a00:1450:400c:c05::236]:41398) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XKRFl-0003dS-QF for qemu-devel@nongnu.org; Thu, 21 Aug 2014 08:14:05 -0400 Received: by mail-wi0-f182.google.com with SMTP id d1so8346589wiv.15 for ; Thu, 21 Aug 2014 05:14:05 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <53F5E289.8050703@redhat.com> Date: Thu, 21 Aug 2014 14:14:01 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1408622216-9578-1-git-send-email-famz@redhat.com> <1408622216-9578-2-git-send-email-famz@redhat.com> In-Reply-To: <1408622216-9578-2-git-send-email-famz@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH 1/9] block: Add bdrv_aio_cancel_async List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , Stefan Hajnoczi Il 21/08/2014 13:56, Fam Zheng ha scritto: > + BlockDriverAIOCB *save = g_new(BlockDriverAIOCB, 1); > + save->cb = acb->cb; > + save->opaque = acb->opaque; > + acb->cb = bdrv_aio_cancel_async_cb; > + acb->opaque = acb; > + acb->cancel_acb_save = save; No need for this extra field. You can make a struct with {acb->cb, acb->opaque, acb} and pass it as the opaque. You also do not need to allocate it on the heap, since everything is synchronous. > + > + /* Use the synchronous version and hope our cb is called. */ > + acb->aiocb_info->cancel(acb); Unfortunately, acb has been freed at this point, so you'll be accessing a dangling pointer. I think you need to modify all cancel implementations. For example: - return whether they have called the callback - only free the acb if they have called the callback - otherwise, free the acb in bdrv_aio_cancel Paolo > + if (acb->cancel_acb_save) { > + /* cb is not called yet, let's call it */ > + bdrv_aio_cancel_async_cb(acb->opaque, -ECANCELED); > + } > + } > +} > + > /**************************************************************/ > /* async block device emulation */ > > diff --git a/include/block/aio.h b/include/block/aio.h > index c23de3c..fcc5c87 100644 > --- a/include/block/aio.h > +++ b/include/block/aio.h > @@ -27,6 +27,7 @@ typedef void BlockDriverCompletionFunc(void *opaque, int ret); > > typedef struct AIOCBInfo { > void (*cancel)(BlockDriverAIOCB *acb); > + void (*cancel_async)(BlockDriverAIOCB *acb); > size_t aiocb_size; > } AIOCBInfo; > > @@ -35,6 +36,7 @@ struct BlockDriverAIOCB { > BlockDriverState *bs; > BlockDriverCompletionFunc *cb; > void *opaque; > + BlockDriverAIOCB *cancel_acb_save; > };