aboutsummaryrefslogtreecommitdiff
path: root/block
diff options
context:
space:
mode:
authorPaolo Bonzini <pbonzini@redhat.com>2012-08-19 00:12:39 +0200
committerPaolo Bonzini <pbonzini@redhat.com>2012-08-20 15:50:45 +0200
commitb20909195745c34a819aed14ae996b60ab0f591f (patch)
tree830f04a59eae29078009cfb513b52caa1f03f23b /block
parente89001f72edde37fb36fa7c964daa1bbeb2eca26 (diff)
Revert "iscsi: Fix NULL dereferences / races between task completion and abort"
This reverts commit 64e69e80920d82df3fa679bc41b13770d2f99360. The commit returned immediately from iscsi_aio_cancel, risking corruption in case the following happens: guest qemu target ========================================================================= send write 1 --------> send write 1 --------> cancel write 1 ------> cancel write 1 ------> <------------------ cancellation processed send write 2 --------> send write 2 --------> <---------------- completed write 2 <------------------ completed write 2 <---------------- completed write 1 <---------------- cancellation not done Here, the guest would see write 2 superseding write 1, when in fact the outcome could have been the opposite. The right behavior is to return only after the target says whether the cancellation was done or not, and it will be implemented by the next three patches. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Diffstat (limited to 'block')
-rw-r--r--block/iscsi.c55
1 files changed, 32 insertions, 23 deletions
diff --git a/block/iscsi.c b/block/iscsi.c
index bb9cf82459..219f927823 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -76,10 +76,6 @@ static void
iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data,
void *private_data)
{
- IscsiAIOCB *acb = (IscsiAIOCB *)private_data;
-
- scsi_free_scsi_task(acb->task);
- acb->task = NULL;
}
static void
@@ -88,15 +84,15 @@ iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
IscsiAIOCB *acb = (IscsiAIOCB *)blockacb;
IscsiLun *iscsilun = acb->iscsilun;
- acb->canceled = 1;
-
acb->common.cb(acb->common.opaque, -ECANCELED);
+ acb->canceled = 1;
- /* send a task mgmt call to the target to cancel the task on the target
- * this also cancels the task in libiscsi
- */
+ /* send a task mgmt call to the target to cancel the task on the target */
iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
- iscsi_abort_task_cb, &acb);
+ iscsi_abort_task_cb, NULL);
+
+ /* then also cancel the task locally in libiscsi */
+ iscsi_scsi_task_cancel(iscsilun->iscsi, acb->task);
}
static AIOPool iscsi_aio_pool = {
@@ -183,18 +179,11 @@ iscsi_readv_writev_bh_cb(void *p)
qemu_bh_delete(acb->bh);
- if (!acb->canceled) {
+ if (acb->canceled == 0) {
acb->common.cb(acb->common.opaque, acb->status);
}
qemu_aio_release(acb);
-
- if (acb->canceled) {
- return;
- }
-
- scsi_free_scsi_task(acb->task);
- acb->task = NULL;
}
@@ -208,8 +197,10 @@ iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status,
g_free(acb->buf);
- if (acb->canceled) {
+ if (acb->canceled != 0) {
qemu_aio_release(acb);
+ scsi_free_scsi_task(acb->task);
+ acb->task = NULL;
return;
}
@@ -221,6 +212,8 @@ iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status,
}
iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+ scsi_free_scsi_task(acb->task);
+ acb->task = NULL;
}
static int64_t sector_qemu2lun(int64_t sector, IscsiLun *iscsilun)
@@ -305,8 +298,10 @@ iscsi_aio_read16_cb(struct iscsi_context *iscsi, int status,
trace_iscsi_aio_read16_cb(iscsi, status, acb, acb->canceled);
- if (acb->canceled) {
+ if (acb->canceled != 0) {
qemu_aio_release(acb);
+ scsi_free_scsi_task(acb->task);
+ acb->task = NULL;
return;
}
@@ -318,6 +313,8 @@ iscsi_aio_read16_cb(struct iscsi_context *iscsi, int status,
}
iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+ scsi_free_scsi_task(acb->task);
+ acb->task = NULL;
}
static BlockDriverAIOCB *
@@ -417,8 +414,10 @@ iscsi_synccache10_cb(struct iscsi_context *iscsi, int status,
{
IscsiAIOCB *acb = opaque;
- if (acb->canceled) {
+ if (acb->canceled != 0) {
qemu_aio_release(acb);
+ scsi_free_scsi_task(acb->task);
+ acb->task = NULL;
return;
}
@@ -430,6 +429,8 @@ iscsi_synccache10_cb(struct iscsi_context *iscsi, int status,
}
iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+ scsi_free_scsi_task(acb->task);
+ acb->task = NULL;
}
static BlockDriverAIOCB *
@@ -467,8 +468,10 @@ iscsi_unmap_cb(struct iscsi_context *iscsi, int status,
{
IscsiAIOCB *acb = opaque;
- if (acb->canceled) {
+ if (acb->canceled != 0) {
qemu_aio_release(acb);
+ scsi_free_scsi_task(acb->task);
+ acb->task = NULL;
return;
}
@@ -480,6 +483,8 @@ iscsi_unmap_cb(struct iscsi_context *iscsi, int status,
}
iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+ scsi_free_scsi_task(acb->task);
+ acb->task = NULL;
}
static BlockDriverAIOCB *
@@ -523,8 +528,10 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status,
{
IscsiAIOCB *acb = opaque;
- if (acb->canceled) {
+ if (acb->canceled != 0) {
qemu_aio_release(acb);
+ scsi_free_scsi_task(acb->task);
+ acb->task = NULL;
return;
}
@@ -553,6 +560,8 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status,
}
iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+ scsi_free_scsi_task(acb->task);
+ acb->task = NULL;
}
static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,