From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38028) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zr9eX-0006eH-R4 for qemu-devel@nongnu.org; Tue, 27 Oct 2015 15:11:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zr9eU-0005ot-JR for qemu-devel@nongnu.org; Tue, 27 Oct 2015 15:11:25 -0400 Received: from e32.co.us.ibm.com ([32.97.110.150]:55559) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zr9eU-0005of-Dj for qemu-devel@nongnu.org; Tue, 27 Oct 2015 15:11:22 -0400 Received: from localhost by e32.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 27 Oct 2015 13:11:20 -0600 Received: from b03cxnp08028.gho.boulder.ibm.com (b03cxnp08028.gho.boulder.ibm.com [9.17.130.20]) by d03dlp03.boulder.ibm.com (Postfix) with ESMTP id 58A1519D803F for ; Tue, 27 Oct 2015 12:59:27 -0600 (MDT) Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by b03cxnp08028.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t9RJ9tGk10092806 for ; Tue, 27 Oct 2015 12:09:55 -0700 Received: from d03av02.boulder.ibm.com (localhost [127.0.0.1]) by d03av02.boulder.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t9RJBGDj021860 for ; Tue, 27 Oct 2015 13:11:16 -0600 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <1445968123-1773-4-git-send-email-den@openvz.org> References: <1445968123-1773-1-git-send-email-den@openvz.org> <1445968123-1773-4-git-send-email-den@openvz.org> Message-ID: <20151027191112.4255.20231@loki> Date: Tue, 27 Oct 2015 14:11:12 -0500 Subject: Re: [Qemu-devel] [PATCH 3/3] qga: set file descriptor in qmp_guest_file_open non-blocking on Win32 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Denis V. Lunev" Cc: Olga Krishtal , Stefan Weil , qemu-devel@nongnu.org Quoting Denis V. Lunev (2015-10-27 12:48:43) > From: Olga Krishtal > = > 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 > Signed-off-by: Denis V. Lunev > CC: Michael Roth > CC: Stefan Weil > --- > 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 h= as_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 =3D 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 t= m *result) > } > #endif /* CONFIG_LOCALTIME_R */ > = > +static void qemu_set_handle_nonblocking(int64_t fh, bool nonblocking) > +{ > + DWORD file_type, pipe_state; > + HANDLE handle =3D (HANDLE)fh; > + file_type =3D GetFileType(handle); > + if (file_type !=3D FILE_TYPE_PIPE) { > + return; > + } > + /* If file_type =3D=3D 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 =3D=3D !(pipe_state & PIPE_NOWAIT)) { > + /* In this case we do not need perform any operation, because > + * nonblocking =3D true and PIPE_NOWAIT is already set or > + * nonblocking =3D false and PIPE_NOWAIT is not set */ > + return; > + } > + > + if (nonblocking) { > + pipe_state |=3D PIPE_NOWAIT; > + } else { > + pipe_state &=3D ~PIPE_NOWAIT; > + } > + > + SetNamedPipeHandleState(handle, &pipe_state, NULL, NULL); > +} > + > void qemu_set_block(int fd) > { > unsigned long opt =3D 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 >=20