From f9e4283df30ad8916878396da449b2e38656b6f7 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Mon, 28 Sep 2015 12:53:42 +0200 Subject: login: simply XDG_RUNTIME_DIR management Lets not pretend we support changing XDG_RUNTIME_DIR via logind state files. There is no reason to ever write the string into /run, as we allocate it statically based on the UID, anyway. Lets stop that and just allocate the runtime_path in "struct User" at all times. We keep writing it into the /run state to make sure pam_systemd of previous installs can still read it. However, pam_systemd is now fixed to allocate it statically as well, so we can safely remove that some time in the future. Last but not least: If software depends on systemd, they're more than free to assume /run/user/$uid is their runtime dir. Lets not require sane applications to query the environment to get their runtime dir. As long as applications know their login-UID, they should be safe to deduce the runtime dir. --- src/login/logind-user.c | 42 ++++++++++++++---------------------------- src/login/pam_systemd.c | 28 ++++++++++------------------ 2 files changed, 24 insertions(+), 46 deletions(-) diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 7bdbe6583c..92f3b75a85 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -68,6 +68,9 @@ User* user_new(Manager *m, uid_t uid, gid_t gid, const char *name) { if (asprintf(&u->state_file, "/run/systemd/users/"UID_FMT, uid) < 0) goto fail; + if (asprintf(&u->runtime_path, "/run/user/"UID_FMT, uid) < 0) + goto fail; + if (hashmap_put(m->users, UID_TO_PTR(uid), u) < 0) goto fail; @@ -78,6 +81,7 @@ User* user_new(Manager *m, uid_t uid, gid_t gid, const char *name) { return u; fail: + free(u->runtime_path); free(u->state_file); free(u->name); free(u); @@ -141,6 +145,7 @@ static int user_save_internal(User *u) { u->name, user_state_to_string(user_get_state(u))); + /* LEGACY: no-one reads RUNTIME= anymore, drop it at some point */ if (u->runtime_path) fprintf(f, "RUNTIME=%s\n", u->runtime_path); @@ -288,7 +293,6 @@ int user_load(User *u) { assert(u); r = parse_env_file(u->state_file, NEWLINE, - "RUNTIME", &u->runtime_path, "SERVICE", &u->service, "SERVICE_JOB", &u->service_job, "SLICE", &u->slice, @@ -327,7 +331,6 @@ int user_load(User *u) { } static int user_mkdir_runtime_path(User *u) { - char *p; int r; assert(u); @@ -336,16 +339,10 @@ static int user_mkdir_runtime_path(User *u) { if (r < 0) return log_error_errno(r, "Failed to create /run/user: %m"); - if (!u->runtime_path) { - if (asprintf(&p, "/run/user/" UID_FMT, u->uid) < 0) - return log_oom(); - } else - p = u->runtime_path; - - if (path_is_mount_point(p, 0) <= 0) { + if (path_is_mount_point(u->runtime_path, 0) <= 0) { _cleanup_free_ char *t = NULL; - (void) mkdir_label(p, 0700); + (void) mkdir_label(u->runtime_path, 0700); if (mac_smack_use()) r = asprintf(&t, "mode=0700,smackfsroot=*,uid=" UID_FMT ",gid=" GID_FMT ",size=%zu", u->uid, u->gid, u->manager->runtime_dir_size); @@ -356,10 +353,10 @@ static int user_mkdir_runtime_path(User *u) { goto fail; } - r = mount("tmpfs", p, "tmpfs", MS_NODEV|MS_NOSUID, t); + r = mount("tmpfs", u->runtime_path, "tmpfs", MS_NODEV|MS_NOSUID, t); if (r < 0) { if (errno != EPERM) { - r = log_error_errno(errno, "Failed to mount per-user tmpfs directory %s: %m", p); + r = log_error_errno(errno, "Failed to mount per-user tmpfs directory %s: %m", u->runtime_path); goto fail; } @@ -367,29 +364,23 @@ static int user_mkdir_runtime_path(User *u) { * CAP_SYS_ADMIN-less container? In this case, * just use a normal directory. */ - r = chmod_and_chown(p, 0700, u->uid, u->gid); + r = chmod_and_chown(u->runtime_path, 0700, u->uid, u->gid); if (r < 0) { log_error_errno(r, "Failed to change runtime directory ownership and mode: %m"); goto fail; } } - r = label_fix(p, false, false); + r = label_fix(u->runtime_path, false, false); if (r < 0) - log_warning_errno(r, "Failed to fix label of '%s', ignoring: %m", p); + log_warning_errno(r, "Failed to fix label of '%s', ignoring: %m", u->runtime_path); } - u->runtime_path = p; return 0; fail: - if (p) { - /* Try to clean up, but ignore errors */ - (void) rmdir(p); - free(p); - } - - u->runtime_path = NULL; + /* Try to clean up, but ignore errors */ + (void) rmdir(u->runtime_path); return r; } @@ -574,9 +565,6 @@ static int user_remove_runtime_path(User *u) { assert(u); - if (!u->runtime_path) - return 0; - r = rm_rf(u->runtime_path, 0); if (r < 0) log_error_errno(r, "Failed to remove runtime directory %s: %m", u->runtime_path); @@ -592,8 +580,6 @@ static int user_remove_runtime_path(User *u) { if (r < 0) log_error_errno(r, "Failed to remove runtime directory %s: %m", u->runtime_path); - u->runtime_path = mfree(u->runtime_path); - return r; } diff --git a/src/login/pam_systemd.c b/src/login/pam_systemd.c index 0d61f528db..3f7cbb78df 100644 --- a/src/login/pam_systemd.c +++ b/src/login/pam_systemd.c @@ -267,29 +267,21 @@ _public_ PAM_EXTERN int pam_sm_open_session( pam_get_item(handle, PAM_SERVICE, (const void**) &service); if (streq_ptr(service, "systemd-user")) { - _cleanup_free_ char *p = NULL, *rt = NULL; + _cleanup_free_ char *rt = NULL; - if (asprintf(&p, "/run/systemd/users/"UID_FMT, pw->pw_uid) < 0) + if (asprintf(&rt, "/run/user/"UID_FMT, pw->pw_uid) < 0) return PAM_BUF_ERR; - r = parse_env_file(p, NEWLINE, - "RUNTIME", &rt, - NULL); - if (r < 0 && r != -ENOENT) - return PAM_SESSION_ERR; - - if (rt) { - r = pam_misc_setenv(handle, "XDG_RUNTIME_DIR", rt, 0); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to set runtime dir."); - return r; - } - - r = export_legacy_dbus_address(handle, pw->pw_uid, rt); - if (r != PAM_SUCCESS) - return r; + r = pam_misc_setenv(handle, "XDG_RUNTIME_DIR", rt, 0); + if (r != PAM_SUCCESS) { + pam_syslog(handle, LOG_ERR, "Failed to set runtime dir."); + return r; } + r = export_legacy_dbus_address(handle, pw->pw_uid, rt); + if (r != PAM_SUCCESS) + return r; + return PAM_SUCCESS; } -- cgit v1.2.3-54-g00ecf From 6230bf750a4c41ff9a7ec291243fc92b059e896b Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Tue, 29 Sep 2015 11:03:04 +0200 Subject: login: keep user->slice constant Currently, we allocate user->slice when starting a slice, but we never release it. This is incompatible if we want to re-use a user object once it was stopped. Hence, make sure user->slice is allocated statically on the user object and use "u->started || u->stopping" as an indication whether the slice is actually available on pid1 or not. --- src/login/logind-user.c | 95 ++++++++++++++++++++----------------------------- 1 file changed, 39 insertions(+), 56 deletions(-) diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 92f3b75a85..d985d19c46 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -52,7 +52,9 @@ #include "util.h" User* user_new(Manager *m, uid_t uid, gid_t gid, const char *name) { + char lu[DECIMAL_STR_MAX(uid_t) + 1]; User *u; + int r; assert(m); assert(name); @@ -71,9 +73,17 @@ User* user_new(Manager *m, uid_t uid, gid_t gid, const char *name) { if (asprintf(&u->runtime_path, "/run/user/"UID_FMT, uid) < 0) goto fail; + sprintf(lu, UID_FMT, uid); + r = slice_build_subslice(SPECIAL_USER_SLICE, lu, &u->slice); + if (r < 0) + goto fail; + if (hashmap_put(m->users, UID_TO_PTR(uid), u) < 0) goto fail; + if (hashmap_put(m->user_units, u->slice, u) < 0) + goto fail; + u->manager = m; u->uid = uid; u->gid = gid; @@ -81,6 +91,10 @@ User* user_new(Manager *m, uid_t uid, gid_t gid, const char *name) { return u; fail: + if (u->slice) + hashmap_remove(m->user_units, u->slice); + hashmap_remove(m->users, UID_TO_PTR(uid)); + free(u->slice); free(u->runtime_path); free(u->state_file); free(u->name); @@ -98,23 +112,20 @@ void user_free(User *u) { while (u->sessions) session_free(u->sessions); - if (u->slice) { - hashmap_remove(u->manager->user_units, u->slice); - free(u->slice); - } - if (u->service) { hashmap_remove(u->manager->user_units, u->service); free(u->service); } + hashmap_remove(u->manager->user_units, u->slice); + hashmap_remove(u->manager->users, UID_TO_PTR(u->uid)); + free(u->slice_job); free(u->service_job); + free(u->slice); free(u->runtime_path); - hashmap_remove(u->manager->users, UID_TO_PTR(u->uid)); - free(u->name); free(u->state_file); free(u); @@ -154,8 +165,6 @@ static int user_save_internal(User *u) { if (u->service_job) fprintf(f, "SERVICE_JOB=%s\n", u->service_job); - if (u->slice) - fprintf(f, "SLICE=%s\n", u->slice); if (u->slice_job) fprintf(f, "SLICE_JOB=%s\n", u->slice_job); @@ -295,7 +304,6 @@ int user_load(User *u) { r = parse_env_file(u->state_file, NEWLINE, "SERVICE", &u->service, "SERVICE_JOB", &u->service_job, - "SLICE", &u->slice, "SLICE_JOB", &u->slice_job, "DISPLAY", &display, "REALTIME", &realtime, @@ -385,52 +393,33 @@ fail: } static int user_start_slice(User *u) { + _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL; + const char *description; + char *job; int r; assert(u); - if (!u->slice) { - _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL; - char lu[DECIMAL_STR_MAX(uid_t) + 1], *slice, *job; - const char *description; - - u->slice_job = mfree(u->slice_job); - - xsprintf(lu, UID_FMT, u->uid); - r = slice_build_subslice(SPECIAL_USER_SLICE, lu, &slice); - if (r < 0) - return log_error_errno(r, "Failed to build slice name: %m"); - - description = strjoina("User Slice of ", u->name); - - r = manager_start_slice( - u->manager, - slice, - description, - "systemd-logind.service", - "systemd-user-sessions.service", - u->manager->user_tasks_max, - &error, - &job); - if (r < 0) { - - if (sd_bus_error_has_name(&error, BUS_ERROR_UNIT_EXISTS)) - /* The slice already exists? If so, that's fine, let's just reuse it */ - u->slice = slice; - else { - log_error_errno(r, "Failed to start user slice %s, ignoring: %s (%s)", slice, bus_error_message(&error, r), error.name); - free(slice); - /* we don't fail due to this, let's try to continue */ - } - } else { - u->slice = slice; - u->slice_job = job; - } + u->slice_job = mfree(u->slice_job); + description = strjoina("User Slice of ", u->name); + + r = manager_start_slice( + u->manager, + u->slice, + description, + "systemd-logind.service", + "systemd-user-sessions.service", + u->manager->user_tasks_max, + &error, + &job); + if (r < 0) { + /* we don't fail due to this, let's try to continue */ + if (!sd_bus_error_has_name(&error, BUS_ERROR_UNIT_EXISTS)) + log_error_errno(r, "Failed to start user slice %s, ignoring: %s (%s)", u->slice, bus_error_message(&error, r), error.name); + } else { + u->slice_job = job; } - if (u->slice) - (void) hashmap_put(u->manager->user_units, u->slice, u); - return 0; } @@ -523,9 +512,6 @@ static int user_stop_slice(User *u) { assert(u); - if (!u->slice) - return 0; - r = manager_stop_unit(u->manager, u->slice, &error, &job); if (r < 0) { log_error("Failed to stop user slice: %s", bus_error_message(&error, r)); @@ -771,9 +757,6 @@ UserState user_get_state(User *u) { int user_kill(User *u, int signo) { assert(u); - if (!u->slice) - return -ESRCH; - return manager_kill_unit(u->manager, u->slice, KILL_ALL, signo, NULL); } -- cgit v1.2.3-54-g00ecf From 157f50577fbee094eb8ca18f3f0af4e82af8558f Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Tue, 29 Sep 2015 11:10:01 +0200 Subject: login: make user_new() and user_free() follow coding-style Few changes to user_new() and user_free(): - Use _cleanup_(user_freep) in constructor - return 'int' from user_new() - make user_free() deal with partially initialized objects - keep reverse-order in user_free() compared to user_new() - make user_free() return NULL - make user_free() accept NULL as no-op --- src/login/logind-core.c | 7 +++-- src/login/logind-user.c | 83 +++++++++++++++++++++++-------------------------- src/login/logind-user.h | 7 +++-- 3 files changed, 48 insertions(+), 49 deletions(-) diff --git a/src/login/logind-core.c b/src/login/logind-core.c index b3f30c8dc9..38c426c1aa 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -98,15 +98,16 @@ int manager_add_session(Manager *m, const char *id, Session **_session) { int manager_add_user(Manager *m, uid_t uid, gid_t gid, const char *name, User **_user) { User *u; + int r; assert(m); assert(name); u = hashmap_get(m->users, UID_TO_PTR(uid)); if (!u) { - u = user_new(m, uid, gid, name); - if (!u) - return -ENOMEM; + r = user_new(&u, m, uid, gid, name); + if (r < 0) + return r; } if (_user) diff --git a/src/login/logind-user.c b/src/login/logind-user.c index d985d19c46..292a583103 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -51,60 +51,54 @@ #include "user-util.h" #include "util.h" -User* user_new(Manager *m, uid_t uid, gid_t gid, const char *name) { +int user_new(User **out, Manager *m, uid_t uid, gid_t gid, const char *name) { + _cleanup_(user_freep) User *u = NULL; char lu[DECIMAL_STR_MAX(uid_t) + 1]; - User *u; int r; + assert(out); assert(m); assert(name); u = new0(User, 1); if (!u) - return NULL; + return -ENOMEM; + + u->manager = m; + u->uid = uid; + u->gid = gid; u->name = strdup(name); if (!u->name) - goto fail; + return -ENOMEM; if (asprintf(&u->state_file, "/run/systemd/users/"UID_FMT, uid) < 0) - goto fail; + return -ENOMEM; if (asprintf(&u->runtime_path, "/run/user/"UID_FMT, uid) < 0) - goto fail; + return -ENOMEM; - sprintf(lu, UID_FMT, uid); + xsprintf(lu, UID_FMT, uid); r = slice_build_subslice(SPECIAL_USER_SLICE, lu, &u->slice); if (r < 0) - goto fail; - - if (hashmap_put(m->users, UID_TO_PTR(uid), u) < 0) - goto fail; - - if (hashmap_put(m->user_units, u->slice, u) < 0) - goto fail; + return r; - u->manager = m; - u->uid = uid; - u->gid = gid; + r = hashmap_put(m->users, UID_TO_PTR(uid), u); + if (r < 0) + return r; - return u; + r = hashmap_put(m->user_units, u->slice, u); + if (r < 0) + return r; -fail: - if (u->slice) - hashmap_remove(m->user_units, u->slice); - hashmap_remove(m->users, UID_TO_PTR(uid)); - free(u->slice); - free(u->runtime_path); - free(u->state_file); - free(u->name); - free(u); - - return NULL; + *out = u; + u = NULL; + return 0; } -void user_free(User *u) { - assert(u); +User *user_free(User *u) { + if (!u) + return NULL; if (u->in_gc_queue) LIST_REMOVE(gc_queue, u->manager->user_gc_queue, u); @@ -112,23 +106,24 @@ void user_free(User *u) { while (u->sessions) session_free(u->sessions); - if (u->service) { - hashmap_remove(u->manager->user_units, u->service); - free(u->service); - } + if (u->service) + hashmap_remove_value(u->manager->user_units, u->service, u); - hashmap_remove(u->manager->user_units, u->slice); - hashmap_remove(u->manager->users, UID_TO_PTR(u->uid)); + if (u->slice) + hashmap_remove_value(u->manager->user_units, u->slice, u); - free(u->slice_job); - free(u->service_job); + hashmap_remove_value(u->manager->users, UID_TO_PTR(u->uid), u); + + u->slice_job = mfree(u->slice_job); + u->service_job = mfree(u->service_job); - free(u->slice); - free(u->runtime_path); + u->service = mfree(u->service); + u->slice = mfree(u->slice); + u->runtime_path = mfree(u->runtime_path); + u->state_file = mfree(u->state_file); + u->name = mfree(u->name); - free(u->name); - free(u->state_file); - free(u); + return mfree(u); } static int user_save_internal(User *u) { diff --git a/src/login/logind-user.h b/src/login/logind-user.h index 722247806b..11d28d2997 100644 --- a/src/login/logind-user.h +++ b/src/login/logind-user.h @@ -65,8 +65,11 @@ struct User { LIST_FIELDS(User, gc_queue); }; -User* user_new(Manager *m, uid_t uid, gid_t gid, const char *name); -void user_free(User *u); +int user_new(User **out, Manager *m, uid_t uid, gid_t gid, const char *name); +User *user_free(User *u); + +DEFINE_TRIVIAL_CLEANUP_FUNC(User *, user_free); + bool user_check_gc(User *u, bool drop_not_started); void user_add_to_gc_queue(User *u); int user_start(User *u); -- cgit v1.2.3-54-g00ecf From b690ef12b59187074cf5a9c02a69d06fa1398789 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Tue, 29 Sep 2015 11:18:46 +0200 Subject: login: make user->service static Just like user->slice, there is no reason to store the unit name in /run, nor should we allocate it dynamically on job instantiation/removal. Just keep it statically around at all times and rely on user->started || user->stopping to figure out whether the unit exists or not. --- src/login/logind-user.c | 52 +++++++++++++++++++------------------------------ 1 file changed, 20 insertions(+), 32 deletions(-) diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 292a583103..b9d9d537e2 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -67,6 +67,7 @@ int user_new(User **out, Manager *m, uid_t uid, gid_t gid, const char *name) { u->manager = m; u->uid = uid; u->gid = gid; + xsprintf(lu, UID_FMT, uid); u->name = strdup(name); if (!u->name) @@ -78,11 +79,14 @@ int user_new(User **out, Manager *m, uid_t uid, gid_t gid, const char *name) { if (asprintf(&u->runtime_path, "/run/user/"UID_FMT, uid) < 0) return -ENOMEM; - xsprintf(lu, UID_FMT, uid); r = slice_build_subslice(SPECIAL_USER_SLICE, lu, &u->slice); if (r < 0) return r; + r = unit_name_build("user", lu, ".service", &u->service); + if (r < 0) + return r; + r = hashmap_put(m->users, UID_TO_PTR(uid), u); if (r < 0) return r; @@ -91,6 +95,10 @@ int user_new(User **out, Manager *m, uid_t uid, gid_t gid, const char *name) { if (r < 0) return r; + r = hashmap_put(m->user_units, u->service, u); + if (r < 0) + return r; + *out = u; u = NULL; return 0; @@ -155,8 +163,6 @@ static int user_save_internal(User *u) { if (u->runtime_path) fprintf(f, "RUNTIME=%s\n", u->runtime_path); - if (u->service) - fprintf(f, "SERVICE=%s\n", u->service); if (u->service_job) fprintf(f, "SERVICE_JOB=%s\n", u->service_job); @@ -297,7 +303,6 @@ int user_load(User *u) { assert(u); r = parse_env_file(u->state_file, NEWLINE, - "SERVICE", &u->service, "SERVICE_JOB", &u->service_job, "SLICE_JOB", &u->slice_job, "DISPLAY", &display, @@ -425,34 +430,20 @@ static int user_start_service(User *u) { assert(u); - if (!u->service) { - char lu[DECIMAL_STR_MAX(uid_t) + 1], *service; - - xsprintf(lu, UID_FMT, u->uid); - r = unit_name_build("user", lu, ".service", &service); - if (r < 0) - return log_error_errno(r, "Failed to build service name: %m"); + u->service_job = mfree(u->service_job); - r = manager_start_unit( - u->manager, - service, - &error, - &job); - if (r < 0) { - log_error_errno(r, "Failed to start user service, ignoring: %s", bus_error_message(&error, r)); - free(service); - /* we don't fail due to this, let's try to continue */ - } else { - u->service = service; - - free(u->service_job); - u->service_job = job; - } + r = manager_start_unit( + u->manager, + u->service, + &error, + &job); + if (r < 0) { + /* we don't fail due to this, let's try to continue */ + log_error_errno(r, "Failed to start user service, ignoring: %s", bus_error_message(&error, r)); + } else { + u->service_job = job; } - if (u->service) - (void) hashmap_put(u->manager->user_units, u->service, u); - return 0; } @@ -526,9 +517,6 @@ static int user_stop_service(User *u) { assert(u); - if (!u->service) - return 0; - r = manager_stop_unit(u->manager, u->service, &error, &job); if (r < 0) { log_error("Failed to stop user service: %s", bus_error_message(&error, r)); -- cgit v1.2.3-54-g00ecf From 090d92bd6f66afd7c6bf18ef07315058ce826f01 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Tue, 29 Sep 2015 11:20:52 +0200 Subject: login: group static fields in "struct User" Make sure to put static fields together in "struct User". This makes it easier to figure out the lifetime of each field. --- src/login/logind-user.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/login/logind-user.h b/src/login/logind-user.h index 11d28d2997..de99cf47b4 100644 --- a/src/login/logind-user.h +++ b/src/login/logind-user.h @@ -39,16 +39,13 @@ typedef enum UserState { struct User { Manager *manager; - uid_t uid; gid_t gid; char *name; - char *state_file; char *runtime_path; - - char *service; char *slice; + char *service; char *service_job; char *slice_job; -- cgit v1.2.3-54-g00ecf From a832ab6f9953d070ee8f5cf2c7869425760b2645 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Tue, 29 Sep 2015 11:36:18 +0200 Subject: login: fix re-use of users If the last reference to a user is released, we queue stop-jobs for the user-service and slice. Only once those are finished, we drop the user-object. However, if a new session is opened before the user object is fully dropped, we currently incorrectly re-use the object. This has the effect, that we get stale sessions without a valid "systemd --user" instance. Fix this by properly allowing user_start() to be called, even if user->stopping is true. --- src/login/logind-user.c | 43 ++++++++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/src/login/logind-user.c b/src/login/logind-user.c index b9d9d537e2..778f19b50d 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -452,15 +452,32 @@ int user_start(User *u) { assert(u); - if (u->started) + if (u->started && !u->stopping) return 0; - log_debug("New user %s logged in.", u->name); - - /* Make XDG_RUNTIME_DIR */ - r = user_mkdir_runtime_path(u); - if (r < 0) - return r; + /* + * If u->stopping is set, the user is marked for removal and the slice + * and service stop-jobs are queued. We have to clear that flag before + * queing the start-jobs again. If they succeed, the user object can be + * re-used just fine (pid1 takes care of job-ordering and proper + * restart), but if they fail, we want to force another user_stop() so + * possibly pending units are stopped. + * Note that we don't clear u->started, as we have no clue what state + * the user is in on failure here. Hence, we pretend the user is + * running so it will be properly taken down by GC. However, we clearly + * return an error from user_start() in that case, so no further + * reference to the user is taken. + */ + u->stopping = false; + + if (!u->started) { + log_debug("New user %s logged in.", u->name); + + /* Make XDG_RUNTIME_DIR */ + r = user_mkdir_runtime_path(u); + if (r < 0) + return r; + } /* Create cgroup */ r = user_start_slice(u); @@ -478,16 +495,16 @@ int user_start(User *u) { if (r < 0) return r; - if (!dual_timestamp_is_set(&u->timestamp)) - dual_timestamp_get(&u->timestamp); - - u->started = true; + if (!u->started) { + if (!dual_timestamp_is_set(&u->timestamp)) + dual_timestamp_get(&u->timestamp); + user_send_signal(u, true); + u->started = true; + } /* Save new user data */ user_save(u); - user_send_signal(u, true); - return 0; } -- cgit v1.2.3-54-g00ecf From 79ee4ad108adc55c44891dc0058568d1068f311e Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Mon, 16 Nov 2015 15:43:18 +0100 Subject: login: make sure to replace existing units When queuing unit jobs, we should rather replace existing units than fail. This is especially important when we queued a user-shutdown and a new login is encountered. In this case, we better raplce the shutdown jobs. systemd takes care of everything else. --- src/login/logind-dbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 7cc0044bab..e36217c5b2 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -2964,7 +2964,7 @@ int manager_start_unit(Manager *manager, const char *unit, sd_bus_error *error, "StartUnit", error, &reply, - "ss", unit, "fail"); + "ss", unit, "replace"); if (r < 0) return r; -- cgit v1.2.3-54-g00ecf From 2f157acdae14962ed7b7f7734f3e3547477aea0a Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Mon, 16 Nov 2015 15:45:02 +0100 Subject: login: ignore JobRemoved of old jobs If we requeue jobs, we are no longer interested in old jobs. Hence, we better ignore any JobRemoved signals for old jobs and concentrate on our replacements. --- src/login/logind-dbus.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index e36217c5b2..e507a19aef 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -2620,11 +2620,8 @@ int match_job_removed(sd_bus_message *message, void *userdata, sd_bus_error *err } session = hashmap_get(m->session_units, unit); - if (session) { - - if (streq_ptr(path, session->scope_job)) - session->scope_job = mfree(session->scope_job); - + if (session && streq_ptr(path, session->scope_job)) { + session->scope_job = mfree(session->scope_job); session_jobs_reply(session, unit, result); session_save(session); @@ -2633,7 +2630,9 @@ int match_job_removed(sd_bus_message *message, void *userdata, sd_bus_error *err } user = hashmap_get(m->user_units, unit); - if (user) { + if (user && + (streq_ptr(path, user->service_job) || + streq_ptr(path, user->slice_job))) { if (streq_ptr(path, user->service_job)) user->service_job = mfree(user->service_job); -- cgit v1.2.3-54-g00ecf