From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59177) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UCvJJ-0005k3-VE for qemu-devel@nongnu.org; Tue, 05 Mar 2013 12:05:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UCvJC-0008Ux-IO for qemu-devel@nongnu.org; Tue, 05 Mar 2013 12:05:53 -0500 Sender: Paolo Bonzini From: Paolo Bonzini Date: Tue, 5 Mar 2013 18:05:25 +0100 Message-Id: <1362503125-27057-7-git-send-email-pbonzini@redhat.com> In-Reply-To: <1362503125-27057-1-git-send-email-pbonzini@redhat.com> References: <1362503125-27057-1-git-send-email-pbonzini@redhat.com> Subject: [Qemu-devel] [PATCH 6/6] scsi-disk: handle io_canceled uniformly and correctly List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: qemu-stable@nongnu.org Always check it immediately after calling bdrv_acct_done, and always do a "goto done" in case the "done" label has to free some memory---as is the case for scsi_unmap_complete in the previous patch. This patch could fix problems that happen when a request is split into multiple parts, and one of them is canceled. Then the next part is fired, but the HBA's cancellation callbacks have fired already. Whether this happens or not, depends on how the block/ driver implements AIO cancellation. It it does a simple bdrv_drain_all() or similar, then it will not have a problem. If it only cancels the given AIOCB, this scenario could happen. Cc: qemu-stable@nongnu.org Signed-off-by: Paolo Bonzini --- hw/scsi-disk.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 6c0ddff..4a0673c 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -178,6 +178,9 @@ static void scsi_aio_complete(void *opaque, int ret) assert(r->req.aiocb != NULL); r->req.aiocb = NULL; bdrv_acct_done(s->qdev.conf.bs, &r->acct); + if (r->req.io_canceled) { + goto done; + } if (ret < 0) { if (scsi_handle_rw_error(r, -ret)) { @@ -223,6 +226,10 @@ static void scsi_write_do_fua(SCSIDiskReq *r) { SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev); + if (r->req.io_canceled) { + goto done; + } + if (scsi_is_cmd_fua(&r->req.cmd)) { bdrv_acct_start(s->qdev.conf.bs, &r->acct, 0, BDRV_ACCT_FLUSH); r->req.aiocb = bdrv_aio_flush(s->qdev.conf.bs, scsi_aio_complete, r); @@ -230,6 +237,8 @@ static void scsi_write_do_fua(SCSIDiskReq *r) } scsi_req_complete(&r->req, GOOD); + +done: if (!r->req.io_canceled) { scsi_req_unref(&r->req); } @@ -243,6 +252,9 @@ static void scsi_dma_complete(void *opaque, int ret) assert(r->req.aiocb != NULL); r->req.aiocb = NULL; bdrv_acct_done(s->qdev.conf.bs, &r->acct); + if (r->req.io_canceled) { + goto done; + } if (ret < 0) { if (scsi_handle_rw_error(r, -ret)) { @@ -274,6 +286,9 @@ static void scsi_read_complete(void * opaque, int ret) assert(r->req.aiocb != NULL); r->req.aiocb = NULL; bdrv_acct_done(s->qdev.conf.bs, &r->acct); + if (r->req.io_canceled) { + goto done; + } if (ret < 0) { if (scsi_handle_rw_error(r, -ret)) { @@ -305,6 +320,9 @@ static void scsi_do_read(void *opaque, int ret) r->req.aiocb = NULL; bdrv_acct_done(s->qdev.conf.bs, &r->acct); } + if (r->req.io_canceled) { + goto done; + } if (ret < 0) { if (scsi_handle_rw_error(r, -ret)) { @@ -312,10 +330,6 @@ 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); @@ -423,6 +437,9 @@ static void scsi_write_complete(void * opaque, int ret) r->req.aiocb = NULL; bdrv_acct_done(s->qdev.conf.bs, &r->acct); } + if (r->req.io_canceled) { + goto done; + } if (ret < 0) { if (scsi_handle_rw_error(r, -ret)) { -- 1.8.1.2