OpenE2K
/
qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

iscsi: fix races between task completion and abort 

This patch fixes two main issues with block/iscsi.c:

1) iscsi_task_mgmt_abort_task_async calls iscsi_scsi_task_cancel which
was also directly called in iscsi_aio_cancel

2) a race between task completion and task abortion could happen cause
the scsi_free_scsi_task were done before iscsi_schedule_bh has finished.
To fix this, all the freeing of IscsiTasks and releasing of the AIOCBs
is centralized in iscsi_bh_cb, independent of whether the SCSI command
has completed or was cancelled.

3) iscsi_aio_cancel was not synchronously waiting for the end of the
command.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
Paolo Bonzini 2012-08-18 23:35:49 +02:00
parent cfb3f5064a
commit 1bd075f29e
1 changed files with 30 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
block/iscsi.c
View File

@ -83,12 +83,20 @@ iscsi_bh_cb(void *p)
acb->common.cb(acb->common.opaque, acb->status); acb->common.cb(acb->common.opaque, acb->status);
} }
if (acb->task != NULL) {
scsi_free_scsi_task(acb->task);
acb->task = NULL;
}
qemu_aio_release(acb); qemu_aio_release(acb);
} }
static void static void
iscsi_schedule_bh(IscsiAIOCB *acb) iscsi_schedule_bh(IscsiAIOCB *acb)
{ {
if (acb->bh) {
return;
}
acb->bh = qemu_bh_new(iscsi_bh_cb, acb); acb->bh = qemu_bh_new(iscsi_bh_cb, acb);
qemu_bh_schedule(acb->bh); qemu_bh_schedule(acb->bh);
} }
@ -98,6 +106,10 @@ static void
iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data, iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data,
void *private_data) void *private_data)
{ {
IscsiAIOCB *acb = private_data;
acb->status = -ECANCELED;
iscsi_schedule_bh(acb);
} }
static void static void
@ -106,15 +118,19 @@ iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
IscsiAIOCB *acb = (IscsiAIOCB *)blockacb; IscsiAIOCB *acb = (IscsiAIOCB *)blockacb;
IscsiLun *iscsilun = acb->iscsilun; IscsiLun *iscsilun = acb->iscsilun;
acb->common.cb(acb->common.opaque, -ECANCELED); if (acb->status != -EINPROGRESS) {
return;
}
acb->canceled = 1; acb->canceled = 1;
/* send a task mgmt call to the target to cancel the task on the target */ /* 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_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
iscsi_abort_task_cb, NULL); iscsi_abort_task_cb, acb);
/* then also cancel the task locally in libiscsi */ while (acb->status == -EINPROGRESS) {
iscsi_scsi_task_cancel(iscsilun->iscsi, acb->task); qemu_aio_wait();
}
} }
static AIOPool iscsi_aio_pool = { static AIOPool iscsi_aio_pool = {
@ -192,9 +208,6 @@ iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status,
g_free(acb->buf); g_free(acb->buf);
if (acb->canceled != 0) { if (acb->canceled != 0) {
qemu_aio_release(acb);
scsi_free_scsi_task(acb->task);
acb->task = NULL;
return; return;
} }
@ -206,8 +219,6 @@ iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status,
} }
iscsi_schedule_bh(acb); iscsi_schedule_bh(acb);
scsi_free_scsi_task(acb->task);
acb->task = NULL;
} }
static int64_t sector_qemu2lun(int64_t sector, IscsiLun *iscsilun) static int64_t sector_qemu2lun(int64_t sector, IscsiLun *iscsilun)
@ -236,6 +247,8 @@ iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
acb->qiov = qiov; acb->qiov = qiov;
acb->canceled = 0; acb->canceled = 0;
acb->bh = NULL;
acb->status = -EINPROGRESS;
/* XXX we should pass the iovec to write16 to avoid the extra copy */ /* XXX we should pass the iovec to write16 to avoid the extra copy */
/* this will allow us to get rid of 'buf' completely */ /* this will allow us to get rid of 'buf' completely */
@ -293,9 +306,6 @@ iscsi_aio_read16_cb(struct iscsi_context *iscsi, int status,
trace_iscsi_aio_read16_cb(iscsi, status, acb, acb->canceled); trace_iscsi_aio_read16_cb(iscsi, status, acb, acb->canceled);
if (acb->canceled != 0) { if (acb->canceled != 0) {
qemu_aio_release(acb);
scsi_free_scsi_task(acb->task);
acb->task = NULL;
return; return;
} }
@ -307,8 +317,6 @@ iscsi_aio_read16_cb(struct iscsi_context *iscsi, int status,
} }
iscsi_schedule_bh(acb); iscsi_schedule_bh(acb);
scsi_free_scsi_task(acb->task);
acb->task = NULL;
} }
static BlockDriverAIOCB * static BlockDriverAIOCB *
@ -334,6 +342,8 @@ iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
acb->qiov = qiov; acb->qiov = qiov;
acb->canceled = 0; acb->canceled = 0;
acb->bh = NULL;
acb->status = -EINPROGRESS;
acb->read_size = qemu_read_size; acb->read_size = qemu_read_size;
acb->buf = NULL; acb->buf = NULL;
@ -409,9 +419,6 @@ iscsi_synccache10_cb(struct iscsi_context *iscsi, int status,
IscsiAIOCB *acb = opaque; IscsiAIOCB *acb = opaque;
if (acb->canceled != 0) { if (acb->canceled != 0) {
qemu_aio_release(acb);
scsi_free_scsi_task(acb->task);
acb->task = NULL;
return; return;
} }
@ -423,8 +430,6 @@ iscsi_synccache10_cb(struct iscsi_context *iscsi, int status,
} }
iscsi_schedule_bh(acb); iscsi_schedule_bh(acb);
scsi_free_scsi_task(acb->task);
acb->task = NULL;
} }
static BlockDriverAIOCB * static BlockDriverAIOCB *
@ -439,6 +444,8 @@ iscsi_aio_flush(BlockDriverState *bs,
acb->iscsilun = iscsilun; acb->iscsilun = iscsilun;
acb->canceled = 0; acb->canceled = 0;
acb->bh = NULL;
acb->status = -EINPROGRESS;
acb->task = iscsi_synchronizecache10_task(iscsi, iscsilun->lun, acb->task = iscsi_synchronizecache10_task(iscsi, iscsilun->lun,
0, 0, 0, 0, 0, 0, 0, 0,
@ -463,9 +470,6 @@ iscsi_unmap_cb(struct iscsi_context *iscsi, int status,
IscsiAIOCB *acb = opaque; IscsiAIOCB *acb = opaque;
if (acb->canceled != 0) { if (acb->canceled != 0) {
qemu_aio_release(acb);
scsi_free_scsi_task(acb->task);
acb->task = NULL;
return; return;
} }
@ -477,8 +481,6 @@ iscsi_unmap_cb(struct iscsi_context *iscsi, int status,
} }
iscsi_schedule_bh(acb); iscsi_schedule_bh(acb);
scsi_free_scsi_task(acb->task);
acb->task = NULL;
} }
static BlockDriverAIOCB * static BlockDriverAIOCB *
@ -495,6 +497,8 @@ iscsi_aio_discard(BlockDriverState *bs,
acb->iscsilun = iscsilun; acb->iscsilun = iscsilun;
acb->canceled = 0; acb->canceled = 0;
acb->bh = NULL;
acb->status = -EINPROGRESS;
list[0].lba = sector_qemu2lun(sector_num, iscsilun); list[0].lba = sector_qemu2lun(sector_num, iscsilun);
list[0].num = nb_sectors * BDRV_SECTOR_SIZE / iscsilun->block_size; list[0].num = nb_sectors * BDRV_SECTOR_SIZE / iscsilun->block_size;
@ -523,9 +527,6 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status,
IscsiAIOCB *acb = opaque; IscsiAIOCB *acb = opaque;
if (acb->canceled != 0) { if (acb->canceled != 0) {
qemu_aio_release(acb);
scsi_free_scsi_task(acb->task);
acb->task = NULL;
return; return;
} }
@ -554,8 +555,6 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status,
} }
iscsi_schedule_bh(acb); iscsi_schedule_bh(acb);
scsi_free_scsi_task(acb->task);
acb->task = NULL;
} }
static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
@ -573,6 +572,8 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
acb->iscsilun = iscsilun; acb->iscsilun = iscsilun;
acb->canceled = 0; acb->canceled = 0;
acb->bh = NULL;
acb->status = -EINPROGRESS;
acb->buf = NULL; acb->buf = NULL;
acb->ioh = buf; acb->ioh = buf;