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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org