mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Revert "iscsi: Fix NULL dereferences / races between task completion and abort"

Browse Source 
This reverts commit 64e69e8092.  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>
This commit is contained in:
Paolo Bonzini 2012-08-19 00:12:39 +02:00
parent e89001f72e
commit b209091957
1 changed files with 32 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
block/iscsi.c
View File

@ -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->common.cb(acb->common.opaque, -ECANCELED);
acb->canceled = 1;
acb->common.cb(acb->common.opaque, -ECANCELED);
/* 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,