Merge pull request #29333 from YHNdnzj/systemctl-warn-half-masked

systemctl-enable: warn if disabled/masked unit has active triggering units
This commit is contained in:
Mike Yuan
2023-09-28 17:58:21 +08:00
committed by GitHub
6 changed files with 115 additions and 52 deletions
+17 -1
View File
@@ -443,6 +443,10 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err
It will <emphasis>not</emphasis> fail if any of the commands configured to stop the unit
(<varname>ExecStop=</varname>, etc.) fail, because the manager will still forcibly terminate the
unit.</para>
<para>If a unit that gets stopped can still be triggered by other units, a warning containing
the names of the triggering units is shown. <option>--no-warn</option> can be used to suppress
the warning.</para>
</listitem>
</varlistentry>
<varlistentry>
@@ -892,6 +896,10 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err
executed. This output may be suppressed by passing <option>--quiet</option>.
</para>
<para>If a unit gets disabled but its triggering units are still active, a warning containing
the names of the triggering units is shown. <option>--no-warn</option> can be used to suppress
the warning.</para>
<para>When this command is used with <option>--user</option>, the units being operated on might
still be enabled in global scope, and thus get started automatically even after a successful
disablement in user scope. In this case, a warning about it is shown, which can be suppressed
@@ -1079,6 +1087,10 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err
<option>--user</option> mode, in which case the directories are below the user's home directory
however.</para>
<para>If a unit gets masked but its triggering units are still active, a warning containing
the names of the triggering units is shown. <option>--no-warn</option> can be used to suppress
the warning.</para>
<xi:include href="version-info.xml" xpointer="v238"/>
</listitem>
</varlistentry>
@@ -2233,7 +2245,11 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err
</listitem>
<listitem>
<para>when using <command>disable</command> combined with <option>--user</option> on units
that are enabled in global scope.</para>
that are enabled in global scope,</para>
</listitem>
<listitem>
<para>when a <command>stop</command>-ped, <command>disable</command>-d, or <command>mask</command>-ed
unit still has active triggering units.</para>
</listitem>
</itemizedlist>
</para>
+1 -6
View File
@@ -318,7 +318,6 @@ int verb_edit(int argc, char *argv[], void *userdata) {
.remove_parent = !arg_full,
.overwrite_with_origin = true,
};
_cleanup_(lookup_paths_free) LookupPaths lp = {};
_cleanup_strv_free_ char **names = NULL;
sd_bus *bus;
int r;
@@ -329,10 +328,6 @@ int verb_edit(int argc, char *argv[], void *userdata) {
if (arg_transport != BUS_TRANSPORT_LOCAL)
return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Cannot edit units remotely.");
r = lookup_paths_init_or_warn(&lp, arg_runtime_scope, 0, arg_root);
if (r < 0)
return r;
r = mac_init();
if (r < 0)
return r;
@@ -348,7 +343,7 @@ int verb_edit(int argc, char *argv[], void *userdata) {
return log_error_errno(SYNTHETIC_ERRNO(ENOENT), "No units matched the specified patterns.");
STRV_FOREACH(tmp, names) {
r = unit_is_masked(bus, &lp, *tmp);
r = unit_is_masked(bus, *tmp);
if (r < 0)
return r;
if (r > 0)
+22 -10
View File
@@ -138,7 +138,8 @@ int verb_enable(int argc, char *argv[], void *userdata) {
_cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
bool expect_carries_install_info = false;
bool send_runtime = true, send_force = true, send_preset_mode = false;
const char *method;
const char *method, *warn_trigger_operation = NULL;
bool warn_trigger_ignore_masked = true; /* suppress "used uninitialized" warning */
sd_bus *bus;
if (STR_IN_SET(verb, "mask", "unmask")) {
@@ -170,6 +171,9 @@ int verb_enable(int argc, char *argv[], void *userdata) {
method = "DisableUnitFilesWithFlagsAndInstallInfo";
expect_carries_install_info = true;
send_force = false;
warn_trigger_operation = "Disabling";
warn_trigger_ignore_masked = true;
} else if (streq(verb, "reenable")) {
method = "ReenableUnitFiles";
expect_carries_install_info = true;
@@ -185,9 +189,12 @@ int verb_enable(int argc, char *argv[], void *userdata) {
expect_carries_install_info = true;
ignore_carries_install_info = true;
} else if (streq(verb, "mask"))
} else if (streq(verb, "mask")) {
method = "MaskUnitFiles";
else if (streq(verb, "unmask")) {
warn_trigger_operation = "Masking";
warn_trigger_ignore_masked = false;
} else if (streq(verb, "unmask")) {
method = "UnmaskUnitFiles";
send_force = false;
} else if (streq(verb, "revert")) {
@@ -245,6 +252,10 @@ int verb_enable(int argc, char *argv[], void *userdata) {
if (r < 0)
return r;
}
if (warn_trigger_operation && !arg_quiet && !arg_no_warn)
STRV_FOREACH(unit, names)
warn_triggering_units(bus, *unit, warn_trigger_operation, warn_trigger_ignore_masked);
}
if (carries_install_info == 0 && !ignore_carries_install_info)
@@ -254,7 +265,7 @@ int verb_enable(int argc, char *argv[], void *userdata) {
" \n" /* trick: the space is needed so that the line does not get stripped from output */
"Possible reasons for having this kind of units are:\n"
"%1$s A unit may be statically enabled by being symlinked from another unit's\n"
" .wants/ or .requires/ directory.\n"
" .wants/, .requires/, or .upholds/ directory.\n"
"%1$s A unit's purpose may be to act as a helper for some other unit which has\n"
" a requirement dependency on it.\n"
"%1$s A unit may be started when needed via activation (socket, path, timer,\n"
@@ -267,7 +278,8 @@ int verb_enable(int argc, char *argv[], void *userdata) {
/* If some of the units are disabled in user scope but still enabled in global scope,
* we emit a warning for that. */
_cleanup_strv_free_ char **enabled_in_global_scope = NULL;
/* No strv_free here, strings are owned by 'names' */
_cleanup_free_ char **enabled_in_global_scope = NULL;
STRV_FOREACH(name, names) {
UnitFileState state;
@@ -279,24 +291,24 @@ int verb_enable(int argc, char *argv[], void *userdata) {
return log_error_errno(r, "Failed to get unit file state for %s: %m", *name);
if (IN_SET(state, UNIT_FILE_ENABLED, UNIT_FILE_ENABLED_RUNTIME)) {
r = strv_extend(&enabled_in_global_scope, *name);
r = strv_push(&enabled_in_global_scope, *name);
if (r < 0)
return log_oom();
}
}
if (!strv_isempty(enabled_in_global_scope)) {
_cleanup_free_ char *units = NULL;
_cleanup_free_ char *joined = NULL;
units = strv_join(enabled_in_global_scope, ", ");
if (!units)
joined = strv_join(enabled_in_global_scope, ", ");
if (!joined)
return log_oom();
log_notice("The following unit files have been enabled in global scope. This means\n"
"they will still be started automatically after a successful disablement\n"
"in user scope:\n"
"%s",
units);
joined);
}
}
+3 -3
View File
@@ -392,9 +392,9 @@ int verb_start(int argc, char *argv[], void *userdata) {
/* When stopping units, warn if they can still be triggered by
* another active unit (socket, path, timer) */
if (!arg_quiet)
STRV_FOREACH(name, stopped_units)
(void) check_triggering_units(bus, *name);
if (!arg_quiet && !arg_no_warn)
STRV_FOREACH(unit, stopped_units)
warn_triggering_units(bus, *unit, "Stopping", /* ignore_masked = */ true);
}
if (arg_wait) {
+69 -30
View File
@@ -315,26 +315,33 @@ int expand_unit_names(sd_bus *bus, char **names, const char* suffix, char ***ret
return 0;
}
int check_triggering_units(sd_bus *bus, const char *unit) {
int get_active_triggering_units(sd_bus *bus, const char *unit, bool ignore_masked, char ***ret) {
_cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
_cleanup_free_ char *n = NULL, *dbus_path = NULL, *load_state = NULL;
_cleanup_strv_free_ char **triggered_by = NULL;
_cleanup_strv_free_ char **triggered_by = NULL, **active = NULL;
_cleanup_free_ char *name = NULL, *dbus_path = NULL;
int r;
r = unit_name_mangle(unit, 0, &n);
if (r < 0)
return log_error_errno(r, "Failed to mangle unit name: %m");
assert(bus);
assert(unit);
assert(ret);
r = unit_load_state(bus, n, &load_state);
r = unit_name_mangle(unit, 0, &name);
if (r < 0)
return r;
if (streq(load_state, "masked"))
return 0;
if (ignore_masked) {
r = unit_is_masked(bus, name);
if (r < 0)
return r;
if (r > 0) {
*ret = NULL;
return 0;
}
}
dbus_path = unit_dbus_path_from_name(n);
dbus_path = unit_dbus_path_from_name(name);
if (!dbus_path)
return log_oom();
return -ENOMEM;
r = sd_bus_get_property_strv(
bus,
@@ -345,9 +352,9 @@ int check_triggering_units(sd_bus *bus, const char *unit) {
&error,
&triggered_by);
if (r < 0)
return log_error_errno(r, "Failed to get triggered by array of %s: %s", n, bus_error_message(&error, r));
return log_debug_errno(r, "Failed to get TriggeredBy property of unit '%s': %s",
name, bus_error_message(&error, r));
bool first = true;
STRV_FOREACH(i, triggered_by) {
UnitActiveState active_state;
@@ -358,17 +365,43 @@ int check_triggering_units(sd_bus *bus, const char *unit) {
if (!IN_SET(active_state, UNIT_ACTIVE, UNIT_RELOADING))
continue;
if (first) {
log_warning("Warning: Stopping %s, but it can still be activated by:", n);
first = false;
}
log_warning(" %s", *i);
r = strv_extend(&active, *i);
if (r < 0)
return r;
}
*ret = TAKE_PTR(active);
return 0;
}
void warn_triggering_units(sd_bus *bus, const char *unit, const char *operation, bool ignore_masked) {
_cleanup_strv_free_ char **triggered_by = NULL;
_cleanup_free_ char *joined = NULL;
int r;
assert(bus);
assert(unit);
assert(operation);
r = get_active_triggering_units(bus, unit, ignore_masked, &triggered_by);
if (r < 0) {
log_warning_errno(r,
"Failed to get triggering units for '%s', ignoring: %m", unit);
return;
}
if (strv_isempty(triggered_by))
return;
joined = strv_join(triggered_by, ", ");
if (!joined)
return (void) log_oom();
log_warning("%s '%s', but its triggering units are still active:\n"
"%s",
operation, unit, joined);
}
int need_daemon_reload(sd_bus *bus, const char *unit) {
_cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
const char *path;
@@ -624,25 +657,31 @@ static int unit_find_template_path(
return r;
}
int unit_is_masked(sd_bus *bus, LookupPaths *lp, const char *name) {
int unit_is_masked(sd_bus *bus, const char *unit) {
_cleanup_free_ char *load_state = NULL;
int r;
if (unit_name_is_valid(name, UNIT_NAME_TEMPLATE)) {
_cleanup_free_ char *path = NULL;
assert(bus);
assert(unit);
/* A template cannot be loaded, but it can be still masked, so
* we need to use a different method. */
if (unit_name_is_valid(unit, UNIT_NAME_TEMPLATE)) {
_cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
_cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
const char *state;
r = unit_file_find_path(lp, name, &path);
r = bus_call_method(bus, bus_systemd_mgr, "GetUnitFileState", &error, &reply, "s", unit);
if (r < 0)
return r;
if (r == 0)
return false;
return null_or_empty_path(path);
return log_debug_errno(r, "Failed to get UnitFileState for '%s': %s",
unit, bus_error_message(&error, r));
r = sd_bus_message_read(reply, "s", &state);
if (r < 0)
return bus_log_parse_error_debug(r);
return STR_IN_SET(state, "masked", "masked-runtime");
}
r = unit_load_state(bus, name, &load_state);
r = unit_load_state(bus, unit, &load_state);
if (r < 0)
return r;
+3 -2
View File
@@ -26,7 +26,8 @@ int get_sub_state_one_unit(sd_bus *bus, const char *unit, char **ret_sub_state);
int get_unit_list(sd_bus *bus, const char *machine, char **patterns, UnitInfo **unit_infos, int c, sd_bus_message **ret_reply);
int expand_unit_names(sd_bus *bus, char **names, const char* suffix, char ***ret, bool *ret_expanded);
int check_triggering_units(sd_bus *bus, const char *unit);
int get_active_triggering_units(sd_bus *bus, const char *unit, bool ignore_masked, char ***ret);
void warn_triggering_units(sd_bus *bus, const char *unit, const char *operation, bool ignore_masked);
int need_daemon_reload(sd_bus *bus, const char *unit);
@@ -38,7 +39,7 @@ int maybe_extend_with_unit_dependencies(sd_bus *bus, char ***list);
int unit_file_find_path(LookupPaths *lp, const char *unit_name, char **ret_unit_path);
int unit_find_paths(sd_bus *bus, const char *unit_name, LookupPaths *lp, bool force_client_side, Hashmap **cached_id_map, Hashmap **cached_name_map, char **ret_fragment_path, char ***ret_dropin_paths);
int unit_is_masked(sd_bus *bus, LookupPaths *lp, const char *name);
int unit_is_masked(sd_bus *bus, const char *unit);
int unit_exists(LookupPaths *lp, const char *unit);
int unit_get_dependencies(sd_bus *bus, const char *name, char ***ret);