Merge pull request #19383 from keszybz/test58-fixes

Fixes for TEST-58-REPART and ExecStart deserialization logic

TODO

@@ -89,6 +89,14 @@ Features:
   or so. (this is useful to factory reset an image, then putting it into
   another machine, ensuring that luks key is generated on new machine, not old)
 
+* systemd-repart: support setting up dm-integrity with HMAC
+
+* systemd-repart: maybe remove half-initialized image on failure. It fails
+  if the output file exists, so a repeated invocation will usually fail if
+  something goes wrong on the way.
+
+* systemd-repart: drop pager mode on normal operation?
+
 * move logind udev rules to top-level rule.d/ directory
 
 * move multiseat vid/pid matches from logind udev rule to hwdb
@@ -242,8 +250,6 @@ Features:
 * when main nspawn supervisor process gets suspended due to SIGSTOP/SIGTTOU or
   so, freeze the payload too.
 
-* repart: support setting up dm-integrity with HMAC
-
 * add /etc/integritytab, to support dm-integrity setups. In particular those
   with HMAC as hash function, so that we can have a protected /home without
   encryption (leaving encryption to the individual dirs/homed).

src/core/main.c

@@ -1198,8 +1198,8 @@ static void bump_file_max_and_nr_open(void) {
 #endif
 
 #if BUMP_PROC_SYS_FS_FILE_MAX
-        /* The maximum the kernel allows for this since 5.2 is LONG_MAX, use that. (Previously thing where
-         * different but the operation would fail silently.) */
+        /* The maximum the kernel allows for this since 5.2 is LONG_MAX, use that. (Previously things were
+         * different, but the operation would fail silently.) */
         r = sysctl_writef("fs/file-max", "%li\n", LONG_MAX);
         if (r < 0)
                 log_full_errno(IN_SET(r, -EROFS, -EPERM, -EACCES) ? LOG_DEBUG : LOG_WARNING, r, "Failed to bump fs.file-max, ignoring: %m");

src/core/service.c

@@ -2725,7 +2725,7 @@ static int service_serialize(Unit *u, FILE *f, FDSet *fds) {
         return 0;
 }
 
-static int service_deserialize_exec_command(
+int service_deserialize_exec_command(
                 Unit *u,
                 const char *key,
                 const char *value) {
@@ -2783,9 +2783,6 @@ static int service_deserialize_exec_command(
                 case STATE_EXEC_COMMAND_PATH:
                         path = TAKE_PTR(arg);
                         state = STATE_EXEC_COMMAND_ARGS;
-
-                        if (!path_is_absolute(path))
-                                return -EINVAL;
                         break;
                 case STATE_EXEC_COMMAND_ARGS:
                         r = strv_extend(&argv, arg);
@@ -2793,13 +2790,14 @@ static int service_deserialize_exec_command(
                                 return -ENOMEM;
                         break;
                 default:
-                        assert_not_reached("Unknown error at deserialization of exec command");
-                        break;
+                        assert_not_reached("Logic error in exec command deserialization");
                 }
         }
 
         if (state != STATE_EXEC_COMMAND_ARGS)
                 return -EINVAL;
+        if (strv_isempty(argv))
+                return -EINVAL; /* At least argv[0] must be always present. */
 
         /* Let's check whether exec command on given offset matches data that we just deserialized */
         for (command = s->exec_command[id], i = 0; command; command = command->command_next, i++) {

src/core/service.h

@@ -255,3 +255,6 @@ ServiceTimeoutFailureMode service_timeout_failure_mode_from_string(const char *s
 DEFINE_CAST(SERVICE, Service);
 
 #define STATUS_TEXT_MAX (16U*1024U)
+
+/* Only exported for unit tests */
+int service_deserialize_exec_command(Unit *u, const char *key, const char *value);

src/dissect/dissect.c

@@ -774,7 +774,7 @@ static int run(int argc, char *argv[]) {
                         FLAGS_SET(arg_flags, DISSECT_IMAGE_NO_PARTITION_TABLE) ? 0 : LO_FLAGS_PARTSCAN,
                         &d);
         if (r < 0)
-                return log_error_errno(r, "Failed to set up loopback device: %m");
+                return log_error_errno(r, "Failed to set up loopback device for %s: %m", arg_image);
 
         r = dissect_image_and_warn(
                         d->fd,

src/network/networkd-dhcp4.c

@@ -143,19 +143,6 @@ static int dhcp4_route_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *li
         return 1;
 }
 
-static int route_scope_from_address(const Route *route, const struct in_addr *self_addr) {
-        assert(route);
-        assert(self_addr);
-
-        if (in4_addr_is_localhost(&route->dst.in) ||
-            (in4_addr_is_set(self_addr) && in4_addr_equal(&route->dst.in, self_addr)))
-                return RT_SCOPE_HOST;
-        else if (in4_addr_is_null(&route->gw.in))
-                return RT_SCOPE_LINK;
-        else
-                return RT_SCOPE_UNIVERSE;
-}
-
 static int dhcp_route_configure(Route *route, Link *link) {
         Route *ret;
         int r;
@@ -215,6 +202,7 @@ static int link_set_dhcp_prefix_route(Link *link) {
         route->scope = RT_SCOPE_LINK;
         route->protocol = RTPROT_DHCP;
         route->table = link_get_dhcp_route_table(link);
+        route->mtu = link->network->dhcp_route_mtu;
 
         return dhcp_route_configure(route, link);
 }
@@ -249,23 +237,105 @@ static int link_set_dhcp_route_to_gateway(Link *link, const struct in_addr *gw)
         return dhcp_route_configure(route, link);
 }
 
-static int link_set_dhcp_static_routes(Link *link) {
-        _cleanup_free_ sd_dhcp_route **static_routes = NULL;
-        bool classless_route = false, static_route = false;
-        _cleanup_(route_freep) Route *route = NULL;
-        struct in_addr address;
-        int n, r;
+static int dhcp_route_configure_auto(
+                Route *route,
+                Link *link,
+                const struct in_addr *gw) {
 
+        struct in_addr address, netmask, prefix;
+        uint8_t prefixlen;
+        int r;
+
+        assert(route);
         assert(link);
         assert(link->dhcp_lease);
+        assert(gw);
 
-        if (!link->network->dhcp_use_routes)
-                return 0;
+        /* The route object may be reused in an iteration. All elements must be set or cleared. */
 
         r = sd_dhcp_lease_get_address(link->dhcp_lease, &address);
         if (r < 0)
                 return r;
 
+        r = sd_dhcp_lease_get_netmask(link->dhcp_lease, &netmask);
+        if (r < 0)
+                return r;
+
+        prefix.s_addr = address.s_addr & netmask.s_addr;
+        prefixlen = in4_addr_netmask_to_prefixlen(&netmask);
+
+        if (in4_addr_is_localhost(&route->dst.in)) {
+                if (in4_addr_is_set(gw))
+                        log_link_debug(link, "DHCP: requested route destination "IPV4_ADDRESS_FMT_STR"/%u is localhost, "
+                                       "ignoring gateway address "IPV4_ADDRESS_FMT_STR,
+                                       IPV4_ADDRESS_FMT_VAL(route->dst.in), route->dst_prefixlen, IPV4_ADDRESS_FMT_VAL(*gw));
+
+                route->scope = RT_SCOPE_HOST;
+                route->gw_family = AF_UNSPEC;
+                route->gw = IN_ADDR_NULL;
+                route->prefsrc = IN_ADDR_NULL;
+
+        } else if (in4_addr_equal(&route->dst.in, &address)) {
+                if (in4_addr_is_set(gw))
+                        log_link_debug(link, "DHCP: requested route destination "IPV4_ADDRESS_FMT_STR"/%u is equivalent to the acquired address, "
+                                       "ignoring gateway address "IPV4_ADDRESS_FMT_STR,
+                                       IPV4_ADDRESS_FMT_VAL(route->dst.in), route->dst_prefixlen, IPV4_ADDRESS_FMT_VAL(*gw));
+
+                route->scope = RT_SCOPE_HOST;
+                route->gw_family = AF_UNSPEC;
+                route->gw = IN_ADDR_NULL;
+                route->prefsrc.in = address;
+
+        } else if (route->dst_prefixlen >= prefixlen &&
+                   (route->dst.in.s_addr & netmask.s_addr) == prefix.s_addr) {
+                if (in4_addr_is_set(gw))
+                        log_link_debug(link, "DHCP: requested route destination "IPV4_ADDRESS_FMT_STR"/%u is in the assigned network "
+                                       IPV4_ADDRESS_FMT_STR"/%u, ignoring gateway address "IPV4_ADDRESS_FMT_STR,
+                                       IPV4_ADDRESS_FMT_VAL(route->dst.in), route->dst_prefixlen,
+                                       IPV4_ADDRESS_FMT_VAL(prefix), prefixlen,
+                                       IPV4_ADDRESS_FMT_VAL(*gw));
+
+                route->scope = RT_SCOPE_LINK;
+                route->gw_family = AF_UNSPEC;
+                route->gw = IN_ADDR_NULL;
+                route->prefsrc.in = address;
+
+        } else {
+                if (in4_addr_is_null(gw)) {
+                        log_link_debug(link, "DHCP: requested route destination "IPV4_ADDRESS_FMT_STR"/%u is not in the assigned network "
+                                       IPV4_ADDRESS_FMT_STR"/%u, but no gateway is specified, ignoring.",
+                                       IPV4_ADDRESS_FMT_VAL(route->dst.in), route->dst_prefixlen,
+                                       IPV4_ADDRESS_FMT_VAL(prefix), prefixlen);
+                        return 0;
+                }
+
+                r = link_set_dhcp_route_to_gateway(link, gw);
+                if (r < 0)
+                        return r;
+
+                route->scope = RT_SCOPE_UNIVERSE;
+                route->gw_family = AF_INET;
+                route->gw.in = *gw;
+                route->prefsrc.in = address;
+        }
+
+        return dhcp_route_configure(route, link);
+}
+
+static int link_set_dhcp_static_routes(Link *link, struct in_addr *ret_default_gw) {
+        _cleanup_free_ sd_dhcp_route **static_routes = NULL;
+        bool classless_route = false, static_route = false;
+        _cleanup_(route_freep) Route *route = NULL;
+        struct in_addr default_gw = {};
+        int n, r;
+
+        assert(link);
+        assert(link->dhcp_lease);
+        assert(ret_default_gw);
+
+        if (!link->network->dhcp_use_routes)
+                return 0;
+
         n = sd_dhcp_lease_get_routes(link->dhcp_lease, &static_routes);
         if (IN_SET(n, 0, -ENODATA)) {
                 log_link_debug(link, "DHCP: No static routes received from DHCP server.");
@@ -302,11 +372,13 @@ static int link_set_dhcp_static_routes(Link *link) {
         route->mtu = link->network->dhcp_route_mtu;
 
         for (int i = 0; i < n; i++) {
+                struct in_addr gw;
+
                 if (sd_dhcp_route_get_option(static_routes[i]) !=
                     (classless_route ? SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE : SD_DHCP_OPTION_STATIC_ROUTE))
                         continue;
 
-                r = sd_dhcp_route_get_gateway(static_routes[i], &route->gw.in);
+                r = sd_dhcp_route_get_gateway(static_routes[i], &gw);
                 if (r < 0)
                         return r;
 
@@ -318,21 +390,24 @@ static int link_set_dhcp_static_routes(Link *link) {
                 if (r < 0)
                         return r;
 
-                route->scope = route_scope_from_address(route, &address);
-                if (IN_SET(route->scope, RT_SCOPE_LINK, RT_SCOPE_UNIVERSE))
-                        route->prefsrc.in = address;
-                else
-                        route->prefsrc = IN_ADDR_NULL;
+                /* When classless static routes are provided, then router option will be ignored. To
+                 * use the default gateway later in other routes, e.g., routes to dns servers, here we
+                 * need to find the default gateway in the classless static routes. */
+                if (classless_route &&
+                    in4_addr_is_null(&route->dst.in) && route->dst_prefixlen == 0 &&
+                    in4_addr_is_null(&default_gw))
+                        default_gw = gw;
 
-                r = dhcp_route_configure(route, link);
+                r = dhcp_route_configure_auto(route, link, &gw);
                 if (r < 0)
                         return r;
         }
 
+        *ret_default_gw = default_gw;
         return classless_route;
 }
 
-static int link_set_dhcp_gateway(Link *link) {
+static int link_set_dhcp_gateway(Link *link, struct in_addr *ret_gw) {
         _cleanup_(route_freep) Route *route = NULL;
         const struct in_addr *router;
         struct in_addr address;
@@ -341,6 +416,7 @@ static int link_set_dhcp_gateway(Link *link) {
 
         assert(link);
         assert(link->dhcp_lease);
+        assert(ret_gw);
 
         if (!link->network->dhcp_use_gateway)
                 return 0;
@@ -407,18 +483,19 @@ static int link_set_dhcp_gateway(Link *link) {
                         return r;
         }
 
+        *ret_gw = router[0];
         return 0;
 }
 
-static int link_set_dns_routes(Link *link) {
+static int link_set_dns_routes(Link *link, const struct in_addr *gw) {
         _cleanup_(route_freep) Route *route = NULL;
         const struct in_addr *dns;
-        struct in_addr address;
         int n, r;
 
         assert(link);
         assert(link->dhcp_lease);
         assert(link->network);
+        assert(gw);
 
         if (!link->network->dhcp_use_dns ||
             !link->network->dhcp_routes_to_dns)
@@ -430,26 +507,24 @@ static int link_set_dns_routes(Link *link) {
         if (n < 0)
                 return n;
 
-        r = sd_dhcp_lease_get_address(link->dhcp_lease, &address);
-        if (r < 0)
-                return r;
-
         r = route_new(&route);
         if (r < 0)
                 return r;
 
         route->family = AF_INET;
         route->dst_prefixlen = 32;
-        route->prefsrc.in = address;
-        route->scope = RT_SCOPE_LINK;
         route->protocol = RTPROT_DHCP;
         route->priority = link->network->dhcp_route_metric;
         route->table = link_get_dhcp_route_table(link);
+        route->mtu = link->network->dhcp_route_mtu;
 
         for (int i = 0; i < n; i ++) {
+                if (in4_addr_is_null(&dns[i]))
+                        continue;
+
                 route->dst.in = dns[i];
 
-                r = dhcp_route_configure(route, link);
+                r = dhcp_route_configure_auto(route, link, gw);
                 if (r < 0)
                         return r;
         }
@@ -458,6 +533,7 @@ static int link_set_dns_routes(Link *link) {
 }
 
 static int link_set_dhcp_routes(Link *link) {
+        struct in_addr gw = {};
         Route *rt;
         int r;
 
@@ -484,18 +560,18 @@ static int link_set_dhcp_routes(Link *link) {
         if (r < 0)
                 return log_link_error_errno(link, r, "DHCP error: Could not set prefix route: %m");
 
-        r = link_set_dhcp_static_routes(link);
+        r = link_set_dhcp_static_routes(link, &gw);
         if (r < 0)
                 return log_link_error_errno(link, r, "DHCP error: Could not set static routes: %m");
         if (r == 0) {
                 /* According to RFC 3442: If the DHCP server returns both a Classless Static Routes option and
                  * a Router option, the DHCP client MUST ignore the Router option. */
-                r = link_set_dhcp_gateway(link);
+                r = link_set_dhcp_gateway(link, &gw);
                 if (r < 0)
                         return log_link_error_errno(link, r, "DHCP error: Could not set gateway: %m");
         }
 
-        r = link_set_dns_routes(link);
+        r = link_set_dns_routes(link, &gw);
         if (r < 0)
                 return log_link_error_errno(link, r, "DHCP error: Could not set routes to DNS servers: %m");
 

src/portable/portable.c

@@ -376,7 +376,7 @@ static int portable_extract_by_path(
                         return r;
 
         } else if (r < 0)
-                return log_debug_errno(r, "Failed to set up loopback device: %m");
+                return log_debug_errno(r, "Failed to set up loopback device for %s: %m", path);
         else {
                 _cleanup_(dissected_image_unrefp) DissectedImage *m = NULL;
                 _cleanup_(rmdir_and_freep) char *tmpdir = NULL;

src/shared/dissect-image.c

@@ -2741,7 +2741,7 @@ int mount_image_privately_interactively(
                         FLAGS_SET(flags, DISSECT_IMAGE_NO_PARTITION_TABLE) ? 0 : LO_FLAGS_PARTSCAN,
                         &d);
         if (r < 0)
-                return log_error_errno(r, "Failed to set up loopback device: %m");
+                return log_error_errno(r, "Failed to set up loopback device for %s: %m", image);
 
         r = dissect_image_and_warn(d->fd, image, &verity, NULL, d->uevent_seqnum_not_before, d->timestamp_not_before, flags, &dissected_image);
         if (r < 0)

src/sysext/sysext.c

@@ -525,7 +525,7 @@ static int merge_subprocess(Hashmap *images, const char *workspace) {
 
                         r = loop_device_make_by_path(img->path, O_RDONLY, 0, &d);
                         if (r < 0)
-                                return log_error_errno(r, "Failed to set up loopback device: %m");
+                                return log_error_errno(r, "Failed to set up loopback device for %s: %m", img->path);
 
                         r = dissect_image_and_warn(
                                         d->fd,

src/test/meson.build

@@ -133,6 +133,17 @@ tests += [
 
         [['src/test/test-serialize.c']],
 
+        [['src/test/test-unit-serialize.c'],
+         [libcore,
+          libshared],
+         [threads,
+          librt,
+          libseccomp,
+          libselinux,
+          libmount,
+          libblkid],
+         core_includes],
+
         [['src/test/test-utf8.c']],
 
         [['src/test/test-dev-setup.c']],

src/test/test-unit-serialize.c

@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include "rm-rf.h"
+#include "service.h"
+#include "tests.h"
+
+#define EXEC_START_ABSOLUTE \
+        "ExecStart 0 /bin/sh \"sh\" \"-e\" \"-x\" \"-c\" \"systemctl --state=failed --no-legend --no-pager >/failed ; systemctl daemon-reload ; echo OK >/testok\""
+#define EXEC_START_RELATIVE \
+        "ExecStart 0 sh \"sh\" \"-e\" \"-x\" \"-c\" \"systemctl --state=failed --no-legend --no-pager >/failed ; systemctl daemon-reload ; echo OK >/testok\""
+
+static void test_deserialize_exec_command_one(Manager *m, const char *key, const char *line, int expected) {
+        _cleanup_(unit_freep) Unit *u = NULL;
+        int r;
+
+        assert_se(unit_new_for_name(m, sizeof(Service), "test.service", &u) >= 0);
+
+        r = service_deserialize_exec_command(u, key, line);
+        log_debug("[%s] → %d (expected: %d)", line, r, expected);
+        assert(r == expected);
+
+        /* Note that the command doesn't match any command in the empty list of commands in 's', so it is
+         * always rejected with "Current command vanished from the unit file", and we don't leak anything. */
+}
+
+static void test_deserialize_exec_command(void) {
+        _cleanup_(manager_freep) Manager *m = NULL;
+        int r;
+
+        log_info("/* %s */", __func__);
+
+        r = manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_MINIMAL, &m);
+        if (manager_errno_skip_test(r)) {
+                log_notice_errno(r, "Skipping test: manager_new: %m");
+                return;
+        }
+
+        assert_se(r >= 0);
+
+        test_deserialize_exec_command_one(m, "main-command", EXEC_START_ABSOLUTE, 0);
+        test_deserialize_exec_command_one(m, "main-command", EXEC_START_RELATIVE, 0);
+        test_deserialize_exec_command_one(m, "control-command", EXEC_START_ABSOLUTE, 0);
+        test_deserialize_exec_command_one(m, "control-command", EXEC_START_RELATIVE, 0);
+
+        test_deserialize_exec_command_one(m, "control-command", "ExecStart 0 /bin/sh \"sh\"", 0);
+        test_deserialize_exec_command_one(m, "control-command", "ExecStart 0 /no/command ", -EINVAL);
+        test_deserialize_exec_command_one(m, "control-command", "ExecStart 0 /bad/quote \"", -EINVAL);
+        test_deserialize_exec_command_one(m, "control-command", "ExecStart s /bad/id x y z", -EINVAL);
+        test_deserialize_exec_command_one(m, "control-command", "ExecStart 11", -EINVAL);
+        test_deserialize_exec_command_one(m, "control-command", "ExecWhat 11 /a/b c d e", -EINVAL);
+}
+
+int main(int argc, char *argv[]) {
+        _cleanup_(rm_rf_physical_and_freep) char *runtime_dir = NULL;
+        int r;
+
+        test_setup_logging(LOG_DEBUG);
+
+        r = enter_cgroup_subroot(NULL);
+        if (r == -ENOMEDIUM)
+                return log_tests_skipped("cgroupfs not available");
+
+        assert_se(runtime_dir = setup_fake_runtime_dir());
+
+        test_deserialize_exec_command();
+
+        return EXIT_SUCCESS;
+}

test/TEST-02-UNITTESTS/test.sh

@@ -39,7 +39,7 @@ check_result_nspawn() {
     save_journal "$workspace/var/log/journal"
     _umount_dir "${initdir:?}"
 
-    [[ -n "${TIMED_OUT:=}" ]] && ret=$((ret + 1))
+    [[ -n "${TIMED_OUT:=}" ]] && ret=1
     return $ret
 }
 
@@ -67,7 +67,7 @@ check_result_qemu() {
     save_journal "$initdir/var/log/journal"
     _umount_dir "$initdir"
 
-    [[ -n "${TIMED_OUT:=}" ]] && ret=$((ret + 1))
+    [[ -n "${TIMED_OUT:=}" ]] && ret=1
     return $ret
 }
 

test/TEST-24-CRYPTSETUP/test.sh

@@ -26,7 +26,7 @@ check_result_qemu() {
     [[ -f "$TESTDIR/failed" ]] && cat "$TESTDIR/failed"
     echo "${JOURNAL_LIST:-No journals were saved}"
 
-    test -s "$TESTDIR/failed" && ret=$((ret + 1))
+    test -s "$TESTDIR/failed" && ret=1
     return $ret
 }
 

test/TEST-55-OOMD/test.sh

@@ -19,52 +19,4 @@ EOF
     )
 }
 
-check_result_nspawn() {
-    local workspace="${1:?}"
-    local ret=1
-    local journald_report=""
-    local pids=""
-
-    [[ -e "$workspace/testok" ]] && ret=0
-    if [[ -e "$workspace/skipped" ]]; then
-        echo "TEST-56-OOMD was skipped:"
-        cat "$workspace/skipped"
-        ret=0
-    fi
-
-    [[ -f "$workspace/failed" ]] && cp -a "$workspace/failed" "${TESTDIR:?}"
-    save_journal "$workspace/var/log/journal"
-    [[ -f "$TESTDIR/failed" ]] && cat "$TESTDIR/failed"
-    echo "${JOURNAL_LIST:-No journals were saved}"
-
-    test -s "$TESTDIR/failed" && ret=$((ret + 1))
-    [ -n "${TIMED_OUT:=}" ] && ret=$((ret + 1))
-    check_asan_reports "$workspace" || ret=$((ret + 1))
-    _umount_dir "${initdir:?}"
-    return $ret
-}
-
-check_result_qemu() {
-    local ret=1
-
-    mount_initdir
-    [[ -e "${initdir:?}/testok" ]] && ret=0
-    if [[ -e "$initdir/skipped" ]]; then
-        echo "TEST-56-OOMD was skipped:"
-        cat "$initdir/skipped"
-        ret=0
-    fi
-
-    [[ -f "$initdir/failed" ]] && cp -a "$initdir/failed" "${TESTDIR:?}"
-    save_journal "$initdir/var/log/journal"
-    check_asan_reports "$initdir" || ret=$((ret + 1))
-    _umount_dir "$initdir"
-    [[ -f "$TESTDIR/failed" ]] && cat "$TESTDIR/failed"
-    echo "${JOURNAL_LIST:-No journals were saved}"
-
-    test -s "$TESTDIR/failed" && ret=$((ret + 1))
-    [ -n "${TIMED_OUT:=}" ] && ret=$((ret + 1))
-    return $ret
-}
-
 do_test "$@" 55

test/TEST-58-REPART/test.sh

@@ -1,6 +1,7 @@
 #!/usr/bin/env bash
 set -e
 TEST_DESCRIPTION="test systemd-repart"
+TEST_NO_NSPAWN=1
 . $TEST_BASE_DIR/test-functions
 
-do_test "$@" 56
+do_test "$@" 58

test/test-functions

@@ -632,7 +632,8 @@ setup_basic_environment() {
     install_keymaps
     install_terminfo
     install_execs
-    install_fsck
+    install_fs_tools
+    install_modules
     install_plymouth
     install_debug_tools
     install_ld_so_conf
@@ -833,13 +834,28 @@ EOF
     chmod 0755 "$strace_wrapper"
 }
 
-install_fsck() {
+install_fs_tools() {
     dinfo "Install fsck"
     dracut_install /sbin/fsck*
     dracut_install -o /bin/fsck*
 
     # fskc.reiserfs calls reiserfsck. so, install it
     dracut_install -o reiserfsck
+
+    # we use mkfs in system-repart tests
+    dracut_install /sbin/mkfs.ext4
+    dracut_install /sbin/mkfs.vfat
+}
+
+install_modules() {
+    dinfo "Install modules"
+
+    instmods loop
+    instmods vfat
+
+    if [[ "$LOOKS_LIKE_SUSE" ]]; then
+        instmods ext4
+    fi
 }
 
 install_dmevent() {
@@ -1102,42 +1118,66 @@ save_journal() {
     JOURNAL_LIST="$(ls -l "$dest"*)"
 }
 
-check_result_nspawn() {
+check_result_common() {
     local workspace="${1:?}"
-    local ret=1
-    local journald_report=""
-    local pids=""
-    [[ -e "$workspace/testok" ]] && ret=0
-    [[ -f "$workspace/failed" ]] && cp -a "$workspace/failed" "${TESTDIR:?}"
+    local ret
+
+    if [ -s "$workspace/failed" ]; then
+        # …/failed only counts if non-empty
+        ls -l "$workspace/failed"
+        cp -a "$workspace/failed" "${TESTDIR:?}/"
+        ret=1
+    elif [ -e "$workspace/testok" ]; then
+        # …/testok always counts (but with lower priority than …/failed)
+        ret=0
+    elif [ -e "$workspace/skipped" ]; then
+        # …/skipped always counts (a message is expected)
+        echo "${TESTNAME:?} was skipped:"
+        cat "$workspace/skipped"
+        ret=0
+    elif [ -n "$TIMED_OUT" ]; then
+        echo "${TESTNAME:?} timed out!"
+        ret=2
+    else
+        echo "${TESTNAME:?} did not report a result!"
+    fi
+
     save_journal "$workspace/var/log/journal"
-    [[ -f "$TESTDIR/failed" ]] && cat "$TESTDIR/failed"
-    echo "${JOURNAL_LIST:-"No journals were saved"}"
-    test -s "$TESTDIR/failed" && ret=$((ret+1))
-    [ -n "$TIMED_OUT" ] && ret=$((ret+1))
-    check_asan_reports "$workspace" || ret=$((ret+1))
+
+    check_asan_reports "$workspace" || ret=3
+
     if [ -d "${ARTIFACT_DIRECTORY}" ] && [ -f "$workspace/strace.out" ]; then
         cp "$workspace/strace.out" "${ARTIFACT_DIRECTORY}/"
     fi
+
+    [ -f "$TESTDIR/failed" ] && cat "$TESTDIR/failed"
+    echo "${JOURNAL_LIST:-"No journals were saved"}"
+
+    return $ret
+}
+
+check_result_nspawn() {
+    local workspace="${1:?}"
+    local ret
+
+    check_result_common "${workspace}"
+    ret=$?
+
     _umount_dir "${initdir:?}"
+
     return $ret
 }
 
 # can be overridden in specific test
 check_result_qemu() {
-    local ret=1
+    local ret
     mount_initdir
-    [[ -e "${initdir:?}/testok" ]] && ret=0
-    [[ -f "$initdir/failed" ]] && cp -a "$initdir/failed" "${TESTDIR:?}"
-    save_journal "$initdir/var/log/journal"
-    check_asan_reports "$initdir" || ret=$((ret+1))
-    if [ -d "${ARTIFACT_DIRECTORY}" ] && [ -f "$initdir/strace.out" ]; then
-        cp "$initdir/strace.out" "${ARTIFACT_DIRECTORY}/"
-    fi
-    _umount_dir "$initdir"
-    [[ -f "$TESTDIR/failed" ]] && cat "$TESTDIR/failed"
-    echo "${JOURNAL_LIST:-"No journals were saved"}"
-    test -s "$TESTDIR/failed" && ret=$((ret+1))
-    [ -n "$TIMED_OUT" ] && ret=$((ret+1))
+
+    check_result_common "${initdir:?}"
+    ret=$?
+
+    _umount_dir "${initdir:?}"
+
     return $ret
 }
 
@@ -2192,7 +2232,7 @@ instmods() {
                 ((ret+=$?))
                 ;;
         esac
-        return $ret
+        return "$ret"
     }
 
     local mod mpargs
@@ -2220,7 +2260,6 @@ setup_suse() {
     ln -fs ../usr/bin/systemctl "${initdir:?}/bin/"
     ln -fs ../usr/lib/systemd "$initdir/lib/"
     inst_simple "/usr/lib/systemd/system/haveged.service"
-    instmods ext4
 }
 
 _umount_dir() {

test/test-network/systemd-networkd-tests.py

@@ -461,7 +461,7 @@ def remove_unit_from_networkd_path(units):
                 shutil.rmtree(os.path.join(network_unit_file_path, unit + '.d'))
 
 def start_dnsmasq(additional_options='', ipv4_range='192.168.5.10,192.168.5.200', ipv6_range='2600::10,2600::20', lease_time='1h'):
-    dnsmasq_command = f'dnsmasq -8 /var/run/networkd-ci/test-dnsmasq-log-file --log-queries=extra --log-dhcp --pid-file=/var/run/networkd-ci/test-test-dnsmasq.pid --conf-file=/dev/null --interface=veth-peer --enable-ra --dhcp-range={ipv6_range},{lease_time} --dhcp-range={ipv4_range},{lease_time} -R --dhcp-leasefile=/var/run/networkd-ci/lease --dhcp-option=26,1492 --dhcp-option=option:router,192.168.5.1 --dhcp-option=33,192.168.5.4,192.168.5.5 --port=0 ' + additional_options
+    dnsmasq_command = f'dnsmasq -8 /var/run/networkd-ci/test-dnsmasq-log-file --log-queries=extra --log-dhcp --pid-file=/var/run/networkd-ci/test-test-dnsmasq.pid --conf-file=/dev/null --interface=veth-peer --enable-ra --dhcp-range={ipv6_range},{lease_time} --dhcp-range={ipv4_range},{lease_time} -R --dhcp-leasefile=/var/run/networkd-ci/lease --dhcp-option=26,1492 --dhcp-option=option:router,192.168.5.1 --port=0 ' + additional_options
     check_output(dnsmasq_command)
 
 def stop_dnsmasq(pid_file):
@@ -3852,55 +3852,71 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities):
         self.assertRegex(output, r'192.168.5.8 proto dhcp scope link src 192.168.5.181 metric 1024')
 
     def test_dhcp_client_ipv4_use_routes_gateway(self):
-        for (routes, gateway, dnsroutes) in itertools.product([True, False, None], repeat=3):
+        for (routes, gateway, dnsroutes, classless) in itertools.product([True, False], repeat=4):
             self.setUp()
-            with self.subTest(routes=routes, gateway=gateway, dnsroutes=dnsroutes):
-                self._test_dhcp_client_ipv4_use_routes_gateway(routes, gateway, dnsroutes)
+            with self.subTest(routes=routes, gateway=gateway, dnsroutes=dnsroutes, classless=classless):
+                self._test_dhcp_client_ipv4_use_routes_gateway(routes, gateway, dnsroutes, classless)
             self.tearDown()
 
-    def _test_dhcp_client_ipv4_use_routes_gateway(self, routes, gateway, dnsroutes):
+    def _test_dhcp_client_ipv4_use_routes_gateway(self, use_routes, use_gateway, dnsroutes, classless):
         testunit = 'dhcp-client-ipv4-use-routes-use-gateway.network'
         testunits = ['25-veth.netdev', 'dhcp-server-veth-peer.network', testunit]
-        if routes != None:
-            testunits.append(f'{testunit}.d/use-routes-{routes}.conf');
-        if gateway != None:
-            testunits.append(f'{testunit}.d/use-gateway-{gateway}.conf');
-        if dnsroutes != None:
-            testunits.append(f'{testunit}.d/use-dns-routes-{dnsroutes}.conf');
+        testunits.append(f'{testunit}.d/use-routes-{use_routes}.conf');
+        testunits.append(f'{testunit}.d/use-gateway-{use_gateway}.conf');
+        testunits.append(f'{testunit}.d/use-dns-routes-{dnsroutes}.conf');
         copy_unit_to_networkd_unit_path(*testunits, dropins=False)
 
         start_networkd()
         self.wait_online(['veth-peer:carrier'])
-        start_dnsmasq(additional_options='--dhcp-option=option:dns-server,192.168.5.6,192.168.5.7', lease_time='2m')
+        additional_options = '--dhcp-option=option:dns-server,192.168.5.10,8.8.8.8 --dhcp-option=option:static-route,192.168.5.100,192.168.5.2,8.8.8.8,192.168.5.3'
+        if classless:
+            additional_options += ' --dhcp-option=option:classless-static-route,0.0.0.0/0,192.168.5.4,8.0.0.0/8,192.168.5.5'
+        start_dnsmasq(additional_options=additional_options, lease_time='2m')
         self.wait_online(['veth99:routable', 'veth-peer:routable'])
 
-        output = check_output('ip route show dev veth99')
+        output = check_output('ip -4 route show dev veth99')
         print(output)
 
-        # UseRoutes= defaults to true
-        useroutes = routes in [True, None]
-        # UseGateway= defaults to useroutes
-        usegateway = useroutes if gateway == None else gateway
-
         # Check UseRoutes=
-        if useroutes:
-            self.assertRegex(output, r'192.168.5.0/24 via 192.168.5.5 proto dhcp src 192.168.5.181 metric 1024')
+        if use_routes:
+            if classless:
+                self.assertRegex(output, r'default via 192.168.5.4 proto dhcp src 192.168.5.[0-9]* metric 1024')
+                self.assertRegex(output, r'8.0.0.0/8 via 192.168.5.5 proto dhcp src 192.168.5.[0-9]* metric 1024')
+                self.assertRegex(output, r'192.168.5.4 proto dhcp scope link src 192.168.5.[0-9]* metric 1024')
+                self.assertRegex(output, r'192.168.5.5 proto dhcp scope link src 192.168.5.[0-9]* metric 1024')
+            else:
+                self.assertRegex(output, r'192.168.5.0/24 proto dhcp scope link src 192.168.5.[0-9]* metric 1024')
+                self.assertRegex(output, r'8.0.0.0/8 via 192.168.5.3 proto dhcp src 192.168.5.[0-9]* metric 1024')
+                self.assertRegex(output, r'192.168.5.3 proto dhcp scope link src 192.168.5.[0-9]* metric 1024')
         else:
-            self.assertNotRegex(output, r'192.168.5.5')
+            self.assertNotRegex(output, r'default via 192.168.5.4 proto dhcp src 192.168.5.[0-9]* metric 1024')
+            self.assertNotRegex(output, r'8.0.0.0/8 via 192.168.5.5 proto dhcp src 192.168.5.[0-9]* metric 1024')
+            self.assertNotRegex(output, r'192.168.5.4 proto dhcp scope link src 192.168.5.[0-9]* metric 1024')
+            self.assertNotRegex(output, r'192.168.5.5 proto dhcp scope link src 192.168.5.[0-9]* metric 1024')
+            self.assertNotRegex(output, r'192.168.5.0/24 proto dhcp scope link src 192.168.5.[0-9]* metric 1024')
+            self.assertNotRegex(output, r'8.0.0.0/8 via 192.168.5.3 proto dhcp src 192.168.5.[0-9]* metric 1024')
+            self.assertNotRegex(output, r'192.168.5.3 proto dhcp scope link src 192.168.5.[0-9]* metric 1024')
 
         # Check UseGateway=
-        if usegateway:
-            self.assertRegex(output, r'default via 192.168.5.1 proto dhcp src 192.168.5.181 metric 1024')
+        if use_gateway and (not classless or not use_routes):
+            self.assertRegex(output, r'default via 192.168.5.1 proto dhcp src 192.168.5.[0-9]* metric 1024')
+            self.assertRegex(output, r'192.168.5.1 proto dhcp scope link src 192.168.5.[0-9]* metric 1024')
         else:
-            self.assertNotRegex(output, r'default via 192.168.5.1')
+            self.assertNotRegex(output, r'default via 192.168.5.1 proto dhcp src 192.168.5.[0-9]* metric 1024')
+            self.assertNotRegex(output, r'192.168.5.1 proto dhcp scope link src 192.168.5.[0-9]* metric 1024')
 
-        # Check RoutesToDNS=, which defaults to false
+        # Check RoutesToDNS=
         if dnsroutes:
-            self.assertRegex(output, r'192.168.5.6 proto dhcp scope link src 192.168.5.181 metric 1024')
-            self.assertRegex(output, r'192.168.5.7 proto dhcp scope link src 192.168.5.181 metric 1024')
+            self.assertRegex(output, r'192.168.5.10 proto dhcp scope link src 192.168.5.[0-9]* metric 1024')
+            if classless and use_routes:
+                self.assertRegex(output, r'8.8.8.8 via 192.168.5.4 proto dhcp src 192.168.5.[0-9]* metric 1024')
+            elif use_gateway:
+                self.assertRegex(output, r'8.8.8.8 via 192.168.5.1 proto dhcp src 192.168.5.[0-9]* metric 1024')
+            else:
+                self.assertNotRegex(output, r'8.8.8.8 via 192.168.5.[0-9]* proto dhcp src 192.168.5.[0-9]* metric 1024')
         else:
-            self.assertNotRegex(output, r'192.168.5.6')
-            self.assertNotRegex(output, r'192.168.5.7')
+            self.assertNotRegex(output, r'192.168.5.10 proto dhcp scope link src 192.168.5.[0-9]* metric 1024')
+            self.assertNotRegex(output, r'8.8.8.8 via 192.168.5.[0-9]* proto dhcp src 192.168.5.[0-9]* metric 1024')
 
     def test_dhcp_client_ipv4_ipv6(self):
         copy_unit_to_networkd_unit_path('25-veth.netdev', 'dhcp-server-veth-peer.network', 'dhcp-client-ipv6-only.network',
@@ -3948,7 +3964,6 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities):
         self.assertRegex(output, 'default via 192.168.5.1 proto dhcp')
         if main_table_is_empty:
             self.assertRegex(output, '192.168.5.0/24 proto dhcp')
-        self.assertRegex(output, '192.168.5.0/24 via 192.168.5.5 proto dhcp')
         self.assertRegex(output, '192.168.5.1 proto dhcp scope link')
 
         print('## dnsmasq log')
@@ -4020,7 +4035,6 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities):
         print(output)
         self.assertRegex(output, r'default via 192.168.5.1 proto dhcp src 192.168.5.[0-9]* metric 1024')
         self.assertRegex(output, r'192.168.5.0/24 proto kernel scope link src 192.168.5.250')
-        self.assertRegex(output, r'192.168.5.0/24 via 192.168.5.5 proto dhcp src 192.168.5.[0-9]* metric 1024')
         self.assertRegex(output, r'192.168.5.1 proto dhcp scope link src 192.168.5.[0-9]* metric 1024')
 
     def test_dhcp_route_table_id(self):
@@ -4273,7 +4287,6 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities):
         print(output)
         self.assertRegex(output, 'default via 192.168.5.1 dev veth99 proto dhcp src 192.168.5.')
         self.assertRegex(output, '192.168.5.0/24 dev veth99 proto kernel scope link src 192.168.5')
-        self.assertRegex(output, '192.168.5.0/24 via 192.168.5.5 dev veth99 proto dhcp')
         self.assertRegex(output, '192.168.5.1 dev veth99 proto dhcp scope link src 192.168.5')
 
         print('## ip route show table main dev veth99')

test/units/testsuite-58.service

@@ -1,5 +1,5 @@
 [Unit]
-Description=TEST-56-EXIT-TYPE
+Description=TEST-58-REPART
 
 [Service]
 ExecStart=/usr/lib/systemd/tests/testdata/units/%N.sh

test/units/testsuite-58.sh

@@ -1,21 +1,28 @@
 #!/usr/bin/env bash
 set -eux
+set -o pipefail
+
+if ! command -v systemd-repart &>/dev/null; then
+    echo "no systemd-repart" >/skipped
+    exit 0
+fi
 
 export SYSTEMD_LOG_LEVEL=debug
 export PAGER=cat
 
+rm -f /var/tmp/testsuite-58.img /var/tmp/testsuite-58.2.img /tmp/testsuite-58.dump
 mkdir -p /tmp/testsuite-58-defs/
 
 # First part: create a disk image and verify its in order
 
-cat > /tmp/testsuite-58-defs/esp.conf <<EOF
+cat >/tmp/testsuite-58-defs/esp.conf <<EOF
 [Partition]
 Type=esp
 SizeMinBytes=10M
 Format=vfat
 EOF
 
-cat > /tmp/testsuite-58-defs/usr.conf <<EOF
+cat >/tmp/testsuite-58-defs/usr.conf <<EOF
 [Partition]
 Type=usr
 SizeMinBytes=10M
@@ -23,7 +30,7 @@ Format=ext4
 ReadOnly=yes
 EOF
 
-cat > /tmp/testsuite-58-defs/root.conf <<EOF
+cat >/tmp/testsuite-58-defs/root.conf <<EOF
 [Partition]
 Type=root
 SizeMinBytes=10M
@@ -31,9 +38,13 @@ Format=ext4
 MakeDirectories=/usr /efi
 EOF
 
-systemd-repart --definitions=/tmp/testsuite-58-defs/ --empty=create --size=auto --seed=750b6cd5c4ae4012a15e7be3c29e6a47 /var/tmp/testsuite-58.img
+systemd-repart --definitions=/tmp/testsuite-58-defs/ \
+               --empty=create \
+               --size=auto \
+               --seed=750b6cd5c4ae4012a15e7be3c29e6a47 \
+               /var/tmp/testsuite-58.img
 
-sfdisk --dump /var/tmp/testsuite-58.img > /tmp/testsuite-58.dump
+sfdisk --dump /var/tmp/testsuite-58.img >/tmp/testsuite-58.dump
 
 grep -qxF '/var/tmp/testsuite-58.img1 : start=        2048, size=       20480, type=C12A7328-F81F-11D2-BA4B-00A0C93EC93B, uuid=39107B09-615D-48FB-BA37-C663885FCE67, name="esp"' /tmp/testsuite-58.dump
 grep -qxF '/var/tmp/testsuite-58.img2 : start=       22528, size=       20480, type=4F68BCE3-E8CD-4DB1-96E7-FBCAF984B709, uuid=60F33797-1D71-4DCB-AA6F-20564F036CD0, name="root-x86-64"' /tmp/testsuite-58.dump
@@ -41,31 +52,36 @@ grep -qxF '/var/tmp/testsuite-58.img3 : start=       43008, size=       20480, t
 
 # Second part, duplicate it with CopyBlocks=auto
 
-cat > /tmp/testsuite-58-defs/esp.conf <<EOF
+cat >/tmp/testsuite-58-defs/esp.conf <<EOF
 [Partition]
 Type=esp
 CopyBlocks=auto
 EOF
 
-cat > /tmp/testsuite-58-defs/usr.conf <<EOF
+cat >/tmp/testsuite-58-defs/usr.conf <<EOF
 [Partition]
 Type=usr
 ReadOnly=yes
 CopyBlocks=auto
 EOF
 
-cat > /tmp/testsuite-58-defs/root.conf <<EOF
+cat >/tmp/testsuite-58-defs/root.conf <<EOF
 [Partition]
 Type=root
 CopyBlocks=auto
 EOF
 
-systemd-repart --definitions=/tmp/testsuite-58-defs/ --empty=create --size=auto --seed=750b6cd5c4ae4012a15e7be3c29e6a47 --image=/var/tmp/testsuite-58.img /var/tmp/testsuite-58.2.img
+systemd-repart --definitions=/tmp/testsuite-58-defs/ \
+               --empty=create \
+               --size=auto \
+               --seed=750b6cd5c4ae4012a15e7be3c29e6a47 \
+               --image=/var/tmp/testsuite-58.img \
+               /var/tmp/testsuite-58.2.img
 
 cmp /var/tmp/testsuite-58.img /var/tmp/testsuite-58.2.img
 
-rm -f /var/tmp/testsuite-58.img /var/tmp/testsuite-58.2.img /tmp/testsuite-58.dump
-rm -rf /tmp/testsuite-58-defs/
+rm /var/tmp/testsuite-58.img /var/tmp/testsuite-58.2.img /tmp/testsuite-58.dump
+rm -r /tmp/testsuite-58-defs/
 
 echo OK >/testok