Quoting Michael Roth (2015-10-27 14:11:12) > 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.
Actually that would be silly since win32 has FILE as well. So I think this interface will only ever make sense for HANDLE so let's be explicit about it. > > > > > 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 > >