[Qemu-devel] [PATCH 1/6] util, qga: drop guest_file_toggle_flags

[Denis V. Lunev]

From: Olga Krishtal <address@hidden>

guest_file_toggle_flags is a copy from semi-portable qemu_set_nonblock.
The latter is not working properly for Windows due to reduced Windows
Posix implementation.

On Windows OS there is a separate API for changing flags of file, pipes
and sockets. Portable way to change file descriptor flags requires
to detect file descriptor type and proper actions depending of that
type. The patch adds wrapper qemu_set_fd_blocking into Windows specific
code to handle this stuff properly.

The only problem is that qemu_set_nonblock is void but this should not
be a problem.

Signed-off-by: Olga Krishtal <address@hidden>
Signed-off-by: Denis V. Lunev <address@hidden>
CC: Roman Kagan <address@hidden>
CC: Eric Blake <address@hidden>
CC: Michael Roth <address@hidden>
---
 qga/commands-posix.c | 27 ++-------------------------
 util/oslib-win32.c   | 52 +++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 49 insertions(+), 30 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index ba8de62..d0f371b 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -28,6 +28,7 @@
 #include "qapi/qmp/qerror.h"
 #include "qemu/queue.h"
 #include "qemu/host-utils.h"
+#include "qemu/sockets.h"
 
 #ifndef CONFIG_HAS_ENVIRON
 #ifdef __APPLE__
@@ -376,27 +377,6 @@ safe_open_or_create(const char *path, const char *mode, 
Error **errp)
     return NULL;
 }
 
-static int guest_file_toggle_flags(int fd, int flags, bool set, Error **err)
-{
-    int ret, old_flags;
-
-    old_flags = fcntl(fd, F_GETFL);
-    if (old_flags == -1) {
-        error_set_errno(err, errno, QERR_QGA_COMMAND_FAILED,
-                        "failed to fetch filehandle flags");
-        return -1;
-    }
-
-    ret = fcntl(fd, F_SETFL, set ? (old_flags | flags) : (old_flags & ~flags));
-    if (ret == -1) {
-        error_set_errno(err, errno, QERR_QGA_COMMAND_FAILED,
-                        "failed to set filehandle flags");
-        return -1;
-    }
-
-    return ret;
-}
-
 int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode,
                             Error **errp)
 {
@@ -417,10 +397,7 @@ int64_t qmp_guest_file_open(const char *path, bool 
has_mode, const char *mode,
     /* set fd non-blocking to avoid common use cases (like reading from a
      * named pipe) from hanging the agent
      */
-    if (guest_file_toggle_flags(fileno(fh), O_NONBLOCK, true, errp) < 0) {
-        fclose(fh);
-        return -1;
-    }
+    qemu_set_nonblock(fileno(fh));
 
     handle = guest_file_handle_add(fh, errp);
     if (handle < 0) {
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 87cfbe0..58a2d5b 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -119,17 +119,59 @@ struct tm *localtime_r(const time_t *timep, struct tm 
*result)
     return p;
 }
 
-void qemu_set_block(int fd)
+static void qemu_set_fd_blocking(int fd, bool blocking)
 {
-    unsigned long opt = 0;
-    WSAEventSelect(fd, NULL, 0);
+    HANDLE handle;
+    DWORD file_type, pipe_state;
+
+    handle = (HANDLE)_get_osfhandle(fd);
+    if (handle == INVALID_HANDLE_VALUE) {
+        return;
+    }
+
+    file_type = GetFileType(handle);
+    if (file_type != FILE_TYPE_PIPE) {
+        return;
+    }
+
+    /* If file_type == FILE_TYPE_PIPE, according to msdn
+     * the specified file is socket or named pipe */
+    if (GetNamedPipeHandleState(handle, &pipe_state, NULL,
+                                NULL, NULL, NULL, 0)) {
+        /* The fd is named pipe fd */
+        if (!!blocking == !(pipe_state & PIPE_NOWAIT)) {
+            /* In this case we do not need perform any operation, because
+             * blocking = true and PIPE_NOWAIT is already set or
+             * blocking = false and PIPE_NOWAIT is not set */
+            return;
+        }
+
+        if (blocking) {
+            pipe_state |= PIPE_NOWAIT;
+        } else {
+            pipe_state &= ~PIPE_NOWAIT;
+        }
+
+        SetNamedPipeHandleState(handle, &pipe_state, NULL, NULL);
+        return;
+    }
+
+    /* The fd is socket fd */
+    unsigned long opt = (unsigned long)blocking;
+    if (!blocking) {
+        WSAEventSelect(fd, NULL, 0);
+    }
     ioctlsocket(fd, FIONBIO, &opt);
 }
 
+void qemu_set_block(int fd)
+{
+    qemu_set_fd_blocking(fd, false);
+}
+
 void qemu_set_nonblock(int fd)
 {
-    unsigned long opt = 1;
-    ioctlsocket(fd, FIONBIO, &opt);
+    qemu_set_fd_blocking(fd, true);
     qemu_fd_register(fd);
 }
 
-- 
1.9.1