From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39222) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zr9hh-0008HB-BJ for qemu-devel@nongnu.org; Tue, 27 Oct 2015 15:14:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zr9hc-0006sJ-BX for qemu-devel@nongnu.org; Tue, 27 Oct 2015 15:14:41 -0400 Received: from e19.ny.us.ibm.com ([129.33.205.209]:60406) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zr9hc-0006s7-84 for qemu-devel@nongnu.org; Tue, 27 Oct 2015 15:14:36 -0400 Received: from localhost by e19.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 27 Oct 2015 15:14:35 -0400 Received: from b01cxnp22036.gho.pok.ibm.com (b01cxnp22036.gho.pok.ibm.com [9.57.198.26]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id 0CBA338C8054 for ; Tue, 27 Oct 2015 15:14:32 -0400 (EDT) Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by b01cxnp22036.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t9RJEVE856099032 for ; Tue, 27 Oct 2015 19:14:31 GMT Received: from d01av03.pok.ibm.com (localhost [127.0.0.1]) by d01av03.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t9RJEVg6031118 for ; Tue, 27 Oct 2015 15:14:31 -0400 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <20151027191112.4255.20231@loki> References: <1445968123-1773-1-git-send-email-den@openvz.org> <1445968123-1773-4-git-send-email-den@openvz.org> <20151027191112.4255.20231@loki> Message-ID: <20151027191426.4255.81022@loki> Date: Tue, 27 Oct 2015 14:14:26 -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 Michael Roth (2015-10-27 14:11:12) > 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. 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 > > 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= has_mode, > > return -1; > > } > > = > > + /* set fd non-blocking to avoid common use cases (like reading fro= m 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= tm *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 sti= ll in > > -- = > > 2.1.4 > >=20