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

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

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 0fcf5977677792a2b0d820a8cc3d98a6ec87214a..effbd5c07ca6437f56d586bc460d90a813544f65 100644 (file)
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -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 (dropin_paths && !strv_isempty(dropins)) {
-                *dropin_paths = TAKE_PTR(dropins);
-                r = 1;
+        if (ret_dropin_paths) {
+                if (!strv_isempty(dropins)) {
+                        *ret_dropin_paths = TAKE_PTR(dropins);
+                        r = 1;
+                } else
+                        *ret_dropin_paths = NULL;
         }
+
  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;