SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

shared/install: rework error propagation again

Browse Source 
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.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek
2026-01-29 08:49:44 +01:00
parent c67d64add6
commit 64eb78135c
7 changed files with 20 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/shared/install.c
+12 -6
View File
@@ -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;
}
src/shared/install.h
+1 -1
View File
@@ -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,