From 0377cd2936ae5cac0c9d76a4b58889f121c097c4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Sep 2020 19:49:33 +0200 Subject: [PATCH] core: propagate triggered unit in more load states In 4c2ef3276735ad9f7fccf33f5bdcbe7d8751e7ec we enabled propagating triggered unit state to the triggering unit for service units in more load states, so that we don't accidentally stop tracking state correctly. Do the same for our other triggering unit states: automounts, paths, and timers. Also, make this an assertion rather than a simple test. After all it should never happen that we get called for half-loaded units or units of the wrong type. The load routines should already have made this impossible. --- src/core/automount.c | 4 ++-- src/core/path.c | 7 +++---- src/core/socket.c | 9 ++------- src/core/timer.c | 4 ++-- src/core/transaction.c | 2 +- src/core/unit.h | 4 ++++ 6 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/core/automount.c b/src/core/automount.c index 782f7680aa..4db763f84e 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -507,8 +507,8 @@ static void automount_trigger_notify(Unit *u, Unit *other) { assert(other); /* Filter out invocations with bogus state */ - if (other->load_state != UNIT_LOADED || other->type != UNIT_MOUNT) - return; + assert(UNIT_IS_LOAD_COMPLETE(other->load_state)); + assert(other->type == UNIT_MOUNT); /* Don't propagate state changes from the mount if we are already down */ if (!IN_SET(a->state, AUTOMOUNT_WAITING, AUTOMOUNT_RUNNING)) diff --git a/src/core/path.c b/src/core/path.c index 1c3c28e341..8ffec72ede 100644 --- a/src/core/path.c +++ b/src/core/path.c @@ -748,11 +748,10 @@ static void path_trigger_notify(Unit *u, Unit *other) { assert(u); assert(other); - /* Invoked whenever the unit we trigger changes state or gains - * or loses a job */ + /* Invoked whenever the unit we trigger changes state or gains or loses a job */ - if (other->load_state != UNIT_LOADED) - return; + /* Filter out invocations with bogus state */ + assert(UNIT_IS_LOAD_COMPLETE(other->load_state)); if (p->state == PATH_RUNNING && UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(other))) { diff --git a/src/core/socket.c b/src/core/socket.c index 02841f0e24..be7d364084 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -3265,13 +3265,8 @@ static void socket_trigger_notify(Unit *u, Unit *other) { assert(other); /* Filter out invocations with bogus state */ - if (!IN_SET(other->load_state, - UNIT_LOADED, - UNIT_NOT_FOUND, - UNIT_BAD_SETTING, - UNIT_ERROR, - UNIT_MASKED) || other->type != UNIT_SERVICE) - return; + assert(UNIT_IS_LOAD_COMPLETE(other->load_state)); + assert(other->type == UNIT_SERVICE); /* Don't propagate state changes from the service if we are already down */ if (!IN_SET(s->state, SOCKET_RUNNING, SOCKET_LISTENING)) diff --git a/src/core/timer.c b/src/core/timer.c index 03a9c14f76..94388f0727 100644 --- a/src/core/timer.c +++ b/src/core/timer.c @@ -746,8 +746,8 @@ static void timer_trigger_notify(Unit *u, Unit *other) { assert(u); assert(other); - if (other->load_state != UNIT_LOADED) - return; + /* Filter out invocations with bogus state */ + assert(UNIT_IS_LOAD_COMPLETE(other->load_state)); /* Reenable all timers that depend on unit state */ LIST_FOREACH(value, v, t->values) diff --git a/src/core/transaction.c b/src/core/transaction.c index 6cf305f4b5..f4cdbfe6f5 100644 --- a/src/core/transaction.c +++ b/src/core/transaction.c @@ -938,7 +938,7 @@ int transaction_add_job_and_dependencies( /* Safety check that the unit is a valid state, i.e. not in UNIT_STUB or UNIT_MERGED which should only be set * temporarily. */ - if (!IN_SET(unit->load_state, UNIT_LOADED, UNIT_ERROR, UNIT_NOT_FOUND, UNIT_BAD_SETTING, UNIT_MASKED)) + if (!UNIT_IS_LOAD_COMPLETE(unit->load_state)) return sd_bus_error_setf(e, BUS_ERROR_LOAD_FAILED, "Unit %s is not loaded properly.", unit->id); if (type != JOB_STOP) { diff --git a/src/core/unit.h b/src/core/unit.h index 2205b7b163..35873d57bc 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -49,6 +49,10 @@ static inline bool UNIT_IS_INACTIVE_OR_FAILED(UnitActiveState t) { return IN_SET(t, UNIT_INACTIVE, UNIT_FAILED); } +static inline bool UNIT_IS_LOAD_COMPLETE(UnitLoadState t) { + return t >= 0 && t < _UNIT_LOAD_STATE_MAX && t != UNIT_STUB && t != UNIT_MERGED; +} + /* Stores the 'reason' a dependency was created as a bit mask, i.e. due to which configuration source it came to be. We * use this so that we can selectively flush out parts of dependencies again. Note that the same dependency might be * created as a result of multiple "reasons", hence the bitmask. */