From 9f9f034271cf719ba7d6b763130ad7013844207c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 20 Nov 2017 21:01:13 +0100 Subject: [PATCH 1/6] manager: rework the timestamps logic, so that they are an enum-index array This makes things quite a bit more systematic I think, as we can systematically operate on all timestamps, for example for the purpose of serialization/deserialization. This rework doesn't necessarily make things shorter in the individual lines, but it does reduce the line count a bit. (This is useful particularly when we want to add additional timestamps, for example to solve #7023) --- src/core/dbus-manager.c | 26 ++++---- src/core/main.c | 10 +-- src/core/manager.c | 133 ++++++++++++++++++++++------------------ src/core/manager.h | 35 +++++++---- src/core/timer.c | 2 +- 5 files changed, 113 insertions(+), 93 deletions(-) diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 3c043ba91a..b98351e84e 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -323,7 +323,7 @@ static int property_get_progress( assert(reply); assert(m); - if (dual_timestamp_is_set(&m->finish_timestamp)) + if (dual_timestamp_is_set(m->timestamps + MANAGER_TIMESTAMP_FINISH)) d = 1.0; else d = 1.0 - ((double) hashmap_size(m->jobs) / (double) m->n_installed_jobs); @@ -2413,18 +2413,18 @@ const sd_bus_vtable bus_manager_vtable[] = { SD_BUS_PROPERTY("Virtualization", "s", property_get_virtualization, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Architecture", "s", property_get_architecture, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Tainted", "s", property_get_tainted, 0, SD_BUS_VTABLE_PROPERTY_CONST), - BUS_PROPERTY_DUAL_TIMESTAMP("FirmwareTimestamp", offsetof(Manager, firmware_timestamp), SD_BUS_VTABLE_PROPERTY_CONST), - BUS_PROPERTY_DUAL_TIMESTAMP("LoaderTimestamp", offsetof(Manager, loader_timestamp), SD_BUS_VTABLE_PROPERTY_CONST), - BUS_PROPERTY_DUAL_TIMESTAMP("KernelTimestamp", offsetof(Manager, kernel_timestamp), SD_BUS_VTABLE_PROPERTY_CONST), - BUS_PROPERTY_DUAL_TIMESTAMP("InitRDTimestamp", offsetof(Manager, initrd_timestamp), SD_BUS_VTABLE_PROPERTY_CONST), - BUS_PROPERTY_DUAL_TIMESTAMP("UserspaceTimestamp", offsetof(Manager, userspace_timestamp), SD_BUS_VTABLE_PROPERTY_CONST), - BUS_PROPERTY_DUAL_TIMESTAMP("FinishTimestamp", offsetof(Manager, finish_timestamp), SD_BUS_VTABLE_PROPERTY_CONST), - BUS_PROPERTY_DUAL_TIMESTAMP("SecurityStartTimestamp", offsetof(Manager, security_start_timestamp), SD_BUS_VTABLE_PROPERTY_CONST), - BUS_PROPERTY_DUAL_TIMESTAMP("SecurityFinishTimestamp", offsetof(Manager, security_finish_timestamp), SD_BUS_VTABLE_PROPERTY_CONST), - BUS_PROPERTY_DUAL_TIMESTAMP("GeneratorsStartTimestamp", offsetof(Manager, generators_start_timestamp), SD_BUS_VTABLE_PROPERTY_CONST), - BUS_PROPERTY_DUAL_TIMESTAMP("GeneratorsFinishTimestamp", offsetof(Manager, generators_finish_timestamp), SD_BUS_VTABLE_PROPERTY_CONST), - BUS_PROPERTY_DUAL_TIMESTAMP("UnitsLoadStartTimestamp", offsetof(Manager, units_load_start_timestamp), SD_BUS_VTABLE_PROPERTY_CONST), - BUS_PROPERTY_DUAL_TIMESTAMP("UnitsLoadFinishTimestamp", offsetof(Manager, units_load_finish_timestamp), SD_BUS_VTABLE_PROPERTY_CONST), + BUS_PROPERTY_DUAL_TIMESTAMP("FirmwareTimestamp", offsetof(Manager, timestamps[MANAGER_TIMESTAMP_FIRMWARE]), SD_BUS_VTABLE_PROPERTY_CONST), + BUS_PROPERTY_DUAL_TIMESTAMP("LoaderTimestamp", offsetof(Manager, timestamps[MANAGER_TIMESTAMP_LOADER]), SD_BUS_VTABLE_PROPERTY_CONST), + BUS_PROPERTY_DUAL_TIMESTAMP("KernelTimestamp", offsetof(Manager, timestamps[MANAGER_TIMESTAMP_KERNEL]), SD_BUS_VTABLE_PROPERTY_CONST), + BUS_PROPERTY_DUAL_TIMESTAMP("InitRDTimestamp", offsetof(Manager, timestamps[MANAGER_TIMESTAMP_INITRD]), SD_BUS_VTABLE_PROPERTY_CONST), + BUS_PROPERTY_DUAL_TIMESTAMP("UserspaceTimestamp", offsetof(Manager, timestamps[MANAGER_TIMESTAMP_USERSPACE]), SD_BUS_VTABLE_PROPERTY_CONST), + BUS_PROPERTY_DUAL_TIMESTAMP("FinishTimestamp", offsetof(Manager, timestamps[MANAGER_TIMESTAMP_FINISH]), SD_BUS_VTABLE_PROPERTY_CONST), + BUS_PROPERTY_DUAL_TIMESTAMP("SecurityStartTimestamp", offsetof(Manager, timestamps[MANAGER_TIMESTAMP_SECURITY_START]), SD_BUS_VTABLE_PROPERTY_CONST), + BUS_PROPERTY_DUAL_TIMESTAMP("SecurityFinishTimestamp", offsetof(Manager, timestamps[MANAGER_TIMESTAMP_SECURITY_FINISH]), SD_BUS_VTABLE_PROPERTY_CONST), + BUS_PROPERTY_DUAL_TIMESTAMP("GeneratorsStartTimestamp", offsetof(Manager, timestamps[MANAGER_TIMESTAMP_GENERATORS_START]), SD_BUS_VTABLE_PROPERTY_CONST), + BUS_PROPERTY_DUAL_TIMESTAMP("GeneratorsFinishTimestamp", offsetof(Manager, timestamps[MANAGER_TIMESTAMP_GENERATORS_FINISH]), SD_BUS_VTABLE_PROPERTY_CONST), + BUS_PROPERTY_DUAL_TIMESTAMP("UnitsLoadStartTimestamp", offsetof(Manager, timestamps[MANAGER_TIMESTAMP_UNITS_LOAD_START]), SD_BUS_VTABLE_PROPERTY_CONST), + BUS_PROPERTY_DUAL_TIMESTAMP("UnitsLoadFinishTimestamp", offsetof(Manager, timestamps[MANAGER_TIMESTAMP_UNITS_LOAD_FINISH]), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_WRITABLE_PROPERTY("LogLevel", "s", property_get_log_level, property_set_log_level, 0, 0), SD_BUS_WRITABLE_PROPERTY("LogTarget", "s", property_get_log_target, property_set_log_target, 0, 0), SD_BUS_PROPERTY("NNames", "u", property_get_n_names, 0, 0), diff --git a/src/core/main.c b/src/core/main.c index ee9594a00f..6a9a297e5f 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1995,11 +1995,11 @@ int main(int argc, char *argv[]) { goto finish; } - m->userspace_timestamp = userspace_timestamp; - m->kernel_timestamp = kernel_timestamp; - m->initrd_timestamp = initrd_timestamp; - m->security_start_timestamp = security_start_timestamp; - m->security_finish_timestamp = security_finish_timestamp; + m->timestamps[MANAGER_TIMESTAMP_KERNEL] = kernel_timestamp; + m->timestamps[MANAGER_TIMESTAMP_INITRD] = initrd_timestamp; + m->timestamps[MANAGER_TIMESTAMP_USERSPACE] = userspace_timestamp; + m->timestamps[MANAGER_TIMESTAMP_SECURITY_START] = security_start_timestamp; + m->timestamps[MANAGER_TIMESTAMP_SECURITY_FINISH] = security_finish_timestamp; set_manager_defaults(m); set_manager_settings(m); diff --git a/src/core/manager.c b/src/core/manager.c index 67fe3d2120..059482ff1d 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -624,7 +624,9 @@ int manager_new(UnitFileScope scope, unsigned test_run_flags, Manager **_m) { #if ENABLE_EFI if (MANAGER_IS_SYSTEM(m) && detect_container() <= 0) - boot_timestamps(&m->userspace_timestamp, &m->firmware_timestamp, &m->loader_timestamp); + boot_timestamps(m->timestamps + MANAGER_TIMESTAMP_USERSPACE, + m->timestamps + MANAGER_TIMESTAMP_FIRMWARE, + m->timestamps + MANAGER_TIMESTAMP_LOADER); #endif /* Prepare log fields we can use for structured logging */ @@ -1340,9 +1342,9 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { return log_error_errno(r, "Failed to create transient generator directory \"%s\": %m", m->lookup_paths.transient); - dual_timestamp_get(&m->generators_start_timestamp); + dual_timestamp_get(m->timestamps + MANAGER_TIMESTAMP_GENERATORS_START); r = manager_run_generators(m); - dual_timestamp_get(&m->generators_finish_timestamp); + dual_timestamp_get(m->timestamps + MANAGER_TIMESTAMP_GENERATORS_FINISH); if (r < 0) return r; @@ -1369,9 +1371,9 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { m->n_reloading++; /* First, enumerate what we can from all config files */ - dual_timestamp_get(&m->units_load_start_timestamp); + dual_timestamp_get(m->timestamps + MANAGER_TIMESTAMP_UNITS_LOAD_START); manager_enumerate(m); - dual_timestamp_get(&m->units_load_finish_timestamp); + dual_timestamp_get(m->timestamps + MANAGER_TIMESTAMP_UNITS_LOAD_FINISH); /* Second, deserialize if there is something to deserialize */ if (serialization) { @@ -2577,9 +2579,10 @@ int manager_open_serialization(Manager *m, FILE **_f) { } int manager_serialize(Manager *m, FILE *f, FDSet *fds, bool switching_root) { + ManagerTimestamp q; + const char *t; Iterator i; Unit *u; - const char *t; int r; assert(m); @@ -2594,20 +2597,17 @@ int manager_serialize(Manager *m, FILE *f, FDSet *fds, bool switching_root) { fprintf(f, "taint-usr=%s\n", yes_no(m->taint_usr)); fprintf(f, "ready-sent=%s\n", yes_no(m->ready_sent)); - dual_timestamp_serialize(f, "firmware-timestamp", &m->firmware_timestamp); - dual_timestamp_serialize(f, "loader-timestamp", &m->loader_timestamp); - dual_timestamp_serialize(f, "kernel-timestamp", &m->kernel_timestamp); - dual_timestamp_serialize(f, "initrd-timestamp", &m->initrd_timestamp); + for (q = 0; q < _MANAGER_TIMESTAMP_MAX; q++) { + /* The userspace and finish timestamps only apply to the host system, hence only serialize them there */ + if (in_initrd() && IN_SET(q, MANAGER_TIMESTAMP_USERSPACE, MANAGER_TIMESTAMP_FINISH)) + continue; - if (!in_initrd()) { - dual_timestamp_serialize(f, "userspace-timestamp", &m->userspace_timestamp); - dual_timestamp_serialize(f, "finish-timestamp", &m->finish_timestamp); - dual_timestamp_serialize(f, "security-start-timestamp", &m->security_start_timestamp); - dual_timestamp_serialize(f, "security-finish-timestamp", &m->security_finish_timestamp); - dual_timestamp_serialize(f, "generators-start-timestamp", &m->generators_start_timestamp); - dual_timestamp_serialize(f, "generators-finish-timestamp", &m->generators_finish_timestamp); - dual_timestamp_serialize(f, "units-load-start-timestamp", &m->units_load_start_timestamp); - dual_timestamp_serialize(f, "units-load-finish-timestamp", &m->units_load_finish_timestamp); + t = manager_timestamp_to_string(q); + { + char field[strlen(t) + strlen("-timestamp") + 1]; + strcpy(stpcpy(field, t), "-timestamp"); + dual_timestamp_serialize(f, field, m->timestamps + q); + } } if (!switching_root) @@ -2758,31 +2758,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) { else m->ready_sent = m->ready_sent || b; - } else if ((val = startswith(l, "firmware-timestamp="))) - dual_timestamp_deserialize(val, &m->firmware_timestamp); - else if ((val = startswith(l, "loader-timestamp="))) - dual_timestamp_deserialize(val, &m->loader_timestamp); - else if ((val = startswith(l, "kernel-timestamp="))) - dual_timestamp_deserialize(val, &m->kernel_timestamp); - else if ((val = startswith(l, "initrd-timestamp="))) - dual_timestamp_deserialize(val, &m->initrd_timestamp); - else if ((val = startswith(l, "userspace-timestamp="))) - dual_timestamp_deserialize(val, &m->userspace_timestamp); - else if ((val = startswith(l, "finish-timestamp="))) - dual_timestamp_deserialize(val, &m->finish_timestamp); - else if ((val = startswith(l, "security-start-timestamp="))) - dual_timestamp_deserialize(val, &m->security_start_timestamp); - else if ((val = startswith(l, "security-finish-timestamp="))) - dual_timestamp_deserialize(val, &m->security_finish_timestamp); - else if ((val = startswith(l, "generators-start-timestamp="))) - dual_timestamp_deserialize(val, &m->generators_start_timestamp); - else if ((val = startswith(l, "generators-finish-timestamp="))) - dual_timestamp_deserialize(val, &m->generators_finish_timestamp); - else if ((val = startswith(l, "units-load-start-timestamp="))) - dual_timestamp_deserialize(val, &m->units_load_start_timestamp); - else if ((val = startswith(l, "units-load-finish-timestamp="))) - dual_timestamp_deserialize(val, &m->units_load_finish_timestamp); - else if (startswith(l, "env=")) { + } else if (startswith(l, "env=")) { r = deserialize_environment(&m->environment, l); if (r == -ENOMEM) goto finish; @@ -2845,9 +2821,24 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) { if (strv_extend(&m->deserialized_subscribed, val) < 0) log_oom(); + } else { + ManagerTimestamp q; - } else if (!startswith(l, "kdbus-fd=")) /* ignore this one */ - log_notice("Unknown serialization item '%s'", l); + for (q = 0; q < _MANAGER_TIMESTAMP_MAX; q++) { + val = startswith(l, manager_timestamp_to_string(q)); + if (!val) + continue; + + val = startswith(val, "-timestamp="); + if (val) + break; + } + + if (q < _MANAGER_TIMESTAMP_MAX) /* found it */ + dual_timestamp_deserialize(val, m->timestamps + q); + else if (!startswith(l, "kdbus-fd=")) /* ignore kdbus */ + log_notice("Unknown serialization item '%s'", l); + } } for (;;) { @@ -3033,20 +3024,20 @@ static void manager_notify_finished(Manager *m) { if (MANAGER_IS_SYSTEM(m) && detect_container() <= 0) { - /* Note that m->kernel_usec.monotonic is always at 0, - * and m->firmware_usec.monotonic and - * m->loader_usec.monotonic should be considered + /* Note that MANAGER_TIMESTAMP_KERNEL's monotonic value is always at 0, and + * MANAGER_TIMESTAMP_FIRMWARE's and MANAGER_TIMESTAMP_LOADER's monotonic value should be considered * negative values. */ - firmware_usec = m->firmware_timestamp.monotonic - m->loader_timestamp.monotonic; - loader_usec = m->loader_timestamp.monotonic - m->kernel_timestamp.monotonic; - userspace_usec = m->finish_timestamp.monotonic - m->userspace_timestamp.monotonic; - total_usec = m->firmware_timestamp.monotonic + m->finish_timestamp.monotonic; + firmware_usec = m->timestamps[MANAGER_TIMESTAMP_FIRMWARE].monotonic - m->timestamps[MANAGER_TIMESTAMP_LOADER].monotonic; + loader_usec = m->timestamps[MANAGER_TIMESTAMP_LOADER].monotonic - m->timestamps[MANAGER_TIMESTAMP_KERNEL].monotonic; + userspace_usec = m->timestamps[MANAGER_TIMESTAMP_FINISH].monotonic - m->timestamps[MANAGER_TIMESTAMP_USERSPACE].monotonic; + total_usec = m->timestamps[MANAGER_TIMESTAMP_FIRMWARE].monotonic + m->timestamps[MANAGER_TIMESTAMP_FINISH].monotonic; - if (dual_timestamp_is_set(&m->initrd_timestamp)) { + if (dual_timestamp_is_set(&m->timestamps[MANAGER_TIMESTAMP_INITRD])) { - kernel_usec = m->initrd_timestamp.monotonic - m->kernel_timestamp.monotonic; - initrd_usec = m->userspace_timestamp.monotonic - m->initrd_timestamp.monotonic; + /* The initrd case on bare-metal*/ + kernel_usec = m->timestamps[MANAGER_TIMESTAMP_INITRD].monotonic - m->timestamps[MANAGER_TIMESTAMP_KERNEL].monotonic; + initrd_usec = m->timestamps[MANAGER_TIMESTAMP_USERSPACE].monotonic - m->timestamps[MANAGER_TIMESTAMP_INITRD].monotonic; log_struct(LOG_INFO, "MESSAGE_ID=" SD_MESSAGE_STARTUP_FINISHED_STR, @@ -3060,7 +3051,9 @@ static void manager_notify_finished(Manager *m) { format_timespan(sum, sizeof(sum), total_usec, USEC_PER_MSEC)), NULL); } else { - kernel_usec = m->userspace_timestamp.monotonic - m->kernel_timestamp.monotonic; + /* The initrd-less case on bare-metal*/ + + kernel_usec = m->timestamps[MANAGER_TIMESTAMP_USERSPACE].monotonic - m->timestamps[MANAGER_TIMESTAMP_KERNEL].monotonic; initrd_usec = 0; log_struct(LOG_INFO, @@ -3074,8 +3067,9 @@ static void manager_notify_finished(Manager *m) { NULL); } } else { + /* The container case */ firmware_usec = loader_usec = initrd_usec = kernel_usec = 0; - total_usec = userspace_usec = m->finish_timestamp.monotonic - m->userspace_timestamp.monotonic; + total_usec = userspace_usec = m->timestamps[MANAGER_TIMESTAMP_FINISH].monotonic - m->timestamps[MANAGER_TIMESTAMP_USERSPACE].monotonic; log_struct(LOG_INFO, "MESSAGE_ID=" SD_MESSAGE_USER_STARTUP_FINISHED_STR, @@ -3142,10 +3136,10 @@ void manager_check_finished(Manager *m) { /* This is no longer the first boot */ manager_set_first_boot(m, false); - if (dual_timestamp_is_set(&m->finish_timestamp)) + if (dual_timestamp_is_set(m->timestamps + MANAGER_TIMESTAMP_FINISH)) return; - dual_timestamp_get(&m->finish_timestamp); + dual_timestamp_get(m->timestamps + MANAGER_TIMESTAMP_FINISH); manager_notify_finished(m); @@ -3500,7 +3494,7 @@ ManagerState manager_state(Manager *m) { assert(m); /* Did we ever finish booting? If not then we are still starting up */ - if (!dual_timestamp_is_set(&m->finish_timestamp)) { + if (!dual_timestamp_is_set(m->timestamps + MANAGER_TIMESTAMP_FINISH)) { u = manager_get_unit(m, SPECIAL_BASIC_TARGET); if (!u || !UNIT_IS_ACTIVE_OR_RELOADING(unit_active_state(u))) @@ -3836,3 +3830,20 @@ static const char *const manager_state_table[_MANAGER_STATE_MAX] = { }; DEFINE_STRING_TABLE_LOOKUP(manager_state, ManagerState); + +static const char *const manager_timestamp_table[_MANAGER_TIMESTAMP_MAX] = { + [MANAGER_TIMESTAMP_FIRMWARE] = "firmware", + [MANAGER_TIMESTAMP_LOADER] = "loader", + [MANAGER_TIMESTAMP_KERNEL] = "kernel", + [MANAGER_TIMESTAMP_INITRD] = "initrd", + [MANAGER_TIMESTAMP_USERSPACE] = "userspace", + [MANAGER_TIMESTAMP_FINISH] = "finish", + [MANAGER_TIMESTAMP_SECURITY_START] = "security-start", + [MANAGER_TIMESTAMP_SECURITY_FINISH] = "security-finish", + [MANAGER_TIMESTAMP_GENERATORS_START] = "generators-start", + [MANAGER_TIMESTAMP_GENERATORS_FINISH] = "generators-finish", + [MANAGER_TIMESTAMP_UNITS_LOAD_START] = "units-load-start", + [MANAGER_TIMESTAMP_UNITS_LOAD_FINISH] = "units-load-finish", +}; + +DEFINE_STRING_TABLE_LOOKUP(manager_timestamp, ManagerTimestamp); diff --git a/src/core/manager.h b/src/core/manager.h index 8f1357d37e..1812eb37ea 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -70,6 +70,24 @@ typedef enum StatusType { STATUS_TYPE_EMERGENCY, } StatusType; +typedef enum ManagerTimestamp { + MANAGER_TIMESTAMP_FIRMWARE, + MANAGER_TIMESTAMP_LOADER, + MANAGER_TIMESTAMP_KERNEL, + MANAGER_TIMESTAMP_INITRD, + MANAGER_TIMESTAMP_USERSPACE, + MANAGER_TIMESTAMP_FINISH, + + MANAGER_TIMESTAMP_SECURITY_START, + MANAGER_TIMESTAMP_SECURITY_FINISH, + MANAGER_TIMESTAMP_GENERATORS_START, + MANAGER_TIMESTAMP_GENERATORS_FINISH, + MANAGER_TIMESTAMP_UNITS_LOAD_START, + MANAGER_TIMESTAMP_UNITS_LOAD_FINISH, + _MANAGER_TIMESTAMP_MAX, + _MANAGER_TIMESTAMP_INVALID = -1, +} ManagerTimestamp; + #include "execute.h" #include "job.h" #include "path-lookup.h" @@ -171,19 +189,7 @@ struct Manager { usec_t runtime_watchdog; usec_t shutdown_watchdog; - dual_timestamp firmware_timestamp; - dual_timestamp loader_timestamp; - dual_timestamp kernel_timestamp; - dual_timestamp initrd_timestamp; - dual_timestamp userspace_timestamp; - dual_timestamp finish_timestamp; - - dual_timestamp security_start_timestamp; - dual_timestamp security_finish_timestamp; - dual_timestamp generators_start_timestamp; - dual_timestamp generators_finish_timestamp; - dual_timestamp units_load_start_timestamp; - dual_timestamp units_load_finish_timestamp; + dual_timestamp timestamps[_MANAGER_TIMESTAMP_MAX]; struct udev* udev; @@ -431,3 +437,6 @@ ManagerState manager_state_from_string(const char *s) _pure_; const char *manager_get_confirm_spawn(Manager *m); bool manager_is_confirm_spawn_disabled(Manager *m); void manager_disable_confirm_spawn(void); + +const char *manager_timestamp_to_string(ManagerTimestamp m) _const_; +ManagerTimestamp manager_timestamp_from_string(const char *s) _pure_; diff --git a/src/core/timer.c b/src/core/timer.c index 27f4f74b49..55197faa59 100644 --- a/src/core/timer.c +++ b/src/core/timer.c @@ -419,7 +419,7 @@ static void timer_enter_waiting(Timer *t, bool initial) { * our own startup. */ /* fall through */ case TIMER_STARTUP: - base = UNIT(t)->manager->userspace_timestamp.monotonic; + base = UNIT(t)->manager->timestamps[MANAGER_TIMESTAMP_USERSPACE].monotonic; break; case TIMER_UNIT_ACTIVE: From ad75b9e765147ce7a8d6afe382fa3cbb663a30a9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 20 Nov 2017 21:11:32 +0100 Subject: [PATCH 2/6] core: add manager_dump() call, and make it output timestamp data It's a wrapper around manager_dump_units() and manager_dump_jobs(), and outputs some additional timestamp data. Also, port two users of this over. --- src/core/dbus-manager.c | 3 +-- src/core/manager.c | 23 +++++++++++++++++++++-- src/core/manager.h | 1 + 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index b98351e84e..3261e1e86e 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -1307,8 +1307,7 @@ static int method_dump(sd_bus_message *message, void *userdata, sd_bus_error *er if (!f) return -ENOMEM; - manager_dump_units(m, f, NULL); - manager_dump_jobs(m, f, NULL); + manager_dump(m, f, NULL); r = fflush_and_check(f); if (r < 0) diff --git a/src/core/manager.c b/src/core/manager.c index 059482ff1d..b99bf22e23 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1711,6 +1711,26 @@ void manager_dump_units(Manager *s, FILE *f, const char *prefix) { unit_dump(u, f, prefix); } +void manager_dump(Manager *m, FILE *f, const char *prefix) { + ManagerTimestamp q; + + assert(m); + assert(f); + + for (q = 0; q < _MANAGER_TIMESTAMP_MAX; q++) { + char buf[FORMAT_TIMESTAMP_MAX]; + + if (dual_timestamp_is_set(m->timestamps + q)) + fprintf(f, "%sTimestamp %s: %s\n", + strempty(prefix), + manager_timestamp_to_string(q), + format_timestamp(buf, sizeof(buf), m->timestamps[q].realtime)); + } + + manager_dump_units(m, f, prefix); + manager_dump_jobs(m, f, prefix); +} + void manager_clear_jobs(Manager *m) { Job *j; @@ -2168,8 +2188,7 @@ static int manager_dispatch_signal_fd(sd_event_source *source, int fd, uint32_t break; } - manager_dump_units(m, f, "\t"); - manager_dump_jobs(m, f, "\t"); + manager_dump(m, f, NULL); r = fflush_and_check(f); if (r < 0) { diff --git a/src/core/manager.h b/src/core/manager.h index 1812eb37ea..eec063e465 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -374,6 +374,7 @@ int manager_propagate_reload(Manager *m, Unit *unit, JobMode mode, sd_bus_error void manager_dump_units(Manager *s, FILE *f, const char *prefix); void manager_dump_jobs(Manager *s, FILE *f, const char *prefix); +void manager_dump(Manager *s, FILE *f, const char *prefix); void manager_clear_jobs(Manager *m); From 713f6f901db285602ff88ebf1dcd4631293e85c1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 20 Nov 2017 21:20:44 +0100 Subject: [PATCH 3/6] manager: add manager_get_dump_string() It's like manager_dump(), but returns a string. This allows us to reduce some duplicate code. Also, while we are at it, turn off stdio locking while we write to the memory FILE *f. --- src/core/dbus-manager.c | 10 +--------- src/core/manager.c | 44 ++++++++++++++++++++++++++++++----------- src/core/manager.h | 1 + 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 3261e1e86e..5029f49a01 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -1289,9 +1289,7 @@ static int method_unsubscribe(sd_bus_message *message, void *userdata, sd_bus_er static int method_dump(sd_bus_message *message, void *userdata, sd_bus_error *error) { _cleanup_free_ char *dump = NULL; - _cleanup_fclose_ FILE *f = NULL; Manager *m = userdata; - size_t size; int r; assert(message); @@ -1303,13 +1301,7 @@ static int method_dump(sd_bus_message *message, void *userdata, sd_bus_error *er if (r < 0) return r; - f = open_memstream(&dump, &size); - if (!f) - return -ENOMEM; - - manager_dump(m, f, NULL); - - r = fflush_and_check(f); + r = manager_get_dump_string(m, &dump); if (r < 0) return r; diff --git a/src/core/manager.c b/src/core/manager.c index b99bf22e23..97ca77c9fa 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -1731,6 +1732,35 @@ void manager_dump(Manager *m, FILE *f, const char *prefix) { manager_dump_jobs(m, f, prefix); } +int manager_get_dump_string(Manager *m, char **ret) { + _cleanup_free_ char *dump = NULL; + _cleanup_fclose_ FILE *f = NULL; + size_t size; + int r; + + assert(m); + assert(ret); + + f = open_memstream(&dump, &size); + if (!f) + return -errno; + + __fsetlocking(f, FSETLOCKING_BYCALLER); + + manager_dump(m, f, NULL); + + r = fflush_and_check(f); + if (r < 0) + return r; + + f = safe_fclose(f); + + *ret = dump; + dump = NULL; + + return 0; +} + void manager_clear_jobs(Manager *m) { Job *j; @@ -2179,20 +2209,10 @@ static int manager_dispatch_signal_fd(sd_event_source *source, int fd, uint32_t case SIGUSR2: { _cleanup_free_ char *dump = NULL; - _cleanup_fclose_ FILE *f = NULL; - size_t size; - f = open_memstream(&dump, &size); - if (!f) { - log_warning_errno(errno, "Failed to allocate memory stream: %m"); - break; - } - - manager_dump(m, f, NULL); - - r = fflush_and_check(f); + r = manager_get_dump_string(m, &dump); if (r < 0) { - log_warning_errno(r, "Failed to write status stream: %m"); + log_warning_errno(errno, "Failed to acquire manager dump: %m"); break; } diff --git a/src/core/manager.h b/src/core/manager.h index eec063e465..4271e0c6c0 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -375,6 +375,7 @@ int manager_propagate_reload(Manager *m, Unit *unit, JobMode mode, sd_bus_error void manager_dump_units(Manager *s, FILE *f, const char *prefix); void manager_dump_jobs(Manager *s, FILE *f, const char *prefix); void manager_dump(Manager *s, FILE *f, const char *prefix); +int manager_get_dump_string(Manager *m, char **ret); void manager_clear_jobs(Manager *m); From 49d5666cc500d0efa2fd800403ac735707f976ed Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 20 Nov 2017 21:24:59 +0100 Subject: [PATCH 4/6] manager: introduce MANAGER_IS_FINISHED() macro Let's make our finished checks a bit more readable. Checking the timestamp is not entirely obvious, hence let's abstract that a bit by adding a macro that shows what we are doing here, not how we doing it. This is particularly useful if we want to change the definition of "finished" later on, in particular, when we try to fix #7023. --- src/core/dbus-manager.c | 2 +- src/core/manager.c | 4 ++-- src/core/manager.h | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 5029f49a01..e5b899fc01 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -323,7 +323,7 @@ static int property_get_progress( assert(reply); assert(m); - if (dual_timestamp_is_set(m->timestamps + MANAGER_TIMESTAMP_FINISH)) + if (MANAGER_IS_FINISHED(m)) d = 1.0; else d = 1.0 - ((double) hashmap_size(m->jobs) / (double) m->n_installed_jobs); diff --git a/src/core/manager.c b/src/core/manager.c index 97ca77c9fa..0340b41ef9 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3175,7 +3175,7 @@ void manager_check_finished(Manager *m) { /* This is no longer the first boot */ manager_set_first_boot(m, false); - if (dual_timestamp_is_set(m->timestamps + MANAGER_TIMESTAMP_FINISH)) + if (MANAGER_IS_FINISHED(m)) return; dual_timestamp_get(m->timestamps + MANAGER_TIMESTAMP_FINISH); @@ -3533,7 +3533,7 @@ ManagerState manager_state(Manager *m) { assert(m); /* Did we ever finish booting? If not then we are still starting up */ - if (!dual_timestamp_is_set(m->timestamps + MANAGER_TIMESTAMP_FINISH)) { + if (!MANAGER_IS_FINISHED(m)) { u = manager_get_unit(m, SPECIAL_BASIC_TARGET); if (!u || !UNIT_IS_ACTIVE_OR_RELOADING(unit_active_state(u))) diff --git a/src/core/manager.h b/src/core/manager.h index 4271e0c6c0..779e09bc97 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -352,6 +352,8 @@ struct Manager { #define MANAGER_IS_RELOADING(m) ((m)->n_reloading > 0) +#define MANAGER_IS_FINISHED(m) (dual_timestamp_is_set((m)->timestamps + MANAGER_TIMESTAMP_FINISH)) + int manager_new(UnitFileScope scope, unsigned test_run_flags, Manager **m); Manager* manager_free(Manager *m); From e68537f0ba1a4433ecdf58e609b1701ed7091abc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Nov 2017 10:10:28 +0100 Subject: [PATCH 5/6] core: make use of unit_active_or_pending() where we can Let's make use of unit_active_or_pending() where we can. Note that this change changes beaviour in one specific case: when shutdown.target is active we'll now also return that the system is in "stopping" state, not only when we try to get into it. That makes sense as shutdown.target is ordered before the actually shutdown units such as "systemd-poweroff.service", and if the state is queried between reaching those we should also report "stopping". --- src/core/manager.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index 0340b41ef9..d81502a92c 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3542,20 +3542,18 @@ ManagerState manager_state(Manager *m) { return MANAGER_STARTING; } - /* Is the special shutdown target queued? If so, we are in shutdown state */ + /* Is the special shutdown target active or queued? If so, we are in shutdown state */ u = manager_get_unit(m, SPECIAL_SHUTDOWN_TARGET); - if (u && u->job && IN_SET(u->job->type, JOB_START, JOB_RESTART, JOB_RELOAD_OR_START)) + if (unit_active_or_pending(u)) return MANAGER_STOPPING; /* Are the rescue or emergency targets active or queued? If so we are in maintenance state */ u = manager_get_unit(m, SPECIAL_RESCUE_TARGET); - if (u && (UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(u)) || - (u->job && IN_SET(u->job->type, JOB_START, JOB_RESTART, JOB_RELOAD_OR_START)))) + if (unit_active_or_pending(u)) return MANAGER_MAINTENANCE; u = manager_get_unit(m, SPECIAL_EMERGENCY_TARGET); - if (u && (UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(u)) || - (u->job && IN_SET(u->job->type, JOB_START, JOB_RESTART, JOB_RELOAD_OR_START)))) + if (unit_active_or_pending(u)) return MANAGER_MAINTENANCE; /* Are there any failed units? If so, we are in degraded mode */ From 213242a36b6b9b89cf0bec59f72a9ecc5b4812b6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Nov 2017 10:16:12 +0100 Subject: [PATCH 6/6] man: bootup(7) is one of our own man pages --- man/systemd.special.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/systemd.special.xml b/man/systemd.special.xml index 0033332d6d..18689a0a9e 100644 --- a/man/systemd.special.xml +++ b/man/systemd.special.xml @@ -1072,7 +1072,7 @@ PartOf=graphical-session.target systemd.socket5, systemd.target5, systemd.slice5, - bootup7, + bootup7, systemd-fstab-generator8