Merge pull request #17911 from yuwata/log-cli-program-use-journal

log: open journal when cli program run in a service unit

src/basic/async.h

@@ -1,7 +1,13 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 #pragma once
 
+#include <sys/types.h>
+
+#include "macro.h"
+
 int asynchronous_job(void* (*func)(void *p), void *arg);
 
 int asynchronous_sync(pid_t *ret_pid);
 int asynchronous_close(int fd);
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(int, asynchronous_close);

src/basic/log.c

@@ -51,7 +51,7 @@ static int journal_fd = -1;
 
 static bool syslog_is_stream = false;
 
-static bool show_color = false;
+static int show_color = -1; /* tristate */
 static bool show_location = false;
 static bool show_time = false;
 static bool show_tid = false;
@@ -387,11 +387,10 @@ static int write_to_console(
                 iovec[n++] = IOVEC_MAKE_STRING(prefix);
         }
 
-        if (show_time) {
-                if (format_timestamp(header_time, sizeof(header_time), now(CLOCK_REALTIME))) {
-                        iovec[n++] = IOVEC_MAKE_STRING(header_time);
-                        iovec[n++] = IOVEC_MAKE_STRING(" ");
-                }
+        if (show_time &&
+            format_timestamp(header_time, sizeof(header_time), now(CLOCK_REALTIME))) {
+                iovec[n++] = IOVEC_MAKE_STRING(header_time);
+                iovec[n++] = IOVEC_MAKE_STRING(" ");
         }
 
         if (show_tid) {
@@ -399,12 +398,12 @@ static int write_to_console(
                 iovec[n++] = IOVEC_MAKE_STRING(tid_string);
         }
 
-        if (show_color)
+        if (log_get_show_color())
                 get_log_colors(LOG_PRI(level), &on, &off, NULL);
 
         if (show_location) {
                 const char *lon = "", *loff = "";
-                if (show_color) {
+                if (log_get_show_color()) {
                         lon = ANSI_HIGHLIGHT_YELLOW4;
                         loff = ANSI_NORMAL;
                 }
@@ -1222,7 +1221,7 @@ void log_show_color(bool b) {
 }
 
 bool log_get_show_color(void) {
-        return show_color;
+        return show_color > 0; /* Defaults to false. */
 }
 
 void log_show_location(bool b) {
@@ -1485,7 +1484,9 @@ void log_setup_service(void) {
 void log_setup_cli(void) {
         /* Sets up logging the way it is most appropriate for running a program as a CLI utility. */
 
-        log_show_color(true);
+        log_set_target(LOG_TARGET_AUTO);
         log_parse_environment_cli();
         (void) log_open();
+        if (log_on_console() && show_color < 0)
+                log_show_color(true);
 }

src/core/cgroup.c

@@ -1836,6 +1836,10 @@ int unit_pick_cgroup_path(Unit *u) {
         return 0;
 }
 
+static int cg_v1_errno_to_log_level(int r) {
+        return r == -EROFS ? LOG_DEBUG : LOG_WARNING;
+}
+
 static int unit_update_cgroup(
                 Unit *u,
                 CGroupMask target_mask,
@@ -1893,16 +1897,30 @@ static int unit_update_cgroup(
          * We perform migration also with whole slices for cases when users don't care about leave
          * granularity. Since delegated_mask is subset of target mask, we won't trim slice subtree containing
          * delegated units.
+         *
+         * If we're in an nspawn container and using legacy cgroups, the controller hierarchies are mounted
+         * read-only into the container. We skip migration/trim in this scenario since it would fail
+         * regardless with noisy "Read-only filesystem" warnings.
          */
         if (cg_all_unified() == 0) {
                 r = cg_migrate_v1_controllers(u->manager->cgroup_supported, migrate_mask, u->cgroup_path, migrate_callback, u);
                 if (r < 0)
-                        log_unit_warning_errno(u, r, "Failed to migrate controller cgroups from %s, ignoring: %m", u->cgroup_path);
+                        log_unit_full_errno(
+                                u,
+                                cg_v1_errno_to_log_level(r),
+                                r,
+                                "Failed to migrate controller cgroups from %s, ignoring: %m",
+                                u->cgroup_path);
 
                 is_root_slice = unit_has_name(u, SPECIAL_ROOT_SLICE);
                 r = cg_trim_v1_controllers(u->manager->cgroup_supported, ~target_mask, u->cgroup_path, !is_root_slice);
                 if (r < 0)
-                        log_unit_warning_errno(u, r, "Failed to delete controller cgroups %s, ignoring: %m", u->cgroup_path);
+                        log_unit_full_errno(
+                                u,
+                                cg_v1_errno_to_log_level(r),
+                                r,
+                                "Failed to delete controller cgroups %s, ignoring: %m",
+                                u->cgroup_path);
         }
 
         /* Set attributes */

src/core/manager.c

@@ -2937,8 +2937,10 @@ int manager_loop(Manager *m) {
                 usec_t wait_usec, watchdog_usec;
 
                 watchdog_usec = manager_get_watchdog(m, WATCHDOG_RUNTIME);
-                if (timestamp_is_set(watchdog_usec))
+                if (m->runtime_watchdog_running)
                         (void) watchdog_ping();
+                else if (timestamp_is_set(watchdog_usec))
+                        manager_retry_runtime_watchdog(m);
 
                 if (!ratelimit_below(&rl)) {
                         /* Yay, something is going seriously wrong, pause a little */
@@ -3408,14 +3410,18 @@ void manager_set_watchdog(Manager *m, WatchdogType t, usec_t timeout) {
 
         if (t == WATCHDOG_RUNTIME)
                 if (!timestamp_is_set(m->watchdog_overridden[WATCHDOG_RUNTIME])) {
-                        if (timestamp_is_set(timeout))
+                        if (timestamp_is_set(timeout)) {
                                 r = watchdog_set_timeout(&timeout);
-                        else
+
+                                if (r >= 0)
+                                        m->runtime_watchdog_running = true;
+                        } else {
                                 watchdog_close(true);
+                                m->runtime_watchdog_running = false;
+                        }
                 }
 
-        if (r >= 0)
-                m->watchdog[t] = timeout;
+        m->watchdog[t] = timeout;
 }
 
 int manager_override_watchdog(Manager *m, WatchdogType t, usec_t timeout) {
@@ -3433,18 +3439,36 @@ int manager_override_watchdog(Manager *m, WatchdogType t, usec_t timeout) {
                 usec_t *p;
 
                 p = timestamp_is_set(timeout) ? &timeout : &m->watchdog[t];
-                if (timestamp_is_set(*p))
+                if (timestamp_is_set(*p)) {
                         r = watchdog_set_timeout(p);
-                else
+
+                        if (r >= 0)
+                                m->runtime_watchdog_running = true;
+                } else {
                         watchdog_close(true);
+                        m->runtime_watchdog_running = false;
+                }
         }
 
-        if (r >= 0)
-                m->watchdog_overridden[t] = timeout;
+        m->watchdog_overridden[t] = timeout;
 
         return 0;
 }
 
+void manager_retry_runtime_watchdog(Manager *m) {
+        int r = 0;
+
+        assert(m);
+
+        if (timestamp_is_set(m->watchdog_overridden[WATCHDOG_RUNTIME]))
+                r = watchdog_set_timeout(&m->watchdog_overridden[WATCHDOG_RUNTIME]);
+        else
+                r = watchdog_set_timeout(&m->watchdog[WATCHDOG_RUNTIME]);
+
+        if (r >= 0)
+                m->runtime_watchdog_running = true;
+}
+
 static void manager_deserialize_uid_refs_one_internal(
                 Manager *m,
                 Hashmap** uid_refs,

src/core/manager.h

@@ -241,6 +241,8 @@ struct Manager {
         usec_t watchdog[_WATCHDOG_TYPE_MAX];
         usec_t watchdog_overridden[_WATCHDOG_TYPE_MAX];
 
+        bool runtime_watchdog_running; /* Whether the runtime HW watchdog was started, so we know if we still need to get the real timeout from the hardware */
+
         dual_timestamp timestamps[_MANAGER_TIMESTAMP_MAX];
 
         /* Data specific to the device subsystem */
@@ -562,6 +564,7 @@ ManagerTimestamp manager_timestamp_initrd_mangle(ManagerTimestamp s);
 usec_t manager_get_watchdog(Manager *m, WatchdogType t);
 void manager_set_watchdog(Manager *m, WatchdogType t, usec_t timeout);
 int manager_override_watchdog(Manager *m, WatchdogType t, usec_t timeout);
+void manager_retry_runtime_watchdog(Manager *m);
 
 const char* oom_policy_to_string(OOMPolicy i) _const_;
 OOMPolicy oom_policy_from_string(const char *s) _pure_;

src/login/logind-button.c

@@ -8,12 +8,12 @@
 #include "sd-messages.h"
 
 #include "alloc-util.h"
+#include "async.h"
 #include "fd-util.h"
 #include "logind-button.h"
 #include "missing_input.h"
 #include "string-util.h"
 #include "util.h"
-#include "async.h"
 
 #define CONST_MAX5(a, b, c, d, e) CONST_MAX(CONST_MAX(a, b), CONST_MAX(CONST_MAX(c, d), e))
 
@@ -60,11 +60,7 @@ void button_free(Button *b) {
         sd_event_source_unref(b->io_event_source);
         sd_event_source_unref(b->check_event_source);
 
-        if (b->fd >= 0)
-                /* If the device has been unplugged close() returns
-                 * ENODEV, let's ignore this, hence we don't use
-                 * safe_close() */
-                (void) asynchronous_close(b->fd);
+        asynchronous_close(b->fd);
 
         free(b->name);
         free(b->seat);
@@ -72,19 +68,9 @@ void button_free(Button *b) {
 }
 
 int button_set_seat(Button *b, const char *sn) {
-        char *s;
-
         assert(b);
-        assert(sn);
 
-        s = strdup(sn);
-        if (!s)
-                return -ENOMEM;
-
-        free(b->seat);
-        b->seat = s;
-
-        return 0;
+        return free_and_strdup(&b->seat, sn);
 }
 
 static void button_lid_switch_handle_action(Manager *manager, bool is_edge) {
@@ -327,14 +313,14 @@ static int button_set_mask(const char *name, int fd) {
 }
 
 int button_open(Button *b) {
-        _cleanup_close_ int fd = -1;
+        _cleanup_(asynchronous_closep) int fd = -1;
         const char *p;
         char name[256];
         int r;
 
         assert(b);
 
-        b->fd = safe_close(b->fd);
+        b->fd = asynchronous_close(b->fd);
 
         p = strjoina("/dev/input/", b->name);
 
@@ -345,11 +331,9 @@ int button_open(Button *b) {
         r = button_suitable(fd);
         if (r < 0)
                 return log_warning_errno(r, "Failed to determine whether input device %s is relevant to us: %m", p);
-        if (r == 0) {
-                b->fd = TAKE_FD(fd);
+        if (r == 0)
                 return log_debug_errno(SYNTHETIC_ERRNO(EADDRNOTAVAIL),
                                        "Device %s does not expose keys or switches relevant to us, ignoring.", p);
-        }
 
         if (ioctl(fd, EVIOCGNAME(sizeof name), name) < 0)
                 return log_error_errno(errno, "Failed to get input name for %s: %m", p);