debug-generator: rework from post-merge review #35410 (#35696)

- https://github.com/systemd/systemd/pull/35410#discussion_r1893664993
- https://github.com/systemd/systemd/pull/35410#discussion_r1893667581

6 files changed, 45 insertions, 29 deletions

diff --git a/man/systemd-debug-generator.xml b/man/systemd-debug-generator.xml
index 529d834b83..e93ee17460 100644
--- a/man/systemd-debug-generator.xml
+++ b/man/systemd-debug-generator.xml
@@ -56,7 +56,7 @@
         <command>mask</command> command. This is useful to boot with certain units removed from the initial
         boot transaction for debugging system startup. May be specified more than once. The option prefixed
         with <literal>rd.</literal> is honored only in the initrd, while the one without prefix is only
-        honored in the main system.</para>
+        honored on the host.</para>
 
         <xi:include href="version-info.xml" xpointer="v215"/></listitem>
       </varlistentry>
@@ -68,7 +68,7 @@
         <listitem><para>These options take a unit name as argument. A start job for this unit is added to the
         initial transaction. This is useful to start one or more additional units at boot. May be specified
         more than once. The option prefixed with <literal>rd.</literal> is honored only in the initrd, while
-        the one that is not prefixed only in the main system.</para>
+        the one that is not prefixed only on the host.</para>
 
         <xi:include href="version-info.xml" xpointer="v215"/></listitem>
       </varlistentry>
@@ -89,7 +89,7 @@
         shell may also be turned on persistently by enabling it with
         <citerefentry><refentrytitle>systemctl</refentrytitle><manvolnum>1</manvolnum></citerefentry>'s
         <command>enable</command> command. The options prefixed with <literal>rd.</literal> are honored only
-        in the initrd, while the ones without prefix are only honored in the main system.</para>
+        in the initrd, while the ones without prefix are only honored on the host.</para>
 
         <xi:include href="version-info.xml" xpointer="v215"/></listitem>
       </varlistentry>
@@ -103,8 +103,8 @@
         <literal>rd.</literal> option). It also accepts multiple values separated by comma
         (<literal>,</literal>). These options allow to pause the boot process at a certain point and spawn a
         debug shell. After exiting this shell, the system will resume booting. The option prefixed with
-        <literal>rd.</literal> is honored only in the initrd, while the one without prefix is only honored in
-        the main system.</para>
+        <literal>rd.</literal> is honored only in the initrd, while the one without prefix is only honored on
+        the host.</para>
 
         <table>
           <title>Available breakpoints</title>
@@ -113,13 +113,13 @@
             <colspec colname='breakpoint' />
             <colspec colname='description' />
             <colspec colname='initrd' />
-            <colspec colname='main' />
+            <colspec colname='host' />
             <thead>
               <row>
                 <entry>Breakpoints</entry>
                 <entry>Description</entry>
                 <entry>Can be used in the initrd</entry>
-                <entry>Can be used in the main system</entry>
+                <entry>Can be used on the host</entry>
               </row>
             </thead>
             <tbody>
diff --git a/src/debug-generator/debug-generator.c b/src/debug-generator/debug-generator.c
index edc8e5f5f4..098e06ca5d 100644
--- a/src/debug-generator/debug-generator.c
+++ b/src/debug-generator/debug-generator.c
@@ -66,10 +66,23 @@ static const struct BreakpointInfo breakpoint_info_table[_BREAKPOINT_TYPE_MAX] =
         { BREAKPOINT_PRE_SWITCH_ROOT,   "pre-switch-root", "breakpoint-pre-switch-root.service", BREAKPOINT_IN_INITRD | BREAKPOINT_DEFAULT },
 };
 
+static bool breakpoint_applies(const BreakpointInfo *info, int log_level) {
+        assert(info);
+
+        if (in_initrd() && !FLAGS_SET(info->validity, BREAKPOINT_IN_INITRD))
+                log_full(log_level, "Breakpoint '%s' not valid in the initrd, ignoring.", info->name);
+        else if (!in_initrd() && !FLAGS_SET(info->validity, BREAKPOINT_ON_HOST))
+                log_full(log_level, "Breakpoint '%s' not valid on the host, ignoring.", info->name);
+        else
+                return true;
+
+        return false;
+}
+
 static BreakpointType parse_breakpoint_from_string_one(const char *s) {
         assert(s);
 
-        FOREACH_ARRAY(i, breakpoint_info_table, ELEMENTSOF(breakpoint_info_table))
+        FOREACH_ELEMENT(i, breakpoint_info_table)
                 if (streq(i->name, s))
                         return i->type;
 
@@ -84,14 +97,18 @@ static int parse_breakpoint_from_string(const char *s, uint32_t *ret_breakpoints
 
         /* Empty value? set default breakpoint */
         if (isempty(s)) {
-                if (in_initrd()) {
-                        FOREACH_ARRAY(i, breakpoint_info_table, ELEMENTSOF(breakpoint_info_table))
-                                if (i->validity & BREAKPOINT_DEFAULT) {
-                                        breakpoints |= 1 << i->type;
-                                        break;
-                                }
-                } else
-                        log_warning("No default breakpoint defined on the host, ignoring breakpoint request from kernel command line.");
+                bool found_default = false;
+
+                FOREACH_ELEMENT(i, breakpoint_info_table)
+                        if (FLAGS_SET(i->validity, BREAKPOINT_DEFAULT) && breakpoint_applies(i, INT_MAX)) {
+                                breakpoints |= 1 << i->type;
+                                found_default = true;
+                                break;
+                        }
+
+                if (!found_default)
+                        log_warning("No default breakpoint defined %s, ignoring.",
+                                    in_initrd() ? "in the initrd" : "on the host");
         } else
                 for (;;) {
                         _cleanup_free_ char *t = NULL;
@@ -109,11 +126,7 @@ static int parse_breakpoint_from_string(const char *s, uint32_t *ret_breakpoints
                                 continue;
                         }
 
-                        if (in_initrd() && !FLAGS_SET(breakpoint_info_table[tt].validity, BREAKPOINT_IN_INITRD))
-                                log_warning("Breakpoint '%s' not valid in the initrd, ignoring.", t);
-                        else if (!in_initrd() && !FLAGS_SET(breakpoint_info_table[tt].validity, BREAKPOINT_ON_HOST))
-                                log_warning("Breakpoint '%s' not valid on the host, ignoring.", t);
-                        else
+                        if (breakpoint_applies(&breakpoint_info_table[tt], LOG_WARNING))
                                 breakpoints |= 1 << tt;
                 }
 
diff --git a/units/breakpoint-pre-basic.service.in b/units/breakpoint-pre-basic.service.in
index 69a406149d..e9df12c993 100644
--- a/units/breakpoint-pre-basic.service.in
+++ b/units/breakpoint-pre-basic.service.in
@@ -8,12 +8,13 @@
 #  (at your option) any later version.
 
 [Unit]
-Description=Breakpoint Before Basic System
+Description=Breakpoint Before basic.target
 Documentation=man:systemd-debug-generator(8)
 DefaultDependencies=no
+RefuseManualStart=yes
 Conflicts=shutdown.target emergency.target
 After=sysinit.target sockets.target paths.target slices.target tmp.mount systemd-vconsole-setup.service
-Before=basic.target
+Before=basic.target initrd-root-fs.target sysroot.mount shutdown.target emergency.target
 
 [Service]
 Environment=SHELL_PROMPT_PREFIX="pre-basic "
diff --git a/units/breakpoint-pre-mount.service.in b/units/breakpoint-pre-mount.service.in
index b50c780f01..55551dbbf2 100644
--- a/units/breakpoint-pre-mount.service.in
+++ b/units/breakpoint-pre-mount.service.in
@@ -8,13 +8,14 @@
 #  (at your option) any later version.
 
 [Unit]
-Description=Breakpoint Before Mounting the Root Filesystem on /sysroot
+Description=Breakpoint Before Mounting the Root Filesystem on /sysroot/
 Documentation=man:systemd-debug-generator(8)
 AssertPathExists=/etc/initrd-release
 DefaultDependencies=no
+RefuseManualStart=yes
 Conflicts=shutdown.target emergency.target
 After=basic.target systemd-vconsole-setup.service
-Before=initrd-root-fs.target sysroot.mount systemd-fsck-root.service
+Before=initrd-root-fs.target sysroot.mount systemd-fsck-root.service shutdown.target emergency.target
 
 [Service]
 Environment=SHELL_PROMPT_PREFIX="pre-mount "
diff --git a/units/breakpoint-pre-switch-root.service.in b/units/breakpoint-pre-switch-root.service.in
index 76eaa8039a..4c6334662c 100644
--- a/units/breakpoint-pre-switch-root.service.in
+++ b/units/breakpoint-pre-switch-root.service.in
@@ -12,10 +12,10 @@ Description=Breakpoint Before Switching Root
 Documentation=man:systemd-debug-generator(8)
 AssertPathExists=/etc/initrd-release
 DefaultDependencies=no
+RefuseManualStart=yes
 Conflicts=shutdown.target emergency.target
-Wants=remote-fs.target
-After=initrd.target initrd-parse-etc.service sysroot.mount remote-fs.target systemd-vconsole-setup.service
-Before=initrd-cleanup.service
+After=initrd.target initrd-parse-etc.service remote-fs.target systemd-vconsole-setup.service
+Before=initrd-cleanup.service shutdown.target emergency.target
 
 [Service]
 Environment=SHELL_PROMPT_PREFIX="pre-switch-root "
diff --git a/units/breakpoint-pre-udev.service.in b/units/breakpoint-pre-udev.service.in
index baf0e03351..6ef41e7ff9 100644
--- a/units/breakpoint-pre-udev.service.in
+++ b/units/breakpoint-pre-udev.service.in
@@ -11,10 +11,11 @@
 Description=Breakpoint Before Starting to Process Kernel uevents
 Documentation=man:systemd-debug-generator(8)
 DefaultDependencies=no
+RefuseManualStart=yes
 Conflicts=shutdown.target emergency.target
 Wants=systemd-journald.socket
 After=systemd-journald.socket systemd-vconsole-setup.service
-Before=systemd-udevd.service systemd-udev-trigger.service
+Before=systemd-udevd.service systemd-udev-trigger.service shutdown.target emergency.target
 
 [Service]
 Environment=SHELL_PROMPT_PREFIX="pre-udev "