cpu-set-util: introduce config_parse_cpu_set()

Then, make parse_cpu_set() as a tiny wrapper of it.

Note, previously when an invalid CPU range, e.g. "3-0", is specified,
we ignore the range but allocate an empty set. But, with this commit,
now the conf parser simply ignore it without no side effect.
This potentially changes behavior of a system with such invalid setting,
but the change should be favorable for consistency with other parsers.

diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index ed677aa31fb163ac7c7b9e014824ab782600b571..fd78e4374fdec797fd282d0df48463ecc8425ffa 100644 (file)
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -1572,15 +1572,9 @@ int config_parse_numa_mask(
         }
 
         /* When parsing system.conf or user.conf, rather than unit files, userdata is NULL. */
-        if (!userdata) {
-                r = parse_cpu_set_extend(rvalue, cpu_set, true, unit, filename, line, lvalue);
-                if (r < 0)
-                        log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse NUMA node mask, ignoring: %s", rvalue);
-
-                return 0;
-        }
-
-        return config_parse_unit_cpu_set(unit, filename, line, section, section_line, lvalue, ltype, rvalue, data, userdata);
+        return userdata ?
+                config_parse_unit_cpu_set(unit, filename, line, section, section_line, lvalue, ltype, rvalue, data, userdata) :
+                config_parse_cpu_set(unit, filename, line, section, section_line, lvalue, ltype, rvalue, data, userdata);
 }
 
 int config_parse_exec_cpu_sched_prio(const char *unit,
@@ -3783,8 +3777,7 @@ int config_parse_unit_cpu_set(
                 void *data,
                 void *userdata) {
 
-        CPUSet *c = data;
-        const Unit *u = userdata;
+        const Unit *u = ASSERT_PTR(userdata);
         _cleanup_free_ char *k = NULL;
         int r;
 
@@ -3800,11 +3793,7 @@ int config_parse_unit_cpu_set(
                 return 0;
         }
 
-        r = parse_cpu_set_extend(k, c, true, unit, filename, line, lvalue);
-        if (r < 0)
-                return 0;
-
-        return 1;
+        return config_parse_cpu_set(unit, filename, line, section, section_line, lvalue, ltype, k, data, userdata);
 }
 
 int config_parse_memory_limit(
@@ -6274,25 +6263,6 @@ void unit_dump_config_items(FILE *f) {
         }
 }
 
-int config_parse_cpu_affinity2(
-                const char *unit,
-                const char *filename,
-                unsigned line,
-                const char *section,
-                unsigned section_line,
-                const char *lvalue,
-                int ltype,
-                const char *rvalue,
-                void *data,
-                void *userdata) {
-
-        CPUSet *affinity = ASSERT_PTR(data);
-
-        (void) parse_cpu_set_extend(rvalue, affinity, true, unit, filename, line, lvalue);
-
-        return 0;
-}
-
 int config_parse_show_status(
                 const char* unit,
                 const char *filename,

diff --git a/src/core/load-fragment.h b/src/core/load-fragment.h
index 1365174de607281e1fa820988a78b00e722a6f9b..d87bff0996880ee16d3089084be2c091ca4f8df0 100644 (file)
--- a/src/core/load-fragment.h
+++ b/src/core/load-fragment.h
@@ -140,7 +140,6 @@ CONFIG_PARSER_PROTOTYPE(config_parse_oom_policy);
 CONFIG_PARSER_PROTOTYPE(config_parse_numa_policy);
 CONFIG_PARSER_PROTOTYPE(config_parse_numa_mask);
 CONFIG_PARSER_PROTOTYPE(config_parse_ip_filter_bpf_progs);
-CONFIG_PARSER_PROTOTYPE(config_parse_cpu_affinity2);
 CONFIG_PARSER_PROTOTYPE(config_parse_show_status);
 CONFIG_PARSER_PROTOTYPE(config_parse_status_unit_format);
 CONFIG_PARSER_PROTOTYPE(config_parse_output_restricted);

diff --git a/src/core/main.c b/src/core/main.c
index 5046d05b103747b58b68ae78e9a7f63bcbc5a4cb..ee980ed189c9dceddb2aa792da12889f223bbeaf 100644 (file)
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -741,7 +741,7 @@ static int parse_config_file(void) {
                 { "Manager", "CrashAction",                  config_parse_crash_action,          0,                        &arg_crash_action                 },
                 { "Manager", "ShowStatus",                   config_parse_show_status,           0,                        &arg_show_status                  },
                 { "Manager", "StatusUnitFormat",             config_parse_status_unit_format,    0,                        &arg_status_unit_format           },
-                { "Manager", "CPUAffinity",                  config_parse_cpu_affinity2,         0,                        &arg_cpu_affinity                 },
+                { "Manager", "CPUAffinity",                  config_parse_cpu_set,               0,                        &arg_cpu_affinity                 },
                 { "Manager", "NUMAPolicy",                   config_parse_numa_policy,           0,                        &arg_numa_policy.type             },
                 { "Manager", "NUMAMask",                     config_parse_numa_mask,             0,                        &arg_numa_policy.nodes            },
                 { "Manager", "JoinControllers",              config_parse_warn_compat,           DISABLED_LEGACY,          NULL                              },

diff --git a/src/nspawn/nspawn-gperf.gperf b/src/nspawn/nspawn-gperf.gperf
index 883ca97bc89d04e48cf9e5deb16e66e26ed46747..b735093691cd0bc3eda0bc5f352782468d730282 100644 (file)
--- a/src/nspawn/nspawn-gperf.gperf
+++ b/src/nspawn/nspawn-gperf.gperf
@@ -55,7 +55,7 @@ Exec.LimitRTTIME,             config_parse_rlimit,         RLIMIT_RTTIME,
 Exec.Hostname,                config_parse_hostname,       0,                        offsetof(Settings, hostname)
 Exec.NoNewPrivileges,         config_parse_tristate,       0,                        offsetof(Settings, no_new_privileges)
 Exec.OOMScoreAdjust,          config_parse_oom_score_adjust, 0,                      0
-Exec.CPUAffinity,             config_parse_cpu_affinity,   0,                        0
+Exec.CPUAffinity,             config_parse_cpu_set,        0,                        offsetof(Settings, cpu_set)
 Exec.ResolvConf,              config_parse_resolv_conf,    0,                        offsetof(Settings, resolv_conf)
 Exec.LinkJournal,             config_parse_link_journal,   0,                        0
 Exec.Timezone,                config_parse_timezone_mode,  0,                        offsetof(Settings, timezone)

diff --git a/src/nspawn/nspawn-settings.c b/src/nspawn/nspawn-settings.c
index 98b31d5d40cf5c603de16df2066f1958a620c396..035796994c2a1ce007ab92c0809731b9307b47d6 100644 (file)
--- a/src/nspawn/nspawn-settings.c
+++ b/src/nspawn/nspawn-settings.c
@@ -817,25 +817,6 @@ int config_parse_oom_score_adjust(
         return 0;
 }
 
-int config_parse_cpu_affinity(
-                const char *unit,
-                const char *filename,
-                unsigned line,
-                const char *section,
-                unsigned section_line,
-                const char *lvalue,
-                int ltype,
-                const char *rvalue,
-                void *data,
-                void *userdata) {
-
-        Settings *settings = ASSERT_PTR(data);
-
-        assert(rvalue);
-
-        return parse_cpu_set_extend(rvalue, &settings->cpu_set, true, unit, filename, line, lvalue);
-}
-
 DEFINE_CONFIG_PARSE_ENUM(config_parse_resolv_conf, resolv_conf_mode, ResolvConfMode);
 
 static const char *const resolv_conf_mode_table[_RESOLV_CONF_MODE_MAX] = {

diff --git a/src/nspawn/nspawn-settings.h b/src/nspawn/nspawn-settings.h
index f1c43f50f723f0902506e7b24fff4942c9c134c0..607db87feb382859a4408791de99c0a83e1c9acc 100644 (file)
--- a/src/nspawn/nspawn-settings.h
+++ b/src/nspawn/nspawn-settings.h
@@ -264,7 +264,6 @@ CONFIG_PARSER_PROTOTYPE(config_parse_pid2);
 CONFIG_PARSER_PROTOTYPE(config_parse_private_users);
 CONFIG_PARSER_PROTOTYPE(config_parse_syscall_filter);
 CONFIG_PARSER_PROTOTYPE(config_parse_oom_score_adjust);
-CONFIG_PARSER_PROTOTYPE(config_parse_cpu_affinity);
 CONFIG_PARSER_PROTOTYPE(config_parse_resolv_conf);
 CONFIG_PARSER_PROTOTYPE(config_parse_link_journal);
 CONFIG_PARSER_PROTOTYPE(config_parse_timezone_mode);

diff --git a/src/shared/cpu-set-util.c b/src/shared/cpu-set-util.c
index 174d284b45707750952ba7c594567630bc750478..c5277f11af8e190961575992569812f3a992074c 100644 (file)
--- a/src/shared/cpu-set-util.c
+++ b/src/shared/cpu-set-util.c
@@ -209,91 +209,118 @@ int cpu_set_add_all(CPUSet *c) {
         return cpu_set_add_range(c, 0, m - 1);
 }
 
-int parse_cpu_set_full(
-                const char *rvalue,
-                CPUSet *cpu_set,
-                bool warn,
+int config_parse_cpu_set(
                 const char *unit,
                 const char *filename,
                 unsigned line,
-                const char *lvalue) {
+                const char *section,
+                unsigned section_line,
+                const char *lvalue,
+                int ltype, /* 0 when used as conf parser, 1 when used as usual parser */
+                const char *rvalue,
+                void *data,
+                void *userdata) {
 
-        _cleanup_(cpu_set_done) CPUSet c = {};
-        const char *p = ASSERT_PTR(rvalue);
+        CPUSet *c = ASSERT_PTR(data);
+        int r, level = ltype ? LOG_DEBUG : LOG_DEBUG;
+        bool critical = ltype;
 
-        assert(cpu_set);
+        assert(critical || lvalue);
 
-        for (;;) {
+        if (isempty(rvalue)) {
+                cpu_set_done(c);
+                return 1;
+        }
+
+        _cleanup_(cpu_set_done) CPUSet cpuset = {};
+        for (const char *p = rvalue;;) {
                 _cleanup_free_ char *word = NULL;
-                unsigned cpu_lower, cpu_upper;
-                int r;
 
                 r = extract_first_word(&p, &word, WHITESPACE ",", EXTRACT_UNQUOTE);
                 if (r == -ENOMEM)
-                        return warn ? log_oom() : -ENOMEM;
-                if (r < 0)
-                        return warn ? log_syntax(unit, LOG_ERR, filename, line, r, "Invalid value for %s: %s", lvalue, rvalue) : r;
+                        return log_oom_full(level);
+                if (r < 0) {
+                        if (critical)
+                                return log_debug_errno(r, "Failed to parse CPU set: %s", rvalue);
+
+                        log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse %s= setting, ignoring assignment: %s",
+                                   lvalue, rvalue);
+                        return 0;
+                }
                 if (r == 0)
                         break;
 
-                r = parse_range(word, &cpu_lower, &cpu_upper);
-                if (r < 0)
-                        return warn ? log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse CPU affinity '%s'", word) : r;
+                unsigned lower, upper;
+                r = parse_range(word, &lower, &upper);
+                if (r < 0) {
+                        if (critical)
+                                return log_debug_errno(r, "Failed to parse CPU range: %s", word);
 
-                if (cpu_lower > cpu_upper) {
-                        if (warn)
-                                log_syntax(unit, LOG_WARNING, filename, line, 0, "Range '%s' is invalid, %u > %u, ignoring.",
-                                           word, cpu_lower, cpu_upper);
+                        log_syntax(unit, LOG_WARNING, filename, line, r,
+                                   "Failed to parse CPU range, ignoring assignment: %s", word);
+                        continue;
+                }
 
-                        /* Make sure something is allocated, to distinguish this from the empty case */
-                        r = cpu_set_realloc(&c, 1);
-                        if (r < 0)
-                                return r;
+                if (lower > upper) {
+                        if (critical)
+                                return log_debug_errno(SYNTHETIC_ERRNO(EINVAL),
+                                                       "Invalid CPU range (%u > %u): %s",
+                                                       lower, upper, word);
+
+                        log_syntax(unit, LOG_WARNING, filename, line, r,
+                                   "Invalid CPU range (%u > %u), ignoring assignment: %s",
+                                   lower, upper, word);
+                        continue;
                 }
 
-                for (unsigned cpu_p1 = MIN(cpu_upper, UINT_MAX-1) + 1; cpu_p1 > cpu_lower; cpu_p1--) {
-                        r = cpu_set_add(&c, cpu_p1 - 1);
-                        if (r < 0)
-                                return warn ? log_syntax(unit, LOG_ERR, filename, line, r,
-                                                         "Cannot add CPU %u to set: %m", cpu_p1 - 1) : r;
+                r = cpu_set_add_range(&cpuset, lower, upper);
+                if (r == -ENOMEM)
+                        return log_oom_full(level);
+                if (r < 0) {
+                        if (critical)
+                                return log_debug_errno(r, "Failed to set CPU(s) '%s': %m", word);
+
+                        log_syntax(unit, LOG_WARNING, filename, line, r,
+                                   "Failed to set CPU(s), ignoring assignment: %s", word);
                 }
         }
 
-        *cpu_set = TAKE_STRUCT(c);
+        if (!c->set) {
+                *c = TAKE_STRUCT(cpuset);
+                return 1;
+        }
+
+        r = cpu_set_add_set(c, &cpuset);
+        if (r == -ENOMEM)
+                return log_oom_full(level);
+        assert(r >= 0);
 
-        return 0;
+        return 1;
 }
 
-int parse_cpu_set_extend(
-                const char *rvalue,
-                CPUSet *old,
-                bool warn,
-                const char *unit,
-                const char *filename,
-                unsigned line,
-                const char *lvalue) {
-
-        _cleanup_(cpu_set_done) CPUSet cpuset = {};
+int parse_cpu_set(const char *s, CPUSet *ret) {
+        _cleanup_(cpu_set_done) CPUSet c = {};
         int r;
 
-        assert(old);
+        assert(s);
+        assert(ret);
 
-        r = parse_cpu_set_full(rvalue, &cpuset, true, unit, filename, line, lvalue);
+        r = config_parse_cpu_set(
+                        /* unit = */ NULL,
+                        /* filename = */ NULL,
+                        /* line = */ 0,
+                        /* section = */ NULL,
+                        /* section_line = */ 0,
+                        /* lvalue = */ NULL,
+                        /* ltype = */ 1,
+                        /* rvalue = */ s,
+                        /* data = */ &c,
+                        /* userdata = */ NULL);
         if (r < 0)
                 return r;
 
-        if (!cpuset.set) {
-                /* An empty assignment clears the CPU list */
-                cpu_set_done(old);
-                return 0;
-        }
-
-        if (!old->set) {
-                *old = TAKE_STRUCT(cpuset);
-                return 1;
-        }
-
-        return cpu_set_add_set(old, &cpuset);
+        *ret = TAKE_STRUCT(c);
+        return 0;
 }
 
 int cpus_in_affinity_mask(void) {

diff --git a/src/shared/cpu-set-util.h b/src/shared/cpu-set-util.h
index 8696b39de19391a5dfbe77428d321a9a9f470812..84a21809ca0b2a9c5561c3837820122f5ebb79ba 100644 (file)
--- a/src/shared/cpu-set-util.h
+++ b/src/shared/cpu-set-util.h
@@ -3,6 +3,7 @@
 
 #include <sched.h>
 
+#include "conf-parser-forward.h"
 #include "forward.h"
 
 /* This wraps the libc interface with a variable to keep the allocated size. */
@@ -29,25 +30,8 @@ char* cpu_set_to_string(const CPUSet *c);
 char* cpu_set_to_range_string(const CPUSet *c);
 char* cpu_set_to_mask_string(const CPUSet *c);
 
-int parse_cpu_set_full(
-                const char *rvalue,
-                CPUSet *cpu_set,
-                bool warn,
-                const char *unit,
-                const char *filename, unsigned line,
-                const char *lvalue);
-int parse_cpu_set_extend(
-                const char *rvalue,
-                CPUSet *old,
-                bool warn,
-                const char *unit,
-                const char *filename,
-                unsigned line,
-                const char *lvalue);
-
-static inline int parse_cpu_set(const char *rvalue, CPUSet *cpu_set){
-        return parse_cpu_set_full(rvalue, cpu_set, false, NULL, NULL, 0, NULL);
-}
+CONFIG_PARSER_PROTOTYPE(config_parse_cpu_set);
+int parse_cpu_set(const char *s, CPUSet *ret);
 
 int cpu_set_to_dbus(const CPUSet *c, uint8_t **ret, size_t *ret_size);
 int cpu_set_from_dbus(const uint8_t *bits, size_t size, CPUSet *ret);

diff --git a/src/test/test-cpu-set-util.c b/src/test/test-cpu-set-util.c
index 9254a3a1eca4363c8bc9205b905b555a5f26a96d..e3a960ec742379cbde57dea6c273fcec3484e071 100644 (file)
--- a/src/test/test-cpu-set-util.c
+++ b/src/test/test-cpu-set-util.c
@@ -132,12 +132,6 @@ TEST(parse_cpu_set) {
         ASSERT_CPUSET_STRING(c, "0 1 2 3 8 9 10 11", "0-3 8-11", "f0f");
         cpu_set_done(&c);
 
-        /* Negative range (returns empty cpu_set) */
-        ASSERT_OK(parse_cpu_set("3-0", &c));
-        ASSERT_CPUSET_COUNT(c, 0);
-        ASSERT_CPUSET_STRING(c, "", "", "0");
-        cpu_set_done(&c);
-
         /* Overlapping ranges */
         ASSERT_OK(parse_cpu_set("0-7 4-11", &c));
         ASSERT_CPUSET_COUNT(c, 12);
@@ -156,6 +150,10 @@ TEST(parse_cpu_set) {
         ASSERT_CPUSET_STRING(c, "0 2 4 5 6 7 8 9 10 11", "0 2 4-11", "ff5");
         cpu_set_done(&c);
 
+        /* Negative range */
+        ASSERT_ERROR(parse_cpu_set("3-0", &c), EINVAL);
+        ASSERT_CPUSET_EMPTY(c);
+
         /* Garbage */
         ASSERT_ERROR(parse_cpu_set("0 1 2 3 garbage", &c), EINVAL);
         ASSERT_CPUSET_EMPTY(c);
@@ -188,18 +186,31 @@ TEST(parse_cpu_set) {
         cpu_set_done(&c);
 }
 
-TEST(parse_cpu_set_extend) {
+#define parse(str, c)                                                   \
+        config_parse_cpu_set(                                           \
+                        "unit",                                         \
+                        "filename",                                     \
+                        /* line = */ 0,                                 \
+                        "[Section]",                                    \
+                        /* section_line = */ 0,                         \
+                        "CPUAffinity",                                  \
+                        /* ltype = */ 0,                                \
+                        str,                                            \
+                        c,                                              \
+                        /* userdata = */ NULL)
+
+TEST(config_parse_cpu_set) {
         CPUSet c = {};
 
-        ASSERT_OK_POSITIVE(parse_cpu_set_extend("1 3", &c, true, NULL, "fake", 1, "CPUAffinity"));
+        ASSERT_OK_POSITIVE(parse("1 3", &c));
         ASSERT_CPUSET_COUNT(c, 2);
         ASSERT_CPUSET_STRING(c, "1 3", "1 3", "a");
 
-        ASSERT_OK_POSITIVE(parse_cpu_set_extend("4", &c, true, NULL, "fake", 1, "CPUAffinity"));
+        ASSERT_OK_POSITIVE(parse("4", &c));
         ASSERT_CPUSET_COUNT(c, 3);
         ASSERT_CPUSET_STRING(c, "1 3 4", "1 3-4", "1a");
 
-        ASSERT_OK_ZERO(parse_cpu_set_extend("", &c, true, NULL, "fake", 1, "CPUAffinity"));
+        ASSERT_OK_POSITIVE(parse("", &c));
         ASSERT_CPUSET_EMPTY(c);
 }
 
@@ -254,4 +265,4 @@ TEST(print_cpu_alloc_size) {
         log_info("CPU_ALLOC_SIZE(8191) = %zu", CPU_ALLOC_SIZE(8191));
 }
 
-DEFINE_TEST_MAIN(LOG_INFO);
+DEFINE_TEST_MAIN(LOG_DEBUG);

diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c
index 0363c32ea2b3cac8282c3b390aa17048539d1090..59ee406f494068a541ba7c382a5f46bd295787fb 100644 (file)
--- a/src/udev/net/link-config.c
+++ b/src/udev/net/link-config.c
@@ -1371,15 +1371,8 @@ int config_parse_rps_cpu_mask(
         CPUSet *mask = ASSERT_PTR(data);
         int r;
 
-        assert(filename);
-        assert(lvalue);
         assert(rvalue);
 
-        if (isempty(rvalue)) {
-                cpu_set_done(mask);
-                return 0;
-        }
-
         if (streq(rvalue, "disable")) {
                 _cleanup_(cpu_set_done) CPUSet c = {};
 
@@ -1403,11 +1396,7 @@ int config_parse_rps_cpu_mask(
                 return cpu_set_done_and_replace(*mask, c);
         }
 
-        r = parse_cpu_set_extend(rvalue, mask, /* warn= */ true, unit, filename, line, lvalue);
-        if (r < 0)
-                return 0;
-
-        return 0;
+        return config_parse_cpu_set(unit, filename, line, section, section_line, lvalue, ltype, rvalue, data, userdata);
 }
 
 static const char* const mac_address_policy_table[_MAC_ADDRESS_POLICY_MAX] = {