From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:36144) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfYGz-00050s-3v for qemu-devel@nongnu.org; Fri, 15 Jun 2012 11:17:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SfYGs-0007U7-OW for qemu-devel@nongnu.org; Fri, 15 Jun 2012 11:17:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60241) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SfYGs-0007Sd-GB for qemu-devel@nongnu.org; Fri, 15 Jun 2012 11:17:10 -0400 Message-ID: <4FDB51E8.8060406@redhat.com> Date: Fri, 15 Jun 2012 09:16:56 -0600 From: Eric Blake MIME-Version: 1.0 References: <1339689305-27031-1-git-send-email-coreyb@linux.vnet.ibm.com> <1339689305-27031-4-git-send-email-coreyb@linux.vnet.ibm.com> In-Reply-To: <1339689305-27031-4-git-send-email-coreyb@linux.vnet.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------enig159BA171E41DBFD5E7CBFD1D" Subject: Re: [Qemu-devel] [PATCH v3 3/5] osdep: Enable qemu_open to dup pre-opened fd List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Corey Bryant Cc: kwolf@redhat.com, aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com, libvir-list@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, pbonzini@redhat.com This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig159BA171E41DBFD5E7CBFD1D Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 06/14/2012 09:55 AM, Corey Bryant wrote: > This patch adds support to qemu_open to dup(fd) a pre-opened file > descriptor if the filename is of the format /dev/fd/X. >=20 > +++ b/osdep.c > @@ -82,6 +82,19 @@ int qemu_open(const char *name, int flags, ...) > int ret; > int mode =3D 0; > =20 > +#ifndef _WIN32 > + const char *p; > + > + /* Attempt dup of fd for pre-opened file */ > + if (strstart(name, "/dev/fd/", &p)) { > + ret =3D qemu_parse_fd(p); > + if (ret =3D=3D -1) { > + return -1; > + } > + return dup(ret); I think you need to honor flags so that the end use of the fd will be as if qemu had directly opened the file, rather than just doing a blind dup with a resulting fd that is in a different state than the caller expected. I can think of at least the following cases (there may be more= ): if (flags & O_TRUNCATE || ((flags & (O_CREAT|O_EXCL)) =3D=3D (O_CREAT|O_EXCL)) ftruncate(ret, 0); Why do I truncate on O_CREAT|O_EXCL? Because that's a case where open() guarantees that the file will have just been created empty. if (flags & O_CLOEXEC) use fcntl(F_DUPFD_CLOEXEC) instead of dup(), if possible else fallback to fcntl(F_GETFD/F_SETFD) to set FD_CLOEXEC non-atomical= ly Oh, and are we using MSG_CMSG_CLOEXEC where possible (and where not possible, falling back to fcntl(F_GETFD/F_SETFD) to set FD_CLOEXEC) on all fds received by 'getfd' and 'pass-fd'? I can't think of any reason why 'migrate fd:name' would need to be inheritable, and in the case of /dev/fd/ parsing, while the dup() result may need to be inheritable, the original that we are dup'ing from should most certainly be cloexec. if (flags & O_NONBLOCK) use fcntl(F_GETFL/F_SETFL) to set O_NONBLOCK else use fcntl(F_GETFL/F_SETFL) to clear O_NONBLOCK or maybe we document that callers of pass-fd must always pass fds with O_NONBLOCK clear instead of clearing it ourselves. Or maybe we make sure part of the process of tying name with fd in the lookup list of named fds is determining the current O_NONBLOCK state in case future qemu_open() need it in the opposite state. Treat O_APPEND similarly to O_NONBLOCK (with fcntl(F_GETFL/F_SETFL) to match the desired open() setting). --=20 Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --------------enig159BA171E41DBFD5E7CBFD1D Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBCAAGBQJP21HpAAoJEKeha0olJ0NqoMwH/iw3anhNMrMfWS83fPrAH2/W l6/f27ZcbTkWo1HZyqsv6xgvB0NU/vTIkRuJis7loOr0yqWqe9vsqVgUvpcI8jXQ seXqdG5cMS2GmN87sGCEfe6NGa0FhoJHLRHZB5O5QITFAaV9GcZknmTcUfKwupmD cCdnMHfXDaOWiAtQUFd3P/EC0WoLyq7YCh4HndBh7xffsLCf/STg4OsKHQBvTLDT pbwFpbED5sEU65D5Yr2a+Fe7bivmH+aVDq0m802Ja+D9ol/5+zaADHwORvmDBGai +moupGkw1ydednVpSNe4nj0pxq4Et+nl6NXoj0gyuluTbfGo4eTVV0TZ639QX1g= =Ccse -----END PGP SIGNATURE----- --------------enig159BA171E41DBFD5E7CBFD1D--