[Qemu-devel] [PATCH v2 1/2] char: fix broken EAGAIN retry on OS-X due to errno clobbering

[Daniel P. Berrange]

Some of the chardev I/O paths really want to write the
complete data buffer even though the channel is in
non-blocking mode. To achieve this they look for EAGAIN
and g_usleep() for 100ms. Unfortunately the code is set
to check errno == EAGAIN a second time, after the g_usleep()
call has completed. On OS-X at least, g_usleep clobbers
errno to ETIMEDOUT, causing the retry to be skipped.

This failure to retry means the full data isn't written
to the chardev backend, which causes various failures
including making the tests/ahci-test qtest hang.

Rather than playing games trying to reset errno just
simplify the code to use a goto to retry instead of a
a loop.

Signed-off-by: Daniel P. Berrange <address@hidden>
---
 qemu-char.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 270819a..93fd733 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -225,12 +225,12 @@ static void qemu_chr_fe_write_log(CharDriverState *s,
     }
 
     while (done < len) {
-        do {
-            ret = write(s->logfd, buf + done, len - done);
-            if (ret == -1 && errno == EAGAIN) {
-                g_usleep(100);
-            }
-        } while (ret == -1 && errno == EAGAIN);
+    retry:
+        ret = write(s->logfd, buf + done, len - done);
+        if (ret == -1 && errno == EAGAIN) {
+            g_usleep(100);
+            goto retry;
+        }
 
         if (ret <= 0) {
             return;
@@ -246,12 +246,12 @@ static int qemu_chr_fe_write_buffer(CharDriverState *s, 
const uint8_t *buf, int
 
     qemu_mutex_lock(&s->chr_write_lock);
     while (*offset < len) {
-        do {
-            res = s->chr_write(s, buf + *offset, len - *offset);
-            if (res == -1 && errno == EAGAIN) {
-                g_usleep(100);
-            }
-        } while (res == -1 && errno == EAGAIN);
+    retry:
+        res = s->chr_write(s, buf + *offset, len - *offset);
+        if (res < 0 && errno == EAGAIN) {
+            g_usleep(100);
+            goto retry;
+        }
 
         if (res <= 0) {
             break;
@@ -333,12 +333,12 @@ int qemu_chr_fe_read_all(CharDriverState *s, uint8_t 
*buf, int len)
     }
 
     while (offset < len) {
-        do {
-            res = s->chr_sync_read(s, buf + offset, len - offset);
-            if (res == -1 && errno == EAGAIN) {
-                g_usleep(100);
-            }
-        } while (res == -1 && errno == EAGAIN);
+    retry:
+        res = s->chr_sync_read(s, buf + offset, len - offset);
+        if (res == -1 && errno == EAGAIN) {
+            g_usleep(100);
+            goto retry;
+        }
 
         if (res == 0) {
             break;
-- 
2.5.5