shared/install: rework error propagation again

The immediate impulse for this change is the fedora scriptlet which called:
  /usr/lib/systemd/systemd-update-helper install-system-units cryptsetup-pre.target cryptsetup.target getty@.service ... system-systemd\x2dcryptsetup.slice system-systemd\x2dveritysetup.slice ...
which called
  systemctl preset cryptsetup-pre.target cryptsetup.target getty@.service ... system-systemd\x2dcryptsetup.slice system-systemd\x2dveritysetup.slice ...
which threw an error that system-systemdx2dcryptsetup.slice does not exist
and did nothing at all. (The backslash is consumed by the shell.)
The obvious fix here is to figure out more levels of escaping… But we should
do something more robust in such cases.

If we fail in processing of a single unit, let preset all continue processing
units, report the failure through 'changes'. At the end, return failure. In
general, for operations which operate on a list of units specified by the user,
fail the whole operation if any of the individual operations failed. The only
operation where we don't do this is 'preset-all'.

$ SYSTEMD_LOG_LEVEL=debug build/systemctl --root=/ preset asdf1.servie asdf2.path; echo $?
Cannot find unit asdf1.servie.service.
Cannot find unit asdf2.path.
Failed to preset unit: Unit asdf1.servie.service does not exist
Failed to preset unit: Unit asdf2.path does not exist
1

While at it, fix double logging in the manager: dump_unit_changes() already
logs about errors, so the manager should only log on success.

diff --git a/src/core/manager.c b/src/core/manager.c
index 508b9fc9345600aef789dd4076d6887f20105db4..277935bab2074c2fee65ac12937700cfac72547a 100644 (file)
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1946,11 +1946,8 @@ static void manager_preset_all(Manager *m) {
         log_info("Applying preset policy.");
         r = unit_file_preset_all(RUNTIME_SCOPE_SYSTEM, /* file_flags= */ 0,
                                  /* root_dir= */ NULL, mode, &changes, &n_changes);
-        install_changes_dump(r, "preset", changes, n_changes, /* quiet= */ false);
-        if (r < 0)
-                log_full_errno(r == -EEXIST ? LOG_NOTICE : LOG_WARNING, r,
-                               "Failed to populate /etc with preset unit settings, ignoring: %m");
-        else
+        r = install_changes_dump(r, "preset all", changes, n_changes, /* quiet= */ false);
+        if (r >= 0)
                 log_info("Populated /etc with preset unit settings.");
 }
 

diff --git a/src/shared/install.c b/src/shared/install.c
index 7e3469efb065ab80063ef872985ff620af7faf7f..dc27d53f0984ec51fa4e7bf96a5b0747a19985a6 100644 (file)
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -480,7 +480,7 @@ int install_change_dump_error(const InstallChange *change, char **ret_errmsg, co
         return 0;
 }
 
-void install_changes_dump(
+int install_changes_dump(
                 int error,
                 const char *verb,
                 const InstallChange *changes,
@@ -494,6 +494,8 @@ void install_changes_dump(
         assert(verb || error >= 0);
         assert(changes || n_changes == 0);
 
+        /* An error is returned if 'error' contains an error or if any of the changes failed. */
+
         FOREACH_ARRAY(i, changes, n_changes)
                 if (i->type >= 0) {
                         if (!quiet)
@@ -505,17 +507,21 @@ void install_changes_dump(
 
                         r = install_change_dump_error(i, &err_message, /* ret_bus_error= */ NULL);
                         if (r == -ENOMEM)
-                                return (void) log_oom();
+                                return log_oom();
                         if (r < 0)
-                                log_error_errno(r, "Failed to %s unit %s: %m", verb, i->path);
+                                RET_GATHER(error,
+                                           log_error_errno(r, "Failed to %s unit %s: %m", verb, i->path));
                         else
-                                log_error_errno(i->type, "Failed to %s unit: %s", verb, err_message);
+                                RET_GATHER(error,
+                                           log_error_errno(i->type, "Failed to %s unit: %s", verb, err_message));
 
                         err_logged = true;
                 }
 
         if (error < 0 && !err_logged)
-                log_error_errno(error, "Failed to %s unit: %m.", verb);
+                log_error_errno(error, "Failed to %s units: %m.", verb);
+
+        return error;
 }
 
 /**
@@ -3692,7 +3698,7 @@ int unit_file_preset(
 
         STRV_FOREACH(name, names) {
                 r = preset_prepare_one(scope, &plus, &minus, &lp, *name, &presets, changes, n_changes);
-                if (r < 0)
+                if (r < 0 && !ERRNO_IS_NEG_UNIT_ISSUE(r))
                         return r;
         }
 

diff --git a/src/shared/install.h b/src/shared/install.h
index aa2e7c7f59e61d138269516e4893bdcb689bf7b2..c4592fed358ea9b661ded2caa65b6ddaafdabff3 100644 (file)
--- a/src/shared/install.h
+++ b/src/shared/install.h
@@ -192,7 +192,7 @@ InstallChangeType install_changes_add(InstallChange **changes, size_t *n_changes
 void install_changes_free(InstallChange *changes, size_t n_changes);
 
 int install_change_dump_error(const InstallChange *change, char **ret_errmsg, const char **ret_bus_error);
-void install_changes_dump(
+int install_changes_dump(
                 int error,
                 const char *verb,
                 const InstallChange *changes,

diff --git a/src/systemctl/systemctl-add-dependency.c b/src/systemctl/systemctl-add-dependency.c
index b1fd9242092baf5e4632c3bdfbb5057b810d1df2..bc1a1f00f69e5497b3a7fb0c405f016f63438f2e 100644 (file)
--- a/src/systemctl/systemctl-add-dependency.c
+++ b/src/systemctl/systemctl-add-dependency.c
@@ -51,7 +51,7 @@ int verb_add_dependency(int argc, char *argv[], void *userdata) {
                 CLEANUP_ARRAY(changes, n_changes, install_changes_free);
 
                 r = unit_file_add_dependency(arg_runtime_scope, unit_file_flags_from_args(), arg_root, names, target, dep, &changes, &n_changes);
-                install_changes_dump(r, "add dependency on", changes, n_changes, arg_quiet);
+                r = install_changes_dump(r, "add dependency on", changes, n_changes, arg_quiet);
                 if (r < 0)
                         return r;
         } else {

diff --git a/src/systemctl/systemctl-enable.c b/src/systemctl/systemctl-enable.c
index 72e222c2cb81469a74f5c95e88a3328334bee89f..f6277b7287135d1778f11f8d4d98a45e0d1bb2cd 100644 (file)
--- a/src/systemctl/systemctl-enable.c
+++ b/src/systemctl/systemctl-enable.c
@@ -258,7 +258,7 @@ int verb_enable(int argc, char *argv[], void *userdata) {
                 else
                         assert_not_reached();
 
-                install_changes_dump(r, verb, changes, n_changes, arg_quiet);
+                r = install_changes_dump(r, verb, changes, n_changes, arg_quiet);
                 if (r < 0)
                         return r;
         }

diff --git a/src/systemctl/systemctl-preset-all.c b/src/systemctl/systemctl-preset-all.c
index bf681a6d36a1dcb5552bb614920109927f3bcf39..bcc001fab77632cb595038d42a1978e4ad41bf3c 100644 (file)
--- a/src/systemctl/systemctl-preset-all.c
+++ b/src/systemctl/systemctl-preset-all.c
@@ -26,7 +26,8 @@ int verb_preset_all(int argc, char *argv[], void *userdata) {
                 CLEANUP_ARRAY(changes, n_changes, install_changes_free);
 
                 r = unit_file_preset_all(arg_runtime_scope, unit_file_flags_from_args(), arg_root, arg_preset_mode, &changes, &n_changes);
-                install_changes_dump(r, "preset", changes, n_changes, arg_quiet);
+                /* We do not treat propagate failure of individual units here. */
+                (void) install_changes_dump(r, "preset all", changes, n_changes, arg_quiet);
                 if (r < 0)
                         return r;
         } else {

diff --git a/src/systemctl/systemctl-set-default.c b/src/systemctl/systemctl-set-default.c
index 3a8e3969e2047afe2165784f3f1feb8d0a4439da..c4faa31b4c17b70f20696736439b65ed62f3a51e 100644 (file)
--- a/src/systemctl/systemctl-set-default.c
+++ b/src/systemctl/systemctl-set-default.c
@@ -123,7 +123,7 @@ int verb_set_default(int argc, char *argv[], void *userdata) {
                 CLEANUP_ARRAY(changes, n_changes, install_changes_free);
 
                 r = unit_file_set_default(arg_runtime_scope, UNIT_FILE_FORCE, arg_root, unit, &changes, &n_changes);
-                install_changes_dump(r, "set default", changes, n_changes, arg_quiet);
+                r = install_changes_dump(r, "set default", changes, n_changes, arg_quiet);
                 if (r < 0)
                         return r;
         } else {