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 ..." > > 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 > > 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 <[email protected]> > --- > 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 <[email protected]> > + */ > + > +#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()? 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 > > _______________________________________________ > [email protected]: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel > _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
