Quoting Denis V. Lunev (2015-10-27 12:48:43) > From: Olga Krishtal <okrish...@parallels.com> > > 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 analog of qemu_set_non/block 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.
If this is specific to HANDLEs, why do we need to cast back and forth between int64_t and HANDLE? I haven't build tested, but it seems like this would break for 32-bit mingw builds. I would define these as qemu_set_*_by_handle(HANDLE fh, ...) instead and make them win32 only. If someone wants to introduce a FILE* variant for posix they can introduce it as qemu_set_*_by_handle(FILE *fh, ...) rather than us needing to abstract away the handle type. > > Signed-off-by: Olga Krishtal <okrish...@parallels.com> > Signed-off-by: Denis V. Lunev <d...@openvz.org> > CC: Michael Roth <mdr...@linux.vnet.ibm.com> > CC: Stefan Weil <s...@weilnetz.de> > --- > include/qemu/sockets.h | 2 ++ > qga/commands-win32.c | 6 ++++++ > util/oslib-win32.c | 41 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 49 insertions(+) > > diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h > index 5a183c5..e2323f4 100644 > --- a/include/qemu/sockets.h > +++ b/include/qemu/sockets.h > @@ -39,6 +39,8 @@ int socket_set_cork(int fd, int v); > int socket_set_nodelay(int fd); > void qemu_set_block(int fd); > void qemu_set_nonblock(int fd); > +void qemu_set_nonblock_by_handle(int64_t fh); > +void qemu_set_block_by_handle(int64_t fh); > int socket_set_fast_reuse(int fd); > int send_all(int fd, const void *buf, int len1); > int recv_all(int fd, void *buf, int len1, bool single_read); > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > index 97f19d5..f959d75 100644 > --- a/qga/commands-win32.c > +++ b/qga/commands-win32.c > @@ -34,6 +34,7 @@ > #include "qapi/qmp/qerror.h" > #include "qemu/queue.h" > #include "qemu/host-utils.h" > +#include "qemu/sockets.h" > > #ifndef SHTDN_REASON_FLAG_PLANNED > #define SHTDN_REASON_FLAG_PLANNED 0x80000000 > @@ -158,6 +159,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 > + */ > + qemu_set_nonblock_by_handle((int64_t)fh); > + > fd = guest_file_handle_add(fh, errp); > if (fd < 0) { > CloseHandle(fh); > diff --git a/util/oslib-win32.c b/util/oslib-win32.c > index 09f9e98..4a9fd8e 100644 > --- a/util/oslib-win32.c > +++ b/util/oslib-win32.c > @@ -121,6 +121,37 @@ struct tm *localtime_r(const time_t *timep, struct tm > *result) > } > #endif /* CONFIG_LOCALTIME_R */ > > +static void qemu_set_handle_nonblocking(int64_t fh, bool nonblocking) > +{ > + DWORD file_type, pipe_state; > + HANDLE handle = (HANDLE)fh; > + 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)) { > + return; > + } > + /* The fd is named pipe fd */ > + if (!nonblocking == !(pipe_state & PIPE_NOWAIT)) { > + /* In this case we do not need perform any operation, because > + * nonblocking = true and PIPE_NOWAIT is already set or > + * nonblocking = false and PIPE_NOWAIT is not set */ > + return; > + } > + > + if (nonblocking) { > + pipe_state |= PIPE_NOWAIT; > + } else { > + pipe_state &= ~PIPE_NOWAIT; > + } > + > + SetNamedPipeHandleState(handle, &pipe_state, NULL, NULL); > +} > + > void qemu_set_block(int fd) > { > unsigned long opt = 0; > @@ -135,6 +166,16 @@ void qemu_set_nonblock(int fd) > qemu_fd_register(fd); > } > > +void qemu_set_block_by_handle(int64_t fh) > +{ > + qemu_set_handle_nonblocking(fh, false); > +} > + > +void qemu_set_nonblock_by_handle(int64_t fh) > +{ > + qemu_set_handle_nonblocking(fh, true); > +} > + > int socket_set_fast_reuse(int fd) > { > /* Enabling the reuse of an endpoint that was used by a socket still in > -- > 2.1.4 >