system/systemd
system/systemd
1
0
Fork 0
mirror of https://github.com/systemd/systemd synced 2024-10-02 22:37:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

shared/specifier: provide proper error messages when specifiers fail to read files

Browse source 
ENOENT is easily confused with the file that we're working on not being
present, e.g. when the file contains %o or something else that requires
os-release to be present. Let's use -EUNATCH instead to reduce that chances of
confusion if the context of the error is lost.

And once we have pinpointed the reason, let's provide a proper error message:

+ build/systemctl --root=/tmp/systemctl-test.TO7Mcb enable some-some-link6@.socket
/tmp/systemctl-test.TO7Mcb/etc/systemd/system/some-some-link6@.socket: Failed to resolve alias "target@A:%A.socket": Protocol driver not attached
Failed to enable unit, cannot resolve specifiers in "target@A:%A.socket".
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2022-03-08 12:08:00 +01:00
parent 7962116fc8
commit 6ec4c852c9
4 changed files with 82 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
src/shared/install.c
View file

@ -374,6 +374,7 @@ void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *chang
verb, changes[i].path);
logged = true;
break;
case -EADDRNOTAVAIL:
log_error_errno(changes[i].type_or_errno, "Failed to %s unit, unit %s is transient or generated.",
verb, changes[i].path);
@ -401,6 +402,12 @@ void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *chang
logged = true;
break;
case -EUNATCH:
log_error_errno(changes[i].type_or_errno, "Failed to %s unit, cannot resolve specifiers in \"%s\".",
verb, changes[i].path);
logged = true;
break;
default:
assert(changes[i].type_or_errno < 0);
log_error_errno(changes[i].type_or_errno, "Failed to %s unit, file \"%s\": %m",
@ -1151,7 +1158,8 @@ static int config_parse_also(
r = install_name_printf(info, word, info->root, &printed);
if (r < 0)
return r;
return log_syntax(unit, LOG_WARNING, filename, line, r,
"Failed to resolve unit name in Also=\"%s\": %m", word);
r = install_info_add(c, printed, NULL, info->root, /* auxiliary= */ true, NULL);
if (r < 0)
@ -1198,14 +1206,13 @@ static int config_parse_default_instance(
r = install_name_printf(i, rvalue, i->root, &printed);
if (r < 0)
return r;
return log_syntax(unit, LOG_WARNING, filename, line, r,
"Failed to resolve instance name in DefaultInstance=\"%s\": %m", rvalue);
if (isempty(printed)) {
i->default_instance = mfree(i->default_instance);
return 0;
}
if (isempty(printed))
printed = mfree(printed);
if (!unit_instance_is_valid(printed))
if (printed && !unit_instance_is_valid(printed))
return log_syntax(unit, LOG_WARNING, filename, line, SYNTHETIC_ERRNO(EINVAL),
"Invalid DefaultInstance= value \"%s\".", printed);
@ -1768,8 +1775,10 @@ static int install_info_symlink_alias(
_cleanup_free_ char *alias_path = NULL, *dst = NULL, *dst_updated = NULL;
q = install_name_printf(i, *s, i->root, &dst);
if (q < 0)
if (q < 0) {
unit_file_changes_add(changes, n_changes, q, *s, NULL);
return q;
}
q = unit_file_verify_alias(i, dst, &dst_updated);
if (q < 0)
@ -1852,8 +1861,10 @@ static int install_info_symlink_wants(
_cleanup_free_ char *path = NULL, *dst = NULL;
q = install_name_printf(i, *s, i->root, &dst);
if (q < 0)
if (q < 0) {
unit_file_changes_add(changes, n_changes, q, *s, NULL);
return q;
}
if (!unit_name_is_valid(dst, valid_dst_type)) {
/* Generate a proper error here: EUCLEAN if the name is generally bad, EIDRM if the
@ -3307,7 +3318,7 @@ int unit_file_preset_all(
r = preset_prepare_one(scope, &plus, &minus, &lp, de->d_name, &presets, changes, n_changes);
if (r < 0 &&
!IN_SET(r, -EEXIST, -ERFKILL, -EADDRNOTAVAIL, -EIDRM, -EUCLEAN, -ELOOP, -ENOENT))
!IN_SET(r, -EEXIST, -ERFKILL, -EADDRNOTAVAIL, -EIDRM, -EUCLEAN, -ELOOP, -ENOENT, -EUNATCH))
/* Ignore generated/transient/missing/invalid units when applying preset, propagate other errors.
* Coordinate with unit_file_dump_changes() above. */
return r;

33
src/shared/specifier.c
View file

@ -162,7 +162,8 @@ int specifier_machine_id(char specifier, const void *data, const char *root, con
fd = chase_symlinks_and_open("/etc/machine-id", root, CHASE_PREFIX_ROOT, O_RDONLY|O_CLOEXEC|O_NOCTTY, NULL);
if (fd < 0)
return fd;
/* Translate error for missing os-release file to EUNATCH. */
return fd == -ENOENT ? -EUNATCH : fd;
r = id128_read_fd(fd, ID128_PLAIN, &id);
} else
@ -270,37 +271,41 @@ int specifier_architecture(char specifier, const void *data, const char *root, c
/* Note: fields in /etc/os-release might quite possibly be missing, even if everything is entirely valid
* otherwise. We'll return an empty value or NULL in that case from the functions below. But if the
* os-release file is missing, we'll return -ENOENT. This means that something is seriously wrong with the
* os-release file is missing, we'll return -EUNATCH. This means that something is seriously wrong with the
* installation. */
int specifier_os_id(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
static int parse_os_release_specifier(const char *root, const char *id, char **ret) {
int r;
assert(ret);
return parse_os_release(root, "ID", ret);
/* Translate error for missing os-release file to EUNATCH. */
r = parse_os_release(root, id, ret);
return r == -ENOENT ? -EUNATCH : r;
}
int specifier_os_id(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
return parse_os_release_specifier(root, "ID", ret);
}
int specifier_os_version_id(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
assert(ret);
return parse_os_release(root, "VERSION_ID", ret);
return parse_os_release_specifier(root, "VERSION_ID", ret);
}
int specifier_os_build_id(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
assert(ret);
return parse_os_release(root, "BUILD_ID", ret);
return parse_os_release_specifier(root, "BUILD_ID", ret);
}
int specifier_os_variant_id(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
assert(ret);
return parse_os_release(root, "VARIANT_ID", ret);
return parse_os_release_specifier(root, "VARIANT_ID", ret);
}
int specifier_os_image_id(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
assert(ret);
return parse_os_release(root, "IMAGE_ID", ret);
return parse_os_release_specifier(root, "IMAGE_ID", ret);
}
int specifier_os_image_version(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
assert(ret);
return parse_os_release(root, "IMAGE_VERSION", ret);
return parse_os_release_specifier(root, "IMAGE_VERSION", ret);
}
int specifier_group_name(char specifier, const void *data, const char *root, const void *userdata, char **ret) {

2
src/test/test-specifier.c
View file

@ -146,7 +146,7 @@ TEST(specifiers_missing_data_ok) {
assert_se(streq(resolved, "-----"));
assert_se(setenv("SYSTEMD_OS_RELEASE", "/nosuchfileordirectory", 1) == 0);
assert_se(specifier_printf("%A-%B-%M-%o-%w-%W", SIZE_MAX, specifier_table, NULL, NULL, &resolved) == -ENOENT);
assert_se(specifier_printf("%A-%B-%M-%o-%w-%W", SIZE_MAX, specifier_table, NULL, NULL, &resolved) == -EUNATCH);
assert_se(streq(resolved, "-----"));
assert_se(unsetenv("SYSTEMD_OS_RELEASE") == 0);

55
test/test-systemctl-enable.sh
View file

@ -445,13 +445,46 @@ EOF
check_alias a "$(uname -m | tr '_' '-')"
# FIXME: when os-release is not found, we fail we a cryptic error
# Alias=target@%A.socket
test ! -e "$root/etc/os-release"
test ! -e "$root/usr/lib/os-release"
check_alias A '' && { echo "Expected failure" >&2; exit 1; }
check_alias B '' && { echo "Expected failure" >&2; exit 1; }
check_alias M '' && { echo "Expected failure" >&2; exit 1; }
check_alias o '' && { echo "Expected failure" >&2; exit 1; }
check_alias w '' && { echo "Expected failure" >&2; exit 1; }
check_alias W '' && { echo "Expected failure" >&2; exit 1; }
cat >"$root/etc/os-release" <<EOF
# empty
EOF
check_alias A ''
check_alias B ''
check_alias M ''
check_alias o ''
check_alias w ''
check_alias W ''
cat >"$root/etc/os-release" <<EOF
ID='the-id'
VERSION_ID=39a
BUILD_ID=build-id
VARIANT_ID=wrong
VARIANT_ID=right
IMAGE_ID="foobar"
IMAGE_VERSION='1-2-3'
EOF
check_alias A '1-2-3'
check_alias B 'build-id'
check_alias M 'foobar'
check_alias o 'the-id'
check_alias w '39a'
check_alias W 'right'
check_alias b "$(systemd-id128 boot-id)"
# Alias=target@%B.socket
# FIXME: Failed to enable: Invalid slot.
# Alias=target@%C.socket
# Alias=target@%E.socket
@ -479,18 +512,15 @@ check_alias l "$(uname -n | sed 's/\..*//')"
# FIXME: Failed to enable: Invalid slot.
# Alias=target@%L.socket
# FIXME: Failed to enable: No such file or directory.
# Alias=target@%m.socket
test ! -e "$root/etc/machine-id"
check_alias m '' && { echo "Expected failure" >&2; exit 1; }
# FIXME: Failed to enable: No such file or directory.
# Alias=target@%M.socket
systemd-id128 new >"$root/etc/machine-id"
check_alias m "$(cat "$root/etc/machine-id")"
check_alias n 'some-some-link6@.socket'
check_alias N 'some-some-link6@'
# FIXME: Failed to enable: No such file or directory.
# Alias=target@%o.socket
check_alias p 'some-some-link6'
# FIXME: Failed to enable: Invalid slot.
@ -509,9 +539,6 @@ check_alias v "$(uname -r)"
# FIXME: Failed to enable: Invalid slot.
# Alias=target@%V.socket
# Alias=target@%w.socket
# Alias=target@%W.socket
check_alias % '%' && { echo "Expected failure because % is not legal in unit name" >&2; exit 1; }
check_alias z 'z' && { echo "Expected failure because %z is not known" >&2; exit 1; }