various tools: be more explicit when a glob is passed when not supported

See https://bugzilla.redhat.com/show_bug.cgi?id=1763488: when we say that
'foo@*.service' is not a valid unit name, this is not clear enough. Let's
include the name of the operation that does not support globbing in the
error message:

$ build/systemctl enable 'foo@*.service'
Glob pattern passed to enable, but globs are not supported for this.
Invalid unit name "foo@*.service" escaped as "foo@\x2a.service".
...

diff --git a/src/analyze/analyze-security.c b/src/analyze/analyze-security.c
index 848aeae..ca023ea 100644 (file)
--- a/src/analyze/analyze-security.c
+++ b/src/analyze/analyze-security.c
@@ -2093,7 +2093,7 @@ int analyze_security(sd_bus *bus, char **units, AnalyzeSecurityFlags flags) {
                                 fflush(stdout);
                         }
 
-                        r = unit_name_mangle_with_suffix(*i, 0, ".service", &mangled);
+                        r = unit_name_mangle(*i, 0, &mangled);
                         if (r < 0)
                                 return log_error_errno(r, "Failed to mangle unit name '%s': %m", *i);
 

diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c
index bcd01f8..3e37e34 100644 (file)
--- a/src/basic/unit-name.c
+++ b/src/basic/unit-name.c
@@ -597,10 +597,10 @@ static bool do_escape_mangle(const char *f, bool allow_globs, char *t) {
  *
  *  If @allow_globs, globs characters are preserved. Otherwise, they are escaped.
  */
-int unit_name_mangle_with_suffix(const char *name, UnitNameMangle flags, const char *suffix, char **ret) {
+int unit_name_mangle_with_suffix(const char *name, const char *operation, UnitNameMangle flags, const char *suffix, char **ret) {
         char *s;
         int r;
-        bool mangled;
+        bool mangled, suggest_escape = true;
 
         assert(name);
         assert(suffix);
@@ -617,10 +617,14 @@ int unit_name_mangle_with_suffix(const char *name, UnitNameMangle flags, const c
                 goto good;
 
         /* Already a fully valid globbing expression? If so, no mangling is necessary either... */
-        if ((flags & UNIT_NAME_MANGLE_GLOB) &&
-            string_is_glob(name) &&
-            in_charset(name, VALID_CHARS_GLOB))
-                goto good;
+        if (string_is_glob(name) && in_charset(name, VALID_CHARS_GLOB)) {
+                if (flags & UNIT_NAME_MANGLE_GLOB)
+                        goto good;
+                log_full(flags & UNIT_NAME_MANGLE_WARN ? LOG_NOTICE : LOG_DEBUG,
+                         "Glob pattern passed%s%s, but globs are not supported for this.",
+                         operation ? " " : "", operation ?: "");
+                suggest_escape = false;
+        }
 
         if (is_device_path(name)) {
                 r = unit_name_from_path(name, ".device", ret);
@@ -645,11 +649,12 @@ int unit_name_mangle_with_suffix(const char *name, UnitNameMangle flags, const c
         mangled = do_escape_mangle(name, flags & UNIT_NAME_MANGLE_GLOB, s);
         if (mangled)
                 log_full(flags & UNIT_NAME_MANGLE_WARN ? LOG_NOTICE : LOG_DEBUG,
-                         "Invalid unit name \"%s\" was escaped as \"%s\" (maybe you should use systemd-escape?)",
-                         name, s);
+                         "Invalid unit name \"%s\" escaped as \"%s\"%s.",
+                         name, s,
+                         suggest_escape ? " (maybe you should use systemd-escape?)" : "");
 
-        /* Append a suffix if it doesn't have any, but only if this is not a glob, so that we can allow "foo.*" as a
-         * valid glob. */
+        /* Append a suffix if it doesn't have any, but only if this is not a glob, so that we can allow
+         * "foo.*" as a valid glob. */
         if ((!(flags & UNIT_NAME_MANGLE_GLOB) || !string_is_glob(s)) && unit_name_to_type(s) < 0)
                 strcat(s, suffix);
 

diff --git a/src/basic/unit-name.h b/src/basic/unit-name.h
index ddcfc1b..4a6f64c 100644 (file)
--- a/src/basic/unit-name.h
+++ b/src/basic/unit-name.h
@@ -52,10 +52,10 @@ typedef enum UnitNameMangle {
         UNIT_NAME_MANGLE_WARN = 1 << 1,
 } UnitNameMangle;
 
-int unit_name_mangle_with_suffix(const char *name, UnitNameMangle flags, const char *suffix, char **ret);
+int unit_name_mangle_with_suffix(const char *name, const char *operation, UnitNameMangle flags, const char *suffix, char **ret);
 
 static inline int unit_name_mangle(const char *name, UnitNameMangle flags, char **ret) {
-        return unit_name_mangle_with_suffix(name, flags, ".service", ret);
+        return unit_name_mangle_with_suffix(name, NULL, flags, ".service", ret);
 }
 
 bool service_unit_name_is_valid(const char *name);

diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c
index 026a25f..90e7237 100644 (file)
--- a/src/fstab-generator/fstab-generator.c
+++ b/src/fstab-generator/fstab-generator.c
@@ -227,7 +227,7 @@ static int write_dependency(FILE *f, const char *opts,
         STRV_FOREACH(s, names) {
                 char *x;
 
-                r = unit_name_mangle_with_suffix(*s, 0, ".mount", &x);
+                r = unit_name_mangle_with_suffix(*s, "as dependency", 0, ".mount", &x);
                 if (r < 0)
                         return log_error_errno(r, "Failed to generate unit name: %m");
                 r = strv_consume(&units, x);

diff --git a/src/nspawn/nspawn-register.c b/src/nspawn/nspawn-register.c
index 8e2c329..b2d931a 100644 (file)
--- a/src/nspawn/nspawn-register.c
+++ b/src/nspawn/nspawn-register.c
@@ -258,7 +258,7 @@ int allocate_scope(
         if (r < 0)
                 return log_error_errno(r, "Could not watch job: %m");
 
-        r = unit_name_mangle_with_suffix(machine_name, 0, ".scope", &scope);
+        r = unit_name_mangle_with_suffix(machine_name, "as machine name", 0, ".scope", &scope);
         if (r < 0)
                 return log_error_errno(r, "Failed to mangle scope name: %m");
 
@@ -350,7 +350,7 @@ int terminate_scope(
         _cleanup_free_ char *scope = NULL;
         int r;
 
-        r = unit_name_mangle_with_suffix(machine_name, 0, ".scope", &scope);
+        r = unit_name_mangle_with_suffix(machine_name, "to terminate", 0, ".scope", &scope);
         if (r < 0)
                 return log_error_errno(r, "Failed to mangle scope name: %m");
 

diff --git a/src/run/run.c b/src/run/run.c
index e7d8b30..afa9d14 100644 (file)
--- a/src/run/run.c
+++ b/src/run/run.c
@@ -641,7 +641,9 @@ static int transient_cgroup_set_properties(sd_bus_message *m) {
         if (!isempty(arg_slice)) {
                 _cleanup_free_ char *slice = NULL;
 
-                r = unit_name_mangle_with_suffix(arg_slice, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".slice", &slice);
+                r = unit_name_mangle_with_suffix(arg_slice, "as slice",
+                                                 arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN,
+                                                 ".slice", &slice);
                 if (r < 0)
                         return log_error_errno(r, "Failed to mangle name '%s': %m", arg_slice);
 
@@ -1112,7 +1114,9 @@ static int start_transient_service(
         }
 
         if (arg_unit) {
-                r = unit_name_mangle_with_suffix(arg_unit, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".service", &service);
+                r = unit_name_mangle_with_suffix(arg_unit, "as unit",
+                                                 arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN,
+                                                 ".service", &service);
                 if (r < 0)
                         return log_error_errno(r, "Failed to mangle unit name: %m");
         } else {
@@ -1355,7 +1359,9 @@ static int start_transient_scope(sd_bus *bus) {
                 return log_oom();
 
         if (arg_unit) {
-                r = unit_name_mangle_with_suffix(arg_unit, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".scope", &scope);
+                r = unit_name_mangle_with_suffix(arg_unit, "as unit",
+                                                 arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN,
+                                                 ".scope", &scope);
                 if (r < 0)
                         return log_error_errno(r, "Failed to mangle scope name: %m");
         } else {
@@ -1530,11 +1536,15 @@ static int start_transient_trigger(
                         break;
 
                 default:
-                        r = unit_name_mangle_with_suffix(arg_unit, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".service", &service);
+                        r = unit_name_mangle_with_suffix(arg_unit, "as unit",
+                                                         arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN,
+                                                         ".service", &service);
                         if (r < 0)
                                 return log_error_errno(r, "Failed to mangle unit name: %m");
 
-                        r = unit_name_mangle_with_suffix(arg_unit, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, suffix, &trigger);
+                        r = unit_name_mangle_with_suffix(arg_unit, "as trigger",
+                                                         arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN,
+                                                         suffix, &trigger);
                         if (r < 0)
                                 return log_error_errno(r, "Failed to mangle unit name: %m");
 

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 0501348..3fabbb6 100644 (file)
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -761,10 +761,7 @@ static int expand_names(sd_bus *bus, char **names, const char* suffix, char ***r
                 char *t;
                 UnitNameMangle options = UNIT_NAME_MANGLE_GLOB | (arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN);
 
-                if (suffix)
-                        r = unit_name_mangle_with_suffix(*name, options, suffix, &t);
-                else
-                        r = unit_name_mangle(*name, options, &t);
+                r = unit_name_mangle_with_suffix(*name, NULL, options, suffix ?: ".service", &t);
                 if (r < 0)
                         return log_error_errno(r, "Failed to mangle name: %m");
 
@@ -2182,7 +2179,9 @@ static int set_default(int argc, char *argv[], void *userdata) {
         assert(argc >= 2);
         assert(argv);
 
-        r = unit_name_mangle_with_suffix(argv[1], arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".target", &unit);
+        r = unit_name_mangle_with_suffix(argv[1], "set-default",
+                                         arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN,
+                                         ".target", &unit);
         if (r < 0)
                 return log_error_errno(r, "Failed to mangle unit name: %m");
 
@@ -6565,7 +6564,7 @@ static int enable_sysv_units(const char *verb, char **args) {
         return r;
 }
 
-static int mangle_names(char **original_names, char ***mangled_names) {
+static int mangle_names(const char *operation, char **original_names, char ***mangled_names) {
         char **i, **l, **name;
         int r;
 
@@ -6585,7 +6584,9 @@ static int mangle_names(char **original_names, char ***mangled_names) {
                                 return log_oom();
                         }
                 } else {
-                        r = unit_name_mangle(*name, arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, i);
+                        r = unit_name_mangle_with_suffix(*name, operation,
+                                                         arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN,
+                                                         ".service", i);
                         if (r < 0) {
                                 *i = NULL;
                                 strv_free(l);
@@ -6696,7 +6697,7 @@ static int enable_unit(int argc, char *argv[], void *userdata) {
         if (!argv[1])
                 return 0;
 
-        r = mangle_names(strv_skip(argv, 1), &names);
+        r = mangle_names("to enable", strv_skip(argv, 1), &names);
         if (r < 0)
                 return r;
 
@@ -6923,11 +6924,13 @@ static int add_dependency(int argc, char *argv[], void *userdata) {
         if (!argv[1])
                 return 0;
 
-        r = unit_name_mangle_with_suffix(argv[1], arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN, ".target", &target);
+        r = unit_name_mangle_with_suffix(argv[1], "as target",
+                                         arg_quiet ? 0 : UNIT_NAME_MANGLE_WARN,
+                                         ".target", &target);
         if (r < 0)
                 return log_error_errno(r, "Failed to mangle unit name: %m");
 
-        r = mangle_names(strv_skip(argv, 2), &names);
+        r = mangle_names("as dependency", strv_skip(argv, 2), &names);
         if (r < 0)
                 return r;
 
@@ -7113,7 +7116,7 @@ static int unit_is_enabled(int argc, char *argv[], void *userdata) {
         char **name;
         int r;
 
-        r = mangle_names(strv_skip(argv, 1), &names);
+        r = mangle_names("to check", strv_skip(argv, 1), &names);
         if (r < 0)
                 return r;