systemctl: if service manager couldn't load unit file, don't rely on it to tell us the fragment path

Previously, "systemctl edit" exclusively used the service manager's
per-unit FragmentPath property to figure out which file to edit, when
operating on a non-template unit. If for some reason loading the unit
file failed entirely though (LoadState=error), then FragmentPath would
be empty, and thus the unit not editable.

Let's fix this, by falling back to client-side unit file searching in
this case.

(Also, various other clean-ups to make the relevant functions follow our
coding style)

Fixes: #9561
This commit is contained in:
Lennart Poettering
2018-11-29 11:25:32 +01:00
parent 85163756da
commit 02a126a33d
+72 -27
View File
@@ -2497,33 +2497,59 @@ static int unit_find_paths(
sd_bus *bus,
const char *unit_name,
LookupPaths *lp,
char **fragment_path,
char ***dropin_paths) {
bool force_client_side,
char **ret_fragment_path,
char ***ret_dropin_paths) {
_cleanup_free_ char *path = NULL;
_cleanup_strv_free_ char **dropins = NULL;
_cleanup_free_ char *path = NULL;
int r;
/**
* Finds where the unit is defined on disk. Returns 0 if the unit
* is not found. Returns 1 if it is found, and sets
* - the path to the unit in *path, if it exists on disk,
* - and a strv of existing drop-ins in *dropins,
* if the arg is not NULL and any dropins were found.
* Finds where the unit is defined on disk. Returns 0 if the unit is not found. Returns 1 if it is found, and
* sets:
* - the path to the unit in *ret_frament_path, if it exists on disk,
* - and a strv of existing drop-ins in *ret_dropin_paths, if the arg is not NULL and any dropins were found.
*
* Returns -ERFKILL if the unit is masked, and -EKEYREJECTED if the unit file could not be loaded for some
* reason (the latter only applies if we are going through the service manager)
*/
assert(unit_name);
assert(fragment_path);
assert(ret_fragment_path);
assert(lp);
if (!install_client_side() && !unit_name_is_valid(unit_name, UNIT_NAME_TEMPLATE)) {
/* Go via the bus to acquire the path, unless we are explicitly told not to, or when the unit name is a template */
if (!force_client_side &&
!install_client_side() &&
!unit_name_is_valid(unit_name, UNIT_NAME_TEMPLATE)) {
_cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
_cleanup_free_ char *unit = NULL;
_cleanup_free_ char *load_state = NULL, *unit = NULL;
unit = unit_dbus_path_from_name(unit_name);
if (!unit)
return log_oom();
r = sd_bus_get_property_string(
bus,
"org.freedesktop.systemd1",
unit,
"org.freedesktop.systemd1.Unit",
"LoadState",
&error,
&load_state);
if (r < 0)
return log_error_errno(r, "Failed to get LoadState: %s", bus_error_message(&error, r));
if (streq(load_state, "masked"))
return -ERFKILL;
if (streq(load_state, "not-found")) {
r = 0;
goto not_found;
}
if (!streq(load_state, "loaded"))
return -EKEYREJECTED;
r = sd_bus_get_property_string(
bus,
"org.freedesktop.systemd1",
@@ -2535,7 +2561,7 @@ static int unit_find_paths(
if (r < 0)
return log_error_errno(r, "Failed to get FragmentPath: %s", bus_error_message(&error, r));
if (dropin_paths) {
if (ret_dropin_paths) {
r = sd_bus_get_property_strv(
bus,
"org.freedesktop.systemd1",
@@ -2558,7 +2584,6 @@ static int unit_find_paths(
r = unit_find_template_path(unit_name, lp, &path, &template);
if (r < 0)
return r;
if (r > 0) {
if (null_or_empty_path(path))
/* The template is masked. Let's cut the process short. */
@@ -2582,25 +2607,29 @@ static int unit_find_paths(
if (r < 0)
return log_error_errno(r, "Failed to add unit name: %m");
if (dropin_paths) {
r = unit_file_find_dropin_conf_paths(arg_root, lp->search_path,
NULL, names, &dropins);
if (ret_dropin_paths) {
r = unit_file_find_dropin_conf_paths(arg_root, lp->search_path, NULL, names, &dropins);
if (r < 0)
return r;
}
}
r = 0;
r = 0;
if (!isempty(path)) {
*fragment_path = TAKE_PTR(path);
*ret_fragment_path = TAKE_PTR(path);
r = 1;
} else
*ret_fragment_path = NULL;
if (ret_dropin_paths) {
if (!strv_isempty(dropins)) {
*ret_dropin_paths = TAKE_PTR(dropins);
r = 1;
} else
*ret_dropin_paths = NULL;
}
if (dropin_paths && !strv_isempty(dropins)) {
*dropin_paths = TAKE_PTR(dropins);
r = 1;
}
not_found:
if (r == 0 && !arg_force)
log_error("No files found for %s.", unit_name);
@@ -5390,9 +5419,16 @@ static int cat(int argc, char *argv[], void *userdata) {
_cleanup_free_ char *fragment_path = NULL;
_cleanup_strv_free_ char **dropin_paths = NULL;
r = unit_find_paths(bus, *name, &lp, &fragment_path, &dropin_paths);
r = unit_find_paths(bus, *name, &lp, false, &fragment_path, &dropin_paths);
if (r == -ERFKILL) {
printf("%s# unit %s is masked%s\n",
printf("%s# Unit %s is masked%s.\n",
ansi_highlight_magenta(),
*name,
ansi_normal());
continue;
}
if (r == -EKEYREJECTED) {
printf("%s# Unit %s could not be loaded.%s\n",
ansi_highlight_magenta(),
*name,
ansi_normal());
@@ -5400,7 +5436,7 @@ static int cat(int argc, char *argv[], void *userdata) {
}
if (r < 0)
return r;
else if (r == 0)
if (r == 0)
return -ENOENT;
if (first)
@@ -6893,6 +6929,7 @@ static int run_editor(char **paths) {
n_editor_args = strv_length(editor_args);
argc += n_editor_args - 1;
}
args = newa(const char*, argc + 1);
if (n_editor_args > 0) {
@@ -6944,7 +6981,15 @@ static int find_paths_to_edit(sd_bus *bus, char **names, char ***paths) {
_cleanup_free_ char *path = NULL, *new_path = NULL, *tmp_path = NULL, *tmp_name = NULL;
const char *unit_name;
r = unit_find_paths(bus, *name, &lp, &path, NULL);
r = unit_find_paths(bus, *name, &lp, false, &path, NULL);
if (r == -EKEYREJECTED) {
/* If loading of the unit failed server side complete, then the server won't tell us the unit
* file path. In that case, find the file client side. */
log_debug_errno(r, "Unit '%s' was not loaded correctly, retrying client-side.", *name);
r = unit_find_paths(bus, *name, &lp, true, &path, NULL);
}
if (r == -ERFKILL)
return log_error_errno(r, "Unit '%s' masked, cannot edit.", *name);
if (r < 0)
return r;
@@ -6995,6 +7040,7 @@ static int find_paths_to_edit(sd_bus *bus, char **names, char ***paths) {
r = strv_push_pair(paths, new_path, tmp_path);
if (r < 0)
return log_oom();
new_path = tmp_path = NULL;
}
@@ -7035,7 +7081,6 @@ static int edit(int argc, char *argv[], void *userdata) {
r = unit_is_masked(bus, &lp, *tmp);
if (r < 0)
return r;
if (r > 0) {
log_error("Cannot edit %s: unit is masked.", *tmp);
return -EINVAL;