From ba64af90ecf48f7653a04bf3af1291385c9a69b8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 10 Nov 2015 20:36:37 +0100 Subject: core: change return value of the unit's enumerate() call to void We cannot handle enumeration failures in a sensible way, hence let's try hard to continue without making such failures fatal, and log about it with precise error messages. --- src/core/device.c | 35 ++++++++++++++++++++++------------- src/core/manager.c | 16 ++++------------ src/core/manager.h | 2 +- src/core/mount.c | 28 +++++++++++++++++++--------- src/core/scope.c | 13 +++++++------ src/core/slice.c | 13 +++++++------ src/core/swap.c | 23 ++++++++++++++++------- src/core/unit.h | 2 +- 8 files changed, 77 insertions(+), 55 deletions(-) (limited to 'src/core') diff --git a/src/core/device.c b/src/core/device.c index 23ee7aee7e..b54352d733 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -602,7 +602,7 @@ static void device_shutdown(Manager *m) { m->devices_by_sysfs = hashmap_free(m->devices_by_sysfs); } -static int device_enumerate(Manager *m) { +static void device_enumerate(Manager *m) { _cleanup_udev_enumerate_unref_ struct udev_enumerate *e = NULL; struct udev_list_entry *item = NULL, *first = NULL; int r; @@ -612,7 +612,7 @@ static int device_enumerate(Manager *m) { if (!m->udev_monitor) { m->udev_monitor = udev_monitor_new_from_netlink(m->udev, "udev"); if (!m->udev_monitor) { - r = -ENOMEM; + log_oom(); goto fail; } @@ -622,37 +622,49 @@ static int device_enumerate(Manager *m) { (void) udev_monitor_set_receive_buffer_size(m->udev_monitor, 128*1024*1024); r = udev_monitor_filter_add_match_tag(m->udev_monitor, "systemd"); - if (r < 0) + if (r < 0) { + log_error_errno(r, "Failed to add udev tag match: %m"); goto fail; + } r = udev_monitor_enable_receiving(m->udev_monitor); - if (r < 0) + if (r < 0) { + log_error_errno(r, "Failed to enable udev event reception: %m"); goto fail; + } r = sd_event_add_io(m->event, &m->udev_event_source, udev_monitor_get_fd(m->udev_monitor), EPOLLIN, device_dispatch_io, m); - if (r < 0) + if (r < 0) { + log_error_errno(r, "Failed to watch udev file descriptor: %m"); goto fail; + } (void) sd_event_source_set_description(m->udev_event_source, "device"); } e = udev_enumerate_new(m->udev); if (!e) { - r = -ENOMEM; + log_oom(); goto fail; } r = udev_enumerate_add_match_tag(e, "systemd"); - if (r < 0) + if (r < 0) { + log_error_errno(r, "Failed to create udev tag enumeration: %m"); goto fail; + } r = udev_enumerate_add_match_is_initialized(e); - if (r < 0) + if (r < 0) { + log_error_errno(r, "Failed to install initialization match into enumeration: %m"); goto fail; + } r = udev_enumerate_scan_devices(e); - if (r < 0) + if (r < 0) { + log_error_errno(r, "Failed to enumerate devices: %m"); goto fail; + } first = udev_enumerate_get_list_entry(e); udev_list_entry_foreach(item, first) { @@ -675,13 +687,10 @@ static int device_enumerate(Manager *m) { device_update_found_by_sysfs(m, sysfs, true, DEVICE_FOUND_UDEV, false); } - return 0; + return; fail: - log_error_errno(r, "Failed to enumerate devices: %m"); - device_shutdown(m); - return r; } static int device_dispatch_io(sd_event_source *source, int fd, uint32_t revents, void *userdata) { diff --git a/src/core/manager.c b/src/core/manager.c index 7c3a020c4a..dbe11c2b4d 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -993,8 +993,7 @@ Manager* manager_free(Manager *m) { return NULL; } -int manager_enumerate(Manager *m) { - int r = 0; +void manager_enumerate(Manager *m) { UnitType c; assert(m); @@ -1002,8 +1001,6 @@ int manager_enumerate(Manager *m) { /* Let's ask every type to load all units from disk/kernel * that it might know */ for (c = 0; c < _UNIT_TYPE_MAX; c++) { - int q; - if (!unit_type_supported(c)) { log_debug("Unit type .%s is not supported on this system.", unit_type_to_string(c)); continue; @@ -1012,13 +1009,10 @@ int manager_enumerate(Manager *m) { if (!unit_vtable[c]->enumerate) continue; - q = unit_vtable[c]->enumerate(m); - if (q < 0) - r = q; + unit_vtable[c]->enumerate(m); } manager_dispatch_load_queue(m); - return r; } static void manager_coldplug(Manager *m) { @@ -1152,7 +1146,7 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { /* First, enumerate what we can from all config files */ dual_timestamp_get(&m->units_load_start_timestamp); - r = manager_enumerate(m); + manager_enumerate(m); dual_timestamp_get(&m->units_load_finish_timestamp); /* Second, deserialize if there is something to deserialize */ @@ -2554,9 +2548,7 @@ int manager_reload(Manager *m) { manager_build_unit_path_cache(m); /* First, enumerate what we can from all config files */ - q = manager_enumerate(m); - if (q < 0 && r >= 0) - r = q; + manager_enumerate(m); /* Second, deserialize our stored data */ q = manager_deserialize(m, f, fds); diff --git a/src/core/manager.h b/src/core/manager.h index 38d2770e97..c57a645c33 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -316,7 +316,7 @@ struct Manager { int manager_new(ManagerRunningAs running_as, bool test_run, Manager **m); Manager* manager_free(Manager *m); -int manager_enumerate(Manager *m); +void manager_enumerate(Manager *m); int manager_startup(Manager *m, FILE *serialization, FDSet *fds); Job *manager_get_job(Manager *m, uint32_t id); diff --git a/src/core/mount.c b/src/core/mount.c index 950d5d76d5..d3766ba934 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1596,7 +1596,7 @@ static int mount_get_timeout(Unit *u, uint64_t *timeout) { return 1; } -static int mount_enumerate(Manager *m) { +static void mount_enumerate(Manager *m) { int r; assert(m); @@ -1608,29 +1608,40 @@ static int mount_enumerate(Manager *m) { m->mount_monitor = mnt_new_monitor(); if (!m->mount_monitor) { - r = -ENOMEM; + log_oom(); goto fail; } r = mnt_monitor_enable_kernel(m->mount_monitor, 1); - if (r < 0) + if (r < 0) { + log_error_errno(r, "Failed to enable watching of kernel mount events: %m"); goto fail; + } + r = mnt_monitor_enable_userspace(m->mount_monitor, 1, NULL); - if (r < 0) + if (r < 0) { + log_error_errno(r, "Failed to enable watching of userspace mount events: %m"); goto fail; + } /* mnt_unref_monitor() will close the fd */ fd = r = mnt_monitor_get_fd(m->mount_monitor); - if (r < 0) + if (r < 0) { + log_error_errno(r, "Failed to acquire watch file descriptor: %m"); goto fail; + } r = sd_event_add_io(m->event, &m->mount_event_source, fd, EPOLLIN, mount_dispatch_io, m); - if (r < 0) + if (r < 0) { + log_error_errno(r, "Failed to watch mount file descriptor: %m"); goto fail; + } r = sd_event_source_set_priority(m->mount_event_source, -10); - if (r < 0) + if (r < 0) { + log_error_errno(r, "Failed to adjust mount watch priority: %m"); goto fail; + } (void) sd_event_source_set_description(m->mount_event_source, "mount-monitor-dispatch"); } @@ -1639,11 +1650,10 @@ static int mount_enumerate(Manager *m) { if (r < 0) goto fail; - return 0; + return; fail: mount_shutdown(m); - return r; } static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents, void *userdata) { diff --git a/src/core/scope.c b/src/core/scope.c index 6bacb226e8..48a5eaae48 100644 --- a/src/core/scope.c +++ b/src/core/scope.c @@ -510,7 +510,7 @@ _pure_ static const char *scope_sub_state_to_string(Unit *u) { return scope_state_to_string(SCOPE(u)->state); } -static int scope_enumerate(Manager *m) { +static void scope_enumerate(Manager *m) { Unit *u; int r; @@ -524,13 +524,16 @@ static int scope_enumerate(Manager *m) { u = manager_get_unit(m, SPECIAL_INIT_SCOPE); if (!u) { u = unit_new(m, sizeof(Scope)); - if (!u) - return log_oom(); + if (!u) { + log_oom(); + return; + } r = unit_add_name(u, SPECIAL_INIT_SCOPE); if (r < 0) { unit_free(u); - return log_error_errno(r, "Failed to add init.scope name"); + log_error_errno(r, "Failed to add init.scope name"); + return; } } @@ -551,8 +554,6 @@ static int scope_enumerate(Manager *m) { unit_add_to_load_queue(u); unit_add_to_dbus_queue(u); - - return 0; } static const char* const scope_result_table[_SCOPE_RESULT_MAX] = { diff --git a/src/core/slice.c b/src/core/slice.c index 4602144150..2aa3fbf8be 100644 --- a/src/core/slice.c +++ b/src/core/slice.c @@ -255,7 +255,7 @@ _pure_ static const char *slice_sub_state_to_string(Unit *u) { return slice_state_to_string(SLICE(u)->state); } -static int slice_enumerate(Manager *m) { +static void slice_enumerate(Manager *m) { Unit *u; int r; @@ -264,13 +264,16 @@ static int slice_enumerate(Manager *m) { u = manager_get_unit(m, SPECIAL_ROOT_SLICE); if (!u) { u = unit_new(m, sizeof(Slice)); - if (!u) - return log_oom(); + if (!u) { + log_oom(); + return; + } r = unit_add_name(u, SPECIAL_ROOT_SLICE); if (r < 0) { unit_free(u); - return log_error_errno(r, "Failed to add -.slice name"); + log_error_errno(r, "Failed to add -.slice name"); + return; } } @@ -288,8 +291,6 @@ static int slice_enumerate(Manager *m) { unit_add_to_load_queue(u); unit_add_to_dbus_queue(u); - - return 0; } const UnitVTable slice_vtable = { diff --git a/src/core/swap.c b/src/core/swap.c index baaa27b6a3..e44cffc2e8 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -1268,26 +1268,36 @@ static void swap_shutdown(Manager *m) { m->swaps_by_devnode = hashmap_free(m->swaps_by_devnode); } -static int swap_enumerate(Manager *m) { +static void swap_enumerate(Manager *m) { int r; assert(m); if (!m->proc_swaps) { m->proc_swaps = fopen("/proc/swaps", "re"); - if (!m->proc_swaps) - return errno == ENOENT ? 0 : -errno; + if (!m->proc_swaps) { + if (errno == ENOENT) + log_debug("Not swap enabled, skipping enumeration"); + else + log_error_errno(errno, "Failed to open /proc/swaps: %m"); + + return; + } r = sd_event_add_io(m->event, &m->swap_event_source, fileno(m->proc_swaps), EPOLLPRI, swap_dispatch_io, m); - if (r < 0) + if (r < 0) { + log_error_errno(r, "Failed to watch /proc/swaps: %m"); goto fail; + } /* Dispatch this before we dispatch SIGCHLD, so that * we always get the events from /proc/swaps before * the SIGCHLD of /sbin/swapon. */ r = sd_event_source_set_priority(m->swap_event_source, -10); - if (r < 0) + if (r < 0) { + log_error_errno(r, "Failed to change /proc/swaps priority: %m"); goto fail; + } (void) sd_event_source_set_description(m->swap_event_source, "swap-proc"); } @@ -1296,11 +1306,10 @@ static int swap_enumerate(Manager *m) { if (r < 0) goto fail; - return 0; + return; fail: swap_shutdown(m); - return r; } int swap_process_device_new(Manager *m, struct udev_device *dev) { diff --git a/src/core/unit.h b/src/core/unit.h index 6f775c5ce1..eb2af229b5 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -389,7 +389,7 @@ struct UnitVTable { * everything that is loaded here should still stay in * inactive state. It is the job of the coldplug() call above * to put the units into the initial state. */ - int (*enumerate)(Manager *m); + void (*enumerate)(Manager *m); /* Type specific cleanups. */ void (*shutdown)(Manager *m); -- cgit v1.2.3-54-g00ecf