From 125053965b05b31427ff5c75dc3b87acaa8d0505 Mon Sep 17 00:00:00 2001 From: "Denis V. Lunev" Date: Wed, 28 Oct 2015 18:13:55 +0300 Subject: [PATCH 1/3] qga: drop hand-made guest_file_toggle_flags helper We'd better use generic qemu_set_nonblock directly. Signed-off-by: Denis V. Lunev Reviewed-by: Yuri Pudgorodskiy Reviewed-by: Eric Blake CC: Michael Roth Signed-off-by: Michael Roth --- qga/commands-posix.c | 27 ++------------------------- 1 file changed, 2 insertions(+), 25 deletions(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 67a173af4f..0ebd47336a 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__ @@ -385,27 +386,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_setg_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_setg_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) { @@ -426,10 +406,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) { From c87d0964ef7534d50a4c729a6ae20045b3a0cd34 Mon Sep 17 00:00:00 2001 From: Olga Krishtal Date: Wed, 28 Oct 2015 18:13:56 +0300 Subject: [PATCH 2/3] qga: fixed CloseHandle in qmp_guest_file_open CloseHandle use HANDLE as an argument, but not *HANDLE Signed-off-by: Olga Krishtal Signed-off-by: Denis V. Lunev Reviewed-by: Stefan Weil CC: Michael Roth Signed-off-by: Michael Roth --- qga/commands-win32.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qga/commands-win32.c b/qga/commands-win32.c index d9de23bbb8..97f19d5392 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -160,7 +160,7 @@ int64_t qmp_guest_file_open(const char *path, bool has_mode, fd = guest_file_handle_add(fh, errp); if (fd < 0) { - CloseHandle(&fh); + CloseHandle(fh); error_setg(errp, "failed to add handle to qmp handle table"); return -1; } From fb68777312887000cd0367d72621fdd67cc4a0a0 Mon Sep 17 00:00:00 2001 From: Olga Krishtal Date: Wed, 28 Oct 2015 18:13:57 +0300 Subject: [PATCH 3/3] qga: set file descriptor in qmp_guest_file_open non-blocking on Win32 Set fd non-blocking to avoid common use cases (like reading from a named pipe) from hanging the agent. This was missed in the original code. The patch introduces qemu_set_handle_nonoblocking, the local analog of qemu_set_nonblock for HANDLES. The usage of handles in qemu_set_non/block is impossible, because for win32 there is a difference between file discriptors and file handles, and all file ops are made via Win32 api. Signed-off-by: Olga Krishtal Signed-off-by: Denis V. Lunev CC: Michael Roth CC: Stefan Weil Signed-off-by: Michael Roth --- qga/commands-win32.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 97f19d5392..a5306e76b0 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -128,6 +128,28 @@ static GuestFileHandle *guest_file_handle_find(int64_t id, Error **errp) return NULL; } +static void handle_set_nonblocking(HANDLE fh) +{ + DWORD file_type, pipe_state; + file_type = GetFileType(fh); + 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(fh, &pipe_state, NULL, + NULL, NULL, NULL, 0)) { + return; + } + /* The fd is named pipe fd */ + if (pipe_state & PIPE_NOWAIT) { + return; + } + + pipe_state |= PIPE_NOWAIT; + SetNamedPipeHandleState(fh, &pipe_state, NULL, NULL); +} + int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, Error **errp) { @@ -158,6 +180,11 @@ int64_t qmp_guest_file_open(const char *path, bool has_mode, return -1; } + /* set fd non-blocking to avoid common use cases (like reading from a + * named pipe) from hanging the agent + */ + handle_set_nonblocking(fh); + fd = guest_file_handle_add(fh, errp); if (fd < 0) { CloseHandle(fh);