nspawn: fix comparisons of versions with non-numerical suffixes

See a2b0cd3f5ab3f450e74e2085ad20372a05451c74. When -Dshared-lib-tag is used,
libsystemd-shared.so and libsystemd-core.so get a suffix which breaks the
parsing done by systemd_installation_has_version(). We can assume that the
tag will be something like "251-rc1-1.fc37" that is currently used in Fedora.
(Anything that does *not* start with the version would be completely crazy.)
By switching to strverscmp_improved() we simplify the code and fix comparisons
with such versions.

$ build/test-nspawn-util /var/lib/machines/rawhide
...
Found libsystemd shared at "/var/lib/machines/rawhide/lib/systemd/libsystemd-shared-251-rc1-1.fc37.so.so", version 251-rc1-1.fc37 (OK).
/var/lib/machines/rawhide has systemd >= 251: yes
...

I noticed this when I started a systemd-nspawn container with Redora rawhide
and got the message "Not running with unified cgroup hierarchy, LSM BPF is not
supported". I thought the message is in error, but it was actually correct:
nspawn was misdetecting that the container does not sport new-enough systemd
to support cgroups-v2.

diff --git a/src/nspawn/nspawn-util.c b/src/nspawn/nspawn-util.c
index 7aacbb80962be6783e6882f76f941a611f0b083b..4b3bf2aa001c3853cedfd75fdfb5897e34d72acd 100644 (file)
--- a/src/nspawn/nspawn-util.c
+++ b/src/nspawn/nspawn-util.c
@@ -9,7 +9,7 @@
 #include "path-util.h"
 #include "string-util.h"
 
-int systemd_installation_has_version(const char *root, unsigned minimal_version) {
+int systemd_installation_has_version(const char *root, const char *minimal_version) {
         const char *pattern;
         int r;
 
@@ -48,7 +48,6 @@ int systemd_installation_has_version(const char *root, unsigned minimal_version)
                 STRV_FOREACH(name, names) {
                         /* This is most likely to run only once, hence let's not optimize anything. */
                         char *t, *t2;
-                        unsigned version;
 
                         t = startswith(*name, path);
                         if (!t)
@@ -57,19 +56,13 @@ int systemd_installation_has_version(const char *root, unsigned minimal_version)
                         t2 = endswith(t, ".so");
                         if (!t2)
                                 continue;
+                        *t2 = '\0';
 
-                        t2[0] = '\0'; /* truncate the suffix */
-
-                        r = safe_atou(t, &version);
-                        if (r < 0) {
-                                log_debug_errno(r, "Found libsystemd shared at \"%s.so\", but failed to parse version: %m", *name);
-                                continue;
-                        }
-
-                        log_debug("Found libsystemd shared at \"%s.so\", version %u (%s).",
-                                  *name, version,
-                                  version >= minimal_version ? "OK" : "too old");
-                        if (version >= minimal_version)
+                        r = strverscmp_improved(t, minimal_version);
+                        log_debug("Found libsystemd shared at \"%s.so\", version %s (%s).",
+                                  *name, t,
+                                  r >= 0 ? "OK" : "too old");
+                        if (r >= 0)
                                 return true;
                 }
         }

diff --git a/src/nspawn/nspawn-util.h b/src/nspawn/nspawn-util.h
index 1e90862c9d0ff6314526f126427eae1cec30537e..e83cd564dad447afa708df283e40595512dada0c 100644 (file)
--- a/src/nspawn/nspawn-util.h
+++ b/src/nspawn/nspawn-util.h
@@ -1,4 +1,4 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 #pragma once
 
-int systemd_installation_has_version(const char *root, unsigned minimal_version);
+int systemd_installation_has_version(const char *root, const char *minimal_version);

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 1929cd12b02840a7cf45b00894f0f6bd7047d8ee..a5b826530361fcaa38c691c3c42c120177ffd7f6 100644 (file)
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -509,7 +509,7 @@ static int detect_unified_cgroup_hierarchy_from_image(const char *directory) {
         if (r > 0) {
                 /* Unified cgroup hierarchy support was added in 230. Unfortunately the detection
                  * routine only detects 231, so we'll have a false negative here for 230. */
-                r = systemd_installation_has_version(directory, 230);
+                r = systemd_installation_has_version(directory, "230");
                 if (r < 0)
                         return log_error_errno(r, "Failed to determine systemd version in container: %m");
                 if (r > 0)
@@ -518,7 +518,7 @@ static int detect_unified_cgroup_hierarchy_from_image(const char *directory) {
                         arg_unified_cgroup_hierarchy = CGROUP_UNIFIED_NONE;
         } else if (cg_unified_controller(SYSTEMD_CGROUP_CONTROLLER) > 0) {
                 /* Mixed cgroup hierarchy support was added in 233 */
-                r = systemd_installation_has_version(directory, 233);
+                r = systemd_installation_has_version(directory, "233");
                 if (r < 0)
                         return log_error_errno(r, "Failed to determine systemd version in container: %m");
                 if (r > 0)

diff --git a/src/nspawn/test-nspawn-util.c b/src/nspawn/test-nspawn-util.c
index 7d55db89340029393d2f3b36019bc679548c3fc7..ed68063630831d4f3633ea9ff5d3a57193b42178 100644 (file)
--- a/src/nspawn/test-nspawn-util.c
+++ b/src/nspawn/test-nspawn-util.c
@@ -2,17 +2,17 @@
 
 #include "nspawn-util.h"
 #include "string-util.h"
+#include "strv.h"
 #include "tests.h"
 
 TEST(systemd_installation_has_version) {
-        static const unsigned versions[] = {0, 231, PROJECT_VERSION, 999};
         int r;
 
-        for (size_t i = 0; i < ELEMENTSOF(versions); i++) {
-                r = systemd_installation_has_version(saved_argv[1], versions[i]);
+        FOREACH_STRING(version, "0", "231", STRINGIFY(PROJECT_VERSION), "999") {
+                r = systemd_installation_has_version(saved_argv[1], version);
                 assert_se(r >= 0);
-                log_info("%s has systemd >= %u: %s",
-                         saved_argv[1] ?: "Current installation", versions[i], yes_no(r));
+                log_info("%s has systemd >= %s: %s",
+                         saved_argv[1] ?: "Current installation", version, yes_no(r));
         }
 }