Quoting Eric Blake (eblake@redhat.com):
[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).
But that gid should be set according to what the guest expects, right? So we actually can't do it at compilation?
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").
I was thinking that should be a separate patch for ease of review, but I'll roll it into my next patch. thanks, -serge