Discussion:
[PATCH] xfree86: Only switch to original VT if it is active.
Michal Srb
2018-10-11 14:45:46 UTC
Permalink
If the X server is terminated while its VT is not active, it should
not change the current VT.
---
Changing the VT in that situation serves no purpose and can be confusing.
For example when a user's graphical session is terminated while other
user is using the computer, it would switch the VT he is working on.

hw/xfree86/common/xf86Events.c | 4 ++++
hw/xfree86/common/xf86Globals.c | 1 +
hw/xfree86/common/xf86Privstr.h | 1 +
hw/xfree86/os-support/linux/lnx_init.c | 4 +++-
hw/xfree86/os-support/linux/systemd-logind.c | 5 ++++-
5 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/hw/xfree86/common/xf86Events.c b/hw/xfree86/common/xf86Events.c
index 80676c669..1e52c4f12 100644
--- a/hw/xfree86/common/xf86Events.c
+++ b/hw/xfree86/common/xf86Events.c
@@ -405,6 +405,8 @@ xf86VTLeave(void)
if (xorgHWAccess)
xf86DisableIO();

+ xf86Info.hasVT = FALSE;
+
xf86UpdateHasVTProperty(FALSE);

return;
@@ -488,6 +490,8 @@ xf86VTEnter(void)
xf86platformVTProbe();
#endif

+ xf86Info.hasVT = TRUE;
+
xf86UpdateHasVTProperty(TRUE);

input_unlock();
diff --git a/hw/xfree86/common/xf86Globals.c b/hw/xfree86/common/xf86Globals.c
index 193f17aec..8bd8f8165 100644
--- a/hw/xfree86/common/xf86Globals.c
+++ b/hw/xfree86/common/xf86Globals.c
@@ -106,6 +106,7 @@ xf86InfoRec xf86Info = {
.dontVTSwitch = FALSE,
.autoVTSwitch = TRUE,
.ShareVTs = FALSE,
+ .hasVT = FALSE,
.dontZap = FALSE,
.dontZoom = FALSE,
.currentScreen = NULL,
diff --git a/hw/xfree86/common/xf86Privstr.h b/hw/xfree86/common/xf86Privstr.h
index 55d1b2455..e4d827dee 100644
--- a/hw/xfree86/common/xf86Privstr.h
+++ b/hw/xfree86/common/xf86Privstr.h
@@ -61,6 +61,7 @@ typedef struct {
Bool dontVTSwitch;
Bool autoVTSwitch;
Bool ShareVTs;
+ Bool hasVT;
Bool dontZap;
Bool dontZoom;

diff --git a/hw/xfree86/os-support/linux/lnx_init.c b/hw/xfree86/os-support/linux/lnx_init.c
index 039dc4a4d..a8c43782a 100644
--- a/hw/xfree86/os-support/linux/lnx_init.c
+++ b/hw/xfree86/os-support/linux/lnx_init.c
@@ -233,6 +233,8 @@ xf86OpenConsole(void)
if (!switch_to(xf86Info.vtno, "xf86OpenConsole"))
FatalError("xf86OpenConsole: Switching VT failed\n");

+ xf86Info.hasVT = TRUE;
+
SYSCALL(ret = ioctl(xf86Info.consoleFd, VT_GETMODE, &VT));
if (ret < 0)
FatalError("xf86OpenConsole: VT_GETMODE failed %s\n",
@@ -334,7 +336,7 @@ xf86CloseConsole(void)
strerror(errno));
}

- if (xf86Info.autoVTSwitch) {
+ if (xf86Info.autoVTSwitch && xf86Info.hasVT) {
/*
* Perform a switch back to the active VT when we were started
*/
diff --git a/hw/xfree86/os-support/linux/systemd-logind.c b/hw/xfree86/os-support/linux/systemd-logind.c
index 13784d15c..0d3f2de27 100644
--- a/hw/xfree86/os-support/linux/systemd-logind.c
+++ b/hw/xfree86/os-support/linux/systemd-logind.c
@@ -37,6 +37,7 @@
#include "linux.h"
#include "xf86.h"
#include "xf86platformBus.h"
+#include "xf86Priv.h"
#include "xf86Xinput.h"
#include "globals.h"

@@ -242,8 +243,10 @@ systemd_logind_vtenter(void)
if (!info->active)
return; /* Session not active */

- if (info->vt_active)
+ if (info->vt_active) {
+ xf86Info.hasVT = TRUE;
return; /* Already did vtenter */
+ }

for (i = 0; i < xf86_num_platform_devices; i++) {
if (xf86_platform_devices[i].flags & XF86_PDEV_PAUSED)
--
2.16.4

_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/x
Adam Jackson
2018-10-12 14:18:11 UTC
Permalink
Post by Michal Srb
If the X server is terminated while its VT is not active, it should
not change the current VT.
---
Changing the VT in that situation serves no purpose and can be confusing.
For example when a user's graphical session is terminated while other
user is using the computer, it would switch the VT he is working on.
Conceptual ack. Would slightly prefer to see this done by
VT_GETSTATE/VT_GETACTIVE ioctl (depending which OS you're on). It
should be simpler, and also...
Post by Michal Srb
diff --git a/hw/xfree86/common/xf86Privstr.h b/hw/xfree86/common/xf86Privstr.h
index 55d1b2455..e4d827dee 100644
--- a/hw/xfree86/common/xf86Privstr.h
+++ b/hw/xfree86/common/xf86Privstr.h
@@ -61,6 +61,7 @@ typedef struct {
Bool dontVTSwitch;
Bool autoVTSwitch;
Bool ShareVTs;
+ Bool hasVT;
Bool dontZap;
Bool dontZoom;
... this struct is ABI (sigh) so the fix would be unsuitable for a
stable release.

- ajax

_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info
Michal Srb
2018-10-16 07:31:09 UTC
Permalink
Post by Adam Jackson
Post by Michal Srb
If the X server is terminated while its VT is not active, it should
not change the current VT.
---
Changing the VT in that situation serves no purpose and can be confusing.
For example when a user's graphical session is terminated while other
user is using the computer, it would switch the VT he is working on.
Conceptual ack. Would slightly prefer to see this done by
VT_GETSTATE/VT_GETACTIVE ioctl (depending which OS you're on). It
should be simpler, and also...
Ok, thank you for your response, that will indeed be simpler.

The VT_GETACTIVE seems to be on BSD while VT_GETSTATE is on Linux. The code
doing the switching back to original VT is only in Linux part, so only that
one needs to be modified.

I'll send version 2.

Michal


_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-deve
Michal Srb
2018-10-16 07:32:13 UTC
Permalink
If the X server is terminated while its VT is not active, it should
not change the current VT.

v2: Query current state in xf86CloseConsole using VT_GETSTATE instead of
keeping track in xf86VTEnter/xf86VTLeave/etc.
---
Changing the VT in that situation serves no purpose and can be confusing.
For example when a user's graphical session is terminated while other
user is using the computer, it would switch the VT he is working on.
hw/xfree86/os-support/linux/lnx_init.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/hw/xfree86/os-support/linux/lnx_init.c b/hw/xfree86/os-support/linux/lnx_init.c
index 039dc4a4d..358d89f0f 100644
--- a/hw/xfree86/os-support/linux/lnx_init.c
+++ b/hw/xfree86/os-support/linux/lnx_init.c
@@ -299,6 +299,7 @@ void
xf86CloseConsole(void)
{
struct vt_mode VT;
+ struct vt_stat vts;
int ret;

if (xf86Info.ShareVTs) {
@@ -336,10 +337,19 @@ xf86CloseConsole(void)

if (xf86Info.autoVTSwitch) {
/*
- * Perform a switch back to the active VT when we were started
- */
+ * Perform a switch back to the active VT when we were started if our
+ * vt is active now.
+ */
if (activeVT >= 0) {
- switch_to(activeVT, "xf86CloseConsole");
+ SYSCALL(ret = ioctl(xf86Info.consoleFd, VT_GETSTATE, &vts));
+ if (ret < 0) {
+ xf86Msg(X_WARNING, "xf86OpenConsole: VT_GETSTATE failed: %s\n",
+ strerror(errno));
+ } else {
+ if (vts.v_active == xf86Info.vtno) {
+ switch_to(activeVT, "xf86CloseConsole");
+ }
+ }
activeVT = -1;
}
}
--
2.16.4

_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-d
Loading...