[Spice-devel] usbredirparser: fix EAGAIN error in *_do_write()

[Frediano Ziglio]

> 
> Hello,
> 
> I'd like to propose a bugfix for usbredirparser.c usbredirparser_do_write()
> procedure, see below.
> 
> 
> Issue description:
> 
> usbredirparser_do_write() does not handle EAGAIN properly.
> In case of large write parser->callb.write_func() returns 0 with
> errno=EAGAIN.
> 
> Then usbredirparser_do_write() code returns an error and, consequently,
> usbredirserver disconnects the client.This commit introduces some delay
> (100ms)
> and then retries the write.
> 
> 
> How to reproduce:
> 
> 
> Trigger some I/O which causes massive read from the USB device.
> For example, this error was initially observed when Windows 7 invoke
> filesystem check on a 8Gb  FAT USB flashdrive. There was a call to
> usbredirparser_do_write() that tried to write ~70Kb of data which
> caused the return value=0 and errno=EAGAIN from write(2).
> Apparently, socket write buffer was full.
> 
> 
> Observed behavior:
> 
> usbredirparser_do_write() returned error and consequently TCP connection
> to the client was closed by usbredirserver.
> 
> Expected behavior:
> 
> usbredirserver should wait for the data to be written, TCP connection
> with the client should not be closed.
> 
> 
> Suggested patch:
> 
> See attached *.patch file.
> 
> 
> 
> Best regards,
> Dmitriy Samborskiy

Hi Dmitriy,
  thanks for the patch.

>From your description looks like the patch is reversed.
This patch should do the job however could have consequences depending
on how the code is handled. If this code is not expected to block and
not executed in a thread specific to usb this will possibly block
other part of the application.
Usually EAGAIN is returned if you don't want to block and in case
you want the specific code to block it's better to use a select/poll
on the device if you get EAGAIN (or before the write) to avoid polling.

I'm not that familiar with usbredir code but looking at the code
if usbredirparser_do_write returns 0 the server should just loop
again. Perhaps write_func is returning -1 instead of 0?
Of the select loop in usbredirserver/usbredirserver.c is polling
for the wrong file descriptor?

Frediano