Re: [PATCH] weston: Don't exit just because tty is in gfx mode already

[Pekka Paalanen]

On Fri, 20 May 2016 11:30:53 +0300
Pekka Paalanen  wrote:

> On Mon, 16 May 2016 15:25:52 -0700
> Florian Hänel  wrote:
> 
> > From: =?UTF-8?q?Florian=20H=C3=A4nel?= 
> > 
> > If weston crashed (or during development) it can leave the tty
> > in graphics mode, which would prevent it from ever coming up again.
> > 
> > Another compositor running should be handled by upstart/systemd
> > and the tty in graphics mode does not prevent us from using it.
> >   
> 
> Hi,
> 
> can you explain more how you figure that to work?
> 
> A supported use case is to start weston with weston-launch, and without
> any systemd, upstart, logind, or any other manager running. What else
> would be preventing Weston from launching in case a display server was
> already running and active? (*)
> 
> Setting DRM master failure probably comes too late as the display
> servers have already started fighting, and then Weston attempts to undo
> the VT setup while the other display server is still running. How could
> that not end up in an even bigger mess?
> 
> How is it even possible to leave the VT in graphics mode? If
> weston crashed, then weston-launch or logind (as launcher-logind.c does
> not seem to touch the VT) will restore the VT.
> 
> (PS. never do 'killall -9 weston', that's a pretty guaranteed way to
> make your VT unusable as it kills both weston and weston-launch before
> either can restore. Logind might save you, though, I suppose.)
> 
> > A informational log message for debugging purposes should be enough.
> > ---
> >   src/launcher-util.c | 1 -
> >   1 file changed, 1 deletion(-)
> > 
> > diff --git a/src/launcher-util.c b/src/launcher-util.c
> > index e89710b..a98ff74 100644
> > --- a/src/launcher-util.c
> > +++ b/src/launcher-util.c
> > @@ -319,7 +319,6 @@ setup_tty(struct weston_launcher *launcher, int tty)
> >   if (kd_mode != KD_TEXT) {
> >   weston_log("%s is already in graphics mode, "
> >  "is another display server running?\n", tty_device);
> > -goto err_close;
> >   }
> > 
> >   ioctl(launcher->tty, VT_ACTIVATE, minor(buf.st_rdev));  
> 
> I do not think this patch applies, there is no such code in
> launcher-util.c anymore.
> 
> Looks like this hunk is now in launcher-direct.c. Ok, so you are
> running weston as root, without weston-launch. Is there any reason you
> cannot use weston-launch? Having a separate process to restore the VT
> is a good idea, even if you run as root.
> 
> OTOH, seeing that this only applies to the case of running weston
> directly as root and without any session manager service or
> weston-launch, maybe it's ok to drop the sanity check and let the user
> shoot his foot if he wants to?
> 
> The very least this patch needs to be rebased for master, and explain
> more why this change is needed and what the effects are, also if there
> really is another display server already running.
> 
> (*) Btw. now that I look at weston-launch.c, it is not doing the kd_mode
> check at all, is it? Makes me wonder if it should - was it just
> forgotten?

Hi,

this issue has been re-raised in
https://gitlab.freedesktop.org/wayland/weston/issues/361
so I'll clean up Patchwork for this patch.


Thanka,
pq


pgprMVisZP9iU.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] weston: Don't exit just because tty is in gfx mode already

[Pekka Paalanen]

On Mon, 16 May 2016 15:25:52 -0700
Florian Hänel  wrote:

> From: =?UTF-8?q?Florian=20H=C3=A4nel?= 
> 
> If weston crashed (or during development) it can leave the tty
> in graphics mode, which would prevent it from ever coming up again.
> 
> Another compositor running should be handled by upstart/systemd
> and the tty in graphics mode does not prevent us from using it.
> 

Hi,

can you explain more how you figure that to work?

A supported use case is to start weston with weston-launch, and without
any systemd, upstart, logind, or any other manager running. What else
would be preventing Weston from launching in case a display server was
already running and active? (*)

Setting DRM master failure probably comes too late as the display
servers have already started fighting, and then Weston attempts to undo
the VT setup while the other display server is still running. How could
that not end up in an even bigger mess?

How is it even possible to leave the VT in graphics mode? If
weston crashed, then weston-launch or logind (as launcher-logind.c does
not seem to touch the VT) will restore the VT.

(PS. never do 'killall -9 weston', that's a pretty guaranteed way to
make your VT unusable as it kills both weston and weston-launch before
either can restore. Logind might save you, though, I suppose.)

> A informational log message for debugging purposes should be enough.
> ---
>   src/launcher-util.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/src/launcher-util.c b/src/launcher-util.c
> index e89710b..a98ff74 100644
> --- a/src/launcher-util.c
> +++ b/src/launcher-util.c
> @@ -319,7 +319,6 @@ setup_tty(struct weston_launcher *launcher, int tty)
>   if (kd_mode != KD_TEXT) {
>   weston_log("%s is already in graphics mode, "
>  "is another display server running?\n", tty_device);
> -goto err_close;
>   }
> 
>   ioctl(launcher->tty, VT_ACTIVATE, minor(buf.st_rdev));

I do not think this patch applies, there is no such code in
launcher-util.c anymore.

Looks like this hunk is now in launcher-direct.c. Ok, so you are
running weston as root, without weston-launch. Is there any reason you
cannot use weston-launch? Having a separate process to restore the VT
is a good idea, even if you run as root.

OTOH, seeing that this only applies to the case of running weston
directly as root and without any session manager service or
weston-launch, maybe it's ok to drop the sanity check and let the user
shoot his foot if he wants to?

The very least this patch needs to be rebased for master, and explain
more why this change is needed and what the effects are, also if there
really is another display server already running.

(*) Btw. now that I look at weston-launch.c, it is not doing the kd_mode
check at all, is it? Makes me wonder if it should - was it just
forgotten?


Thanks,
pq


pgp3nTE5iL6kc.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] weston: Don't exit just because tty is in gfx mode already

[Yong Bakos]

On May 16, 2016, at 5:25 PM, Florian Hänel  wrote:
> 
> From: =?UTF-8?q?Florian=20H=C3=A4nel?= 
> 
> If weston crashed (or during development) it can leave the tty
> in graphics mode, which would prevent it from ever coming up again.
> 
> Another compositor running should be handled by upstart/systemd
> and the tty in graphics mode does not prevent us from using it.
> 
> A informational log message for debugging purposes should be enough.

Hi Florian,
Some quick feedback about the sending of the patch. See:
https://cgit.freedesktop.org/wayland/wayland/tree/doc/Contributing

Ideally, the subject line should be:
[PATCH weston] launcher-util: Don't...

And the commit message should include an "Sob" line (use the -s option with
git commit). And, are you using a non-default encoding config with git
send-email? I'm noticing that your name isn't being formatted correctly.

Regards,
yong


> ---
> src/launcher-util.c | 1 -
> 1 file changed, 1 deletion(-)
> 
> diff --git a/src/launcher-util.c b/src/launcher-util.c
> index e89710b..a98ff74 100644
> --- a/src/launcher-util.c
> +++ b/src/launcher-util.c
> @@ -319,7 +319,6 @@ setup_tty(struct weston_launcher *launcher, int tty)
> if (kd_mode != KD_TEXT) {
> weston_log("%s is already in graphics mode, "
>"is another display server running?\n", tty_device);
> -goto err_close;
> }
> 
> ioctl(launcher->tty, VT_ACTIVATE, minor(buf.st_rdev));
> -- 
> 2.7.4
> 
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH] weston: Don't exit just because tty is in gfx mode already

[Florian Hänel]

From: =?UTF-8?q?Florian=20H=C3=A4nel?= 

If weston crashed (or during development) it can leave the tty
in graphics mode, which would prevent it from ever coming up again.

Another compositor running should be handled by upstart/systemd
and the tty in graphics mode does not prevent us from using it.

A informational log message for debugging purposes should be enough.
---
 src/launcher-util.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/launcher-util.c b/src/launcher-util.c
index e89710b..a98ff74 100644
--- a/src/launcher-util.c
+++ b/src/launcher-util.c
@@ -319,7 +319,6 @@ setup_tty(struct weston_launcher *launcher, int tty)
 if (kd_mode != KD_TEXT) {
 weston_log("%s is already in graphics mode, "
"is another display server running?\n", tty_device);
-goto err_close;
 }

 ioctl(launcher->tty, VT_ACTIVATE, minor(buf.st_rdev));
--
2.7.4

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH] weston: Don't exit just because tty is in gfx mode already

[Florian Hänel]

From: =?UTF-8?q?Florian=20H=C3=A4nel?= 

If weston crashed (or during development) it can leave the tty
in graphics mode, which would prevent it from ever coming up again.

Another compositor running should be handled by upstart/systemd
and the tty in graphics mode does not prevent us from using it.

A informational log message for debugging purposes should be enough.
---
 src/launcher-util.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/launcher-util.c b/src/launcher-util.c
index e89710b..a98ff74 100644
--- a/src/launcher-util.c
+++ b/src/launcher-util.c
@@ -319,7 +319,6 @@ setup_tty(struct weston_launcher *launcher, int tty)
 if (kd_mode != KD_TEXT) {
 weston_log("%s is already in graphics mode, "
"is another display server running?\n", tty_device);
-goto err_close;
 }

 ioctl(launcher->tty, VT_ACTIVATE, minor(buf.st_rdev));
--
2.7.4

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel