Re: [systemd-devel] [PATCH] unit: Set KillMode=mixed for services which use sulogin

2015-05-13 Thread Lennart Poettering
On Tue, 12.05.15 14:36, Michael Biebl (mbi...@gmail.com) wrote:

 2015-05-12 0:03 GMT+02:00 Lennart Poettering lenn...@poettering.net:
  On Tue, 12.05.15 00:01, Lennart Poettering (lenn...@poettering.net) wrote:
 
  On Mon, 11.05.15 23:50, Michael Biebl (mbi...@gmail.com) wrote:
 
   2015-05-08 17:43 GMT+02:00 Michael Biebl mbi...@gmail.com:
See 
http://lists.freedesktop.org/archives/systemd-devel/2015-May/031536.html
  
   Lennart, are you ok if I commit this?
 
  Yes, please!
 
  But actually, I agree with Zbigniew, can you please replace all
  occurences of Killmode=process with KillMode=mixed, with the exception
  of debug-shell.service. Also, if you'd add a comment there explaining
  that that file sticks to KillMode=process to not affect backgrounded
  debugging process, things would even be better!
 
 Hm, I just tested KillMode=mixed with getty@.service on installations
 which didn't have libpam-systemd enabled.

Ah, humm. 

So, we don't really care about installations without pam_systemd
upstream regarding this. However, logind is actually responsible for
killing user process on logout if that's configured via logind.conf.

sulogin generally does not set up a PAM session, and we indeed should
allow processes like screen staying around in such a context. Hence
KillMode=process is actually the right choice for all these services,
indeed.

Hence I figure the status quo for all of this is pretty OK already...

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] unit: Set KillMode=mixed for services which use sulogin

2015-05-13 Thread Lennart Poettering
On Wed, 13.05.15 15:32, Michael Biebl (mbi...@gmail.com) wrote:

 2015-05-13 15:19 GMT+02:00 Lennart Poettering lenn...@poettering.net:
  sulogin generally does not set up a PAM session, and we indeed should
  allow processes like screen staying around in such a context. Hence
  KillMode=process is actually the right choice for all these services,
  indeed.
 
 Do you really think it makes sense to start screen from
 emergency/rescue mode?

No I don't. But I think we shouldn't try to enforce any policy on
process lifetime in debug/emergency/rescue mode... They are supposed
to be low-level recovery features, that give you raw, naked, rough
access to the system guts, really. And hence we probably shouldn't
kill what they leave around...

I mean, if admins do something like this:

 ( while : ; do ps xawuf  /tmp/ps-log ; sleep 10 ; done )  disown

in the debug shell, to debug something, then we should not break that
at logout, really. 

 Imho those are the cases where you don't actually want stuff to stay
 around after you log out.
 
  Hence I figure the status quo for all of this is pretty OK already...
 
 Well, I was intending to commit my original patch, which only uses
 KillMode=mixed for services which use sulogin, i.e.
 emergency.service, rescue.service and console-shell.service.
 
 See the original bug that triggered this patch [1]. We don't really
 want a stray bash process to stay around which potentially fights with
 sulogin over the input.

Well, I am pretty sure that in this case, it should be sulogin that
propagates the shutdown request to the shell it spawned, but we should
not do it otherwise.

Note that by default we don't even clean up processes of unprivileged
users on logout. You have to turn this on via KillUserProcesses=
explicitly. And if we don't do this for unprivileged users, we
certainly shouldn't do it for debug shells either

 [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=784238

That bug reports is long... From what I got this really looks like
something to fix in Debian's sulogin implementation really.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] unit: Set KillMode=mixed for services which use sulogin

2015-05-13 Thread Michael Biebl
2015-05-13 15:19 GMT+02:00 Lennart Poettering lenn...@poettering.net:
 sulogin generally does not set up a PAM session, and we indeed should
 allow processes like screen staying around in such a context. Hence
 KillMode=process is actually the right choice for all these services,
 indeed.

Do you really think it makes sense to start screen from emergency/rescue mode?
Imho those are the cases where you don't actually want stuff to stay
around after you log out.

 Hence I figure the status quo for all of this is pretty OK already...

Well, I was intending to commit my original patch, which only uses
KillMode=mixed for services which use sulogin, i.e.
emergency.service, rescue.service and console-shell.service.

See the original bug that triggered this patch [1]. We don't really
want a stray bash process to stay around which potentially fights with
sulogin over the input.

Michael

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=784238

-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] unit: Set KillMode=mixed for services which use sulogin

2015-05-13 Thread Michael Biebl
2015-05-13 15:45 GMT+02:00 Lennart Poettering lenn...@poettering.net:
 Well, I am pretty sure that in this case, it should be sulogin that
 propagates the shutdown request to the shell it spawned, but we should
 not do it otherwise.

 Note that by default we don't even clean up processes of unprivileged
 users on logout. You have to turn this on via KillUserProcesses=
 explicitly. And if we don't do this for unprivileged users, we
 certainly shouldn't do it for debug shells either

Well, it's not the debug shell, it's emergency/rescue mode.
You typically end up there, because you have a fatal (config) error.
You then fix the problem and exit the shell to boot into the regular mode.
I don't see the point of letting stuff from emergency/rescue mode
continue to run after you've stopped it.

 [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=784238

 That bug reports is long... From what I got this really looks like
 something to fix in Debian's sulogin implementation really.

You are correct. This turned out to be a bug in Debian's sulogin
implementation (which is currently provided by sysvinit-utils). I
filed bugs to have util-linux provide sulogin [1], since I wasn't able
to reproduce the problem with that implementation.

Still, while handling this bug report, I couldn't think of a good
reason to not use KillMode=mixed for emergency/rescue mode, that's why
I proposed this patch.

I see that you don't like it, so I'll probably just ship it downstream
for now. And drop it eventually, once we have u-l's sulogin as
default.

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=784566#77
-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] unit: Set KillMode=mixed for services which use sulogin

2015-05-13 Thread Lennart Poettering
On Wed, 13.05.15 16:18, Michael Biebl (mbi...@gmail.com) wrote:

 2015-05-13 15:45 GMT+02:00 Lennart Poettering lenn...@poettering.net:
  Well, I am pretty sure that in this case, it should be sulogin that
  propagates the shutdown request to the shell it spawned, but we should
  not do it otherwise.
 
  Note that by default we don't even clean up processes of unprivileged
  users on logout. You have to turn this on via KillUserProcesses=
  explicitly. And if we don't do this for unprivileged users, we
  certainly shouldn't do it for debug shells either
 
 Well, it's not the debug shell, it's emergency/rescue mode.
 You typically end up there, because you have a fatal (config) error.

Well, if you in either mode, you ran into problems, and you have to
debug them now. I am pretty sure we should make more people unhappy if
we killed everything they forked off from these recovery shells, then
we'd make happy by cleaning up after them...

KillMode=mixed appears OK as a temporary fix to work-around a broken
sulogin version, but I dont think it makes a good default otherwise.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] unit: Set KillMode=mixed for services which use sulogin

2015-05-12 Thread Michael Biebl
2015-05-12 0:03 GMT+02:00 Lennart Poettering lenn...@poettering.net:
 On Tue, 12.05.15 00:01, Lennart Poettering (lenn...@poettering.net) wrote:

 On Mon, 11.05.15 23:50, Michael Biebl (mbi...@gmail.com) wrote:

  2015-05-08 17:43 GMT+02:00 Michael Biebl mbi...@gmail.com:
   See 
   http://lists.freedesktop.org/archives/systemd-devel/2015-May/031536.html
 
  Lennart, are you ok if I commit this?

 Yes, please!

 But actually, I agree with Zbigniew, can you please replace all
 occurences of Killmode=process with KillMode=mixed, with the exception
 of debug-shell.service. Also, if you'd add a comment there explaining
 that that file sticks to KillMode=process to not affect backgrounded
 debugging process, things would even be better!

Hm, I just tested KillMode=mixed with getty@.service on installations
which didn't have libpam-systemd enabled.
This broke the usage of screen, since it killed the screen sessions.
I don't think we should change the behaviour as users might rely on that.

For sulogin using services, like emergency.service, I guess one can
argue, that screen is not something you typically use here.

Michael


-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] unit: Set KillMode=mixed for services which use sulogin

2015-05-11 Thread Michael Biebl
2015-05-08 17:43 GMT+02:00 Michael Biebl mbi...@gmail.com:
 See http://lists.freedesktop.org/archives/systemd-devel/2015-May/031536.html

Lennart, are you ok if I commit this?

-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] unit: Set KillMode=mixed for services which use sulogin

2015-05-11 Thread Lennart Poettering
On Mon, 11.05.15 23:50, Michael Biebl (mbi...@gmail.com) wrote:

 2015-05-08 17:43 GMT+02:00 Michael Biebl mbi...@gmail.com:
  See http://lists.freedesktop.org/archives/systemd-devel/2015-May/031536.html
 
 Lennart, are you ok if I commit this?

Yes, please!

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] unit: Set KillMode=mixed for services which use sulogin

2015-05-11 Thread Lennart Poettering
On Tue, 12.05.15 00:01, Lennart Poettering (lenn...@poettering.net) wrote:

 On Mon, 11.05.15 23:50, Michael Biebl (mbi...@gmail.com) wrote:
 
  2015-05-08 17:43 GMT+02:00 Michael Biebl mbi...@gmail.com:
   See 
   http://lists.freedesktop.org/archives/systemd-devel/2015-May/031536.html
  
  Lennart, are you ok if I commit this?
 
 Yes, please!

But actually, I agree with Zbigniew, can you please replace all
occurences of Killmode=process with KillMode=mixed, with the exception
of debug-shell.service. Also, if you'd add a comment there explaining
that that file sticks to KillMode=process to not affect backgrounded
debugging process, things would even be better!

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] unit: Set KillMode=mixed for services which use sulogin

2015-05-08 Thread David Herrmann
Hi

On Fri, May 8, 2015 at 7:59 PM, Michael Biebl mbi...@gmail.com wrote:
 2015-05-08 17:43 GMT+02:00 Michael Biebl mbi...@gmail.com:
 See http://lists.freedesktop.org/archives/systemd-devel/2015-May/031536.html

 We probably want to use KillMode=mixed, but let's start with the ones

 We probably want to use KillMode=mixed *as well for other units which
 spawn of child processes*, but ...

..and it should have been the default right from the beginning. I
think there're even plans to switch, but.. backwards compat..

Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] unit: Set KillMode=mixed for services which use sulogin

2015-05-08 Thread Michael Biebl
2015-05-08 17:43 GMT+02:00 Michael Biebl mbi...@gmail.com:
 See http://lists.freedesktop.org/archives/systemd-devel/2015-May/031536.html

 We probably want to use KillMode=mixed, but let's start with the ones

We probably want to use KillMode=mixed *as well for other units which
spawn of child processes*, but ...



-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel