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
diff --git a/usbredirparser/usbredirparser.c b/usbredirparser/usbredirparser.c
index dc72d82..5dfeb9e 100644
--- a/usbredirparser/usbredirparser.c
+++ b/usbredirparser/usbredirparser.c
@@ -24,7 +24,6 @@
 #include <stdlib.h>
 #include <stdarg.h>
 #include <string.h>
-#include <unistd.h>
 #include "usbredirproto-compat.h"
 #include "usbredirparser.h"
 #include "usbredirfilter.h"
@@ -1064,20 +1063,11 @@ int usbredirparser_do_write(struct usbredirparser *parser_pub)
             break;
 
         w = wbuf->len - wbuf->pos;
-        if (w > 0) {
-            w = parser->callb.write_func(parser->callb.priv,
-                                        wbuf->buf + wbuf->pos, w);
-            if (w < 0) {
-                ret = -1;
-                break;
-            }
-            if (w == 0) {
-                UNLOCK(parser);
-                WARNING("usbredirparser_do_write : 0 bytes written out of %d, sleeping for 0.1sec\n", wbuf->len - wbuf->pos);
-                usleep(100000);
-                LOCK(parser);
-                continue;
-            }
+        w = parser->callb.write_func(parser->callb.priv,
+                                     wbuf->buf + wbuf->pos, w);
+        if (w <= 0) {
+            ret = w;
+            break;
         }
 
         /* See usbredirparser_write documentation */
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Reply via email to