* [Qemu-devel] [PATCH v5 1/7] scsi: Drop scsi_req_abort
2014-09-30 3:40 [Qemu-devel] [PATCH v5 0/7] virtio-scsi: Asynchronous cancellation Fam Zheng
@ 2014-09-30 3:40 ` Fam Zheng
2014-09-30 3:40 ` [Qemu-devel] [PATCH v5 2/7] scsi-generic: Handle canceled request in scsi_command_complete Fam Zheng
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Fam Zheng @ 2014-09-30 3:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini
The only user of this function is spapr_vscsi.c. We can convert to
scsi_req_cancel plus adding a check in vscsi_request_cancelled.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
hw/scsi/scsi-bus.c | 15 ---------------
hw/scsi/spapr_vscsi.c | 11 ++++++++---
2 files changed, 8 insertions(+), 18 deletions(-)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index af293b5..f90a204 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1736,21 +1736,6 @@ void scsi_req_cancel(SCSIRequest *req)
scsi_req_unref(req);
}
-void scsi_req_abort(SCSIRequest *req, int status)
-{
- if (!req->enqueued) {
- return;
- }
- scsi_req_ref(req);
- scsi_req_dequeue(req);
- req->io_canceled = true;
- if (req->ops->cancel_io) {
- req->ops->cancel_io(req);
- }
- scsi_req_complete(req, status);
- scsi_req_unref(req);
-}
-
static int scsi_ua_precedence(SCSISense sense)
{
if (sense.key != UNIT_ATTENTION) {
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index 048cfc7..ec5dc0d 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -77,8 +77,9 @@ typedef struct vscsi_req {
SCSIRequest *sreq;
uint32_t qtag; /* qemu tag != srp tag */
bool active;
- uint32_t data_len;
bool writing;
+ bool dma_error;
+ uint32_t data_len;
uint32_t senselen;
uint8_t sense[SCSI_SENSE_BUF_SIZE];
@@ -536,8 +537,8 @@ static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t len)
}
if (rc < 0) {
fprintf(stderr, "VSCSI: RDMA error rc=%d!\n", rc);
- vscsi_makeup_sense(s, req, HARDWARE_ERROR, 0, 0);
- scsi_req_abort(req->sreq, CHECK_CONDITION);
+ req->dma_error = true;
+ scsi_req_cancel(req->sreq);
return;
}
@@ -591,6 +592,10 @@ static void vscsi_request_cancelled(SCSIRequest *sreq)
{
vscsi_req *req = sreq->hba_private;
+ if (req->dma_error) {
+ vscsi_makeup_sense(s, req, HARDWARE_ERROR, 0, 0);
+ vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
+ }
vscsi_put_req(req);
}
--
1.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH v5 2/7] scsi-generic: Handle canceled request in scsi_command_complete
2014-09-30 3:40 [Qemu-devel] [PATCH v5 0/7] virtio-scsi: Asynchronous cancellation Fam Zheng
2014-09-30 3:40 ` [Qemu-devel] [PATCH v5 1/7] scsi: Drop scsi_req_abort Fam Zheng
@ 2014-09-30 3:40 ` Fam Zheng
2014-09-30 3:40 ` [Qemu-devel] [PATCH v5 3/7] scsi-bus: Unify request unref in scsi_req_cancel Fam Zheng
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Fam Zheng @ 2014-09-30 3:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini
Now that we always called the cb in bdrv_aio_cancel, let's make scsi-generic
callbacks check io_canceled flag similarly to scsi-disk.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
hw/scsi/scsi-generic.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 20587b4..2a73a43 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -93,6 +93,9 @@ static void scsi_command_complete(void *opaque, int ret)
SCSIGenericReq *r = (SCSIGenericReq *)opaque;
r->req.aiocb = NULL;
+ if (r->req.io_canceled) {
+ goto done;
+ }
if (r->io_header.driver_status & SG_ERR_DRIVER_SENSE) {
r->req.sense_len = r->io_header.sb_len_wr;
}
@@ -133,6 +136,7 @@ static void scsi_command_complete(void *opaque, int ret)
r, r->req.tag, status);
scsi_req_complete(&r->req, status);
+done:
if (!r->req.io_canceled) {
scsi_req_unref(&r->req);
}
@@ -186,8 +190,7 @@ static void scsi_read_complete(void * opaque, int ret)
int len;
r->req.aiocb = NULL;
- if (ret) {
- DPRINTF("IO error ret %d\n", ret);
+ if (ret || r->req.io_canceled) {
scsi_command_complete(r, ret);
return;
}
@@ -246,8 +249,7 @@ static void scsi_write_complete(void * opaque, int ret)
DPRINTF("scsi_write_complete() ret = %d\n", ret);
r->req.aiocb = NULL;
- if (ret) {
- DPRINTF("IO error\n");
+ if (ret || r->req.io_canceled) {
scsi_command_complete(r, ret);
return;
}
--
1.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH v5 3/7] scsi-bus: Unify request unref in scsi_req_cancel
2014-09-30 3:40 [Qemu-devel] [PATCH v5 0/7] virtio-scsi: Asynchronous cancellation Fam Zheng
2014-09-30 3:40 ` [Qemu-devel] [PATCH v5 1/7] scsi: Drop scsi_req_abort Fam Zheng
2014-09-30 3:40 ` [Qemu-devel] [PATCH v5 2/7] scsi-generic: Handle canceled request in scsi_command_complete Fam Zheng
@ 2014-09-30 3:40 ` Fam Zheng
2014-09-30 3:40 ` [Qemu-devel] [PATCH v5 4/7] scsi: Drop SCSIReqOps.cancel_io Fam Zheng
` (3 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Fam Zheng @ 2014-09-30 3:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini
Before, scsi_req_cancel will take ownership of the canceled request and unref
it. We did this because we didn't know whether AIO CB will be called or not
during the cancelling, so we set the io_canceled flag before calling it, and
skip unref in the potentially called callbacks, which is not very nice.
Now, bdrv_aio_cancel has a stricter contract that the completion callbacks are
always called, so we can remove the checks of req->io_canceled and just unref
it in callbacks.
It will also make implementing asynchronous cancellation easier.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
hw/scsi/scsi-disk.c | 37 ++++++++-----------------------------
hw/scsi/scsi-generic.c | 13 ++-----------
2 files changed, 10 insertions(+), 40 deletions(-)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 9645d01..2e45752 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -113,11 +113,6 @@ static void scsi_cancel_io(SCSIRequest *req)
DPRINTF("Cancel tag=0x%x\n", req->tag);
if (r->req.aiocb) {
bdrv_aio_cancel(r->req.aiocb);
-
- /* This reference was left in by scsi_*_data. We take ownership of
- * it the moment scsi_req_cancel is called, independent of whether
- * bdrv_aio_cancel completes the request or not. */
- scsi_req_unref(&r->req);
}
r->req.aiocb = NULL;
}
@@ -197,9 +192,7 @@ static void scsi_aio_complete(void *opaque, int ret)
scsi_req_complete(&r->req, GOOD);
done:
- if (!r->req.io_canceled) {
- scsi_req_unref(&r->req);
- }
+ scsi_req_unref(&r->req);
}
static bool scsi_is_cmd_fua(SCSICommand *cmd)
@@ -246,9 +239,7 @@ 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);
- }
+ scsi_req_unref(&r->req);
}
static void scsi_dma_complete_noio(void *opaque, int ret)
@@ -280,9 +271,7 @@ static void scsi_dma_complete_noio(void *opaque, int ret)
}
done:
- if (!r->req.io_canceled) {
- scsi_req_unref(&r->req);
- }
+ scsi_req_unref(&r->req);
}
static void scsi_dma_complete(void *opaque, int ret)
@@ -320,9 +309,7 @@ static void scsi_read_complete(void * opaque, int ret)
scsi_req_data(&r->req, r->qiov.size);
done:
- if (!r->req.io_canceled) {
- scsi_req_unref(&r->req);
- }
+ scsi_req_unref(&r->req);
}
/* Actually issue a read to the block device. */
@@ -363,9 +350,7 @@ static void scsi_do_read(void *opaque, int ret)
}
done:
- if (!r->req.io_canceled) {
- scsi_req_unref(&r->req);
- }
+ scsi_req_unref(&r->req);
}
/* Read more data from scsi device into buffer. */
@@ -481,9 +466,7 @@ static void scsi_write_complete(void * opaque, int ret)
}
done:
- if (!r->req.io_canceled) {
- scsi_req_unref(&r->req);
- }
+ scsi_req_unref(&r->req);
}
static void scsi_write_data(SCSIRequest *req)
@@ -1582,9 +1565,7 @@ static void scsi_unmap_complete(void *opaque, int ret)
scsi_req_complete(&r->req, GOOD);
done:
- if (!r->req.io_canceled) {
- scsi_req_unref(&r->req);
- }
+ scsi_req_unref(&r->req);
g_free(data);
}
@@ -1678,9 +1659,7 @@ static void scsi_write_same_complete(void *opaque, int ret)
scsi_req_complete(&r->req, GOOD);
done:
- if (!r->req.io_canceled) {
- scsi_req_unref(&r->req);
- }
+ scsi_req_unref(&r->req);
qemu_vfree(data->iov.iov_base);
g_free(data);
}
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 2a73a43..e92b418 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -137,9 +137,7 @@ static void scsi_command_complete(void *opaque, int ret)
scsi_req_complete(&r->req, status);
done:
- if (!r->req.io_canceled) {
- scsi_req_unref(&r->req);
- }
+ scsi_req_unref(&r->req);
}
/* Cancel a pending data transfer. */
@@ -150,11 +148,6 @@ static void scsi_cancel_io(SCSIRequest *req)
DPRINTF("Cancel tag=0x%x\n", req->tag);
if (r->req.aiocb) {
bdrv_aio_cancel(r->req.aiocb);
-
- /* This reference was left in by scsi_*_data. We take ownership of
- * it independent of whether bdrv_aio_cancel completes the request
- * or not. */
- scsi_req_unref(&r->req);
}
r->req.aiocb = NULL;
}
@@ -214,9 +207,7 @@ static void scsi_read_complete(void * opaque, int ret)
bdrv_set_guest_block_size(s->conf.bs, s->blocksize);
scsi_req_data(&r->req, len);
- if (!r->req.io_canceled) {
- scsi_req_unref(&r->req);
- }
+ scsi_req_unref(&r->req);
}
}
--
1.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH v5 4/7] scsi: Drop SCSIReqOps.cancel_io
2014-09-30 3:40 [Qemu-devel] [PATCH v5 0/7] virtio-scsi: Asynchronous cancellation Fam Zheng
` (2 preceding siblings ...)
2014-09-30 3:40 ` [Qemu-devel] [PATCH v5 3/7] scsi-bus: Unify request unref in scsi_req_cancel Fam Zheng
@ 2014-09-30 3:40 ` Fam Zheng
2014-09-30 3:40 ` [Qemu-devel] [PATCH v5 5/7] scsi: Introduce scsi_req_cancel_complete Fam Zheng
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Fam Zheng @ 2014-09-30 3:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini
The only two implementations are identical to each other, with nothing specific
to device: they only call bdrv_aio_cancel with the SCSIRequest.aiocb.
Let's move it to scsi-bus.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
hw/scsi/scsi-bus.c | 4 ++--
hw/scsi/scsi-disk.c | 14 --------------
hw/scsi/scsi-generic.c | 13 -------------
include/hw/scsi/scsi.h | 1 -
4 files changed, 2 insertions(+), 30 deletions(-)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index f90a204..764f6cf 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1727,8 +1727,8 @@ void scsi_req_cancel(SCSIRequest *req)
scsi_req_ref(req);
scsi_req_dequeue(req);
req->io_canceled = true;
- if (req->ops->cancel_io) {
- req->ops->cancel_io(req);
+ if (req->aiocb) {
+ bdrv_aio_cancel(req->aiocb);
}
if (req->bus->info->cancel) {
req->bus->info->cancel(req);
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 2e45752..ef13e66 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -105,18 +105,6 @@ static void scsi_check_condition(SCSIDiskReq *r, SCSISense sense)
scsi_req_complete(&r->req, CHECK_CONDITION);
}
-/* Cancel a pending data transfer. */
-static void scsi_cancel_io(SCSIRequest *req)
-{
- SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
-
- DPRINTF("Cancel tag=0x%x\n", req->tag);
- if (r->req.aiocb) {
- bdrv_aio_cancel(r->req.aiocb);
- }
- r->req.aiocb = NULL;
-}
-
static uint32_t scsi_init_iovec(SCSIDiskReq *r, size_t size)
{
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
@@ -2325,7 +2313,6 @@ static const SCSIReqOps scsi_disk_emulate_reqops = {
.send_command = scsi_disk_emulate_command,
.read_data = scsi_disk_emulate_read_data,
.write_data = scsi_disk_emulate_write_data,
- .cancel_io = scsi_cancel_io,
.get_buf = scsi_get_buf,
};
@@ -2335,7 +2322,6 @@ static const SCSIReqOps scsi_disk_dma_reqops = {
.send_command = scsi_disk_dma_command,
.read_data = scsi_read_data,
.write_data = scsi_write_data,
- .cancel_io = scsi_cancel_io,
.get_buf = scsi_get_buf,
.load_request = scsi_disk_load_request,
.save_request = scsi_disk_save_request,
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index e92b418..7e85047 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -140,18 +140,6 @@ done:
scsi_req_unref(&r->req);
}
-/* Cancel a pending data transfer. */
-static void scsi_cancel_io(SCSIRequest *req)
-{
- SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
-
- DPRINTF("Cancel tag=0x%x\n", req->tag);
- if (r->req.aiocb) {
- bdrv_aio_cancel(r->req.aiocb);
- }
- r->req.aiocb = NULL;
-}
-
static int execute_command(BlockDriverState *bdrv,
SCSIGenericReq *r, int direction,
BlockDriverCompletionFunc *complete)
@@ -458,7 +446,6 @@ const SCSIReqOps scsi_generic_req_ops = {
.send_command = scsi_send_command,
.read_data = scsi_read_data,
.write_data = scsi_write_data,
- .cancel_io = scsi_cancel_io,
.get_buf = scsi_get_buf,
.load_request = scsi_generic_load_request,
.save_request = scsi_generic_save_request,
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 6271ad3..1118107 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -130,7 +130,6 @@ struct SCSIReqOps {
int32_t (*send_command)(SCSIRequest *req, uint8_t *buf);
void (*read_data)(SCSIRequest *req);
void (*write_data)(SCSIRequest *req);
- void (*cancel_io)(SCSIRequest *req);
uint8_t *(*get_buf)(SCSIRequest *req);
void (*save_request)(QEMUFile *f, SCSIRequest *req);
--
1.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH v5 5/7] scsi: Introduce scsi_req_cancel_complete
2014-09-30 3:40 [Qemu-devel] [PATCH v5 0/7] virtio-scsi: Asynchronous cancellation Fam Zheng
` (3 preceding siblings ...)
2014-09-30 3:40 ` [Qemu-devel] [PATCH v5 4/7] scsi: Drop SCSIReqOps.cancel_io Fam Zheng
@ 2014-09-30 3:40 ` Fam Zheng
2014-09-30 3:40 ` [Qemu-devel] [PATCH v5 6/7] scsi: Introduce scsi_req_cancel_async Fam Zheng
2014-09-30 3:40 ` [Qemu-devel] [PATCH v5 7/7] virtio-scsi: Handle TMF request cancellation asynchronously Fam Zheng
6 siblings, 0 replies; 9+ messages in thread
From: Fam Zheng @ 2014-09-30 3:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini
Let the aio cb do the clean up and notification job after scsi_req_cancel, in
preparation for asynchronous cancellation.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
hw/scsi/scsi-bus.c | 14 ++++++++++----
hw/scsi/scsi-disk.c | 8 ++++++++
hw/scsi/scsi-generic.c | 1 +
include/hw/scsi/scsi.h | 1 +
4 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 764f6cf..c91db63 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1718,6 +1718,16 @@ void scsi_req_complete(SCSIRequest *req, int status)
scsi_req_unref(req);
}
+/* Called by the devices when the request is canceled. */
+void scsi_req_cancel_complete(SCSIRequest *req)
+{
+ assert(req->io_canceled);
+ if (req->bus->info->cancel) {
+ req->bus->info->cancel(req);
+ }
+ scsi_req_unref(req);
+}
+
void scsi_req_cancel(SCSIRequest *req)
{
trace_scsi_req_cancel(req->dev->id, req->lun, req->tag);
@@ -1730,10 +1740,6 @@ void scsi_req_cancel(SCSIRequest *req)
if (req->aiocb) {
bdrv_aio_cancel(req->aiocb);
}
- if (req->bus->info->cancel) {
- req->bus->info->cancel(req);
- }
- scsi_req_unref(req);
}
static int scsi_ua_precedence(SCSISense sense)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index ef13e66..7a7938a 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -168,6 +168,7 @@ static void scsi_aio_complete(void *opaque, int ret)
r->req.aiocb = NULL;
block_acct_done(bdrv_get_stats(s->qdev.conf.bs), &r->acct);
if (r->req.io_canceled) {
+ scsi_req_cancel_complete(&r->req);
goto done;
}
@@ -214,6 +215,7 @@ static void scsi_write_do_fua(SCSIDiskReq *r)
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
if (r->req.io_canceled) {
+ scsi_req_cancel_complete(&r->req);
goto done;
}
@@ -240,6 +242,7 @@ static void scsi_dma_complete_noio(void *opaque, int ret)
block_acct_done(bdrv_get_stats(s->qdev.conf.bs), &r->acct);
}
if (r->req.io_canceled) {
+ scsi_req_cancel_complete(&r->req);
goto done;
}
@@ -280,6 +283,7 @@ static void scsi_read_complete(void * opaque, int ret)
r->req.aiocb = NULL;
block_acct_done(bdrv_get_stats(s->qdev.conf.bs), &r->acct);
if (r->req.io_canceled) {
+ scsi_req_cancel_complete(&r->req);
goto done;
}
@@ -312,6 +316,7 @@ static void scsi_do_read(void *opaque, int ret)
block_acct_done(bdrv_get_stats(s->qdev.conf.bs), &r->acct);
}
if (r->req.io_canceled) {
+ scsi_req_cancel_complete(&r->req);
goto done;
}
@@ -432,6 +437,7 @@ static void scsi_write_complete(void * opaque, int ret)
block_acct_done(bdrv_get_stats(s->qdev.conf.bs), &r->acct);
}
if (r->req.io_canceled) {
+ scsi_req_cancel_complete(&r->req);
goto done;
}
@@ -1524,6 +1530,7 @@ static void scsi_unmap_complete(void *opaque, int ret)
r->req.aiocb = NULL;
if (r->req.io_canceled) {
+ scsi_req_cancel_complete(&r->req);
goto done;
}
@@ -1623,6 +1630,7 @@ static void scsi_write_same_complete(void *opaque, int ret)
r->req.aiocb = NULL;
block_acct_done(bdrv_get_stats(s->qdev.conf.bs), &r->acct);
if (r->req.io_canceled) {
+ scsi_req_cancel_complete(&r->req);
goto done;
}
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 7e85047..01bca08 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -94,6 +94,7 @@ static void scsi_command_complete(void *opaque, int ret)
r->req.aiocb = NULL;
if (r->req.io_canceled) {
+ scsi_req_cancel_complete(&r->req);
goto done;
}
if (r->io_header.driver_status & SG_ERR_DRIVER_SENSE) {
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 1118107..a75a7c8 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -265,6 +265,7 @@ void scsi_req_complete(SCSIRequest *req, int status);
uint8_t *scsi_req_get_buf(SCSIRequest *req);
int scsi_req_get_sense(SCSIRequest *req, uint8_t *buf, int len);
void scsi_req_abort(SCSIRequest *req, int status);
+void scsi_req_cancel_complete(SCSIRequest *req);
void scsi_req_cancel(SCSIRequest *req);
void scsi_req_retry(SCSIRequest *req);
void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense);
--
1.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH v5 6/7] scsi: Introduce scsi_req_cancel_async
2014-09-30 3:40 [Qemu-devel] [PATCH v5 0/7] virtio-scsi: Asynchronous cancellation Fam Zheng
` (4 preceding siblings ...)
2014-09-30 3:40 ` [Qemu-devel] [PATCH v5 5/7] scsi: Introduce scsi_req_cancel_complete Fam Zheng
@ 2014-09-30 3:40 ` Fam Zheng
2014-09-30 3:40 ` [Qemu-devel] [PATCH v5 7/7] virtio-scsi: Handle TMF request cancellation asynchronously Fam Zheng
6 siblings, 0 replies; 9+ messages in thread
From: Fam Zheng @ 2014-09-30 3:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini
Devices will call this function to start an asynchronous cancellation. The
bus->info->cancel will be called after the request is canceled.
Devices will probably need to track a separate TMF request that triggers this
cancellation, and wait until the cancellation is done before completing it. So
we store a notifier list in SCSIRequest and in scsi_req_cancel_complete we
notify them.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
hw/scsi/scsi-bus.c | 26 ++++++++++++++++++++++++++
include/hw/scsi/scsi.h | 3 +++
2 files changed, 29 insertions(+)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index c91db63..0f3e039 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -566,6 +566,7 @@ SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops, SCSIDevice *d,
req->ops = reqops;
object_ref(OBJECT(d));
object_ref(OBJECT(qbus->parent));
+ notifier_list_init(&req->cancel_notifiers);
trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
return req;
}
@@ -1715,6 +1716,9 @@ void scsi_req_complete(SCSIRequest *req, int status)
scsi_req_ref(req);
scsi_req_dequeue(req);
req->bus->info->complete(req, req->status, req->resid);
+
+ /* Cancelled requests might end up being completed instead of cancelled */
+ notifier_list_notify(&req->cancel_notifiers, req);
scsi_req_unref(req);
}
@@ -1725,9 +1729,31 @@ void scsi_req_cancel_complete(SCSIRequest *req)
if (req->bus->info->cancel) {
req->bus->info->cancel(req);
}
+ notifier_list_notify(&req->cancel_notifiers, req);
scsi_req_unref(req);
}
+/* Cancel @req asynchronously. @notifier is added to @req's cancellation
+ * notifier list, the bus will be notified the requests cancellation is
+ * completed.
+ * */
+void scsi_req_cancel_async(SCSIRequest *req, Notifier *notifier)
+{
+ trace_scsi_req_cancel(req->dev->id, req->lun, req->tag);
+ if (notifier) {
+ notifier_list_add(&req->cancel_notifiers, notifier);
+ }
+ if (req->io_canceled) {
+ return;
+ }
+ scsi_req_ref(req);
+ scsi_req_dequeue(req);
+ req->io_canceled = true;
+ if (req->aiocb) {
+ bdrv_aio_cancel_async(req->aiocb);
+ }
+}
+
void scsi_req_cancel(SCSIRequest *req)
{
trace_scsi_req_cancel(req->dev->id, req->lun, req->tag);
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index a75a7c8..c47dc53 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -5,6 +5,7 @@
#include "block/block.h"
#include "hw/block/block.h"
#include "sysemu/sysemu.h"
+#include "qemu/notify.h"
#define MAX_SCSI_DEVS 255
@@ -53,6 +54,7 @@ struct SCSIRequest {
void *hba_private;
size_t resid;
SCSICommand cmd;
+ NotifierList cancel_notifiers;
/* Note:
* - fields before sense are initialized by scsi_req_alloc;
@@ -267,6 +269,7 @@ int scsi_req_get_sense(SCSIRequest *req, uint8_t *buf, int len);
void scsi_req_abort(SCSIRequest *req, int status);
void scsi_req_cancel_complete(SCSIRequest *req);
void scsi_req_cancel(SCSIRequest *req);
+void scsi_req_cancel_async(SCSIRequest *req, Notifier *notifier);
void scsi_req_retry(SCSIRequest *req);
void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense);
void scsi_device_set_ua(SCSIDevice *sdev, SCSISense sense);
--
1.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH v5 7/7] virtio-scsi: Handle TMF request cancellation asynchronously
2014-09-30 3:40 [Qemu-devel] [PATCH v5 0/7] virtio-scsi: Asynchronous cancellation Fam Zheng
` (5 preceding siblings ...)
2014-09-30 3:40 ` [Qemu-devel] [PATCH v5 6/7] scsi: Introduce scsi_req_cancel_async Fam Zheng
@ 2014-09-30 3:40 ` Fam Zheng
2014-09-30 8:44 ` Paolo Bonzini
6 siblings, 1 reply; 9+ messages in thread
From: Fam Zheng @ 2014-09-30 3:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini
For VIRTIO_SCSI_T_TMF_ABORT_TASK and VIRTIO_SCSI_T_TMF_ABORT_TASK_SET,
use scsi_req_cancel_async to start the cancellation.
Because each tmf command may cancel multiple requests, we need to use a
counter to track the number of remaining requests we still need to wait
for.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
hw/scsi/virtio-scsi.c | 64 ++++++++++++++++++++++++++++++++++++-----
include/hw/virtio/virtio-scsi.h | 10 +++++--
2 files changed, 65 insertions(+), 9 deletions(-)
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index fa36e23..c90da38 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -208,12 +208,33 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
return req;
}
-static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
+typedef struct {
+ Notifier notifier;
+ VirtIOSCSIReq *tmf_req;
+} VirtIOSCSICancelNotifier;
+
+static void virtio_scsi_cancel_notify(Notifier *notifier, void *data)
+{
+ VirtIOSCSICancelNotifier *n = container_of(notifier,
+ VirtIOSCSICancelNotifier,
+ notifier);
+
+ if (--n->tmf_req->remaining == 0) {
+ virtio_scsi_complete_req(n->tmf_req);
+ }
+ g_slice_free(VirtIOSCSICancelNotifier, n);
+}
+
+/* Return 0 if the request is ready to be completed and return to guest;
+ * -EINPROGRESS if the request is submitted and will be completed later, in the
+ * case of async cancellation. */
+static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
{
SCSIDevice *d = virtio_scsi_device_find(s, req->req.tmf.lun);
SCSIRequest *r, *next;
BusChild *kid;
int target;
+ int ret = 0;
if (s->dataplane_started && bdrv_get_aio_context(d->conf.bs) != s->ctx) {
aio_context_acquire(s->ctx);
@@ -251,7 +272,14 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
*/
req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
} else {
- scsi_req_cancel(r);
+ VirtIOSCSICancelNotifier *notifier;
+
+ req->remaining = 1;
+ notifier = g_slice_new(VirtIOSCSICancelNotifier);
+ notifier->tmf_req = req;
+ notifier->notifier.notify = virtio_scsi_cancel_notify;
+ scsi_req_cancel_async(r, ¬ifier->notifier);
+ ret = -EINPROGRESS;
}
}
break;
@@ -277,6 +305,13 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
if (d->lun != virtio_scsi_get_lun(req->req.tmf.lun)) {
goto incorrect_lun;
}
+
+ /* Add 1 to "remaining" until virtio_scsi_do_tmf returns.
+ * This way, if the bus starts calling back to the notifiers
+ * even before we finish the loop, virtio_scsi_cancel_notify
+ * will not complete the TMF too early.
+ */
+ req->remaining = 1;
QTAILQ_FOREACH_SAFE(r, &d->requests, next, next) {
if (r->hba_private) {
if (req->req.tmf.subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK_SET) {
@@ -286,10 +321,19 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
break;
} else {
- scsi_req_cancel(r);
+ VirtIOSCSICancelNotifier *notifier;
+
+ req->remaining++;
+ notifier = g_slice_new(VirtIOSCSICancelNotifier);
+ notifier->notifier.notify = virtio_scsi_cancel_notify;
+ notifier->tmf_req = req;
+ scsi_req_cancel_async(r, ¬ifier->notifier);
}
}
}
+ if (--req->remaining > 0) {
+ ret = -EINPROGRESS;
+ }
break;
case VIRTIO_SCSI_T_TMF_I_T_NEXUS_RESET:
@@ -310,20 +354,22 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
break;
}
- return;
+ return ret;
incorrect_lun:
req->resp.tmf.response = VIRTIO_SCSI_S_INCORRECT_LUN;
- return;
+ return ret;
fail:
req->resp.tmf.response = VIRTIO_SCSI_S_BAD_TARGET;
+ return ret;
}
void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
{
VirtIODevice *vdev = (VirtIODevice *)s;
int type;
+ int r = 0;
if (iov_to_buf(req->elem.out_sg, req->elem.out_num, 0,
&type, sizeof(type)) < sizeof(type)) {
@@ -337,7 +383,7 @@ void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
sizeof(VirtIOSCSICtrlTMFResp)) < 0) {
virtio_scsi_bad_req();
} else {
- virtio_scsi_do_tmf(s, req);
+ r = virtio_scsi_do_tmf(s, req);
}
} else if (req->req.tmf.type == VIRTIO_SCSI_T_AN_QUERY ||
@@ -350,7 +396,11 @@ void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
req->resp.an.response = VIRTIO_SCSI_S_OK;
}
}
- virtio_scsi_complete_req(req);
+ if (r == 0) {
+ virtio_scsi_complete_req(req);
+ } else {
+ assert(r = -EINPROGRESS);
+ }
}
static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 6134c0b..d6e5e79 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -213,8 +213,14 @@ typedef struct VirtIOSCSIReq {
VirtQueueElement elem;
/* Set by dataplane code. */
VirtIOSCSIVring *vring;
- /* Used by two stages request submitting */
- QTAILQ_ENTRY(VirtIOSCSIReq) next;
+
+ union {
+ /* Used for two-stage request submission */
+ QTAILQ_ENTRY(VirtIOSCSIReq) next;
+
+ /* Used for cancellation of request during TMFs */
+ int remaining;
+ };
SCSIRequest *sreq;
size_t resp_size;
--
1.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH v5 7/7] virtio-scsi: Handle TMF request cancellation asynchronously
2014-09-30 3:40 ` [Qemu-devel] [PATCH v5 7/7] virtio-scsi: Handle TMF request cancellation asynchronously Fam Zheng
@ 2014-09-30 8:44 ` Paolo Bonzini
0 siblings, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2014-09-30 8:44 UTC (permalink / raw)
To: Fam Zheng, qemu-devel
Il 30/09/2014 05:40, Fam Zheng ha scritto:
> For VIRTIO_SCSI_T_TMF_ABORT_TASK and VIRTIO_SCSI_T_TMF_ABORT_TASK_SET,
> use scsi_req_cancel_async to start the cancellation.
>
> Because each tmf command may cancel multiple requests, we need to use a
> counter to track the number of remaining requests we still need to wait
> for.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> hw/scsi/virtio-scsi.c | 64 ++++++++++++++++++++++++++++++++++++-----
> include/hw/virtio/virtio-scsi.h | 10 +++++--
> 2 files changed, 65 insertions(+), 9 deletions(-)
>
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index fa36e23..c90da38 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -208,12 +208,33 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
> return req;
> }
>
> -static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
> +typedef struct {
> + Notifier notifier;
> + VirtIOSCSIReq *tmf_req;
> +} VirtIOSCSICancelNotifier;
> +
> +static void virtio_scsi_cancel_notify(Notifier *notifier, void *data)
> +{
> + VirtIOSCSICancelNotifier *n = container_of(notifier,
> + VirtIOSCSICancelNotifier,
> + notifier);
> +
> + if (--n->tmf_req->remaining == 0) {
> + virtio_scsi_complete_req(n->tmf_req);
> + }
> + g_slice_free(VirtIOSCSICancelNotifier, n);
> +}
> +
> +/* Return 0 if the request is ready to be completed and return to guest;
> + * -EINPROGRESS if the request is submitted and will be completed later, in the
> + * case of async cancellation. */
> +static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
> {
> SCSIDevice *d = virtio_scsi_device_find(s, req->req.tmf.lun);
> SCSIRequest *r, *next;
> BusChild *kid;
> int target;
> + int ret = 0;
>
> if (s->dataplane_started && bdrv_get_aio_context(d->conf.bs) != s->ctx) {
> aio_context_acquire(s->ctx);
> @@ -251,7 +272,14 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
> */
> req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
> } else {
> - scsi_req_cancel(r);
> + VirtIOSCSICancelNotifier *notifier;
> +
> + req->remaining = 1;
> + notifier = g_slice_new(VirtIOSCSICancelNotifier);
> + notifier->tmf_req = req;
> + notifier->notifier.notify = virtio_scsi_cancel_notify;
> + scsi_req_cancel_async(r, ¬ifier->notifier);
> + ret = -EINPROGRESS;
> }
> }
> break;
> @@ -277,6 +305,13 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
> if (d->lun != virtio_scsi_get_lun(req->req.tmf.lun)) {
> goto incorrect_lun;
> }
> +
> + /* Add 1 to "remaining" until virtio_scsi_do_tmf returns.
> + * This way, if the bus starts calling back to the notifiers
> + * even before we finish the loop, virtio_scsi_cancel_notify
> + * will not complete the TMF too early.
> + */
> + req->remaining = 1;
> QTAILQ_FOREACH_SAFE(r, &d->requests, next, next) {
> if (r->hba_private) {
> if (req->req.tmf.subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK_SET) {
> @@ -286,10 +321,19 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
> req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
> break;
> } else {
> - scsi_req_cancel(r);
> + VirtIOSCSICancelNotifier *notifier;
> +
> + req->remaining++;
> + notifier = g_slice_new(VirtIOSCSICancelNotifier);
> + notifier->notifier.notify = virtio_scsi_cancel_notify;
> + notifier->tmf_req = req;
> + scsi_req_cancel_async(r, ¬ifier->notifier);
> }
> }
> }
> + if (--req->remaining > 0) {
> + ret = -EINPROGRESS;
> + }
> break;
>
> case VIRTIO_SCSI_T_TMF_I_T_NEXUS_RESET:
> @@ -310,20 +354,22 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
> break;
> }
>
> - return;
> + return ret;
>
> incorrect_lun:
> req->resp.tmf.response = VIRTIO_SCSI_S_INCORRECT_LUN;
> - return;
> + return ret;
>
> fail:
> req->resp.tmf.response = VIRTIO_SCSI_S_BAD_TARGET;
> + return ret;
> }
>
> void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
> {
> VirtIODevice *vdev = (VirtIODevice *)s;
> int type;
> + int r = 0;
>
> if (iov_to_buf(req->elem.out_sg, req->elem.out_num, 0,
> &type, sizeof(type)) < sizeof(type)) {
> @@ -337,7 +383,7 @@ void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
> sizeof(VirtIOSCSICtrlTMFResp)) < 0) {
> virtio_scsi_bad_req();
> } else {
> - virtio_scsi_do_tmf(s, req);
> + r = virtio_scsi_do_tmf(s, req);
> }
>
> } else if (req->req.tmf.type == VIRTIO_SCSI_T_AN_QUERY ||
> @@ -350,7 +396,11 @@ void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
> req->resp.an.response = VIRTIO_SCSI_S_OK;
> }
> }
> - virtio_scsi_complete_req(req);
> + if (r == 0) {
> + virtio_scsi_complete_req(req);
> + } else {
> + assert(r = -EINPROGRESS);
Oops. :)
Applied to scsi-next.
Paolo
> + }
> }
>
> static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
> index 6134c0b..d6e5e79 100644
> --- a/include/hw/virtio/virtio-scsi.h
> +++ b/include/hw/virtio/virtio-scsi.h
> @@ -213,8 +213,14 @@ typedef struct VirtIOSCSIReq {
> VirtQueueElement elem;
> /* Set by dataplane code. */
> VirtIOSCSIVring *vring;
> - /* Used by two stages request submitting */
> - QTAILQ_ENTRY(VirtIOSCSIReq) next;
> +
> + union {
> + /* Used for two-stage request submission */
> + QTAILQ_ENTRY(VirtIOSCSIReq) next;
> +
> + /* Used for cancellation of request during TMFs */
> + int remaining;
> + };
>
> SCSIRequest *sreq;
> size_t resp_size;
>
^ permalink raw reply [flat|nested] 9+ messages in thread