From 6f6710aa99ac53b59ff0f14380830cb9ab6bdc14 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 25 Feb 2013 12:12:58 +0100 Subject: scsi: do not call scsi_read_data/scsi_write_data for a canceled request Cc: qemu-stable@nongnu.org Signed-off-by: Paolo Bonzini --- hw/scsi-bus.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'hw') diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c index a97f1cd..01e1dec 100644 --- a/hw/scsi-bus.c +++ b/hw/scsi-bus.c @@ -1508,6 +1508,10 @@ void scsi_req_unref(SCSIRequest *req) will start the next chunk or complete the command. */ void scsi_req_continue(SCSIRequest *req) { + if (req->io_canceled) { + trace_scsi_req_continue_canceled(req->dev->id, req->lun, req->tag); + return; + } trace_scsi_req_continue(req->dev->id, req->lun, req->tag); if (req->cmd.mode == SCSI_XFER_TO_DEV) { req->ops->write_data(req); -- cgit v1.1 From d0242eadc5bba4f3abe34bc5d536bbfb81aa9891 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 25 Feb 2013 12:14:34 +0100 Subject: scsi-disk: do not complete canceled UNMAP requests Canceled requests should never be completed, and doing that could cause accesses to a NULL hba_private field. Cc: qemu-stable@nongnu.org Reported-by: Stefan Priebe Tested-by: Stefan Priebe Signed-off-by: Paolo Bonzini --- hw/scsi-disk.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'hw') diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index d411586..6c0ddff 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1478,13 +1478,17 @@ static void scsi_unmap_complete(void *opaque, int ret) uint32_t nb_sectors; r->req.aiocb = NULL; + if (r->req.io_canceled) { + goto done; + } + if (ret < 0) { if (scsi_handle_rw_error(r, -ret)) { goto done; } } - if (data->count > 0 && !r->req.io_canceled) { + if (data->count > 0) { sector_num = ldq_be_p(&data->inbuf[0]); nb_sectors = ldl_be_p(&data->inbuf[8]) & 0xffffffffULL; if (!check_lba_range(s, sector_num, nb_sectors)) { @@ -1501,10 +1505,9 @@ static void scsi_unmap_complete(void *opaque, int ret) return; } + scsi_req_complete(&r->req, GOOD); + done: - if (data->count == 0) { - scsi_req_complete(&r->req, GOOD); - } if (!r->req.io_canceled) { scsi_req_unref(&r->req); } -- cgit v1.1 From 0c92e0e6b64c9061f7365a2712b9055ea35b52f9 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 25 Feb 2013 12:16:05 +0100 Subject: scsi-disk: handle io_canceled uniformly and correctly 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(-) (limited to 'hw') 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)) { -- cgit v1.1