shaba/systemd
1
0
Fork 0
mirror of https://github.com/systemd/systemd.git synced 2024-10-31 07:51:21 +03:00
Code

shared/install: propagate errors about invalid aliases and such too

Browse Source 
If an invalid arg appears in [Install] Alias=, WantedBy=, RequiredBy=,
we'd warn in the logs, but not propagate this information to the caller,
and in particular not over dbus. But if we call "systemctl enable" on a
unit, and the config if invalid, this information is quite important.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2022-03-10 11:03:41 +01:00
parent 32450f5348
commit cbfdbffb61
6 changed files with 88 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
src/basic/unit-file.c
View File

@ -69,7 +69,7 @@ int unit_symlink_name_compatible(const char *symlink, const char *target, bool i
return 0; return 0;
} }
int unit_validate_alias_symlink_and_warn(const char *filename, const char *target) { int unit_validate_alias_symlink_or_warn(int log_level, const char *filename, const char *target) {
const char *src, *dst; const char *src, *dst;
_cleanup_free_ char *src_instance = NULL, *dst_instance = NULL; _cleanup_free_ char *src_instance = NULL, *dst_instance = NULL;
UnitType src_unit_type, dst_unit_type; UnitType src_unit_type, dst_unit_type;
@ -92,51 +92,51 @@ int unit_validate_alias_symlink_and_warn(const char *filename, const char *targe
src_name_type = unit_name_to_instance(src, &src_instance); src_name_type = unit_name_to_instance(src, &src_instance);
if (src_name_type < 0) if (src_name_type < 0)
return log_notice_errno(src_name_type, return log_full_errno(log_level, src_name_type,
"%s: not a valid unit name \"%s\": %m", filename, src); "%s: not a valid unit name \"%s\": %m", filename, src);
src_unit_type = unit_name_to_type(src); src_unit_type = unit_name_to_type(src);
assert(src_unit_type >= 0); /* unit_name_to_instance() checked the suffix already */ assert(src_unit_type >= 0); /* unit_name_to_instance() checked the suffix already */
if (!unit_type_may_alias(src_unit_type)) if (!unit_type_may_alias(src_unit_type))
return log_notice_errno(SYNTHETIC_ERRNO(EINVAL), return log_full_errno(log_level, SYNTHETIC_ERRNO(EINVAL),
"%s: symlinks are not allowed for units of this type, rejecting.", "%s: symlinks are not allowed for units of this type, rejecting.",
filename); filename);
if (src_name_type != UNIT_NAME_PLAIN && if (src_name_type != UNIT_NAME_PLAIN &&
!unit_type_may_template(src_unit_type)) !unit_type_may_template(src_unit_type))
return log_notice_errno(SYNTHETIC_ERRNO(EINVAL), return log_full_errno(log_level, SYNTHETIC_ERRNO(EINVAL),
"%s: templates not allowed for %s units, rejecting.", "%s: templates not allowed for %s units, rejecting.",
filename, unit_type_to_string(src_unit_type)); filename, unit_type_to_string(src_unit_type));
/* dst checks */ /* dst checks */
dst_name_type = unit_name_to_instance(dst, &dst_instance); dst_name_type = unit_name_to_instance(dst, &dst_instance);
if (dst_name_type < 0) if (dst_name_type < 0)
return log_notice_errno(dst_name_type == -EINVAL ? SYNTHETIC_ERRNO(EXDEV) : dst_name_type, return log_full_errno(log_level, dst_name_type == -EINVAL ? SYNTHETIC_ERRNO(EXDEV) : dst_name_type,
"%s points to \"%s\" which is not a valid unit name: %m", "%s points to \"%s\" which is not a valid unit name: %m",
filename, dst); filename, dst);
if (!(dst_name_type == src_name_type || if (!(dst_name_type == src_name_type ||
(src_name_type == UNIT_NAME_INSTANCE && dst_name_type == UNIT_NAME_TEMPLATE))) (src_name_type == UNIT_NAME_INSTANCE && dst_name_type == UNIT_NAME_TEMPLATE)))
return log_notice_errno(SYNTHETIC_ERRNO(EXDEV), return log_full_errno(log_level, SYNTHETIC_ERRNO(EXDEV),
"%s: symlink target name type \"%s\" does not match source, rejecting.", "%s: symlink target name type \"%s\" does not match source, rejecting.",
filename, dst); filename, dst);
if (dst_name_type == UNIT_NAME_INSTANCE) { if (dst_name_type == UNIT_NAME_INSTANCE) {
assert(src_instance); assert(src_instance);
assert(dst_instance); assert(dst_instance);
if (!streq(src_instance, dst_instance)) if (!streq(src_instance, dst_instance))
return log_notice_errno(SYNTHETIC_ERRNO(EXDEV), return log_full_errno(log_level, SYNTHETIC_ERRNO(EXDEV),
"%s: unit symlink target \"%s\" instance name doesn't match, rejecting.", "%s: unit symlink target \"%s\" instance name doesn't match, rejecting.",
filename, dst); filename, dst);
} }
dst_unit_type = unit_name_to_type(dst); dst_unit_type = unit_name_to_type(dst);
if (dst_unit_type != src_unit_type) if (dst_unit_type != src_unit_type)
return log_notice_errno(SYNTHETIC_ERRNO(EXDEV), return log_full_errno(log_level, SYNTHETIC_ERRNO(EXDEV),
"%s: symlink target \"%s\" has incompatible suffix, rejecting.", "%s: symlink target \"%s\" has incompatible suffix, rejecting.",
filename, dst); filename, dst);
return 0; return 0;
} }
@ -354,7 +354,7 @@ int unit_file_resolve_symlink(
"Suspicious symlink %s/%s→%s, treating as alias.", "Suspicious symlink %s/%s→%s, treating as alias.",
dir, filename, simplified); dir, filename, simplified);
r = unit_validate_alias_symlink_and_warn(filename, simplified); r = unit_validate_alias_symlink_or_warn(LOG_NOTICE, filename, simplified);
if (r < 0) if (r < 0)
return r; return r;

2
src/basic/unit-file.h
View File

@ -41,7 +41,7 @@ bool unit_type_may_alias(UnitType type) _const_;
bool unit_type_may_template(UnitType type) _const_; bool unit_type_may_template(UnitType type) _const_;
int unit_symlink_name_compatible(const char *symlink, const char *target, bool instance_propagation); int unit_symlink_name_compatible(const char *symlink, const char *target, bool instance_propagation);
int unit_validate_alias_symlink_and_warn(const char *filename, const char *target); int unit_validate_alias_symlink_or_warn(int log_level, const char *filename, const char *target);
bool lookup_paths_timestamp_hash_same(const LookupPaths *lp, uint64_t timestamp_hash, uint64_t *ret_new); bool lookup_paths_timestamp_hash_same(const LookupPaths *lp, uint64_t timestamp_hash, uint64_t *ret_new);

61
src/shared/install.c
View File

@ -394,6 +394,14 @@ void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *chang
err = log_error_errno(changes[i].type_or_errno, "Failed to %s unit, refusing to operate on linked unit file %s.", err = log_error_errno(changes[i].type_or_errno, "Failed to %s unit, refusing to operate on linked unit file %s.",
verb, changes[i].path); verb, changes[i].path);
break; break;
case -EXDEV:
if (changes[i].source)
err = log_error_errno(changes[i].type_or_errno, "Failed to %s unit, cannot alias %s as %s.",
verb, changes[i].source, changes[i].path);
else
err = log_error_errno(changes[i].type_or_errno, "Failed to %s unit, invalid unit reference \"%s\".",
verb, changes[i].path);
break;
case -ENOENT: case -ENOENT:
err = log_error_errno(changes[i].type_or_errno, "Failed to %s unit, unit %s does not exist.", err = log_error_errno(changes[i].type_or_errno, "Failed to %s unit, unit %s does not exist.",
verb, changes[i].path); verb, changes[i].path);
@ -1671,7 +1679,13 @@ static int install_info_discover_and_check(
return install_info_may_process(ret ? *ret : NULL, lp, changes, n_changes); return install_info_may_process(ret ? *ret : NULL, lp, changes, n_changes);
} }
int unit_file_verify_alias(const UnitFileInstallInfo *info, const char *dst, char **ret_dst) { int unit_file_verify_alias(
const UnitFileInstallInfo *info,
const char *dst,
char **ret_dst,
UnitFileChange **changes,
size_t *n_changes) {
_cleanup_free_ char *dst_updated = NULL; _cleanup_free_ char *dst_updated = NULL;
int r; int r;
@ -1698,15 +1712,19 @@ int unit_file_verify_alias(const UnitFileInstallInfo *info, const char *dst, cha
p = endswith(dir, ".wants"); p = endswith(dir, ".wants");
if (!p) if (!p)
p = endswith(dir, ".requires"); p = endswith(dir, ".requires");
if (!p) if (!p) {
return log_warning_errno(SYNTHETIC_ERRNO(EXDEV), unit_file_changes_add(changes, n_changes, -EXDEV, dst, NULL);
"Invalid path \"%s\" in alias.", dir); return log_debug_errno(SYNTHETIC_ERRNO(EXDEV), "Invalid path \"%s\" in alias.", dir);
}
*p = '\0'; /* dir should now be a unit name */ *p = '\0'; /* dir should now be a unit name */
UnitNameFlags type = unit_name_classify(dir); UnitNameFlags type = unit_name_classify(dir);
if (type < 0) if (type < 0) {
return log_warning_errno(SYNTHETIC_ERRNO(EXDEV), unit_file_changes_add(changes, n_changes, -EXDEV, dst, NULL);
"Invalid unit name component \"%s\" in alias.", dir); return log_debug_errno(SYNTHETIC_ERRNO(EXDEV),
"Invalid unit name component \"%s\" in alias.", dir);
}
const bool instance_propagation = type == UNIT_NAME_TEMPLATE; const bool instance_propagation = type == UNIT_NAME_TEMPLATE;
@ -1714,10 +1732,12 @@ int unit_file_verify_alias(const UnitFileInstallInfo *info, const char *dst, cha
r = unit_symlink_name_compatible(path_alias, info->name, instance_propagation); r = unit_symlink_name_compatible(path_alias, info->name, instance_propagation);
if (r < 0) if (r < 0)
return log_error_errno(r, "Failed to verify alias validity: %m"); return log_error_errno(r, "Failed to verify alias validity: %m");
if (r == 0) if (r == 0) {
return log_warning_errno(SYNTHETIC_ERRNO(EXDEV), unit_file_changes_add(changes, n_changes, -EXDEV, dst, info->name);
"Invalid unit \"%s\" symlink \"%s\".", return log_debug_errno(SYNTHETIC_ERRNO(EXDEV),
info->name, dst); "Invalid unit \"%s\" symlink \"%s\".",
info->name, dst);
}
} else { } else {
/* If the symlink target has an instance set and the symlink source doesn't, we "propagate /* If the symlink target has an instance set and the symlink source doesn't, we "propagate
@ -1726,8 +1746,10 @@ int unit_file_verify_alias(const UnitFileInstallInfo *info, const char *dst, cha
_cleanup_free_ char *inst = NULL; _cleanup_free_ char *inst = NULL;
UnitNameFlags type = unit_name_to_instance(info->name, &inst); UnitNameFlags type = unit_name_to_instance(info->name, &inst);
if (type < 0) if (type < 0) {
return log_error_errno(type, "Failed to extract instance name from \"%s\": %m", info->name); unit_file_changes_add(changes, n_changes, -EUCLEAN, info->name, NULL);
return log_debug_errno(type, "Failed to extract instance name from \"%s\": %m", info->name);
}
if (type == UNIT_NAME_INSTANCE) { if (type == UNIT_NAME_INSTANCE) {
r = unit_name_replace_instance(dst, inst, &dst_updated); r = unit_name_replace_instance(dst, inst, &dst_updated);
@ -1737,9 +1759,14 @@ int unit_file_verify_alias(const UnitFileInstallInfo *info, const char *dst, cha
} }
} }
r = unit_validate_alias_symlink_and_warn(dst_updated ?: dst, info->name); r = unit_validate_alias_symlink_or_warn(LOG_DEBUG, dst_updated ?: dst, info->name);
if (r < 0) if (r < 0) {
unit_file_changes_add(changes, n_changes,
r == -EINVAL ? -EXDEV : r,
dst_updated ?: dst,
info->name);
return r; return r;
}
} }
*ret_dst = TAKE_PTR(dst_updated); *ret_dst = TAKE_PTR(dst_updated);
@ -1770,7 +1797,7 @@ static int install_info_symlink_alias(
return q; return q;
} }
q = unit_file_verify_alias(info, dst, &dst_updated); q = unit_file_verify_alias(info, dst, &dst_updated, changes, n_changes);
if (q < 0) if (q < 0)
continue; continue;
@ -3307,7 +3334,7 @@ int unit_file_preset_all(
r = preset_prepare_one(scope, &plus, &minus, &lp, de->d_name, &presets, changes, n_changes); r = preset_prepare_one(scope, &plus, &minus, &lp, de->d_name, &presets, changes, n_changes);
if (r < 0 && if (r < 0 &&
!IN_SET(r, -EEXIST, -ERFKILL, -EADDRNOTAVAIL, -EBADSLT, -EIDRM, -EUCLEAN, -ELOOP, -ENOENT, -EUNATCH)) !IN_SET(r, -EEXIST, -ERFKILL, -EADDRNOTAVAIL, -EBADSLT, -EIDRM, -EUCLEAN, -ELOOP, -ENOENT, -EUNATCH, -EXDEV))
/* Ignore generated/transient/missing/invalid units when applying preset, propagate other errors. /* Ignore generated/transient/missing/invalid units when applying preset, propagate other errors.
* Coordinate with unit_file_dump_changes() above. */ * Coordinate with unit_file_dump_changes() above. */
return r; return r;

7
src/shared/install.h
View File

@ -193,7 +193,12 @@ int unit_file_changes_add(UnitFileChange **changes, size_t *n_changes, int type,
void unit_file_changes_free(UnitFileChange *changes, size_t n_changes); void unit_file_changes_free(UnitFileChange *changes, size_t n_changes);
void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *changes, size_t n_changes, bool quiet); void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *changes, size_t n_changes, bool quiet);
int unit_file_verify_alias(const UnitFileInstallInfo *i, const char *dst, char **ret_dst); int unit_file_verify_alias(
const UnitFileInstallInfo *info,
const char *dst,
char **ret_dst,
UnitFileChange **changes,
size_t *n_changes);
typedef struct UnitFilePresetRule UnitFilePresetRule; typedef struct UnitFilePresetRule UnitFilePresetRule;

2
src/test/test-install-root.c
View File

@ -1091,7 +1091,7 @@ static void verify_one(
if (i != last_info) if (i != last_info)
log_info("-- %s --", (last_info = i)->name); log_info("-- %s --", (last_info = i)->name);
r = unit_file_verify_alias(i, alias, &alias2); r = unit_file_verify_alias(i, alias, &alias2, NULL, NULL);
log_info_errno(r, "alias %s ← %s: %d/%m (expected %d)%s%s%s", log_info_errno(r, "alias %s ← %s: %d/%m (expected %d)%s%s%s",
i->name, alias, r, expected, i->name, alias, r, expected,
alias2 ? " [" : "", strempty(alias2), alias2 ? " [" : "", strempty(alias2),

28
src/test/test-unit-file.c
View File

@ -8,20 +8,20 @@
#include "unit-file.h" #include "unit-file.h"
TEST(unit_validate_alias_symlink_and_warn) { TEST(unit_validate_alias_symlink_and_warn) {
assert_se(unit_validate_alias_symlink_and_warn("/path/a.service", "/other/b.service") == 0); assert_se(unit_validate_alias_symlink_or_warn(LOG_INFO, "/path/a.service", "/other/b.service") == 0);
assert_se(unit_validate_alias_symlink_and_warn("/path/a.service", "/other/b.socket") == -EXDEV); assert_se(unit_validate_alias_symlink_or_warn(LOG_INFO, "/path/a.service", "/other/b.socket") == -EXDEV);
assert_se(unit_validate_alias_symlink_and_warn("/path/a.service", "/other/b.foobar") == -EXDEV); assert_se(unit_validate_alias_symlink_or_warn(LOG_INFO, "/path/a.service", "/other/b.foobar") == -EXDEV);
assert_se(unit_validate_alias_symlink_and_warn("/path/a@.service", "/other/b@.service") == 0); assert_se(unit_validate_alias_symlink_or_warn(LOG_INFO, "/path/a@.service", "/other/b@.service") == 0);
assert_se(unit_validate_alias_symlink_and_warn("/path/a@.service", "/other/b@.socket") == -EXDEV); assert_se(unit_validate_alias_symlink_or_warn(LOG_INFO, "/path/a@.service", "/other/b@.socket") == -EXDEV);
assert_se(unit_validate_alias_symlink_and_warn("/path/a@XXX.service", "/other/b@YYY.service") == -EXDEV); assert_se(unit_validate_alias_symlink_or_warn(LOG_INFO, "/path/a@XXX.service", "/other/b@YYY.service") == -EXDEV);
assert_se(unit_validate_alias_symlink_and_warn("/path/a@XXX.service", "/other/b@YYY.socket") == -EXDEV); assert_se(unit_validate_alias_symlink_or_warn(LOG_INFO, "/path/a@XXX.service", "/other/b@YYY.socket") == -EXDEV);
assert_se(unit_validate_alias_symlink_and_warn("/path/a@.service", "/other/b@YYY.service") == -EXDEV); assert_se(unit_validate_alias_symlink_or_warn(LOG_INFO, "/path/a@.service", "/other/b@YYY.service") == -EXDEV);
assert_se(unit_validate_alias_symlink_and_warn("/path/a@XXX.service", "/other/b@XXX.service") == 0); assert_se(unit_validate_alias_symlink_or_warn(LOG_INFO, "/path/a@XXX.service", "/other/b@XXX.service") == 0);
assert_se(unit_validate_alias_symlink_and_warn("/path/a@XXX.service", "/other/b@.service") == 0); assert_se(unit_validate_alias_symlink_or_warn(LOG_INFO, "/path/a@XXX.service", "/other/b@.service") == 0);
assert_se(unit_validate_alias_symlink_and_warn("/path/a@.service", "/other/b.service") == -EXDEV); assert_se(unit_validate_alias_symlink_or_warn(LOG_INFO, "/path/a@.service", "/other/b.service") == -EXDEV);
assert_se(unit_validate_alias_symlink_and_warn("/path/a.service", "/other/b@.service") == -EXDEV); assert_se(unit_validate_alias_symlink_or_warn(LOG_INFO, "/path/a.service", "/other/b@.service") == -EXDEV);
assert_se(unit_validate_alias_symlink_and_warn("/path/a@.slice", "/other/b.slice") == -EINVAL); assert_se(unit_validate_alias_symlink_or_warn(LOG_INFO, "/path/a@.slice", "/other/b.slice") == -EINVAL);
assert_se(unit_validate_alias_symlink_and_warn("/path/a.slice", "/other/b.slice") == -EINVAL); assert_se(unit_validate_alias_symlink_or_warn(LOG_INFO, "/path/a.slice", "/other/b.slice") == -EINVAL);
} }
TEST(unit_file_build_name_map) { TEST(unit_file_build_name_map) {