Re: [systemd-devel] [PATCH] login: Don't stop a running user manager from garbage-collecting the user.

2013-12-15 Thread Thomas Bächler
Am 12.12.2013 15:38, schrieb Lennart Poettering:
> On Wed, 11.12.13 19:56, Thomas Bächler (tho...@archlinux.org) wrote:
> 
>> With the current logic, a user will never be garbage-collected, since its
>> manager will always be around. Change the logic such that a user is
>> garbage-collected when it has no sessions and linger is disabled.
>> ---
>>  src/login/logind-user.c | 12 
>>  1 file changed, 12 deletions(-)
>>
>> diff --git a/src/login/logind-user.c b/src/login/logind-user.c
>> index 6ba8d98..42a7524 100644
>> --- a/src/login/logind-user.c
>> +++ b/src/login/logind-user.c
>> @@ -612,18 +612,6 @@ bool user_check_gc(User *u, bool drop_not_started) {
>>  if (user_check_linger_file(u) > 0)
>>  return true;
>>  
>> -if (u->slice_job && manager_job_is_active(u->manager, u->slice_job))
>> -return true;
>> -
>> -if (u->service_job && manager_job_is_active(u->manager,
>> -u->service_job))
>> -return true;
> 
> Hmm, we probably should stay around as long as the jobs are still
> active. 
> 
>> -
>> -if (u->slice && manager_unit_is_active(u->manager, u->slice) != 0)
>> -return true;
>> -
>> -if (u->service && manager_unit_is_active(u->manager, u->service) != 
>> 0)
>> -return true;
>> -
> 
> THis part should indeed go, yo are right.
> 
> Can you check whether things work correctly for you if you only remove
> the latter two parts, but leave the former two in? I'lll merge the patch then.

Okay, I tested a bit further: I added 'ExecStartPre=/usr/bin/sleep 10'
to user@.service and tested with both versions of the patch. My version
indeed causes problems, but the race condition I suspected does not
occur: the user manager is started and immediately stopped in case I log
out while the service is still starting up.

Sending v2 in a minute.




signature.asc
Description: OpenPGP digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] login: Don't stop a running user manager from garbage-collecting the user.

2013-12-12 Thread Thomas Bächler
Am 12.12.2013 15:38, schrieb Lennart Poettering:
> On Wed, 11.12.13 19:56, Thomas Bächler (tho...@archlinux.org) wrote:
> 
>> With the current logic, a user will never be garbage-collected, since its
>> manager will always be around. Change the logic such that a user is
>> garbage-collected when it has no sessions and linger is disabled.
>> ---
>>  src/login/logind-user.c | 12 
>>  1 file changed, 12 deletions(-)
>>
>> diff --git a/src/login/logind-user.c b/src/login/logind-user.c
>> index 6ba8d98..42a7524 100644
>> --- a/src/login/logind-user.c
>> +++ b/src/login/logind-user.c
>> @@ -612,18 +612,6 @@ bool user_check_gc(User *u, bool drop_not_started) {
>>  if (user_check_linger_file(u) > 0)
>>  return true;
>>  
>> -if (u->slice_job && manager_job_is_active(u->manager, u->slice_job))
>> -return true;
>> -
>> -if (u->service_job && manager_job_is_active(u->manager,
>> -u->service_job))
>> -return true;
> 
> Hmm, we probably should stay around as long as the jobs are still
> active. 
> 
>> -
>> -if (u->slice && manager_unit_is_active(u->manager, u->slice) != 0)
>> -return true;
>> -
>> -if (u->service && manager_unit_is_active(u->manager, u->service) != 
>> 0)
>> -return true;
>> -
> 
> THis part should indeed go, yo are right.
> 
> Can you check whether things work correctly for you if you only remove
> the latter two parts, but leave the former two in? I'lll merge the patch then.

That's rather hard to test. I'd have to terminate the session while the
service or slice job is still running and see what happens.

My guess is that omitting the first two checks will work most of the
time, but fail under rare circumstance: You terminate the last session
while the start job is still active, omit gc'ing the user and never add
it to the gc list again.

On the other hand I think that removing those lines is fine: Stopping
the user will queue a stob job for the service and the slice. The
corresponding start job may still be running at that time, but the
system manager should be capable of dealing with such issues (it's the
same issues that arises when running systemctl stop before a systemctl
start finished).




signature.asc
Description: OpenPGP digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] login: Don't stop a running user manager from garbage-collecting the user.

2013-12-12 Thread Lennart Poettering
On Wed, 11.12.13 19:56, Thomas Bächler (tho...@archlinux.org) wrote:

> With the current logic, a user will never be garbage-collected, since its
> manager will always be around. Change the logic such that a user is
> garbage-collected when it has no sessions and linger is disabled.
> ---
>  src/login/logind-user.c | 12 
>  1 file changed, 12 deletions(-)
> 
> diff --git a/src/login/logind-user.c b/src/login/logind-user.c
> index 6ba8d98..42a7524 100644
> --- a/src/login/logind-user.c
> +++ b/src/login/logind-user.c
> @@ -612,18 +612,6 @@ bool user_check_gc(User *u, bool drop_not_started) {
>  if (user_check_linger_file(u) > 0)
>  return true;
>  
> -if (u->slice_job && manager_job_is_active(u->manager, u->slice_job))
> -return true;
> -
> -if (u->service_job && manager_job_is_active(u->manager,
> -u->service_job))
> -return true;

Hmm, we probably should stay around as long as the jobs are still
active. 

> -
> -if (u->slice && manager_unit_is_active(u->manager, u->slice) != 0)
> -return true;
> -
> -if (u->service && manager_unit_is_active(u->manager, u->service) != 
> 0)
> -return true;
> -

THis part should indeed go, yo are right.

Can you check whether things work correctly for you if you only remove
the latter two parts, but leave the former two in? I'lll merge the patch then.

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] login: Don't stop a running user manager from garbage-collecting the user.

2013-12-11 Thread Thomas Bächler
Am 11.12.2013 19:56, schrieb Thomas Bächler:
> With the current logic, a user will never be garbage-collected, since its
> manager will always be around. Change the logic such that a user is
> garbage-collected when it has no sessions and linger is disabled.

This seems to fix my current problem.

However, if I have two sessions and close one, both the dbus properties
of the user/seat as well as the /run/systemd/users/$UID file still list
the closed session until another session for the user closes or opens.
This seems to be a separate problem, but still a flaw in the logic
somewhere.




signature.asc
Description: OpenPGP digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH] login: Don't stop a running user manager from garbage-collecting the user.

2013-12-11 Thread Thomas Bächler
With the current logic, a user will never be garbage-collected, since its
manager will always be around. Change the logic such that a user is
garbage-collected when it has no sessions and linger is disabled.
---
 src/login/logind-user.c | 12 
 1 file changed, 12 deletions(-)

diff --git a/src/login/logind-user.c b/src/login/logind-user.c
index 6ba8d98..42a7524 100644
--- a/src/login/logind-user.c
+++ b/src/login/logind-user.c
@@ -612,18 +612,6 @@ bool user_check_gc(User *u, bool drop_not_started) {
 if (user_check_linger_file(u) > 0)
 return true;
 
-if (u->slice_job && manager_job_is_active(u->manager, u->slice_job))
-return true;
-
-if (u->service_job && manager_job_is_active(u->manager, 
u->service_job))
-return true;
-
-if (u->slice && manager_unit_is_active(u->manager, u->slice) != 0)
-return true;
-
-if (u->service && manager_unit_is_active(u->manager, u->service) != 0)
-return true;
-
 return false;
 }
 
-- 
1.8.5.1

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