[v2,2/2] systemd-logind: Only use systemd-logind integration together with keeptty

Submitted by Hans de Goede on April 30, 2015, 12:24 p.m.

Details

Message ID 1430396669-5218-2-git-send-email-hdegoede@redhat.com
State Accepted
Headers show

Not browsing as part of any series.

Commit Message

Hans de Goede April 30, 2015, 12:24 p.m.
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.

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.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Document that -keeptty must be passed for logind integration in man page
-Print an INFO message when disabling logind integration due to -keeptty
 not being set
---
 hw/xfree86/man/Xorg.man                      | 6 +++---
 hw/xfree86/os-support/linux/systemd-logind.c | 9 +++++++++
 2 files changed, 12 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/hw/xfree86/man/Xorg.man b/hw/xfree86/man/Xorg.man
index 3ff6aef..0864a58 100644
--- a/hw/xfree86/man/Xorg.man
+++ b/hw/xfree86/man/Xorg.man
@@ -271,9 +271,9 @@  is ignored if
 is anything other than \(oqPCI\(cq.
 .TP 8
 .B \-keeptty
-Prevent the server from detaching its initial controlling terminal.
-This option is only useful when debugging the server.  Not all platforms
-support (or can use) this option.
+Prevent the server from detaching its initial controlling terminal. If you
+want to use systemd-logind integration you must specify this option.
+Not all platorms support (or can use) this option.
 .TP 8
 .BI \-keyboard " keyboard-name"
 Use the xorg.conf(__filemansuffix__) file
diff --git a/hw/xfree86/os-support/linux/systemd-logind.c b/hw/xfree86/os-support/linux/systemd-logind.c
index 4ad41a3..72f1ae3 100644
--- a/hw/xfree86/os-support/linux/systemd-logind.c
+++ b/hw/xfree86/os-support/linux/systemd-logind.c
@@ -34,6 +34,7 @@ 
 
 #include "os.h"
 #include "dbus-core.h"
+#include "linux.h"
 #include "xf86.h"
 #include "xf86platformBus.h"
 #include "xf86Xinput.h"
@@ -596,6 +597,14 @@  static struct dbus_core_hook core_hook = {
 int
 systemd_logind_init(void)
 {
+    linux_parse_vt_settings();
+    if (!linux_get_keeptty()) {
+        LogMessage(X_INFO,
+            "systemd-logind: logind integration requires -keeptty and "
+            "-keeptty was not provided, disabling logind integration\n");
+        return 1;
+    }
+
     return dbus_core_add_hook(&core_hook);
 }
 

Comments

On Thu, Apr 30, 2015 at 02:24:29PM +0200, 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.
> 
> 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.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Document that -keeptty must be passed for logind integration in man page
> -Print an INFO message when disabling logind integration due to -keeptty
>  not being set
> ---
>  hw/xfree86/man/Xorg.man                      | 6 +++---
>  hw/xfree86/os-support/linux/systemd-logind.c | 9 +++++++++
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/xfree86/man/Xorg.man b/hw/xfree86/man/Xorg.man
> index 3ff6aef..0864a58 100644
> --- a/hw/xfree86/man/Xorg.man
> +++ b/hw/xfree86/man/Xorg.man
> @@ -271,9 +271,9 @@ is ignored if
>  is anything other than \(oqPCI\(cq.
>  .TP 8
>  .B \-keeptty
> -Prevent the server from detaching its initial controlling terminal.
> -This option is only useful when debugging the server.  Not all platforms
> -support (or can use) this option.
> +Prevent the server from detaching its initial controlling terminal. If you
> +want to use systemd-logind integration you must specify this option.
> +Not all platorms support (or can use) this option.

typo, platforms

otherwise, Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

Cheers,
   Peter

>  .TP 8
>  .BI \-keyboard " keyboard-name"
>  Use the xorg.conf(__filemansuffix__) file
> diff --git a/hw/xfree86/os-support/linux/systemd-logind.c b/hw/xfree86/os-support/linux/systemd-logind.c
> index 4ad41a3..72f1ae3 100644
> --- a/hw/xfree86/os-support/linux/systemd-logind.c
> +++ b/hw/xfree86/os-support/linux/systemd-logind.c
> @@ -34,6 +34,7 @@
>  
>  #include "os.h"
>  #include "dbus-core.h"
> +#include "linux.h"
>  #include "xf86.h"
>  #include "xf86platformBus.h"
>  #include "xf86Xinput.h"
> @@ -596,6 +597,14 @@ static struct dbus_core_hook core_hook = {
>  int
>  systemd_logind_init(void)
>  {
> +    linux_parse_vt_settings();
> +    if (!linux_get_keeptty()) {
> +        LogMessage(X_INFO,
> +            "systemd-logind: logind integration requires -keeptty and "
> +            "-keeptty was not provided, disabling logind integration\n");
> +        return 1;
> +    }
> +
>      return dbus_core_add_hook(&core_hook);
>  }
>  
> -- 
> 2.3.6
> 
> _______________________________________________
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>
On 04/30/2015 05:24 AM, 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.
>
> 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.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

I can confirm that this fixes VT switching for X servers started from an 
SSH session on Arch Linux, which I use all the time for debugging. So

Tested-by: Aaron Plattner <aplattner@nvidia.com>

I was afraid not using -keeptty was going to break using a debugger, but 
it looks like it works these days. Not using -keeptty breaks ctrl-Z to 
suspend the server, but I'm less concerned about that.

> ---
> Changes in v2:
> -Document that -keeptty must be passed for logind integration in man page
> -Print an INFO message when disabling logind integration due to -keeptty
>   not being set
> ---
>   hw/xfree86/man/Xorg.man                      | 6 +++---
>   hw/xfree86/os-support/linux/systemd-logind.c | 9 +++++++++
>   2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/hw/xfree86/man/Xorg.man b/hw/xfree86/man/Xorg.man
> index 3ff6aef..0864a58 100644
> --- a/hw/xfree86/man/Xorg.man
> +++ b/hw/xfree86/man/Xorg.man
> @@ -271,9 +271,9 @@ is ignored if
>   is anything other than \(oqPCI\(cq.
>   .TP 8
>   .B \-keeptty
> -Prevent the server from detaching its initial controlling terminal.
> -This option is only useful when debugging the server.  Not all platforms
> -support (or can use) this option.
> +Prevent the server from detaching its initial controlling terminal. If you
> +want to use systemd-logind integration you must specify this option.
> +Not all platorms support (or can use) this option.
>   .TP 8
>   .BI \-keyboard " keyboard-name"
>   Use the xorg.conf(__filemansuffix__) file
> diff --git a/hw/xfree86/os-support/linux/systemd-logind.c b/hw/xfree86/os-support/linux/systemd-logind.c
> index 4ad41a3..72f1ae3 100644
> --- a/hw/xfree86/os-support/linux/systemd-logind.c
> +++ b/hw/xfree86/os-support/linux/systemd-logind.c
> @@ -34,6 +34,7 @@
>
>   #include "os.h"
>   #include "dbus-core.h"
> +#include "linux.h"
>   #include "xf86.h"
>   #include "xf86platformBus.h"
>   #include "xf86Xinput.h"
> @@ -596,6 +597,14 @@ static struct dbus_core_hook core_hook = {
>   int
>   systemd_logind_init(void)
>   {
> +    linux_parse_vt_settings();
> +    if (!linux_get_keeptty()) {
> +        LogMessage(X_INFO,
> +            "systemd-logind: logind integration requires -keeptty and "
> +            "-keeptty was not provided, disabling logind integration\n");
> +        return 1;
> +    }
> +
>       return dbus_core_add_hook(&core_hook);
>   }
>
>
Hi,

On 15-05-15 22:21, Aaron Plattner wrote:
> On 04/30/2015 05:24 AM, 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.
>>
>> 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.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> I can confirm that this fixes VT switching for X servers started from an SSH session on Arch Linux, which I use all the time for debugging. So
>
> Tested-by: Aaron Plattner <aplattner@nvidia.com>

Thanks, note that testing in Fedora has found a minor issue when running headless (with the dummy driver),
so a v3 of the patch-set is coming up, see:

https://bugzilla.redhat.com/show_bug.cgi?id=1203780#c8

I'm waiting from feedback from the reporter before posting v3.

In the mean time you can grab v3 of the patch-set here:

http://cgit.freedesktop.org/~jwrdegoede/xserver/

(it is now 3 patches).

Regards,

Hans
On 05/16/2015 01:03 AM, Hans de Goede wrote:
> Hi,
>
> On 15-05-15 22:21, Aaron Plattner wrote:
>> On 04/30/2015 05:24 AM, 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.
>>>
>>> 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.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>
>> I can confirm that this fixes VT switching for X servers started from
>> an SSH session on Arch Linux, which I use all the time for debugging. So
>>
>> Tested-by: Aaron Plattner <aplattner@nvidia.com>
>
> Thanks, note that testing in Fedora has found a minor issue when running
> headless (with the dummy driver),
> so a v3 of the patch-set is coming up, see:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1203780#c8
>
> I'm waiting from feedback from the reporter before posting v3.
>
> In the mean time you can grab v3 of the patch-set here:
>
> http://cgit.freedesktop.org/~jwrdegoede/xserver/
>
> (it is now 3 patches).

Thanks! I can confirm that merging commit 
d7855745890d42ed56ceb97857081e9097acec12 also fixes the problem for me, 
so you can feel free to add my Tested-by line to those as well.