Re: [Qemu-devel] [PATCH] ISCSI: iscsi_process_read callback for when the iscsi socket becomes readable may be invoked by qemu after the fd-is-readable event has cleared.

[ronnie sahlberg]

The idea was that by being very caredul with FIONREAD  I can guarantee
non-blocking behaviour without relying on making the socket
non-blocking.
The only purpose of making the socket non-blocking was to ensure that
writes dont block due to the lack of a FIONWRITE ioctl.


But, the code becomes simpler.

Ill apply your patch to libiscsi.  Thanks. Ignore this patch to qemu.

regards
ronnie sahlberg


On Tue, May 15, 2012 at 1:30 AM, Paolo Bonzini <address@hidden> wrote:
> Il 12/05/2012 00:04, Ronnie Sahlberg ha scritto:
>> Libiscsi treats a situation such as POLLIN was invoked and the socket is 
>> readable but ioctl(...FIONREAD...) returns that there are no bytes available 
>> to read as an error and that the socket is faulty or has been closed.
>> which may trigger a slow process of closing down the socket completely and 
>> trying to reconnect to recover.
>>
>> Update the iscsi fd-is-readable callback  iscsi_process_read to check for 
>> this condition explicitely.
>> If are invoked and getsockopt tells us there is a real socket problem then 
>> we pass POLLIN onto libiscsi and let it try to handle the situation and/or 
>> recover.
>> If there is no error, but ioctl(...FIONREAD...) still indicates that there 
>> were no bytes to read, then we treat this as just a false invokation from 
>> the eventsystem and do nothing.
>> If there are bytes available to read, then we pass POLLIN into libiscsi and 
>> let it read and process the bytes.
>>
>> regards
>> ronnie sahlberg
>
> I can apply this patch as a workaround, but I see that you already
> applied the same thing upstream in libiscsi?
>
> And indeed, I think this is a bug in libiscsi.  You make the socket
> nonblocking (on POSIX only; but you can do the same ffor Win32, please
> see the code in qemu to set the FIONBIO ioctls), but then you do not
> handle EAGAIN.  Furthermore, you confuse read returning 0 (end-of-data)
> with read returning EAGAIN (no data available).  If you fix this, all
> the things you do with FIONREAD are not necessary.  See the attached
> patch, compile-tested only.
>
> Paolo