[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/1] lxc: use our own hand-rolled code in place of unlockpt and grantpt (v2)

On 10/17/2011 03:29 PM, Serge E. Hallyn wrote:
that matter, the whole point of the mount("devpts",...",gid=5")
designation is that the new pty will be owned by gid 5, without
needing to fchown() it.  Are there kernels that are new enough to
support newinstance mounting, yet old enough to not honor gid=5?

No.  Mode and gid precede multiple devpts instances.

Good to hear. In other words, glibc has to worry about it when dealing with "/dev/pts", because it caters to older kernels that lacked multiple devpts, but lxc can take shortcuts based on guarantees present by virtue of the existence of multiple instances. But definitely some comments in the code are called for, explaining why we are taking these shortcuts.

[but we still have to fix the hard-coding of
gid=5 in the mount() option].

I missed something - why do we have to fix that?

We don't have to fix it now, but we should fix it someday. There's nothing that says a distro has to map 'tty' to gid 5, and while most distros do that, we should instead be portable to compilation on a distro where 'tty' is gid 6 (or some other unusual number).

Instead, I'd just do this as:

virAsprintf(ttyName, "/dev/pts/%d", ptyno);

Where virAsprintf will allocate when ttyName starts as null?

Yep - the whole point of virAsprintf is to allocate the string on your behalf, and to gracefully ensure that ttyName is NULL on allocation failure - much more compact than using snprintf yourself, and avoids the waste of a PATH_MAX allocation.

And, while you are at it, what about also fixing src/util/util.c to remove virFileOpenTtyAt (now that no one calls that), by moving its body into virFileOpenTty except for using posix_openpt instead of open("/dev/ptmx").

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

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]