Merge pull request #5303 from poettering/deleted-units

a small number of install and unit management related fixes

4 files changed, 183 insertions, 112 deletions

diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 0136d38833..27f948cbdd 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -848,13 +848,9 @@ static int method_get_unit_processes(sd_bus_message *message, void *userdata, sd
         if (r < 0)
                 return r;
 
-        r = manager_load_unit(m, name, NULL, error, &u);
-        if (r < 0)
-                return r;
-
-        r = bus_unit_check_load_state(u, error);
-        if (r < 0)
-                return r;
+        u = manager_get_unit(m, name);
+        if (!u)
+                return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, "Unit %s not loaded.", name);
 
         return bus_unit_method_get_processes(message, u, error);
 }
@@ -1916,63 +1912,6 @@ static int send_unit_files_changed(sd_bus *bus, void *userdata) {
         return sd_bus_send(bus, message, NULL);
 }
 
-static int reply_unit_file_changes_and_free(
-                Manager *m,
-                sd_bus_message *message,
-                int carries_install_info,
-                UnitFileChange *changes,
-                unsigned n_changes) {
-
-        _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
-        unsigned i;
-        int r;
-
-        if (unit_file_changes_have_modification(changes, n_changes)) {
-                r = bus_foreach_bus(m, NULL, send_unit_files_changed, NULL);
-                if (r < 0)
-                        log_debug_errno(r, "Failed to send UnitFilesChanged signal: %m");
-        }
-
-        r = sd_bus_message_new_method_return(message, &reply);
-        if (r < 0)
-                goto fail;
-
-        if (carries_install_info >= 0) {
-                r = sd_bus_message_append(reply, "b", carries_install_info);
-                if (r < 0)
-                        goto fail;
-        }
-
-        r = sd_bus_message_open_container(reply, 'a', "(sss)");
-        if (r < 0)
-                goto fail;
-
-        for (i = 0; i < n_changes; i++)
-                if (changes[i].type >= 0) {
-                        const char *change = unit_file_change_type_to_string(changes[i].type);
-                        assert(change != NULL);
-
-                        r = sd_bus_message_append(
-                                        reply, "(sss)",
-                                        change,
-                                        changes[i].path,
-                                        changes[i].source);
-                        if (r < 0)
-                                goto fail;
-                }
-
-        r = sd_bus_message_close_container(reply);
-        if (r < 0)
-                goto fail;
-
-        unit_file_changes_free(changes, n_changes);
-        return sd_bus_send(NULL, reply, NULL);
-
-fail:
-        unit_file_changes_free(changes, n_changes);
-        return r;
-}
-
 /* Create an error reply, using the error information from changes[]
  * if possible, and fall back to generating an error from error code c.
  * The error message only describes the first error.
@@ -1986,12 +1925,14 @@ static int install_error(
                 unsigned n_changes) {
         int r;
         unsigned i;
-        assert(c < 0);
 
         for (i = 0; i < n_changes; i++)
+
                 switch(changes[i].type) {
+
                 case 0 ... INT_MAX:
                         continue;
+
                 case -EEXIST:
                         if (changes[i].source)
                                 r = sd_bus_error_setf(error, BUS_ERROR_UNIT_EXISTS,
@@ -2002,29 +1943,106 @@ static int install_error(
                                                       "File %s already exists.",
                                                       changes[i].path);
                         goto found;
+
                 case -ERFKILL:
                         r = sd_bus_error_setf(error, BUS_ERROR_UNIT_MASKED,
                                               "Unit file %s is masked.", changes[i].path);
                         goto found;
+
                 case -EADDRNOTAVAIL:
                         r = sd_bus_error_setf(error, BUS_ERROR_UNIT_GENERATED,
                                               "Unit %s is transient or generated.", changes[i].path);
                         goto found;
+
                 case -ELOOP:
                         r = sd_bus_error_setf(error, BUS_ERROR_UNIT_LINKED,
                                               "Refusing to operate on linked unit file %s", changes[i].path);
                         goto found;
+
+                case -ENOENT:
+                        r = sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, "Unit file %s does not exist.", changes[i].path);
+                        goto found;
+
                 default:
                         r = sd_bus_error_set_errnof(error, changes[i].type, "File %s: %m", changes[i].path);
                         goto found;
                 }
 
-        r = c;
+        r = c < 0 ? c : -EINVAL;
+
  found:
         unit_file_changes_free(changes, n_changes);
         return r;
 }
 
+static int reply_unit_file_changes_and_free(
+                Manager *m,
+                sd_bus_message *message,
+                int carries_install_info,
+                UnitFileChange *changes,
+                unsigned n_changes,
+                sd_bus_error *error) {
+
+        _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
+        bool bad = false, good = false;
+        unsigned i;
+        int r;
+
+        if (unit_file_changes_have_modification(changes, n_changes)) {
+                r = bus_foreach_bus(m, NULL, send_unit_files_changed, NULL);
+                if (r < 0)
+                        log_debug_errno(r, "Failed to send UnitFilesChanged signal: %m");
+        }
+
+        r = sd_bus_message_new_method_return(message, &reply);
+        if (r < 0)
+                goto fail;
+
+        if (carries_install_info >= 0) {
+                r = sd_bus_message_append(reply, "b", carries_install_info);
+                if (r < 0)
+                        goto fail;
+        }
+
+        r = sd_bus_message_open_container(reply, 'a', "(sss)");
+        if (r < 0)
+                goto fail;
+
+        for (i = 0; i < n_changes; i++) {
+
+                if (changes[i].type < 0) {
+                        bad = true;
+                        continue;
+                }
+
+                r = sd_bus_message_append(
+                                reply, "(sss)",
+                                unit_file_change_type_to_string(changes[i].type),
+                                changes[i].path,
+                                changes[i].source);
+                if (r < 0)
+                        goto fail;
+
+                good = true;
+        }
+
+        /* If there was a failed change, and no successful change, then return the first failure as proper method call
+         * error. */
+        if (bad && !good)
+                return install_error(error, 0, changes, n_changes);
+
+        r = sd_bus_message_close_container(reply);
+        if (r < 0)
+                goto fail;
+
+        unit_file_changes_free(changes, n_changes);
+        return sd_bus_send(NULL, reply, NULL);
+
+fail:
+        unit_file_changes_free(changes, n_changes);
+        return r;
+}
+
 static int method_enable_unit_files_generic(
                 sd_bus_message *message,
                 Manager *m,
@@ -2061,7 +2079,7 @@ static int method_enable_unit_files_generic(
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
-        return reply_unit_file_changes_and_free(m, message, carries_install_info ? r : -1, changes, n_changes);
+        return reply_unit_file_changes_and_free(m, message, carries_install_info ? r : -1, changes, n_changes, error);
 }
 
 static int method_enable_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
@@ -2130,7 +2148,7 @@ static int method_preset_unit_files_with_mode(sd_bus_message *message, void *use
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
-        return reply_unit_file_changes_and_free(m, message, r, changes, n_changes);
+        return reply_unit_file_changes_and_free(m, message, r, changes, n_changes, error);
 }
 
 static int method_disable_unit_files_generic(
@@ -2165,7 +2183,7 @@ static int method_disable_unit_files_generic(
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
-        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes);
+        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes, error);
 }
 
 static int method_disable_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
@@ -2200,7 +2218,7 @@ static int method_revert_unit_files(sd_bus_message *message, void *userdata, sd_
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
-        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes);
+        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes, error);
 }
 
 static int method_set_default_target(sd_bus_message *message, void *userdata, sd_bus_error *error) {
@@ -2231,7 +2249,7 @@ static int method_set_default_target(sd_bus_message *message, void *userdata, sd
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
-        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes);
+        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes, error);
 }
 
 static int method_preset_all_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
@@ -2274,7 +2292,7 @@ static int method_preset_all_unit_files(sd_bus_message *message, void *userdata,
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
-        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes);
+        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes, error);
 }
 
 static int method_add_dependency_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
@@ -2314,7 +2332,7 @@ static int method_add_dependency_unit_files(sd_bus_message *message, void *userd
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
-        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes);
+        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes, error);
 }
 
 static int method_get_unit_file_links(sd_bus_message *message, void *userdata, sd_bus_error *error) {
diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index 60e889e1ef..f1306a023f 100644
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -1006,6 +1006,10 @@ int bus_unit_method_get_processes(sd_bus_message *message, void *userdata, sd_bu
 
         assert(message);
 
+        r = mac_selinux_unit_access_check(u, message, "status", error);
+        if (r < 0)
+                return r;
+
         pids = set_new(NULL);
         if (!pids)
                 return -ENOMEM;
diff --git a/src/shared/install.c b/src/shared/install.c
index f25ed685f6..58c8e852b2 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -389,6 +389,12 @@ void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *chang
                                         verb, changes[i].path);
                         logged = true;
                         break;
+
+                case -ENOENT:
+                        log_error_errno(changes[i].type, "Failed to %s unit, unit %s does not exist.", verb, changes[i].path);
+                        logged = true;
+                        break;
+
                 default:
                         assert(changes[i].type < 0);
                         log_error_errno(changes[i].type, "Failed to %s unit, file %s: %m.",
@@ -1807,7 +1813,9 @@ static int install_context_mark_for_removal(
                 InstallContext *c,
                 const LookupPaths *paths,
                 Set **remove_symlinks_to,
-                const char *config_path) {
+                const char *config_path,
+                UnitFileChange **changes,
+                unsigned *n_changes) {
 
         UnitFileInstallInfo *i;
         int r;
@@ -1833,19 +1841,26 @@ static int install_context_mark_for_removal(
 
                 r = install_info_traverse(scope, c, paths, i, SEARCH_LOAD|SEARCH_FOLLOW_CONFIG_SYMLINKS, NULL);
                 if (r == -ENOLINK) {
-                        log_debug_errno(r, "Name %s leads to a dangling symlink, ignoring.", i->name);
-                        continue;
-                } else if (r == -ENOENT && i->auxiliary) {
-                        /* some unit specified in Also= or similar is missing */
-                        log_debug_errno(r, "Auxiliary unit %s not found, ignoring.", i->name);
-                        continue;
-                } else if (r < 0)
-                        return log_debug_errno(r, "Failed to find unit %s: %m", i->name);
+                        log_debug_errno(r, "Name %s leads to a dangling symlink, removing name.", i->name);
+                        unit_file_changes_add(changes, n_changes, UNIT_FILE_IS_DANGLING, i->path ?: i->name, NULL);
+                } else if (r == -ENOENT) {
 
-                if (i->type != UNIT_FILE_TYPE_REGULAR) {
-                        log_debug("Unit %s has type %s, ignoring.",
-                                  i->name,
-                                  unit_file_type_to_string(i->type) ?: "invalid");
+                        if (i->auxiliary)  /* some unit specified in Also= or similar is missing */
+                                log_debug_errno(r, "Auxiliary unit of %s not found, removing name.", i->name);
+                        else {
+                                log_debug_errno(r, "Unit %s not found, removing name.", i->name);
+                                unit_file_changes_add(changes, n_changes, r, i->path ?: i->name, NULL);
+                        }
+
+                } else if (r < 0) {
+                        log_debug_errno(r, "Failed to find unit %s, removing name: %m", i->name);
+                        unit_file_changes_add(changes, n_changes, r, i->path ?: i->name, NULL);
+                } else if (i->type == UNIT_FILE_TYPE_MASKED) {
+                        log_debug("Unit file %s is masked, ignoring.", i->name);
+                        unit_file_changes_add(changes, n_changes, UNIT_FILE_IS_MASKED, i->path ?: i->name, NULL);
+                        continue;
+                } else if (i->type != UNIT_FILE_TYPE_REGULAR) {
+                        log_debug("Unit %s has type %s, ignoring.", i->name, unit_file_type_to_string(i->type) ?: "invalid");
                         continue;
                 }
 
@@ -1878,6 +1893,8 @@ int unit_file_mask(
                 return r;
 
         config_path = (flags & UNIT_FILE_RUNTIME) ? paths.runtime_config : paths.persistent_config;
+        if (!config_path)
+                return -ENXIO;
 
         STRV_FOREACH(i, files) {
                 _cleanup_free_ char *path = NULL;
@@ -1926,6 +1943,9 @@ int unit_file_unmask(
                 return r;
 
         config_path = (flags & UNIT_FILE_RUNTIME) ? paths.runtime_config : paths.persistent_config;
+        if (!config_path)
+                return -ENXIO;
+
         dry_run = !!(flags & UNIT_FILE_DRY_RUN);
 
         STRV_FOREACH(i, files) {
@@ -2015,6 +2035,8 @@ int unit_file_link(
                 return r;
 
         config_path = (flags & UNIT_FILE_RUNTIME) ? paths.runtime_config : paths.persistent_config;
+        if (!config_path)
+                return -ENXIO;
 
         STRV_FOREACH(i, files) {
                 _cleanup_free_ char *full = NULL;
@@ -2282,6 +2304,8 @@ int unit_file_add_dependency(
                 return r;
 
         config_path = (flags & UNIT_FILE_RUNTIME) ? paths.runtime_config : paths.persistent_config;
+        if (!config_path)
+                return -ENXIO;
 
         r = install_info_discover(scope, &c, &paths, target, SEARCH_FOLLOW_CONFIG_SYMLINKS,
                                   &target_info, changes, n_changes);
@@ -2347,6 +2371,8 @@ int unit_file_enable(
                 return r;
 
         config_path = (flags & UNIT_FILE_RUNTIME) ? paths.runtime_config : paths.persistent_config;
+        if (!config_path)
+                return -ENXIO;
 
         STRV_FOREACH(f, files) {
                 r = install_info_discover(scope, &c, &paths, *f, SEARCH_LOAD|SEARCH_FOLLOW_CONFIG_SYMLINKS,
@@ -2391,6 +2417,8 @@ int unit_file_disable(
                 return r;
 
         config_path = (flags & UNIT_FILE_RUNTIME) ? paths.runtime_config : paths.persistent_config;
+        if (!config_path)
+                return -ENXIO;
 
         STRV_FOREACH(i, files) {
                 if (!unit_name_is_valid(*i, UNIT_NAME_ANY))
@@ -2401,7 +2429,7 @@ int unit_file_disable(
                         return r;
         }
 
-        r = install_context_mark_for_removal(scope, &c, &paths, &remove_symlinks_to, config_path);
+        r = install_context_mark_for_removal(scope, &c, &paths, &remove_symlinks_to, config_path, changes, n_changes);
         if (r < 0)
                 return r;
 
@@ -2790,7 +2818,7 @@ static int execute_preset(
         if (mode != UNIT_FILE_PRESET_ENABLE_ONLY) {
                 _cleanup_set_free_free_ Set *remove_symlinks_to = NULL;
 
-                r = install_context_mark_for_removal(scope, minus, paths, &remove_symlinks_to, config_path);
+                r = install_context_mark_for_removal(scope, minus, paths, &remove_symlinks_to, config_path, changes, n_changes);
                 if (r < 0)
                         return r;
 
@@ -2885,6 +2913,8 @@ int unit_file_preset(
                 return r;
 
         config_path = (flags & UNIT_FILE_RUNTIME) ? paths.runtime_config : paths.persistent_config;
+        if (!config_path)
+                return -ENXIO;
 
         r = read_presets(scope, root_dir, &presets);
         if (r < 0)
@@ -2923,6 +2953,8 @@ int unit_file_preset_all(
                 return r;
 
         config_path = (flags & UNIT_FILE_RUNTIME) ? paths.runtime_config : paths.persistent_config;
+        if (!config_path)
+                return -ENXIO;
 
         r = read_presets(scope, root_dir, &presets);
         if (r < 0)
diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c
index 586ef64e72..09a44534e2 100644
--- a/src/shared/path-lookup.c
+++ b/src/shared/path-lookup.c
@@ -33,6 +33,7 @@
 #include "stat-util.h"
 #include "string-util.h"
 #include "strv.h"
+#include "user-util.h"
 #include "util.h"
 
 static int user_runtime_dir(char **ret, const char *suffix) {
@@ -57,6 +58,7 @@ static int user_runtime_dir(char **ret, const char *suffix) {
 static int user_config_dir(char **ret, const char *suffix) {
         const char *e;
         char *j;
+        int r;
 
         assert(ret);
 
@@ -64,11 +66,11 @@ static int user_config_dir(char **ret, const char *suffix) {
         if (e)
                 j = strappend(e, suffix);
         else {
-                const char *home;
+                _cleanup_free_ char *home = NULL;
 
-                home = getenv("HOME");
-                if (!home)
-                        return -ENXIO;
+                r = get_home_dir(&home);
+                if (r < 0)
+                        return r;
 
                 j = strjoin(home, "/.config", suffix);
         }
@@ -83,6 +85,7 @@ static int user_config_dir(char **ret, const char *suffix) {
 static int user_data_dir(char **ret, const char *suffix) {
         const char *e;
         char *j;
+        int r;
 
         assert(ret);
         assert(suffix);
@@ -95,12 +98,11 @@ static int user_data_dir(char **ret, const char *suffix) {
         if (e)
                 j = strappend(e, suffix);
         else {
-                const char *home;
-
-                home = getenv("HOME");
-                if (!home)
-                        return -ENXIO;
+                _cleanup_free_ char *home = NULL;
 
+                r = get_home_dir(&home);
+                if (r < 0)
+                        return r;
 
                 j = strjoin(home, "/.local/share", suffix);
         }
@@ -136,10 +138,10 @@ static char** user_dirs(
                 NULL
         };
 
-        const char *e;
         _cleanup_strv_free_ char **config_dirs = NULL, **data_dirs = NULL;
         _cleanup_free_ char *data_home = NULL;
         _cleanup_strv_free_ char **res = NULL;
+        const char *e;
         char **tmp;
         int r;
 
@@ -186,9 +188,8 @@ static char** user_dirs(
         if (strv_extend(&res, generator_early) < 0)
                 return NULL;
 
-        if (!strv_isempty(config_dirs))
-                if (strv_extend_strv_concat(&res, config_dirs, "/systemd/user") < 0)
-                        return NULL;
+        if (strv_extend_strv_concat(&res, config_dirs, "/systemd/user") < 0)
+                return NULL;
 
         if (strv_extend(&res, persistent_config) < 0)
                 return NULL;
@@ -205,9 +206,8 @@ static char** user_dirs(
         if (strv_extend(&res, data_home) < 0)
                 return NULL;
 
-        if (!strv_isempty(data_dirs))
-                if (strv_extend_strv_concat(&res, data_dirs, "/systemd/user") < 0)
-                        return NULL;
+        if (strv_extend_strv_concat(&res, data_dirs, "/systemd/user") < 0)
+                return NULL;
 
         if (strv_extend_strv(&res, (char**) data_unit_paths, false) < 0)
                 return NULL;
@@ -220,6 +220,7 @@ static char** user_dirs(
 
         tmp = res;
         res = NULL;
+
         return tmp;
 }
 
@@ -328,12 +329,18 @@ static int acquire_config_dirs(UnitFileScope scope, char **persistent, char **ru
 
         case UNIT_FILE_USER:
                 r = user_config_dir(&a, "/systemd/user");
-                if (r < 0)
+                if (r < 0 && r != -ENXIO)
                         return r;
 
                 r = user_runtime_dir(runtime, "/systemd/user");
-                if (r < 0)
-                        return r;
+                if (r < 0) {
+                        if (r != -ENXIO)
+                                return r;
+
+                        /* If XDG_RUNTIME_DIR is not set, don't consider that fatal, simply initialize the runtime
+                         * directory to NULL */
+                        *runtime = NULL;
+                }
 
                 *persistent = a;
                 a = NULL;
@@ -382,12 +389,18 @@ static int acquire_control_dirs(UnitFileScope scope, char **persistent, char **r
 
         case UNIT_FILE_USER:
                 r = user_config_dir(&a, "/systemd/system.control");
-                if (r < 0)
+                if (r < 0 && r != -ENXIO)
                         return r;
 
                 r = user_runtime_dir(runtime, "/systemd/system.control");
-                if (r < 0)
-                        return r;
+                if (r < 0) {
+                        if (r != -ENXIO)
+                                return r;
+
+                        /* If XDG_RUNTIME_DIR is not set, don't consider this fatal, simply initialize the directory to
+                         * NULL */
+                        *runtime = NULL;
+                }
 
                 break;
 
@@ -474,22 +487,26 @@ int lookup_paths_init(
                         return -ENOMEM;
         }
 
+        /* Note: when XDG_RUNTIME_DIR is not set this will not return -ENXIO, but simply set runtime_config to NULL */
         r = acquire_config_dirs(scope, &persistent_config, &runtime_config);
-        if (r < 0 && r != -ENXIO)
+        if (r < 0)
                 return r;
 
         if ((flags & LOOKUP_PATHS_EXCLUDE_GENERATED) == 0) {
+                /* Note: if XDG_RUNTIME_DIR is not set, this will fail completely with ENXIO */
                 r = acquire_generator_dirs(scope, &generator, &generator_early, &generator_late);
                 if (r < 0 && r != -EOPNOTSUPP && r != -ENXIO)
                         return r;
         }
 
+        /* Note: if XDG_RUNTIME_DIR is not set, this will fail completely with ENXIO */
         r = acquire_transient_dir(scope, &transient);
         if (r < 0 && r != -EOPNOTSUPP && r != -ENXIO)
                 return r;
 
+        /* Note: when XDG_RUNTIME_DIR is not set this will not return -ENXIO, but simply set runtime_control to NULL */
         r = acquire_control_dirs(scope, &persistent_control, &runtime_control);
-        if (r < 0 && r != -EOPNOTSUPP && r != -ENXIO)
+        if (r < 0 && r != -EOPNOTSUPP)
                 return r;
 
         /* First priority is whatever has been passed to us via env vars */