* [Qemu-devel] [PATCH] iscsi: fix race between task completition and task abortion
@ 2012-08-14 6:44 Stefan Priebe
2012-08-14 10:35 ` Stefan Hajnoczi
2012-08-14 14:57 ` Kevin Wolf
0 siblings, 2 replies; 7+ messages in thread
From: Stefan Priebe @ 2012-08-14 6:44 UTC (permalink / raw)
To: qemu-devel; +Cc: spriebe
From: spriebe <git@profihost.ag>
---
block/iscsi.c | 36 ++++++++++++++++++++----------------
1 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 12ca76d..257f97f 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -76,6 +76,10 @@ 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
@@ -85,14 +89,19 @@ iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
IscsiLun *iscsilun = acb->iscsilun;
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 */
- iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
- iscsi_abort_task_cb, NULL);
+ if (acb->canceled != 0)
+ return;
+
+ acb->canceled = 1;
- /* then also cancel the task locally in libiscsi */
- iscsi_scsi_task_cancel(iscsilun->iscsi, acb->task);
+ /* send a task mgmt call to the target to cancel the task on the target
+ * this also cancels the task in libiscsi
+ */
+ if (acb->task) {
+ iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
+ iscsi_abort_task_cb, &acb);
+ }
}
static AIOPool iscsi_aio_pool = {
@@ -184,6 +193,11 @@ iscsi_readv_writev_bh_cb(void *p)
}
qemu_aio_release(acb);
+
+ if (acb->task) {
+ scsi_free_scsi_task(acb->task);
+ acb->task = NULL;
+ }
}
@@ -212,8 +226,6 @@ 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)
@@ -313,8 +325,6 @@ 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 *
@@ -429,8 +439,6 @@ 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 *
@@ -483,8 +491,6 @@ 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 *
@@ -560,8 +566,6 @@ 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,
--
1.7.2.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] iscsi: fix race between task completition and task abortion
2012-08-14 6:44 [Qemu-devel] [PATCH] iscsi: fix race between task completition and task abortion Stefan Priebe
@ 2012-08-14 10:35 ` Stefan Hajnoczi
2012-08-14 12:09 ` ronnie sahlberg
2012-08-14 14:57 ` Kevin Wolf
1 sibling, 1 reply; 7+ messages in thread
From: Stefan Hajnoczi @ 2012-08-14 10:35 UTC (permalink / raw)
To: Stefan Priebe; +Cc: spriebe, qemu-devel, ronniesahlberg
On Tue, Aug 14, 2012 at 08:44:46AM +0200, Stefan Priebe wrote:
> From: spriebe <git@profihost.ag>
CCing Ronnie, iSCSI block driver author.
>
> ---
> block/iscsi.c | 36 ++++++++++++++++++++----------------
> 1 files changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 12ca76d..257f97f 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -76,6 +76,10 @@ 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
> @@ -85,14 +89,19 @@ iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
> IscsiLun *iscsilun = acb->iscsilun;
>
> 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 */
> - iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
> - iscsi_abort_task_cb, NULL);
> + if (acb->canceled != 0)
> + return;
> +
> + acb->canceled = 1;
>
> - /* then also cancel the task locally in libiscsi */
> - iscsi_scsi_task_cancel(iscsilun->iscsi, acb->task);
> + /* send a task mgmt call to the target to cancel the task on the target
> + * this also cancels the task in libiscsi
> + */
> + if (acb->task) {
> + iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
> + iscsi_abort_task_cb, &acb);
> + }
> }
>
> static AIOPool iscsi_aio_pool = {
> @@ -184,6 +193,11 @@ iscsi_readv_writev_bh_cb(void *p)
> }
>
> qemu_aio_release(acb);
> +
> + if (acb->task) {
> + scsi_free_scsi_task(acb->task);
> + acb->task = NULL;
> + }
> }
>
>
> @@ -212,8 +226,6 @@ 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)
> @@ -313,8 +325,6 @@ 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 *
> @@ -429,8 +439,6 @@ 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 *
> @@ -483,8 +491,6 @@ 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 *
> @@ -560,8 +566,6 @@ 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,
> --
> 1.7.2.5
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] iscsi: fix race between task completition and task abortion
2012-08-14 10:35 ` Stefan Hajnoczi
@ 2012-08-14 12:09 ` ronnie sahlberg
2012-08-14 12:11 ` Stefan Hajnoczi
0 siblings, 1 reply; 7+ messages in thread
From: ronnie sahlberg @ 2012-08-14 12:09 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: spriebe, qemu-devel, Stefan Priebe
Stefan H
How should I do Acked-by properly,
Is a reply with the text
Acked-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
sufficient ?
regards
ronnie sahlberg
On Tue, Aug 14, 2012 at 8:35 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Tue, Aug 14, 2012 at 08:44:46AM +0200, Stefan Priebe wrote:
>> From: spriebe <git@profihost.ag>
>
> CCing Ronnie, iSCSI block driver author.
>
>>
>> ---
>> block/iscsi.c | 36 ++++++++++++++++++++----------------
>> 1 files changed, 20 insertions(+), 16 deletions(-)
>>
>> diff --git a/block/iscsi.c b/block/iscsi.c
>> index 12ca76d..257f97f 100644
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -76,6 +76,10 @@ 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
>> @@ -85,14 +89,19 @@ iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
>> IscsiLun *iscsilun = acb->iscsilun;
>>
>> 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 */
>> - iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
>> - iscsi_abort_task_cb, NULL);
>> + if (acb->canceled != 0)
>> + return;
>> +
>> + acb->canceled = 1;
>>
>> - /* then also cancel the task locally in libiscsi */
>> - iscsi_scsi_task_cancel(iscsilun->iscsi, acb->task);
>> + /* send a task mgmt call to the target to cancel the task on the target
>> + * this also cancels the task in libiscsi
>> + */
>> + if (acb->task) {
>> + iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
>> + iscsi_abort_task_cb, &acb);
>> + }
>> }
>>
>> static AIOPool iscsi_aio_pool = {
>> @@ -184,6 +193,11 @@ iscsi_readv_writev_bh_cb(void *p)
>> }
>>
>> qemu_aio_release(acb);
>> +
>> + if (acb->task) {
>> + scsi_free_scsi_task(acb->task);
>> + acb->task = NULL;
>> + }
>> }
>>
>>
>> @@ -212,8 +226,6 @@ 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)
>> @@ -313,8 +325,6 @@ 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 *
>> @@ -429,8 +439,6 @@ 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 *
>> @@ -483,8 +491,6 @@ 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 *
>> @@ -560,8 +566,6 @@ 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,
>> --
>> 1.7.2.5
>>
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] iscsi: fix race between task completition and task abortion
2012-08-14 12:09 ` ronnie sahlberg
@ 2012-08-14 12:11 ` Stefan Hajnoczi
2012-08-14 14:08 ` Kevin Wolf
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Hajnoczi @ 2012-08-14 12:11 UTC (permalink / raw)
To: ronnie sahlberg; +Cc: spriebe, qemu-devel, Stefan Priebe
On Tue, Aug 14, 2012 at 1:09 PM, ronnie sahlberg
<ronniesahlberg@gmail.com> wrote:
> Is a reply with the text
>
> Acked-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>
> sufficient ?
Yes
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] iscsi: fix race between task completition and task abortion
2012-08-14 12:11 ` Stefan Hajnoczi
@ 2012-08-14 14:08 ` Kevin Wolf
2012-08-14 20:21 ` Stefan Priebe
0 siblings, 1 reply; 7+ messages in thread
From: Kevin Wolf @ 2012-08-14 14:08 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: spriebe, qemu-devel, ronnie sahlberg, Stefan Priebe
Am 14.08.2012 14:11, schrieb Stefan Hajnoczi:
> On Tue, Aug 14, 2012 at 1:09 PM, ronnie sahlberg
> <ronniesahlberg@gmail.com> wrote:
>> Is a reply with the text
>>
>> Acked-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>>
>> sufficient ?
>
> Yes
But is this only meant as a question or a real Acked-by and I should
pick it up? (Still for 1.2, I guess?)
Kevin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] iscsi: fix race between task completition and task abortion
2012-08-14 6:44 [Qemu-devel] [PATCH] iscsi: fix race between task completition and task abortion Stefan Priebe
2012-08-14 10:35 ` Stefan Hajnoczi
@ 2012-08-14 14:57 ` Kevin Wolf
1 sibling, 0 replies; 7+ messages in thread
From: Kevin Wolf @ 2012-08-14 14:57 UTC (permalink / raw)
To: Stefan Priebe; +Cc: qemu-devel
Am 14.08.2012 08:44, schrieb Stefan Priebe:
> From: spriebe <git@profihost.ag>
>
> ---
> block/iscsi.c | 36 ++++++++++++++++++++----------------
> 1 files changed, 20 insertions(+), 16 deletions(-)
It would be nice to have your full name and a valid email address in the
From: line (needs an update of your git config) and a more detailed
explanation of the problem that you're fixing.
Having a Signed-off-by line, however, is absolutely required and the
patch can't be merged without it.
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 12ca76d..257f97f 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -76,6 +76,10 @@ 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;
Please use scripts/checkpatch.pl. qemu uses an indentation of four
spaces, more coding style violations follow.
Kevin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] iscsi: fix race between task completition and task abortion
2012-08-14 14:08 ` Kevin Wolf
@ 2012-08-14 20:21 ` Stefan Priebe
0 siblings, 0 replies; 7+ messages in thread
From: Stefan Priebe @ 2012-08-14 20:21 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Stefan Hajnoczi, qemu-devel, ronnie sahlberg
Am 14.08.2012 16:08, schrieb Kevin Wolf:
> Am 14.08.2012 14:11, schrieb Stefan Hajnoczi:
>> On Tue, Aug 14, 2012 at 1:09 PM, ronnie sahlberg
>> <ronniesahlberg@gmail.com> wrote:
>>> Is a reply with the text
>>>
>>> Acked-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>>>
>>> sufficient ?
>>
>> Yes
>
> But is this only meant as a question or a real Acked-by and I should
> pick it up? (Still for 1.2, I guess?)
I'll send a new patch in a few minutes. It fixes the signed-off, style
and a logic problem as well.
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-08-14 20:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-14 6:44 [Qemu-devel] [PATCH] iscsi: fix race between task completition and task abortion Stefan Priebe
2012-08-14 10:35 ` Stefan Hajnoczi
2012-08-14 12:09 ` ronnie sahlberg
2012-08-14 12:11 ` Stefan Hajnoczi
2012-08-14 14:08 ` Kevin Wolf
2012-08-14 20:21 ` Stefan Priebe
2012-08-14 14:57 ` Kevin Wolf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).