From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59164) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZCwrA-0004FG-Q4 for qemu-devel@nongnu.org; Wed, 08 Jul 2015 17:26:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZCwr8-0005Dn-2C for qemu-devel@nongnu.org; Wed, 08 Jul 2015 17:26:16 -0400 Received: from e19.ny.us.ibm.com ([129.33.205.209]:48667) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZCwr7-0005DX-UL for qemu-devel@nongnu.org; Wed, 08 Jul 2015 17:26:13 -0400 Received: from /spool/local by e19.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 8 Jul 2015 17:26:13 -0400 Received: from b01cxnp23034.gho.pok.ibm.com (b01cxnp23034.gho.pok.ibm.com [9.57.198.29]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id 1578638C8041 for ; Wed, 8 Jul 2015 17:15:22 -0400 (EDT) Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by b01cxnp23034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t68LQA6b63504602 for ; Wed, 8 Jul 2015 21:26:11 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 t68LQABl010562 for ; Wed, 8 Jul 2015 17:26:10 -0400 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <559B8B7B.6080403@openvz.org> References: <1435659923-2211-1-git-send-email-den@openvz.org> <1435659923-2211-2-git-send-email-den@openvz.org> <559B8B7B.6080403@openvz.org> Message-ID: <20150708212606.23206.7445@loki> Date: Wed, 08 Jul 2015 16:26:06 -0500 Subject: Re: [Qemu-devel] [PATCH 01/10] util, qga: drop guest_file_toggle_flags List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Denis V. Lunev" Cc: Olga Krishtal , qemu-devel@nongnu.org Quoting Denis V. Lunev (2015-07-07 03:19:07) > On 30/06/15 13:25, Denis V. Lunev wrote: > > From: Olga Krishtal > > > > guest_file_toggle_flags is a copy from semi-portable qemu_set_nonblock. > > The latter is not working properly for Windows due to reduced Windows > > Posix implementation. > > > > On Windows OS there is a separate API for changing flags of file, pipes > > and sockets. Portable way to change file descriptor flags requires > > to detect file descriptor type and proper actions depending of that > > type. The patch adds wrapper qemu_set_fd_nonblocking into Windows speci= fic > > code to handle this stuff properly. > > > > The only problem is that qemu_set_nonblock is void but this should not > > be a problem. > > > > Signed-off-by: Olga Krishtal > > Signed-off-by: Denis V. Lunev > > CC: Eric Blake > > CC: Michael Roth > Michael, Eric, > = > can you please consider merging of this patch. This is a > semi-independent cleanup which has sense on its own. I think this is more a "feature" in the sense that it's adding support for pipes to qemu_set_nonblock(). Since the guest-open-pipe* stuff is likely the first user of the feature it should probably wait on those. > = > Den > = > = > > --- > > qga/commands-posix.c | 27 ++------------------------- > > util/oslib-win32.c | 52 +++++++++++++++++++++++++++++++++++++++++++= ++++----- > > 2 files changed, 49 insertions(+), 30 deletions(-) > > > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > > index befd00b..40dbe25 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__ > > @@ -376,27 +377,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 =3D fcntl(fd, F_GETFL); > > - if (old_flags =3D=3D -1) { > > - error_setg_errno(err, errno, QERR_QGA_COMMAND_FAILED, > > - "failed to fetch filehandle flags"); > > - return -1; > > - } > > - > > - ret =3D fcntl(fd, F_SETFL, set ? (old_flags | flags) : (old_flags = & ~flags)); > > - if (ret =3D=3D -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 ch= ar *mode, > > Error **errp) > > { > > @@ -417,10 +397,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 fr= om 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 =3D guest_file_handle_add(fh, errp); > > if (handle < 0) { > > diff --git a/util/oslib-win32.c b/util/oslib-win32.c > > index 730a670..1a6ae72 100644 > > --- a/util/oslib-win32.c > > +++ b/util/oslib-win32.c > > @@ -119,17 +119,59 @@ struct tm *localtime_r(const time_t *timep, struc= t tm *result) > > return p; > > } > > = > > -void qemu_set_block(int fd) > > +static void qemu_set_fd_nonblocking(int fd, bool nonblocking) > > { > > - unsigned long opt =3D 0; > > - WSAEventSelect(fd, NULL, 0); > > + HANDLE handle; > > + DWORD file_type, pipe_state; > > + > > + handle =3D (HANDLE)_get_osfhandle(fd); > > + if (handle =3D=3D INVALID_HANDLE_VALUE) { > > + return; > > + } > > + > > + 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)) { > > + /* The fd is named pipe fd */ > > + if (!nonblocking =3D=3D !(pipe_state & PIPE_NOWAIT)) { > > + /* In this case we do not need perform any operation, beca= use > > + * 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); > > + return; > > + } > > + > > + /* The fd is socket fd */ > > + unsigned long opt =3D (unsigned long)nonblocking; > > + if (!nonblocking) { > > + WSAEventSelect(fd, NULL, 0); > > + } > > ioctlsocket(fd, FIONBIO, &opt); > > } > > = > > +void qemu_set_block(int fd) > > +{ > > + qemu_set_fd_nonblocking(fd, false); > > +} > > + > > void qemu_set_nonblock(int fd) > > { > > - unsigned long opt =3D 1; > > - ioctlsocket(fd, FIONBIO, &opt); > > + qemu_set_fd_nonblocking(fd, true); > > qemu_fd_register(fd); > > } > > = >=20