Merge pull request #3049 from keszybz/preset-fixes

Fixes for preset-all handling and a few related issues

14 files changed, 124 insertions, 74 deletions

diff --git a/src/basic/path-util.c b/src/basic/path-util.c
index 822c09bfba..044a12889d 100644
--- a/src/basic/path-util.c
+++ b/src/basic/path-util.c
@@ -569,10 +569,10 @@ static int binary_is_good(const char *binary) {
         if (r < 0)
                 return r;
 
-        return !path_equal(d, "true") &&
-               !path_equal(d, "/bin/true") &&
-               !path_equal(d, "/usr/bin/true") &&
-               !path_equal(d, "/dev/null");
+        return !PATH_IN_SET(d, "true"
+                               "/bin/true",
+                               "/usr/bin/true",
+                               "/dev/null");
 }
 
 int fsck_exists(const char *fstype) {
diff --git a/src/basic/path-util.h b/src/basic/path-util.h
index 2c2f87a9f2..34d5cd1570 100644
--- a/src/basic/path-util.h
+++ b/src/basic/path-util.h
@@ -48,6 +48,23 @@ bool path_equal(const char *a, const char *b) _pure_;
 bool path_equal_or_files_same(const char *a, const char *b);
 char* path_join(const char *root, const char *path, const char *rest);
 
+static inline bool path_equal_ptr(const char *a, const char *b) {
+        return !!a == !!b && (!a || path_equal(a, b));
+}
+
+/* Note: the search terminates on the first NULL item. */
+#define PATH_IN_SET(p, ...)                                     \
+        ({                                                      \
+                char **s;                                       \
+                bool _found = false;                            \
+                STRV_FOREACH(s, STRV_MAKE(__VA_ARGS__))         \
+                        if (path_equal(p, *s)) {                \
+                               _found = true;                   \
+                               break;                           \
+                        }                                       \
+                _found;                                         \
+        })
+
 int path_strv_make_absolute_cwd(char **l);
 char** path_strv_resolve(char **l, const char *prefix);
 char** path_strv_resolve_uniq(char **l, const char *prefix);
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 2392ed6c16..9a913a6c91 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1565,11 +1565,13 @@ static int reply_unit_file_changes_and_free(
         unsigned i;
         int r;
 
-        if (n_changes > 0) {
-                r = bus_foreach_bus(m, NULL, send_unit_files_changed, NULL);
-                if (r < 0)
-                        log_debug_errno(r, "Failed to send UnitFilesChanged signal: %m");
-        }
+        for (i = 0; i < n_changes; i++)
+                if (unit_file_change_is_modification(changes[i].type)) {
+                        r = bus_foreach_bus(m, NULL, send_unit_files_changed, NULL);
+                        if (r < 0)
+                                log_debug_errno(r, "Failed to send UnitFilesChanged signal: %m");
+                        break;
+                }
 
         r = sd_bus_message_new_method_return(message, &reply);
         if (r < 0)
@@ -1610,7 +1612,7 @@ fail:
 static int install_error(sd_bus_error *error, int c) {
         assert(c < 0);
 
-        if (c == -ESHUTDOWN)
+        if (c == -ERFKILL)
                 return sd_bus_error_setf(error, BUS_ERROR_UNIT_MASKED, "Unit file is masked.");
         if (c == -EADDRNOTAVAIL)
                 return sd_bus_error_setf(error, BUS_ERROR_UNIT_GENERATED, "Unit file is transient or generated.");
diff --git a/src/core/mount.c b/src/core/mount.c
index 74ab54bfd0..632c5c824c 100644
--- a/src/core/mount.c
+++ b/src/core/mount.c
@@ -376,8 +376,7 @@ static int mount_add_quota_links(Mount *m) {
 static bool should_umount(Mount *m) {
         MountParameters *p;
 
-        if (path_equal(m->where, "/") ||
-            path_equal(m->where, "/usr") ||
+        if (PATH_IN_SET(m->where, "/", "/usr") ||
             path_startswith(m->where, "/run/initramfs"))
                 return false;
 
@@ -408,8 +407,7 @@ static int mount_add_default_dependencies(Mount *m) {
          * Also, don't bother with anything mounted below virtual
          * file systems, it's also going to be virtual, and hence
          * not worth the effort. */
-        if (path_equal(m->where, "/") ||
-            path_equal(m->where, "/usr") ||
+        if (PATH_IN_SET(m->where, "/", "/usr") ||
             path_startswith(m->where, "/run/initramfs") ||
             path_startswith(m->where, "/proc") ||
             path_startswith(m->where, "/sys") ||
diff --git a/src/core/transaction.c b/src/core/transaction.c
index dad387749c..d5370b2a14 100644
--- a/src/core/transaction.c
+++ b/src/core/transaction.c
@@ -939,7 +939,7 @@ int transaction_add_job_and_dependencies(
                                 if (r < 0) {
                                         /* unit masked, job type not applicable and unit not found are not considered as errors. */
                                         log_unit_full(dep,
-                                                      IN_SET(r, -ESHUTDOWN, -EBADR, -ENOENT) ? LOG_DEBUG : LOG_WARNING,
+                                                      IN_SET(r, -ERFKILL, -EBADR, -ENOENT) ? LOG_DEBUG : LOG_WARNING,
                                                       r, "Cannot add dependency job, ignoring: %s",
                                                       bus_error_message(e, r));
                                         sd_bus_error_free(e);
diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
index 2939322925..b1d1bf9e14 100644
--- a/src/journal/journald-server.c
+++ b/src/journal/journald-server.c
@@ -492,38 +492,36 @@ static void server_cache_hostname(Server *s) {
 }
 
 static bool shall_try_append_again(JournalFile *f, int r) {
-
-        /* -E2BIG            Hit configured limit
-           -EFBIG            Hit fs limit
-           -EDQUOT           Quota limit hit
-           -ENOSPC           Disk full
-           -EIO              I/O error of some kind (mmap)
-           -EHOSTDOWN        Other machine
-           -EBUSY            Unclean shutdown
-           -EPROTONOSUPPORT  Unsupported feature
-           -EBADMSG          Corrupted
-           -ENODATA          Truncated
-           -ESHUTDOWN        Already archived
-           -EIDRM            Journal file has been deleted */
-
-        if (r == -E2BIG || r == -EFBIG || r == -EDQUOT || r == -ENOSPC)
+        switch(r) {
+        case -E2BIG:           /* Hit configured limit          */
+        case -EFBIG:           /* Hit fs limit                  */
+        case -EDQUOT:          /* Quota limit hit               */
+        case -ENOSPC:          /* Disk full                     */
                 log_debug("%s: Allocation limit reached, rotating.", f->path);
-        else if (r == -EHOSTDOWN)
+                return true;
+        case -EIO:             /* I/O error of some kind (mmap) */
+                log_warning("%s: IO error, rotating.", f->path);
+                return true;
+        case -EHOSTDOWN:       /* Other machine                 */
                 log_info("%s: Journal file from other machine, rotating.", f->path);
-        else if (r == -EBUSY)
+                return true;
+        case -EBUSY:           /* Unclean shutdown              */
                 log_info("%s: Unclean shutdown, rotating.", f->path);
-        else if (r == -EPROTONOSUPPORT)
+                return true;
+        case -EPROTONOSUPPORT: /* Unsupported feature           */
                 log_info("%s: Unsupported feature, rotating.", f->path);
-        else if (r == -EBADMSG || r == -ENODATA || r == ESHUTDOWN)
+                return true;
+        case -EBADMSG:         /* Corrupted                     */
+        case -ENODATA:         /* Truncated                     */
+        case -ESHUTDOWN:       /* Already archived              */
                 log_warning("%s: Journal file corrupted, rotating.", f->path);
-        else if (r == -EIO)
-                log_warning("%s: IO error, rotating.", f->path);
-        else if (r == -EIDRM)
+                return true;
+        case -EIDRM:           /* Journal file has been deleted */
                 log_warning("%s: Journal file has been deleted, rotating.", f->path);
-        else
+                return true;
+        default:
                 return false;
-
-        return true;
+        }
 }
 
 static void write_to_journal(Server *s, uid_t uid, struct iovec *iovec, unsigned n, int priority) {
diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c
index 862f26aad7..04da94e7e3 100644
--- a/src/libsystemd/sd-bus/sd-bus.c
+++ b/src/libsystemd/sd-bus/sd-bus.c
@@ -1778,7 +1778,7 @@ static int bus_send_internal(sd_bus *bus, sd_bus_message *_m, uint64_t *cookie,
 
                 r = bus_write_message(bus, m, hint_sync_call, &idx);
                 if (r < 0) {
-                        if (r == -ENOTCONN || r == -ECONNRESET || r == -EPIPE || r == -ESHUTDOWN) {
+                        if (IN_SET(r, -ENOTCONN, -ECONNRESET, -EPIPE, -ESHUTDOWN)) {
                                 bus_enter_closing(bus);
                                 return -ECONNRESET;
                         }
@@ -2083,7 +2083,7 @@ _public_ int sd_bus_call(
 
                 r = bus_read_message(bus, false, 0);
                 if (r < 0) {
-                        if (r == -ENOTCONN || r == -ECONNRESET || r == -EPIPE || r == -ESHUTDOWN) {
+                        if (IN_SET(r, -ENOTCONN, -ECONNRESET, -EPIPE, -ESHUTDOWN)) {
                                 bus_enter_closing(bus);
                                 r = -ECONNRESET;
                         }
@@ -2116,7 +2116,7 @@ _public_ int sd_bus_call(
 
                 r = dispatch_wqueue(bus);
                 if (r < 0) {
-                        if (r == -ENOTCONN || r == -ECONNRESET || r == -EPIPE || r == -ESHUTDOWN) {
+                        if (IN_SET(r, -ENOTCONN, -ECONNRESET, -EPIPE, -ESHUTDOWN)) {
                                 bus_enter_closing(bus);
                                 r = -ECONNRESET;
                         }
@@ -2766,7 +2766,7 @@ static int bus_process_internal(sd_bus *bus, bool hint_priority, int64_t priorit
 
         case BUS_OPENING:
                 r = bus_socket_process_opening(bus);
-                if (r == -ENOTCONN || r == -ECONNRESET || r == -EPIPE || r == -ESHUTDOWN) {
+                if (IN_SET(r, -ENOTCONN, -ECONNRESET, -EPIPE, -ESHUTDOWN)) {
                         bus_enter_closing(bus);
                         r = 1;
                 } else if (r < 0)
@@ -2777,7 +2777,7 @@ static int bus_process_internal(sd_bus *bus, bool hint_priority, int64_t priorit
 
         case BUS_AUTHENTICATING:
                 r = bus_socket_process_authenticating(bus);
-                if (r == -ENOTCONN || r == -ECONNRESET || r == -EPIPE || r == -ESHUTDOWN) {
+                if (IN_SET(r, -ENOTCONN, -ECONNRESET, -EPIPE, -ESHUTDOWN)) {
                         bus_enter_closing(bus);
                         r = 1;
                 } else if (r < 0)
@@ -2791,7 +2791,7 @@ static int bus_process_internal(sd_bus *bus, bool hint_priority, int64_t priorit
         case BUS_RUNNING:
         case BUS_HELLO:
                 r = process_running(bus, hint_priority, priority, ret);
-                if (r == -ENOTCONN || r == -ECONNRESET || r == -EPIPE || r == -ESHUTDOWN) {
+                if (IN_SET(r, -ENOTCONN, -ECONNRESET, -EPIPE, -ESHUTDOWN)) {
                         bus_enter_closing(bus);
                         r = 1;
 
@@ -2914,7 +2914,7 @@ _public_ int sd_bus_flush(sd_bus *bus) {
         for (;;) {
                 r = dispatch_wqueue(bus);
                 if (r < 0) {
-                        if (r == -ENOTCONN || r == -ECONNRESET || r == -EPIPE || r == -ESHUTDOWN) {
+                        if (IN_SET(r, -ENOTCONN, -ECONNRESET, -EPIPE, -ESHUTDOWN)) {
                                 bus_enter_closing(bus);
                                 return -ECONNRESET;
                         }
diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c
index 2b86b1fcd6..677970b7f0 100644
--- a/src/shared/bus-util.c
+++ b/src/shared/bus-util.c
@@ -2203,11 +2203,17 @@ int bus_deserialize_and_dump_unit_file_changes(sd_bus_message *m, bool quiet, Un
                 if (!quiet) {
                         if (streq(type, "symlink"))
                                 log_info("Created symlink from %s to %s.", path, source);
-                        else
+                        else if (streq(type, "unlink"))
                                 log_info("Removed symlink %s.", path);
+                        else if (streq(type, "masked"))
+                                log_info("Unit %s is masked, ignoring.", path);
+                        else
+                                log_notice("Manager reported unknown change type \"%s\" for %s.", type, path);
                 }
 
-                r = unit_file_changes_add(changes, n_changes, streq(type, "symlink") ? UNIT_FILE_SYMLINK : UNIT_FILE_UNLINK, path, source);
+                r = unit_file_changes_add(changes, n_changes,
+                                          unit_file_change_type_from_string(type),
+                                          path, source);
                 if (r < 0)
                         return r;
         }
diff --git a/src/shared/install.c b/src/shared/install.c
index 8d9f96553b..10c724edbd 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -119,9 +119,9 @@ static int path_is_generator(const LookupPaths *p, const char *path) {
         if (!parent)
                 return -ENOMEM;
 
-        return path_equal(p->generator, parent) ||
-                path_equal(p->generator_early, parent) ||
-                path_equal(p->generator_late, parent);
+        return path_equal_ptr(parent, p->generator) ||
+               path_equal_ptr(parent, p->generator_early) ||
+               path_equal_ptr(parent, p->generator_late);
 }
 
 static int path_is_transient(const LookupPaths *p, const char *path) {
@@ -134,7 +134,7 @@ static int path_is_transient(const LookupPaths *p, const char *path) {
         if (!parent)
                 return -ENOMEM;
 
-        return path_equal(p->transient, parent);
+        return path_equal_ptr(parent, p->transient);
 }
 
 static int path_is_control(const LookupPaths *p, const char *path) {
@@ -147,8 +147,8 @@ static int path_is_control(const LookupPaths *p, const char *path) {
         if (!parent)
                 return -ENOMEM;
 
-        return path_equal(parent, p->persistent_control) ||
-                path_equal(parent, p->runtime_control);
+        return path_equal_ptr(parent, p->persistent_control) ||
+               path_equal_ptr(parent, p->runtime_control);
 }
 
 static int path_is_config(const LookupPaths *p, const char *path) {
@@ -164,8 +164,8 @@ static int path_is_config(const LookupPaths *p, const char *path) {
         if (!parent)
                 return -ENOMEM;
 
-        return path_equal(parent, p->persistent_config) ||
-               path_equal(parent, p->runtime_config);
+        return path_equal_ptr(parent, p->persistent_config) ||
+               path_equal_ptr(parent, p->runtime_config);
 }
 
 static int path_is_runtime(const LookupPaths *p, const char *path) {
@@ -186,12 +186,12 @@ static int path_is_runtime(const LookupPaths *p, const char *path) {
         if (!parent)
                 return -ENOMEM;
 
-        return path_equal(parent, p->runtime_config) ||
-               path_equal(parent, p->generator) ||
-               path_equal(parent, p->generator_early) ||
-               path_equal(parent, p->generator_late) ||
-               path_equal(parent, p->transient) ||
-               path_equal(parent, p->runtime_control);
+        return path_equal_ptr(parent, p->runtime_config) ||
+               path_equal_ptr(parent, p->generator) ||
+               path_equal_ptr(parent, p->generator_early) ||
+               path_equal_ptr(parent, p->generator_late) ||
+               path_equal_ptr(parent, p->transient) ||
+               path_equal_ptr(parent, p->runtime_control);
 }
 
 static int path_is_vendor(const LookupPaths *p, const char *path) {
@@ -754,7 +754,7 @@ static int install_info_may_process(UnitFileInstallInfo *i, const LookupPaths *p
          * not subject to enable/disable operations. */
 
         if (i->type == UNIT_FILE_TYPE_MASKED)
-                return -ESHUTDOWN;
+                return -ERFKILL;
         if (path_is_generator(paths, i->path))
                 return -EADDRNOTAVAIL;
         if (path_is_transient(paths, i->path))
@@ -2539,6 +2539,9 @@ int unit_file_preset_all(
                                 continue;
 
                         r = preset_prepare_one(scope, &plus, &minus, &paths, mode, de->d_name);
+                        if (r == -ERFKILL)
+                                r = unit_file_changes_add(changes, n_changes,
+                                                          UNIT_FILE_IS_MASKED, de->d_name, NULL);
                         if (r < 0)
                                 return r;
                 }
@@ -2652,6 +2655,7 @@ DEFINE_STRING_TABLE_LOOKUP(unit_file_state, UnitFileState);
 static const char* const unit_file_change_type_table[_UNIT_FILE_CHANGE_TYPE_MAX] = {
         [UNIT_FILE_SYMLINK] = "symlink",
         [UNIT_FILE_UNLINK] = "unlink",
+        [UNIT_FILE_IS_MASKED] = "masked",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(unit_file_change_type, UnitFileChangeType);
diff --git a/src/shared/install.h b/src/shared/install.h
index 6f8b45d4f6..6b760def9b 100644
--- a/src/shared/install.h
+++ b/src/shared/install.h
@@ -72,10 +72,15 @@ enum UnitFilePresetMode {
 enum UnitFileChangeType {
         UNIT_FILE_SYMLINK,
         UNIT_FILE_UNLINK,
+        UNIT_FILE_IS_MASKED,
         _UNIT_FILE_CHANGE_TYPE_MAX,
         _UNIT_FILE_CHANGE_TYPE_INVALID = -1
 };
 
+static inline bool unit_file_change_is_modification(UnitFileChangeType type) {
+        return IN_SET(type, UNIT_FILE_SYMLINK, UNIT_FILE_UNLINK);
+}
+
 struct UnitFileChange {
         UnitFileChangeType type;
         char *path;
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 91829fcbcf..0c64cbab80 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -1989,12 +1989,20 @@ static void dump_unit_file_changes(const UnitFileChange *changes, unsigned n_cha
 
         assert(changes || n_changes == 0);
 
-        for (i = 0; i < n_changes; i++) {
-                if (changes[i].type == UNIT_FILE_SYMLINK)
+        for (i = 0; i < n_changes; i++)
+                switch(changes[i].type) {
+                case UNIT_FILE_SYMLINK:
                         log_info("Created symlink %s, pointing to %s.", changes[i].path, changes[i].source);
-                else
+                        break;
+                case UNIT_FILE_UNLINK:
                         log_info("Removed %s.", changes[i].path);
-        }
+                        break;
+                case UNIT_FILE_IS_MASKED:
+                        log_info("Unit %s is masked, ignoring.", changes[i].path);
+                        break;
+                default:
+                        assert_not_reached("bad change type");
+                }
 }
 
 static int set_default(int argc, char *argv[], void *userdata) {
@@ -5272,7 +5280,7 @@ static int enable_sysv_units(const char *verb, char **args) {
                         continue;
 
                 j = unit_file_exists(arg_scope, &paths, name);
-                if (j < 0 && !IN_SET(j, -ELOOP, -ESHUTDOWN, -EADDRNOTAVAIL))
+                if (j < 0 && !IN_SET(j, -ELOOP, -ERFKILL, -EADDRNOTAVAIL))
                         return log_error_errno(j, "Failed to lookup unit file state: %m");
                 found_native = j != 0;
 
@@ -5442,7 +5450,7 @@ static int enable_unit(int argc, char *argv[], void *userdata) {
                 else
                         assert_not_reached("Unknown verb");
 
-                if (r == -ESHUTDOWN)
+                if (r == -ERFKILL)
                         return log_error_errno(r, "Unit file is masked.");
                 if (r == -EADDRNOTAVAIL)
                         return log_error_errno(r, "Unit file is transient or generated.");
@@ -5617,7 +5625,7 @@ static int add_dependency(int argc, char *argv[], void *userdata) {
                 unsigned n_changes = 0;
 
                 r = unit_file_add_dependency(arg_scope, arg_runtime, arg_root, names, target, dep, arg_force, &changes, &n_changes);
-                if (r == -ESHUTDOWN)
+                if (r == -ERFKILL)
                         return log_error_errno(r, "Unit file is masked.");
                 if (r == -EADDRNOTAVAIL)
                         return log_error_errno(r, "Unit file is transient or generated.");
@@ -6718,7 +6726,7 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_ROOT:
-                        r = parse_path_argument_and_warn(optarg, true, &arg_root);
+                        r = parse_path_argument_and_warn(optarg, false, &arg_root);
                         if (r < 0)
                                 return r;
                         break;
diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c
index 9392a3d2b5..fe4bbeeb75 100644
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -807,7 +807,7 @@ static int enumerate_sysv(const LookupPaths *lp, Hashmap *all_services) {
                                 continue;
 
                         r = unit_file_exists(UNIT_FILE_SYSTEM, lp, name);
-                        if (r < 0 && !IN_SET(r, -ELOOP, -ESHUTDOWN, -EADDRNOTAVAIL)) {
+                        if (r < 0 && !IN_SET(r, -ELOOP, -ERFKILL, -EADDRNOTAVAIL)) {
                                 log_debug_errno(r, "Failed to detect whether %s exists, skipping: %m", name);
                                 continue;
                         } else if (r != 0) {
diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c
index 2138655e29..998c345a1a 100644
--- a/src/test/test-install-root.c
+++ b/src/test/test-install-root.c
@@ -80,7 +80,7 @@ static void test_basic_mask_and_enable(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", &state) >= 0 && state == UNIT_FILE_MASKED);
 
         /* Enabling a masked unit should fail! */
-        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, false, root, STRV_MAKE("a.service"), false, &changes, &n_changes) == -ESHUTDOWN);
+        assert_se(unit_file_enable(UNIT_FILE_SYSTEM, false, root, STRV_MAKE("a.service"), false, &changes, &n_changes) == -ERFKILL);
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;
 
diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
index d376dd56c5..5d77e2959c 100644
--- a/src/test/test-path-util.c
+++ b/src/test/test-path-util.c
@@ -90,6 +90,18 @@ static void test_path(void) {
                 assert_se(path_equal(path_kill_slashes(p2), "/aaa/./ccc"));
                 assert_se(path_equal(path_kill_slashes(p3), "/./"));
         }
+
+        assert_se(PATH_IN_SET("/bin", "/", "/bin", "/foo"));
+        assert_se(PATH_IN_SET("/bin", "/bin"));
+        assert_se(PATH_IN_SET("/bin", "/foo/bar", "/bin"));
+        assert_se(PATH_IN_SET("/", "/", "/", "/foo/bar"));
+        assert_se(!PATH_IN_SET("/", "/abc", "/def"));
+
+        assert_se(path_equal_ptr(NULL, NULL));
+        assert_se(path_equal_ptr("/a", "/a"));
+        assert_se(!path_equal_ptr("/a", "/b"));
+        assert_se(!path_equal_ptr("/a", NULL));
+        assert_se(!path_equal_ptr(NULL, "/a"));
 }
 
 static void test_find_binary(const char *self) {