Re: [Qemu-devel] [PATCH] ISCSI: Only set up the read-event if we are actually waiting for data to come back in from the target.

[ronnie sahlberg]

On Fri, May 11, 2012 at 9:27 PM, Paolo Bonzini <address@hidden> wrote:
> Il 11/05/2012 12:22, Ronnie Sahlberg ha scritto:
>> Signed-off-by: Ronnie Sahlberg <address@hidden>
>> ---
>>  block/iscsi.c |    4 +++-
>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/block/iscsi.c b/block/iscsi.c
>> index d37c4ee..989b5e9 100644
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -105,7 +105,9 @@ iscsi_set_events(IscsiLun *iscsilun)
>>  {
>>      struct iscsi_context *iscsi = iscsilun->iscsi;
>>
>> -    qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), iscsi_process_read,
>> +    qemu_aio_set_fd_handler(iscsi_get_fd(iscsi),
>> +                           (iscsi_queue_length(iscsi) > 0)
>> +                           ? iscsi_process_read : NULL,
>>                             (iscsi_which_events(iscsi) & POLLOUT)
>>                             ? iscsi_process_write : NULL,
>>                             iscsi_process_flush, iscsilun);
>
> I wonder if iscsi is also susceptible to the same race condition I saw
> with NBD, where you can have:
>
> 1) select in the iothread exiting and reporting readability
>
> 2) the iothread subsequently blocking on the mutex
>
> 3) a VCPU thread's qemu_aio_wait() calling iscsi_process_read
>
> 4) when the VCPU releases the mutex, the iothread will call
> iscsi_process_read again.
>
> This should be easily reproducible with IDE drives, but the above patch
> would not fix it.  Perhaps it's better to call iscsi_queue_length in
> iscsi_process_read instead.
>

So there is a known condition where the event callbacks can be
triggered like this.
Thanks for confirming!

Let me do a new patch along your suggestion, test it a bit and I will
re-send to the list.


regards
ronnie sahlberg