Merge pull request #19336 from pdmorrow/reloading_restart

core,test: services in reloading state should exit without waiting TimeoutStartSec

meson.build

@@ -1585,6 +1585,9 @@ if get_option('efi')
         elif efi_arch == 'aarch64'
                 EFI_MACHINE_TYPE_NAME = 'aa64'
                 gnu_efi_arch = 'aarch64'
+        elif efi_arch == 'riscv64'
+                EFI_MACHINE_TYPE_NAME = 'riscv64'
+                gnu_efi_arch = 'riscv64'
         else
                 EFI_MACHINE_TYPE_NAME = ''
                 gnu_efi_arch = ''

rules.d/81-net-dhcp.rules

@@ -0,0 +1,14 @@
+# do not edit this file, it will be overwritten on update
+
+ACTION=="remove", GOTO="net_dhcp_end"
+SUBSYSTEM!="net", GOTO="net_dhcp_end"
+
+# Network interfaces requiring DHCPOFFER messages to be broadcast
+# must set ID_NET_DHCP_BROADCAST to "1". This property will be
+# checked by the networkd DHCP4 client to set the DHCP option
+
+# s390 ccwgroup interfaces in layer3 mode need broadcast DHCPOFFER
+# using the link driver to detect this condition
+ENV{ID_NET_DRIVER}=="qeth_l3", ENV{ID_NET_DHCP_BROADCAST}="1"
+
+LABEL="net_dhcp_end"

rules.d/meson.build

@@ -26,6 +26,7 @@ rules = files('''
         75-probe_mtd.rules
         78-sound-card.rules
         80-net-setup-link.rules
+        81-net-dhcp.rules
 '''.split())
 
 if conf.get('HAVE_KMOD') == 1

src/boot/efi/meson.build

@@ -190,9 +190,9 @@ if have_gnu_efi
                        '-znocombreloc',
                        '-L', efi_libdir,
                        efi_crt0]
-        if efi_arch == 'aarch64' or efi_arch == 'arm'
-                # Aarch64 and ARM32 don't have an EFI capable objcopy. Use 'binary'
-                # instead, and add required symbols manually.
+        if efi_arch == 'aarch64' or efi_arch == 'arm' or efi_arch == 'riscv64'
+                # Aarch64, ARM32 and 64bit RISC-V don't have an EFI capable objcopy.
+                # Use 'binary' instead, and add required symbols manually.
                 efi_ldflags += ['--defsym=EFI_SUBSYSTEM=0xa']
                 efi_format = ['-O', 'binary']
         else

src/core/service.c

@@ -3474,6 +3474,15 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) {
 
                         case SERVICE_START_POST:
                         case SERVICE_RELOAD:
+                                /* If neither main nor control processes are running then
+                                 * the current state can never exit cleanly, hence immediately
+                                 * terminate the service. */
+                                if (control_pid_good(s) <= 0)
+                                        service_enter_stop(s, f);
+
+                                /* Otherwise need to wait untill the operation is done. */
+                                break;
+
                         case SERVICE_STOP:
                                 /* Need to wait until the operation is
                                  * done */

src/network/networkd-dhcp4.c

@@ -1318,6 +1318,30 @@ static int dhcp4_set_request_address(Link *link) {
         return sd_dhcp_client_set_request_address(link->dhcp_client, &a->in_addr.in);
 }
 
+static bool link_needs_dhcp_broadcast(Link *link) {
+        const char *val;
+        int r;
+
+        assert(link);
+        assert(link->network);
+
+        /* Return the setting in DHCP[4].RequestBroadcast if specified. Otherwise return the device property
+         * ID_NET_DHCP_BROADCAST setting, which may be set for interfaces requiring that the DHCPOFFER message
+         * is being broadcast because they can't  handle unicast messages while not fully configured.
+         * If neither is set or a failure occurs, return false, which is the default for this flag.
+         */
+        r = link->network->dhcp_broadcast;
+        if (r < 0 && link->sd_device && sd_device_get_property_value(link->sd_device, "ID_NET_DHCP_BROADCAST", &val) >= 0) {
+                r = parse_boolean(val);
+                if (r < 0)
+                        log_link_warning_errno(link, r, "DHCP4 CLIENT: Failed to parse ID_NET_DHCP_BROADCAST, ignoring: %m");
+                else
+                        log_link_debug(link, "DHCP4 CLIENT: Detected ID_NET_DHCP_BROADCAST='%d'.", r);
+
+        }
+        return r == true;
+}
+
 int dhcp4_configure(Link *link) {
         sd_dhcp_option *send_option;
         void *request_options;
@@ -1359,7 +1383,7 @@ int dhcp4_configure(Link *link) {
         if (r < 0)
                 return log_link_warning_errno(link, r, "DHCP4 CLIENT: Failed to set callback: %m");
 
-        r = sd_dhcp_client_set_request_broadcast(link->dhcp_client, link->network->dhcp_broadcast);
+        r = sd_dhcp_client_set_request_broadcast(link->dhcp_client, link_needs_dhcp_broadcast(link));
         if (r < 0)
                 return log_link_warning_errno(link, r, "DHCP4 CLIENT: Failed to set request flag for broadcast: %m");
 

src/network/networkd-network-gperf.gperf

@@ -207,7 +207,7 @@ DHCPv4.RequestOptions,                       config_parse_dhcp_request_options,
 DHCPv4.Anonymize,                            config_parse_bool,                                        0,                             offsetof(Network, dhcp_anonymize)
 DHCPv4.SendHostname,                         config_parse_bool,                                        0,                             offsetof(Network, dhcp_send_hostname)
 DHCPv4.Hostname,                             config_parse_hostname,                                    0,                             offsetof(Network, dhcp_hostname)
-DHCPv4.RequestBroadcast,                     config_parse_bool,                                        0,                             offsetof(Network, dhcp_broadcast)
+DHCPv4.RequestBroadcast,                     config_parse_tristate,                                    0,                             offsetof(Network, dhcp_broadcast)
 DHCPv4.VendorClassIdentifier,                config_parse_string,                                      0,                             offsetof(Network, dhcp_vendor_class_identifier)
 DHCPv4.MUDURL,                               config_parse_dhcp_mud_url,                                0,                             0
 DHCPv4.MaxAttempts,                          config_parse_dhcp_max_attempts,                           0,                             0
@@ -481,7 +481,7 @@ DHCP.UseRoutes,                              config_parse_bool,
 DHCP.Anonymize,                              config_parse_bool,                                        0,                             offsetof(Network, dhcp_anonymize)
 DHCP.SendHostname,                           config_parse_bool,                                        0,                             offsetof(Network, dhcp_send_hostname)
 DHCP.Hostname,                               config_parse_hostname,                                    0,                             offsetof(Network, dhcp_hostname)
-DHCP.RequestBroadcast,                       config_parse_bool,                                        0,                             offsetof(Network, dhcp_broadcast)
+DHCP.RequestBroadcast,                       config_parse_tristate,                                    0,                             offsetof(Network, dhcp_broadcast)
 DHCP.CriticalConnection,                     config_parse_tristate,                                    0,                             offsetof(Network, dhcp_critical)
 DHCP.VendorClassIdentifier,                  config_parse_string,                                      0,                             offsetof(Network, dhcp_vendor_class_identifier)
 DHCP.UserClass,                              config_parse_dhcp_user_or_vendor_class,                   AF_INET,                       offsetof(Network, dhcp_user_class)

src/network/networkd-network.c

@@ -315,6 +315,7 @@ int network_load_one(Manager *manager, OrderedHashmap **networks, const char *fi
                 .dhcp_client_identifier = _DHCP_CLIENT_ID_INVALID,
                 .dhcp_route_table = RT_TABLE_MAIN,
                 .dhcp_ip_service_type = -1,
+                .dhcp_broadcast = -1,
 
                 .dhcp6_use_address = true,
                 .dhcp6_use_dns = true,

src/network/networkd-network.h

@@ -136,7 +136,7 @@ struct Network {
         int dhcp_ip_service_type;
         bool dhcp_anonymize;
         bool dhcp_send_hostname;
-        bool dhcp_broadcast;
+        int dhcp_broadcast;
         bool dhcp_use_dns;
         bool dhcp_use_dns_set;
         bool dhcp_routes_to_dns;

src/partition/repart.c

@@ -2801,15 +2801,6 @@ static int do_copy_files(Partition *p, const char *fs) {
 
         STRV_FOREACH_PAIR(source, target, p->copy_files) {
                 _cleanup_close_ int sfd = -1, pfd = -1, tfd = -1;
-                _cleanup_free_ char *dn = NULL, *fn = NULL;
-
-                r = path_extract_directory(*target, &dn);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to extract directory from '%s': %m", *target);
-
-                r = path_extract_filename(*target, &fn);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to extract filename from '%s': %m", *target);
 
                 sfd = chase_symlinks_and_open(*source, arg_root, CHASE_PREFIX_ROOT|CHASE_WARN, O_CLOEXEC|O_NOCTTY, NULL);
                 if (sfd < 0)
@@ -2823,9 +2814,19 @@ static int do_copy_files(Partition *p, const char *fs) {
                         /* We are looking at a directory */
                         tfd = chase_symlinks_and_open(*target, fs, CHASE_PREFIX_ROOT|CHASE_WARN, O_RDONLY|O_DIRECTORY|O_CLOEXEC, NULL);
                         if (tfd < 0) {
+                                _cleanup_free_ char *dn = NULL, *fn = NULL;
+
                                 if (tfd != -ENOENT)
                                         return log_error_errno(tfd, "Failed to open target directory '%s': %m", *target);
 
+                                r = path_extract_filename(*target, &fn);
+                                if (r < 0)
+                                        return log_error_errno(r, "Failed to extract filename from '%s': %m", *target);
+
+                                r = path_extract_directory(*target, &dn);
+                                if (r < 0)
+                                        return log_error_errno(r, "Failed to extract directory from '%s': %m", *target);
+
                                 r = mkdir_p_root(fs, dn, UID_INVALID, GID_INVALID, 0755);
                                 if (r < 0)
                                         return log_error_errno(r, "Failed to create parent directory '%s': %m", dn);
@@ -2846,10 +2847,23 @@ static int do_copy_files(Partition *p, const char *fs) {
                                                 UID_INVALID, GID_INVALID,
                                                 COPY_REFLINK|COPY_MERGE|COPY_REPLACE|COPY_SIGINT|COPY_HARDLINKS);
                         if (r < 0)
-                                return log_error_errno(r, "Failed to copy %s%s to %s: %m", strempty(arg_root), *source, *target);
+                                return log_error_errno(r, "Failed to copy '%s' to '%s%s': %m", *source, strempty(arg_root), *target);
                 } else {
+                        _cleanup_free_ char *dn = NULL, *fn = NULL;
+
                         /* We are looking at a regular file */
 
+                        r = path_extract_filename(*target, &fn);
+                        if (r == -EADDRNOTAVAIL || r == O_DIRECTORY)
+                                return log_error_errno(SYNTHETIC_ERRNO(EISDIR),
+                                                       "Target path '%s' refers to a directory, but source path '%s' refers to regular file, can't copy.", *target, *source);
+                        if (r < 0)
+                                return log_error_errno(r, "Failed to extract filename from '%s': %m", *target);
+
+                        r = path_extract_directory(*target, &dn);
+                        if (r < 0)
+                                return log_error_errno(r, "Failed to extract directory from '%s': %m", *target);
+
                         r = mkdir_p_root(fs, dn, UID_INVALID, GID_INVALID, 0755);
                         if (r < 0)
                                 return log_error_errno(r, "Failed to create parent directory: %m");
@@ -2858,13 +2872,13 @@ static int do_copy_files(Partition *p, const char *fs) {
                         if (pfd < 0)
                                 return log_error_errno(pfd, "Failed to open parent directory of target: %m");
 
-                        tfd = openat(pfd, basename(*target), O_CREAT|O_EXCL|O_WRONLY|O_CLOEXEC, 0700);
+                        tfd = openat(pfd, fn, O_CREAT|O_EXCL|O_WRONLY|O_CLOEXEC, 0700);
                         if (tfd < 0)
                                 return log_error_errno(errno, "Failed to create target file '%s': %m", *target);
 
                         r = copy_bytes(sfd, tfd, UINT64_MAX, COPY_REFLINK|COPY_SIGINT);
                         if (r < 0)
-                                return log_error_errno(r, "Failed to copy '%s%s' to '%s': %m", strempty(arg_root), *source, *target);
+                                return log_error_errno(r, "Failed to copy '%s' to '%s%s': %m", *source, strempty(arg_root), *target);
 
                         (void) copy_xattr(sfd, tfd);
                         (void) copy_access(sfd, tfd);

src/shared/dissect-image.c

@@ -139,7 +139,11 @@ static int enumerator_for_parent(sd_device *d, sd_device_enumerator **ret) {
         return 0;
 }
 
-static int device_is_partition(sd_device *d, sd_device *expected_parent, blkid_partition pp) {
+static int device_is_partition(
+                sd_device *d,
+                sd_device *expected_parent,
+                blkid_partition pp) {
+
         const char *v, *parent_syspath, *expected_parent_syspath;
         blkid_loff_t bsize, bstart;
         uint64_t size, start;
@@ -174,9 +178,16 @@ static int device_is_partition(sd_device *d, sd_device *expected_parent, blkid_p
         if (!path_equal(parent_syspath, expected_parent_syspath))
                 return false; /* Has a different parent than what we need, not interesting to us */
 
+        /* On kernel uevents we may find the partition number in the PARTN= field. Let's use that preferably,
+         * since it's cheaper and more importantly: the sysfs attribute "partition" appears to become
+         * available late, hence let's use the property instead, which is available at the moment we see the
+         * uevent. */
+        r = sd_device_get_property_value(d, "PARTN", &v);
+        if (r == -ENOENT)
                 r = sd_device_get_sysattr_value(d, "partition", &v);
         if (r < 0)
                 return r;
+
         r = safe_atoi(v, &partno);
         if (r < 0)
                 return r;

test/TEST-59-RELOADING-RESTART/Makefile

@@ -0,0 +1 @@
+../TEST-01-BASIC/Makefile

test/TEST-59-RELOADING-RESTART/test.sh

@@ -0,0 +1,9 @@
+#!/usr/bin/env bash
+set -e
+TEST_DESCRIPTION="Test auto restart of exited services which are stuck in reloading state"
+
+TEST_NO_QEMU=1
+
+. $TEST_BASE_DIR/test-functions
+
+do_test "$@" 59

test/units/testsuite-59.service

@@ -0,0 +1,6 @@
+[Unit]
+Description=TEST-59-RELOADING-RESTART
+
+[Service]
+Type=oneshot
+ExecStart=/usr/lib/systemd/tests/testdata/units/%N.sh

test/units/testsuite-59.sh

@@ -0,0 +1,90 @@
+#!/usr/bin/env bash
+set -eux
+set -o pipefail
+
+fail () {
+    systemd-analyze log-level info
+    exit 1
+}
+
+# Wait for a service to enter a state within a timeout period, if it doesn't
+# enter the desired state within the timeout period then this function will
+# exit the test case with a non zero exit code.
+wait_on_state_or_fail () {
+    service=$1
+    expected_state=$2
+    timeout=$3
+
+    state=$(systemctl show "$service" --property=ActiveState --value)
+    while [ "$state" != "$expected_state" ]; do
+        if [ "$timeout" = "0" ]; then
+            fail
+        fi
+        timeout=$((timeout - 1))
+        sleep 1
+        state=$(systemctl show "$service" --property=ActiveState --value)
+    done
+}
+
+systemd-analyze log-level debug
+systemd-analyze log-target console
+
+
+cat >/run/systemd/system/testservice-fail-59.service <<EOF
+[Unit]
+Description=TEST-59-RELOADING-RESTART Normal exit
+
+[Service]
+Type=notify
+ExecStart=/bin/bash -c "systemd-notify --ready; systemd-notify RELOADING=1; sleep 1; exit 1"
+EOF
+
+cat >/run/systemd/system/testservice-fail-restart-59.service <<EOF
+[Unit]
+Description=TEST-59-RELOADING-RESTART Restart=on-failure
+
+[Service]
+Type=notify
+ExecStart=/bin/bash -c "systemd-notify --ready; systemd-notify RELOADING=1; sleep 1; exit 1"
+Restart=on-failure
+StartLimitBurst=1
+EOF
+
+
+cat >/run/systemd/system/testservice-abort-restart-59.service <<EOF
+[Unit]
+Description=TEST-59-RELOADING-RESTART Restart=on-abort
+
+[Service]
+Type=notify
+ExecStart=/bin/bash -c "systemd-notify --ready; systemd-notify RELOADING=1; sleep 5; exit 1"
+Restart=on-abort
+EOF
+
+systemctl daemon-reload
+
+# This service sends a RELOADING=1 message then exits before it sends a
+# READY=1. Ensure it enters failed state and does not linger in reloading
+# state.
+systemctl start testservice-fail-59.service
+wait_on_state_or_fail "testservice-fail-59.service" "failed" "30"
+
+# This service sends a RELOADING=1 message then exits before it sends a
+# READY=1. It should automatically restart on failure. Ensure it enters failed
+# state and does not linger in reloading state.
+systemctl start testservice-fail-restart-59.service
+wait_on_state_or_fail "testservice-fail-restart-59.service" "failed" "30"
+
+# This service sends a RELOADING=1 message then exits before it sends a
+# READY=1. It should automatically restart on abort. It will sleep for 5s
+# to allow us to send it a SIGABRT. Ensure the service enters the failed state
+# and does not linger in reloading state.
+systemctl start testservice-abort-restart-59.service
+systemctl --signal=SIGABRT kill testservice-abort-restart-59.service
+wait_on_state_or_fail "testservice-abort-restart-59.service" "failed" "30"
+
+systemd-analyze log-level info
+
+echo OK >/testok
+
+exit 0