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

- #35410 (comment) - #35410 (comment)
systemd · Jan 3, 2025 · 36d2096 · 36d2096
2 parents 0726d98 + 5c79396
commit 36d2096
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 29 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 "