From: Zbigniew Jędrzejewski-Szmek Date: Sat, 21 Dec 2019 15:09:35 +0000 (+0100) Subject: core,install: allow one more case of "instance propagation" X-Git-Tag: v245-rc1~137^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=1bf15585521c4d23735f340215331cc333ec3a98;p=thirdparty%2Fsystemd.git core,install: allow one more case of "instance propagation" If we have a template unit template@.service, it should be allowed to specify a dependency on a unit without an instance, bar@.service. When the unit is created, the instance will be propagated into the target, so template@inst.service will depend on bar@inst.service. This commit changes unit_dependency_name_compatible(), which makes the manager accept links like that, and unit_file_verify_alias(), so that the installation function will agree to create a symlink like that, and finally the tests are adjusted to pass. --- diff --git a/src/core/load-dropin.c b/src/core/load-dropin.c index 492e816e91a..f61e9da6f28 100644 --- a/src/core/load-dropin.c +++ b/src/core/load-dropin.c @@ -65,7 +65,7 @@ static int process_deps(Unit *u, UnitDependency dependency, const char *dir_suff /* We don't treat this as an error, especially because we didn't check this for a * long time. Nevertheless, we warn, because such mismatch can be mighty confusing. */ - r = unit_symlink_name_compatible(entry, basename(target)); + r = unit_symlink_name_compatible(entry, basename(target), u->instance); if (r < 0) { log_unit_warning_errno(u, r, "Can't check if names %s and %s are compatible, ignoring: %m", entry, basename(target)); diff --git a/src/shared/install.c b/src/shared/install.c index 711638bd16b..2e3f2d565a6 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -1724,6 +1724,7 @@ int unit_file_verify_alias(const UnitFileInstallInfo *i, const char *dst, char * if (path_alias) { /* This branch covers legacy Alias= function of creating .wants and .requires symlinks. */ _cleanup_free_ char *dir = NULL; + char *p; path_alias ++; /* skip over slash */ @@ -1731,12 +1732,23 @@ int unit_file_verify_alias(const UnitFileInstallInfo *i, const char *dst, char * if (!dir) return log_oom(); - if (!endswith(dir, ".wants") && !endswith(dir, ".requires")) + p = endswith(dir, ".wants"); + if (!p) + p = endswith(dir, ".requires"); + if (!p) return log_warning_errno(SYNTHETIC_ERRNO(EXDEV), "Invalid path \"%s\" in alias.", dir); + *p = '\0'; /* dir should now be a unit name */ + + r = unit_name_classify(dir); + if (r < 0) + return log_warning_errno(SYNTHETIC_ERRNO(EXDEV), + "Invalid unit name component \"%s\" in alias.", dir); + + const bool instance_propagation = r == UNIT_NAME_TEMPLATE; /* That's the name we want to use for verification. */ - r = unit_symlink_name_compatible(path_alias, i->name); + r = unit_symlink_name_compatible(path_alias, i->name, instance_propagation); if (r < 0) return log_error_errno(r, "Failed to verify alias validity: %m"); if (r == 0) diff --git a/src/shared/unit-file.c b/src/shared/unit-file.c index dc0ec784640..7ad0ecc8fdc 100644 --- a/src/shared/unit-file.c +++ b/src/shared/unit-file.c @@ -31,12 +31,15 @@ bool unit_type_may_template(UnitType type) { UNIT_PATH); } -int unit_symlink_name_compatible(const char *symlink, const char *target) { +int unit_symlink_name_compatible(const char *symlink, const char *target, bool instance_propagation) { _cleanup_free_ char *template = NULL; - int r; + int r, un_type1, un_type2; + + un_type1 = unit_name_classify(symlink); - /* The straightforward case: the symlink name matches the target */ - if (streq(symlink, target)) + /* The straightforward case: the symlink name matches the target and we have a valid unit */ + if (streq(symlink, target) && + (un_type1 & (UNIT_NAME_PLAIN | UNIT_NAME_INSTANCE))) return 1; r = unit_name_template(symlink, &template); @@ -45,8 +48,22 @@ int unit_symlink_name_compatible(const char *symlink, const char *target) { if (r < 0) return r; + un_type2 = unit_name_classify(target); + /* An instance name points to a target that is just the template name */ - return streq(template, target); + if (un_type1 == UNIT_NAME_INSTANCE && + un_type2 == UNIT_NAME_TEMPLATE && + streq(template, target)) + return 1; + + /* foo@.target.requires/bar@.service: instance will be propagated */ + if (instance_propagation && + un_type1 == UNIT_NAME_TEMPLATE && + un_type2 == UNIT_NAME_TEMPLATE && + streq(template, target)) + return 1; + + return 0; } int unit_validate_alias_symlink_and_warn(const char *filename, const char *target) { diff --git a/src/shared/unit-file.h b/src/shared/unit-file.h index 83f78618d30..a44ba5b0518 100644 --- a/src/shared/unit-file.h +++ b/src/shared/unit-file.h @@ -39,7 +39,7 @@ enum UnitFileScope { bool unit_type_may_alias(UnitType type) _const_; bool unit_type_may_template(UnitType type) _const_; -int unit_symlink_name_compatible(const char *symlink, const char *target); +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_file_build_name_map( diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c index 5b50f28118b..25498916f11 100644 --- a/src/test/test-install-root.c +++ b/src/test/test-install-root.c @@ -1091,6 +1091,8 @@ static void test_verify_alias(void) { verify_one(&plain_service, "foo.target.requires/plain@.service", -EXDEV, NULL); verify_one(&plain_service, "foo.target.requires/service", -EXDEV, NULL); verify_one(&plain_service, "foo.target.conf/plain.service", -EXDEV, NULL); + verify_one(&plain_service, "foo.service/plain.service", -EXDEV, NULL); /* missing dir suffix */ + verify_one(&plain_service, "asdf.requires/plain.service", -EXDEV, NULL); /* invalid unit name component */ verify_one(&bare_template, "alias.service", -EXDEV, NULL); verify_one(&bare_template, "alias.socket", -EXDEV, NULL); @@ -1114,6 +1116,12 @@ static void test_verify_alias(void) { verify_one(&bare_template, "foo.target.requires/template1@inst.service", 0, NULL); verify_one(&bare_template, "foo.target.requires/service", -EXDEV, NULL); verify_one(&bare_template, "foo.target.conf/plain.service", -EXDEV, NULL); + verify_one(&bare_template, "FOO@.target.requires/plain@.service", -EXDEV, NULL); /* template name mistatch */ + verify_one(&bare_template, "FOO@inst.target.requires/plain@.service", -EXDEV, NULL); + verify_one(&bare_template, "FOO@inst.target.requires/plain@inst.service", -EXDEV, NULL); + verify_one(&bare_template, "FOO@.target.requires/template1@.service", 0, NULL); /* instance propagated */ + verify_one(&bare_template, "FOO@inst.target.requires/template1@.service", -EXDEV, NULL); /* instance missing */ + verify_one(&bare_template, "FOO@inst.target.requires/template1@inst.service", 0, NULL); /* instance provided */ verify_one(&di_template, "alias.service", -EXDEV, NULL); verify_one(&di_template, "alias.socket", -EXDEV, NULL); @@ -1133,6 +1141,7 @@ static void test_verify_alias(void) { verify_one(&di_template, "foo.target.requires/plain@.service", -EXDEV, NULL); verify_one(&di_template, "foo.target.requires/plain@di.service", -EXDEV, NULL); verify_one(&di_template, "foo.target.requires/plain@foo.service", -EXDEV, NULL); + verify_one(&di_template, "foo.target.requires/template2@.service", -EXDEV, NULL); /* instance missing */ verify_one(&di_template, "foo.target.requires/template2@di.service", 0, NULL); verify_one(&di_template, "foo.target.requires/service", -EXDEV, NULL); verify_one(&di_template, "foo.target.conf/plain.service", -EXDEV, NULL); @@ -1161,6 +1170,13 @@ static void test_verify_alias(void) { verify_one(&inst_template, "bar.target.requires/template3@inst.service", 0, NULL); verify_one(&inst_template, "bar.target.requires/service", -EXDEV, NULL); verify_one(&inst_template, "bar.target.conf/plain.service", -EXDEV, NULL); + verify_one(&inst_template, "BAR@.target.requires/plain@.service", -EXDEV, NULL); /* template name mistatch */ + verify_one(&inst_template, "BAR@inst.target.requires/plain@.service", -EXDEV, NULL); + verify_one(&inst_template, "BAR@inst.target.requires/plain@inst.service", -EXDEV, NULL); + verify_one(&inst_template, "BAR@.target.requires/template3@.service", -EXDEV, NULL); /* instance missing */ + verify_one(&inst_template, "BAR@inst.target.requires/template3@.service", -EXDEV, NULL); /* instance missing */ + verify_one(&inst_template, "BAR@inst.target.requires/template3@inst.service", 0, NULL); /* instance provided */ + verify_one(&inst_template, "BAR@inst.target.requires/template3@ins2.service", -EXDEV, NULL); /* instance mismatch */ /* explicit alias overrides DefaultInstance */ verify_one(&di_inst_template, "alias.service", -EXDEV, NULL);