On 11/03/2011 03:03 PM, Eric Blake wrote:
MacOS lacks ptsname_r, and gnulib doesn't (yet) provide it.
But we can avoid it altogether, by using gnulib openpty()
instead.  Note that we do _not_ want the pt_chown module;
all systems that we currently port to can either properly do
openpty() and/or grantpt(), or lack ptys altogether; we are
not porting to any system that requires us to deal with the
hassle of installing a setuid pt_chown helper just to satisfy
gnulib's ability to provide openpty() on even more platforms.

* .gnulib: Update to latest, for openpty fixes
* bootstrap.conf (gnulib_modules): Add openpty, ttyname_r.
(gnulib_tool_option_extras): Exclude pt_chown module.
* src/util/util.c (virFileOpenTty): Rewrite in terms of openpty
and ttyname_r.
* src/util/util.h (virFileOpenTtyAt): Delete dead prototype.
Reported by Justin Clift.
---

I compile-tested this on Linux, but haven't yet tested this on
MacOS, which is the driving force behind this patch.  Meanwhile,
it would be nice to run some LXC tests to ensure this all still
works.  I'm posting it now, to get the review started while I
figure out how to run further tests.

I've now confirmed that this patch is sufficient to get compilation working on FreeBSD 8.2; presumably, since MacOS is BSD-based, it will also help compilation there (although I haven't yet been able to test that).

However, I also finally managed to test an LXC guest on Linux, and found out this patch is incorrect:

@@ -1177,47 +1179,65 @@ int virFileOpenTty(int *ttymaster,
                     char **ttyName,
                     int rawmode)
  {
-    int rc = -1;
-
-    if ((*ttymaster = posix_openpt(O_RDWR|O_NOCTTY|O_NONBLOCK))<  0)

The old code set O_NONBLOCK on the master, but the new code fails to do that, because openpty() lacks a flag argument. Furthermore, neither the old nor new code was setting FD_CLOEXEC on the resulting fd; in general a good thing to do in a multi-threaded app.

I've opened a glibc bug requesting that openpty be extended to give a flags variant that could allow O_NONBLOCK and O_CLOEXEC. Meanwhile, I'm working up a quick gnulib patch to provide the same functionality (although gnulib will eventually want to match the glibc naming convention, if Ulrich agrees to add it to glibc).

But getting a new interface in place is not a pre-req to 0.9.7; so for now, I'll just post a v2 that sets the O_NONBLOCK flag properly.

--
Eric Blake   ebl...@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to