]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
logind: don't start user@UID.service instance for background sessions
authorMichal Sekletar <msekleta@redhat.com>
Fri, 27 May 2022 19:11:37 +0000 (21:11 +0200)
committerYu Watanabe <watanabe.yu+github@gmail.com>
Thu, 7 Jul 2022 01:10:42 +0000 (10:10 +0900)
We have had background session class for a long time (since commit
e2acb67baa), but so far the only difference in handling of background
sessions was logging, i.e. we log some messages with LOG_DEBUG for such
sessions.

Previously there were complains [1] about excessive logging for each
time cron session is started. We used to advise user to enable lingering
for users if they want to avoid these log messages. However, on servers
with a lot of users the extra processes that result from lingering just
adds too much overhead. Hence I think that our current handling of
background sessions is not ideal and we should make better use of this
attribute.

This commit introduces a change in default behavior of logind. Logind is
now not going to start user instance of systemd when background session
is created and that should address excessive logging problem for cron
where background class is used by default. When the same user actually
logs in normally then user instance will be started as previously.

Also note that PAM_TTY variable is now always set to some value for PAM
sessions started via PAMName= option. Otherwise we would categorize such
sessions as "background" and user manager won't be started.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1825942

src/core/execute.c
src/login/logind-dbus.c
src/login/logind-session.c
src/login/logind-user.c
src/login/logind-user.h
src/login/logind.c
src/login/pam_systemd.c

index 05fc00ca1ce56184555d8308184620bab362832f..962edc1eee3c782568b51e3265ad4e0863af6b04 100644 (file)
@@ -1213,13 +1213,15 @@ static int setup_pam(
 
                 if (getttyname_malloc(STDIN_FILENO, &q) >= 0)
                         tty = strjoina("/dev/", q);
+                else
+                        /* If everything else failed then let's just use value "systemd". This will cause that session
+                         * isn't going to be marked as "background" and user manager will be started. */
+                        tty = "systemd";
         }
 
-        if (tty) {
-                pam_code = pam_set_item(handle, PAM_TTY, tty);
-                if (pam_code != PAM_SUCCESS)
-                        goto fail;
-        }
+        pam_code = pam_set_item(handle, PAM_TTY, tty);
+        if (pam_code != PAM_SUCCESS)
+               goto fail;
 
         STRV_FOREACH(nv, *env) {
                 pam_code = pam_putenv(handle, *nv);
index a426842bbb3aa9faf8456465d958626662af72e0..e308e52af3b5f883b9247a300e1720e8928fcb43 100644 (file)
@@ -1305,7 +1305,7 @@ static int method_set_user_linger(sd_bus_message *message, void *userdata, sd_bu
                         return r;
 
                 if (manager_add_user_by_uid(m, uid, &u) >= 0)
-                        user_start(u);
+                        user_start(u, /* want_user_instance= */ true);
 
         } else {
                 User *u;
index 22bb4c371498926a0c9f426ea6d7ca2e3f4813a5..580863050300e205fbf4f434bbb4fda9170f68f6 100644 (file)
@@ -664,12 +664,12 @@ static int session_start_scope(Session *s, sd_bus_message *properties, sd_bus_er
                                 description,
                                 /* These two have StopWhenUnneeded= set, hence add a dep towards them */
                                 STRV_MAKE(s->user->runtime_dir_service,
-                                          s->user->service),
+                                          s->class != SESSION_BACKGROUND ? s->user->service : NULL),
                                 /* And order us after some more */
                                 STRV_MAKE("systemd-logind.service",
                                           "systemd-user-sessions.service",
                                           s->user->runtime_dir_service,
-                                          s->user->service),
+                                          s->class != SESSION_BACKGROUND ? s->user->service : NULL),
                                 user_record_home_directory(s->user->user_record),
                                 properties,
                                 error,
@@ -700,7 +700,7 @@ int session_start(Session *s, sd_bus_message *properties, sd_bus_error *error) {
         if (s->started)
                 return 0;
 
-        r = user_start(s->user);
+        r = user_start(s->user, /* want_user_instance= */ s->class != SESSION_BACKGROUND);
         if (r < 0)
                 return r;
 
index 3f1b8f610ba79a3a61217f39ecaaccc480c288f0..5f6cd590ceaafa81ea8d1b8443f582e658675e14 100644 (file)
@@ -442,7 +442,7 @@ static int user_update_slice(User *u) {
         return 0;
 }
 
-int user_start(User *u) {
+int user_start(User *u, bool want_user_instance) {
         assert(u);
 
         if (u->started && !u->stopping)
@@ -465,7 +465,8 @@ int user_start(User *u) {
         (void) user_update_slice(u);
 
         /* Start user@UID.service */
-        user_start_service(u);
+        if (want_user_instance)
+                user_start_service(u);
 
         if (!u->started) {
                 if (!dual_timestamp_is_set(&u->timestamp))
index 21b9f8f348ec5d73b1eecbfd534ef78e783e1a70..a20775ae7041a9374bc2494a2f8d8aeed48c73d1 100644 (file)
@@ -57,7 +57,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(User *, user_free);
 
 bool user_may_gc(User *u, bool drop_not_started);
 void user_add_to_gc_queue(User *u);
-int user_start(User *u);
+int user_start(User *u, bool need_user_instance);
 int user_stop(User *u, bool force);
 int user_finalize(User *u);
 UserState user_get_state(User *u);
index d14a17274bb0796700b41cf91905806d343324ad..2fdcd1d55ed9a5078468de051f69208db6482a64 100644 (file)
@@ -1041,7 +1041,6 @@ static int manager_startup(Manager *m) {
         int r;
         Seat *seat;
         Session *session;
-        User *user;
         Button *button;
         Inhibitor *inhibitor;
 
@@ -1118,9 +1117,7 @@ static int manager_startup(Manager *m) {
         HASHMAP_FOREACH(seat, m->seats)
                 (void) seat_start(seat);
 
-        HASHMAP_FOREACH(user, m->users)
-                (void) user_start(user);
-
+        /* Users are started by respective sessions */
         HASHMAP_FOREACH(session, m->sessions)
                 (void) session_start(session, NULL, NULL);
 
index cb6a6fb51463f13c3dd4bdb988810ac8cebba28c..677a0140782e0c191d13e571859a7883957ada9b 100644 (file)
@@ -790,7 +790,10 @@ _public_ PAM_EXTERN int pam_sm_open_session(
                              * does the PAM session registration early for new connections, and registers a pty only
                              * much later (this is because it doesn't know yet if it needs one at all, as whether to
                              * register a pty or not is negotiated much later in the protocol). */
-
+        } else if (streq(tty, "systemd")) {
+                if (isempty(class))
+                        class = "user";
+                tty = NULL;
         } else
                 /* Chop off leading /dev prefix that some clients specify, but others do not. */
                 tty = skip_dev_prefix(tty);