[PATCH 1/2] linux: Add linux_get_vtno and linux_get_keeptty helpers
Hans de Goede
hdegoede at redhat.com
Thu Apr 30 03:28:55 PDT 2015
Hi,
So after letting these sit it is time to dust them of, fix the review
comments and get them upstream.
On 23-01-15 05:20, Peter Hutterer wrote:
> On Wed, Jan 21, 2015 at 10:36:15AM +0100, Hans de Goede wrote:
>> systemd-logind integration does not work when starting X on a new tty, as
>> that detaches X from the current session and after hat systemd-logind revokes
>> all rights any already open fds and refuses to open new fds for X.
>
> typo "all rights _on_ any ..."
Will fix.
>> This means that currently e.g. "startx -- vt7" breaks, and breaks badly,
>> requiring ssh access to the system to kill X.
>>
>> The fix for this is easy, we must not use systemd-logind integration when
>> not using KeepTty, or iow we may only use systemd-logind integration together
>> with KeepTty.
>>
>> But the final KeepTty value is not known until the code to chose which vtno to
>> run on has been called, which currently happens after intializing
>> systemd-logind.
>>
>> This commit is step 1 in fixing the "startx -- vt7" breakage, it factors out
>> the linux xf86OpenConsole bits which set xf86Info.vtno and keepTty so that
>> these can be called earlier. Calling this earlier is safe as this code has
>> no side effects other then setting xf86Info.vtno and keepTty.
>
> typo: then -> than
Will fix.
>> Note this basically only moves a large chunk of xf86OpenConsole() into
>> linux_get_vtno() without changing a single line of it, this is hard to see
>> in the diff because the identation level has changed.
>>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>> hw/xfree86/os-support/linux/linux.h | 32 +++++++++
>> hw/xfree86/os-support/linux/lnx_init.c | 117 +++++++++++++++++++--------------
>> 2 files changed, 99 insertions(+), 50 deletions(-)
>> create mode 100644 hw/xfree86/os-support/linux/linux.h
>>
>> diff --git a/hw/xfree86/os-support/linux/linux.h b/hw/xfree86/os-support/linux/linux.h
>> new file mode 100644
>> index 0000000..9613148
>> --- /dev/null
>> +++ b/hw/xfree86/os-support/linux/linux.h
>> @@ -0,0 +1,32 @@
>> +/*
>> + * Copyright © 2015 Red Hat, Inc.
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> + * DEALINGS IN THE SOFTWARE.
>> + *
>> + * Author: Hans de Goede <hdegoede at redhat.com>
>> + */
>> +
>> +#ifndef XF86_LINUX_H
>> +#define XF86_LINUX_H
>> +
>> +void linux_get_vtno(void);
>> +int linux_get_keeptty(void);
>> +
>> +#endif
>> diff --git a/hw/xfree86/os-support/linux/lnx_init.c b/hw/xfree86/os-support/linux/lnx_init.c
>> index 9485307..a6e9ac1 100644
>> --- a/hw/xfree86/os-support/linux/lnx_init.c
>> +++ b/hw/xfree86/os-support/linux/lnx_init.c
>> @@ -31,6 +31,7 @@
>> #include <X11/Xmd.h>
>>
>> #include "compiler.h"
>> +#include "linux.h"
>>
>> #include "xf86.h"
>> #include "xf86Priv.h"
>> @@ -80,71 +81,87 @@ switch_to(int vt, const char *from)
>> #pragma GCC diagnostic ignored "-Wformat-nonliteral"
>>
>> void
>> -xf86OpenConsole(void)
>> +linux_get_vtno(void)
>
> nitpick: I'd expect this to return the vtno, not set vtno and KeepTTTY.
> Maybe use linux_open_vt()?
Well it does not actually open the vt, it temporarily opens /dev/tty0 to find
a free vt, but that is all. As you said it sets vtno and KeepTTTY,
linux_open_vt() is too much like xf86OpenConsole() which does actually
open things.
Thinking more about this linux_parse_vt_settings() is the best name
I can come up with, so I'm going to go with that.
Regards,
Hans
>
> Cheers,
> Peter
>
>> {
>> int i, fd = -1, ret, current_vt = -1;
>> - struct vt_mode VT;
>> struct vt_stat vts;
>> struct stat st;
>> MessageType from = X_PROBED;
>> const char *tty0[] = { "/dev/tty0", "/dev/vc/0", NULL };
>> - const char *vcs[] = { "/dev/vc/%d", "/dev/tty%d", NULL };
>>
>> - if (serverGeneration == 1) {
>> - /*
>> - * setup the virtual terminal manager
>> - */
>> - if (xf86Info.vtno != -1) {
>> - from = X_CMDLINE;
>> - }
>> - else {
>> + /*
>> + * setup the virtual terminal manager
>> + */
>> + if (xf86Info.vtno != -1) {
>> + from = X_CMDLINE;
>> + }
>> + else {
>>
>> - i = 0;
>> - while (tty0[i] != NULL) {
>> - if ((fd = open(tty0[i], O_WRONLY, 0)) >= 0)
>> - break;
>> - i++;
>> - }
>> + i = 0;
>> + while (tty0[i] != NULL) {
>> + if ((fd = open(tty0[i], O_WRONLY, 0)) >= 0)
>> + break;
>> + i++;
>> + }
>>
>> - if (fd < 0)
>> - FatalError("xf86OpenConsole: Cannot open /dev/tty0 (%s)\n",
>> - strerror(errno));
>> + if (fd < 0)
>> + FatalError("xf86OpenConsole: Cannot open /dev/tty0 (%s)\n",
>> + strerror(errno));
>>
>> - if (xf86Info.ShareVTs) {
>> - SYSCALL(ret = ioctl(fd, VT_GETSTATE, &vts));
>> - if (ret < 0)
>> - FatalError("xf86OpenConsole: Cannot find the current"
>> - " VT (%s)\n", strerror(errno));
>> - xf86Info.vtno = vts.v_active;
>> - }
>> - else {
>> - SYSCALL(ret = ioctl(fd, VT_OPENQRY, &xf86Info.vtno));
>> - if (ret < 0)
>> - FatalError("xf86OpenConsole: Cannot find a free VT: "
>> - "%s\n", strerror(errno));
>> - if (xf86Info.vtno == -1)
>> - FatalError("xf86OpenConsole: Cannot find a free VT\n");
>> - }
>> - close(fd);
>> + if (xf86Info.ShareVTs) {
>> + SYSCALL(ret = ioctl(fd, VT_GETSTATE, &vts));
>> + if (ret < 0)
>> + FatalError("xf86OpenConsole: Cannot find the current"
>> + " VT (%s)\n", strerror(errno));
>> + xf86Info.vtno = vts.v_active;
>> }
>> + else {
>> + SYSCALL(ret = ioctl(fd, VT_OPENQRY, &xf86Info.vtno));
>> + if (ret < 0)
>> + FatalError("xf86OpenConsole: Cannot find a free VT: "
>> + "%s\n", strerror(errno));
>> + if (xf86Info.vtno == -1)
>> + FatalError("xf86OpenConsole: Cannot find a free VT\n");
>> + }
>> + close(fd);
>> + }
>>
>> - xf86Msg(from, "using VT number %d\n\n", xf86Info.vtno);
>> + xf86Msg(from, "using VT number %d\n\n", xf86Info.vtno);
>>
>> - /* Some of stdin / stdout / stderr maybe redirected to a file */
>> - for (i = STDIN_FILENO; i <= STDERR_FILENO; i++) {
>> - ret = fstat(i, &st);
>> - if (ret == 0 && S_ISCHR(st.st_mode) && major(st.st_rdev) == 4) {
>> - current_vt = minor(st.st_rdev);
>> - break;
>> - }
>> + /* Some of stdin / stdout / stderr maybe redirected to a file */
>> + for (i = STDIN_FILENO; i <= STDERR_FILENO; i++) {
>> + ret = fstat(i, &st);
>> + if (ret == 0 && S_ISCHR(st.st_mode) && major(st.st_rdev) == 4) {
>> + current_vt = minor(st.st_rdev);
>> + break;
>> }
>> + }
>>
>> - if (!KeepTty && current_vt == xf86Info.vtno) {
>> - xf86Msg(X_PROBED,
>> - "controlling tty is VT number %d, auto-enabling KeepTty\n",
>> - current_vt);
>> - KeepTty = TRUE;
>> - }
>> + if (!KeepTty && current_vt == xf86Info.vtno) {
>> + xf86Msg(X_PROBED,
>> + "controlling tty is VT number %d, auto-enabling KeepTty\n",
>> + current_vt);
>> + KeepTty = TRUE;
>> + }
>> +}
>> +
>> +int
>> +linux_get_keeptty(void)
>> +{
>> + return KeepTty;
>> +}
>> +
>> +void
>> +xf86OpenConsole(void)
>> +{
>> + int i, ret;
>> + struct vt_stat vts;
>> + struct vt_mode VT;
>> + const char *vcs[] = { "/dev/vc/%d", "/dev/tty%d", NULL };
>> +
>> + if (serverGeneration == 1) {
>> + if (xf86Info.vtno == -1)
>> + linux_get_vtno();
>>
>> if (!KeepTty) {
>> pid_t ppid = getppid();
>> --
>> 2.1.0
>>
>> _______________________________________________
>> xorg-devel at lists.x.org: X.Org development
>> Archives: http://lists.x.org/archives/xorg-devel
>> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>>
More information about the xorg-devel
mailing list