Merge pull request #9366 from poettering/udev-wants-dep-fix

udev SYSTEMD_WANTS property fix
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2018-06-21 00:04:03 +02:00 committed by GitHub
commit 88d56f289b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 79 additions and 38 deletions

4
TODO
View file

@ -21,6 +21,10 @@ Janitorial Clean-ups:
Features:
* the stop-when-unneded feature should be reworked: there should be a queue of
units, and we should only enqeueu stop jobs from a defer event that processes
queue instead of right-away when we assume that a unit is now unneeded.
* When reloading configuration PID 1 should reset all its properties to the
original defaults before calling parse_config()

View file

@ -108,10 +108,8 @@ static int verify_socket(Unit *u) {
/* This makes sure instance is created if necessary. */
r = socket_instantiate_service(SOCKET(u));
if (r < 0) {
log_unit_error_errno(u, r, "Socket cannot be started, failed to create instance: %m");
return r;
}
if (r < 0)
return log_unit_error_errno(u, r, "Socket cannot be started, failed to create instance: %m");
/* This checks both type of sockets */
if (UNIT_ISSET(SOCKET(u)->service)) {
@ -227,19 +225,16 @@ static int verify_unit(Unit *u, bool check_man) {
}
int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run_generators) {
_cleanup_free_ char *var = NULL;
Manager *m = NULL;
FILE *serial = NULL;
FDSet *fdset = NULL;
char **filename;
int r = 0, k;
Unit *units[strv_length(filenames)];
int i, count = 0;
const uint8_t flags = MANAGER_TEST_RUN_BASIC |
MANAGER_TEST_RUN_ENV_GENERATORS |
run_generators * MANAGER_TEST_RUN_GENERATORS;
_cleanup_(manager_freep) Manager *m = NULL;
Unit *units[strv_length(filenames)];
_cleanup_free_ char *var = NULL;
int r = 0, k, i, count = 0;
char **filename;
if (strv_isempty(filenames))
return 0;
@ -256,11 +251,9 @@ int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run
log_debug("Starting manager...");
r = manager_startup(m, serial, fdset);
if (r < 0) {
log_error_errno(r, "Failed to start manager: %m");
goto finish;
}
r = manager_startup(m, NULL, NULL);
if (r < 0)
return log_error_errno(r, "Failed to start manager: %m");
manager_clear_jobs(m);
@ -292,8 +285,5 @@ int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run
r = k;
}
finish:
manager_free(m);
return r;
}

View file

@ -6,6 +6,7 @@
#include "libudev.h"
#include "alloc-util.h"
#include "bus-error.h"
#include "dbus-device.h"
#include "device.h"
#include "log.h"
@ -107,6 +108,7 @@ static void device_done(Unit *u) {
assert(d);
device_unset_sysfs(d);
d->wants_property = strv_free(d->wants_property);
}
static void device_set_state(Device *d, DeviceState state) {
@ -276,6 +278,14 @@ static void device_dump(Unit *u, FILE *f, const char *prefix) {
prefix, device_state_to_string(d->state),
prefix, strna(d->sysfs),
prefix, strna(s));
if (!strv_isempty(d->wants_property)) {
char **i;
STRV_FOREACH(i, d->wants_property)
fprintf(f, "%sudev SYSTEMD_WANTS: %s\n",
prefix, *i);
}
}
_pure_ static UnitActiveState device_active_state(Unit *u) {
@ -331,10 +341,12 @@ static int device_update_description(Unit *u, struct udev_device *dev, const cha
}
static int device_add_udev_wants(Unit *u, struct udev_device *dev) {
_cleanup_strv_free_ char **added = NULL;
const char *wants, *property;
Device *d = DEVICE(u);
int r;
assert(u);
assert(d);
assert(dev);
property = MANAGER_IS_USER(u->manager) ? "SYSTEMD_USER_WANTS" : "SYSTEMD_WANTS";
@ -348,21 +360,21 @@ static int device_add_udev_wants(Unit *u, struct udev_device *dev) {
r = extract_first_word(&wants, &word, NULL, EXTRACT_QUOTES);
if (r == 0)
return 0;
break;
if (r == -ENOMEM)
return log_oom();
if (r < 0)
return log_unit_error_errno(u, r, "Failed to parse property %s with value %s: %m", property, wants);
if (unit_name_is_valid(word, UNIT_NAME_TEMPLATE) && DEVICE(u)->sysfs) {
if (unit_name_is_valid(word, UNIT_NAME_TEMPLATE) && d->sysfs) {
_cleanup_free_ char *escaped = NULL;
/* If the unit name is specified as template, then automatically fill in the sysfs path of the
* device as instance name, properly escaped. */
r = unit_name_path_escape(DEVICE(u)->sysfs, &escaped);
r = unit_name_path_escape(d->sysfs, &escaped);
if (r < 0)
return log_unit_error_errno(u, r, "Failed to escape %s: %m", DEVICE(u)->sysfs);
return log_unit_error_errno(u, r, "Failed to escape %s: %m", d->sysfs);
r = unit_name_replace_instance(word, escaped, &k);
if (r < 0)
@ -378,7 +390,43 @@ static int device_add_udev_wants(Unit *u, struct udev_device *dev) {
r = unit_add_dependency_by_name(u, UNIT_WANTS, k, NULL, true, UNIT_DEPENDENCY_UDEV);
if (r < 0)
return log_unit_error_errno(u, r, "Failed to add Wants= dependency: %m");
r = strv_push(&added, k);
if (r < 0)
return log_oom();
k = NULL;
}
if (d->state != DEVICE_DEAD) {
char **i;
/* So here's a special hack, to compensate for the fact that the udev database's reload cycles are not
* synchronized with our own reload cycles: when we detect that the SYSTEMD_WANTS property of a device
* changes while the device unit is already up, let's manually trigger any new units listed in it not
* seen before. This typically appens during the boot-time switch root transition, as udev devices
* will generally already be up in the initrd, but SYSTEMD_WANTS properties get then added through udev
* rules only available on the host system, and thus only when the initial udev coldplug trigger runs.
*
* We do this only if the device has been up already when we parse this, as otherwise the usual
* dependency logic that is run from the dead plugged transition will trigger these deps. */
STRV_FOREACH(i, added) {
_cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
if (strv_contains(d->wants_property, *i)) /* Was this unit already listed before? */
continue;
r = manager_add_job_by_name(u->manager, JOB_START, *i, JOB_FAIL, &error, NULL);
if (r < 0)
log_unit_warning_errno(u, r, "Failed to enqueue SYSTEMD_WANTS= job, ignoring: %s", bus_error_message(&error, r));
}
}
strv_free(d->wants_property);
d->wants_property = TAKE_PTR(added);
return 0;
}
static bool device_is_bound_by_mounts(Device *d, struct udev_device *dev) {

View file

@ -30,6 +30,9 @@ struct Device {
DeviceFound found, deserialized_found, enumerated_found;
bool bind_mounts;
/* The SYSTEMD_WANTS udev property for this device the last time we saw it */
char **wants_property;
};
extern const UnitVTable device_vtable;

View file

@ -3108,8 +3108,9 @@ int manager_serialize(Manager *m, FILE *f, FDSet *fds, bool switching_root) {
assert(m->n_reloading > 0);
m->n_reloading--;
if (ferror(f))
return -EIO;
r = fflush_and_check(f);
if (r < 0)
return r;
r = bus_fdset_add_all(m, fds);
if (r < 0)

View file

@ -17,8 +17,6 @@ static int test_cgroup_mask(void) {
_cleanup_(rm_rf_physical_and_freep) char *runtime_dir = NULL;
_cleanup_(manager_freep) Manager *m = NULL;
Unit *son, *daughter, *parent, *root, *grandchild, *parent_deep;
FILE *serial = NULL;
FDSet *fdset = NULL;
int r;
r = enter_cgroup_subroot();
@ -48,7 +46,7 @@ static int test_cgroup_mask(void) {
m->default_tasks_max = (uint64_t) -1;
assert_se(r >= 0);
assert_se(manager_startup(m, serial, fdset) >= 0);
assert_se(manager_startup(m, NULL, NULL) >= 0);
/* Load units and verify hierarchy. */
assert_se(manager_load_startable_unit_or_warn(m, "parent.slice", NULL, &parent) >= 0);

View file

@ -15,8 +15,6 @@ int main(int argc, char *argv[]) {
_cleanup_(sd_bus_error_free) sd_bus_error err = SD_BUS_ERROR_NULL;
_cleanup_(manager_freep) Manager *m = NULL;
Unit *a = NULL, *b = NULL, *c = NULL, *d = NULL, *e = NULL, *g = NULL, *h = NULL, *unit_with_multiple_dashes = NULL;
FILE *serial = NULL;
FDSet *fdset = NULL;
Job *j;
int r;
@ -39,7 +37,7 @@ int main(int argc, char *argv[]) {
return EXIT_TEST_SKIP;
}
assert_se(r >= 0);
assert_se(manager_startup(m, serial, fdset) >= 0);
assert_se(manager_startup(m, NULL, NULL) >= 0);
printf("Load1:\n");
assert_se(manager_load_startable_unit_or_warn(m, "a.service", NULL, &a) >= 0);

View file

@ -17,8 +17,6 @@ int main(int argc, char *argv[]) {
_cleanup_(manager_freep) Manager *m = NULL;
Unit *idle_ok, *idle_bad, *rr_ok, *rr_bad, *rr_sched;
Service *ser;
FILE *serial = NULL;
FDSet *fdset = NULL;
int r;
r = enter_cgroup_subroot();
@ -36,7 +34,7 @@ int main(int argc, char *argv[]) {
return EXIT_TEST_SKIP;
}
assert_se(r >= 0);
assert_se(manager_startup(m, serial, fdset) >= 0);
assert_se(manager_startup(m, NULL, NULL) >= 0);
/* load idle ok */
assert_se(manager_load_startable_unit_or_warn(m, "sched_idle_ok.service", NULL, &idle_ok) >= 0);

View file

@ -19,4 +19,5 @@ ConditionPathIsReadWrite=/sys
[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=@rootbindir@/udevadm trigger --type=subsystems --action=add ; @rootbindir@/udevadm trigger --type=devices --action=add
ExecStart=@rootbindir@/udevadm trigger --type=subsystems --action=add
ExecStart=@rootbindir@/udevadm trigger --type=devices --action=add