parse-argument: make parse_tristate_argument() do something useful (#40652)

Alternative to #37751

man/bootctl.xml

@@ -407,10 +407,11 @@
       </varlistentry>
 
       <varlistentry>
-        <term><option>--variables=yes|no</option></term>
+        <term><option>--variables=yes|no|auto</option></term>
         <listitem><para>Controls whether to touch the firmware's boot loader list stored in EFI variables,
-        and other EFI variables. If not specified defaults to no when execution in a container runtime is
+        and other EFI variables. If not specified or set to <option>auto</option>, EFI variables will not
-        detected, yes otherwise.</para>
+        be modified when execution in a container runtime is detected. <option>yes</option> may be used to
+        explicit override the check.</para>
 
         <xi:include href="version-info.xml" xpointer="v258"/></listitem>
       </varlistentry>

man/run0.xml

@@ -255,8 +255,9 @@
         <term><option>--lightweight=<replaceable>BOOLEAN</replaceable></option></term>
 
         <listitem><para>Controls whether to activate the per-user service manager for the target user. By
-        default if the target user is <literal>root</literal> or a system user the per-user service manager
+        default (unset or set to <option>auto</option>), if the target user is <literal>root</literal> or
-        is not activated as effect of the <command>run0</command> invocation, otherwise it is.</para>
+        a system user the per-user service manager is not activated as effect of the <command>run0</command>
+        invocation, otherwise it is.</para>
 
         <para>This ultimately controls the <varname>$XDG_SESSION_CLASS</varname> environment variable
         <citerefentry><refentrytitle>pam_systemd</refentrytitle><manvolnum>8</manvolnum></citerefentry>

man/systemd-dissect.xml

@@ -555,10 +555,10 @@
         <term><option>--copy-ownership=</option></term>
 
         <listitem><para>Controls whether file ownership (user and group) is preserved when copying files
-        with <option>--copy-from</option> or <option>--copy-to</option>. Takes a boolean. If
+        with <option>--copy-from</option> or <option>--copy-to</option>. Takes a boolean, or <option>auto</option>.
-        <literal>yes</literal>, ownership is always preserved. If <literal>no</literal>, ownership is never
+        If <literal>yes</literal>, ownership is always preserved. If <literal>no</literal>, ownership is never
-        preserved and the current user's UID/GID is used instead. If not specified, ownership is preserved
+        preserved and the current user's UID/GID is used instead. If not specified or <option>auto</option>,
-        when copying directory trees, but not when copying individual regular files.
+        ownership is preserved when copying directory trees, but not when copying individual regular files.
         </para>
 
         <xi:include href="version-info.xml" xpointer="v260"/></listitem>

man/systemd-vmspawn.xml

@@ -166,8 +166,8 @@
         <varlistentry>
           <term><option>--kvm=<replaceable>BOOL</replaceable></option></term>
 
-          <listitem><para>If <option>--kvm=</option> is not specified, KVM support will be
+          <listitem><para>Controls whether to enable KVM acceleration. If not specified or set to <option>auto</option>,
-          detected automatically. If true, KVM is always used, and if false, KVM is never used.</para>
+          KVM support will be detected automatically. If true, KVM is insisted on. If false, disable KVM.</para>
 
           <xi:include href="version-info.xml" xpointer="v255"/></listitem>
         </varlistentry>
@@ -175,8 +175,9 @@
         <varlistentry>
           <term><option>--vsock=<replaceable>BOOL</replaceable></option></term>
 
-          <listitem><para>If <option>--vsock=</option> is not specified, VSOCK networking support will be
+          <listitem><para>Controls whether to allocate a VSOCK socket for guest. If not specified or set to
-          detected automatically. If true, VSOCK networking is always used, and if false, VSOCK networking is never used.</para>
+          <option>auto</option>, VSOCK networking support will be detected automatically. If true, VSOCK
+          is insisted on. If false, VSOCK networking is disabled.</para>
 
           <xi:include href="version-info.xml" xpointer="v255"/></listitem>
         </varlistentry>
@@ -198,12 +199,10 @@
           <term><option>--tpm=<replaceable>BOOL</replaceable></option></term>
 
           <listitem>
-            <para>If <option>--tpm=</option> is not specified, vmspawn will detect the presence of <citerefentry project='debian'>
+            <para>Controls whether to serve a TPM device for guest, via <citerefentry project='debian'>
-            <refentrytitle>swtpm</refentrytitle><manvolnum>8</manvolnum></citerefentry> and use it if
+            <refentrytitle>swtpm</refentrytitle><manvolnum>8</manvolnum></citerefentry>. If not specified or
-            available.  If yes is specified <citerefentry
+            set to <option>auto</option>, vmspawn will detect the presence of swtpm binary automatically.
-            project='debian'><refentrytitle>swtpm</refentrytitle><manvolnum>8</manvolnum></citerefentry> is
+            If yes, swtpm support is insisted on. If no, TPM is disabled.</para>
-            always used, and if no is set <citerefentry project='debian'><refentrytitle>swtpm</refentrytitle>
-            <manvolnum>8</manvolnum></citerefentry> is never used.</para>
 
             <xi:include href="version-info.xml" xpointer="v256"/>
           </listitem>
@@ -311,9 +310,9 @@
 
           <listitem><para>Configure whether to search for firmware which supports Secure Boot.</para>
 
-          <para>If the option is not specified, the first firmware which is detected will be used.
+          <para>If the option is not specified or set to <option>auto</option>, the first firmware detected
-          If the option is set to yes, then the first firmware with Secure Boot support will be selected.
+          will be used. If the option is set to yes, then the first firmware with Secure Boot support will
-          If no is specified, then the first firmware without Secure Boot will be selected.</para>
+          be selected. If no is specified, then the first firmware without Secure Boot will be selected.</para>
 
           <xi:include href="version-info.xml" xpointer="v255"/></listitem>
         </varlistentry>

src/bootctl/bootctl.c

@@ -521,7 +521,7 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_VARIABLES:
-                        r = parse_tristate_argument("--variables=", optarg, &arg_touch_variables);
+                        r = parse_tristate_argument_with_auto("--variables=", optarg, &arg_touch_variables);
                         if (r < 0)
                                 return r;
                         break;

src/dissect/dissect.c

@@ -637,7 +637,7 @@ static int parse_argv(int argc, char *argv[]) {
                 }
 
                 case ARG_COPY_OWNERSHIP:
-                        r = parse_tristate_argument("--copy-ownership=", optarg, &arg_copy_ownership);
+                        r = parse_tristate_argument_with_auto("--copy-ownership=", optarg, &arg_copy_ownership);
                         if (r < 0)
                                 return r;
                         break;

src/run/run.c

@@ -1083,7 +1083,7 @@ static int parse_argv_sudo_mode(int argc, char *argv[]) {
                         break;
 
                 case ARG_LIGHTWEIGHT:
-                        r = parse_tristate_argument("--lightweight=", optarg, &arg_lightweight);
+                        r = parse_tristate_argument_with_auto("--lightweight=", optarg, &arg_lightweight);
                         if (r < 0)
                                 return r;
                         break;

src/shared/parse-argument.c

@@ -20,10 +20,12 @@ int parse_boolean_argument(const char *optname, const char *s, bool *ret) {
 
         /* Returns the result through *ret and the return value. */
 
+        assert(optname);
+
         if (s) {
                 r = parse_boolean(s);
                 if (r < 0)
-                        return log_error_errno(r, "Failed to parse boolean argument to %s: %s.", optname, s);
+                        return log_error_errno(r, "Failed to parse boolean argument to '%s': %s", optname, s);
 
                 if (ret)
                         *ret = r;
@@ -36,25 +38,21 @@ int parse_boolean_argument(const char *optname, const char *s, bool *ret) {
         }
 }
 
-int parse_tristate_argument(const char *optname, const char *s, int *ret) {
+int parse_tristate_argument_with_auto(const char *optname, const char *s, int *ret) {
         int r;
 
-        if (s) {
+        assert(optname);
-                r = parse_boolean(s);
+        assert(s); /* We refuse NULL optarg here, since that would be ambiguous on cmdline:
+                      for --enable-a[=BOOL], --enable-a is intuitively interpreted as true rather than "auto"
+                      (parse_boolean_argument() does exactly that). IOW, tristate options should require
+                      arguments. */
+
+        r = parse_tristate_full(s, "auto", ret);
         if (r < 0)
-                        return log_error_errno(r, "Failed to parse boolean argument to %s: %s.", optname, s);
+                return log_error_errno(r, "Failed to parse tristate argument to '%s': %s", optname, s);
-
-                if (ret)
-                        *ret = r;
-
-                return r;
-        } else {
-                if (ret)
-                        *ret = -1;
 
         return 0;
 }
-}
 
 int parse_json_argument(const char *s, sd_json_format_flags_t *ret) {
         assert(s);

src/shared/parse-argument.h

@@ -4,7 +4,7 @@
 #include "shared-forward.h"
 
 int parse_boolean_argument(const char *optname, const char *s, bool *ret);
-int parse_tristate_argument(const char *optname, const char *s, int *ret);
+int parse_tristate_argument_with_auto(const char *optname, const char *s, int *ret);
 int parse_json_argument(const char *s, sd_json_format_flags_t *ret);
 int parse_path_argument(const char *path, bool suppress_root, char **arg);
 int parse_signal_argument(const char *s, int *ret);

src/systemctl/systemctl.c

@@ -830,9 +830,9 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_CHECK_INHIBITORS:
-                        r = parse_tristate_full(optarg, "auto", &arg_check_inhibitors);
+                        r = parse_tristate_argument_with_auto("--check-inhibitors=", optarg, &arg_check_inhibitors);
                         if (r < 0)
-                                return log_error_errno(r, "Failed to parse --check-inhibitors= argument: %s", optarg);
+                                return r;
                         break;
 
                 case ARG_PLAIN:

src/vmspawn/vmspawn.c

@@ -455,15 +455,15 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_KVM:
-                        r = parse_tristate(optarg, &arg_kvm);
+                        r = parse_tristate_argument_with_auto("--kvm=", optarg, &arg_kvm);
                         if (r < 0)
-                            return log_error_errno(r, "Failed to parse --kvm=%s: %m", optarg);
+                                return r;
                         break;
 
                 case ARG_VSOCK:
-                        r = parse_tristate(optarg, &arg_vsock);
+                        r = parse_tristate_argument_with_auto("--vsock=", optarg, &arg_vsock);
                         if (r < 0)
-                                return log_error_errno(r, "Failed to parse --vsock=%s: %m", optarg);
+                                return r;
                         break;
 
                 case ARG_VSOCK_CID:
@@ -483,9 +483,9 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_TPM:
-                        r = parse_tristate(optarg, &arg_tpm);
+                        r = parse_tristate_argument_with_auto("--tpm=", optarg, &arg_tpm);
                         if (r < 0)
-                                return log_error_errno(r, "Failed to parse --tpm=%s: %m", optarg);
+                                return r;
                         break;
 
                 case ARG_LINUX:
@@ -587,9 +587,9 @@ static int parse_argv(int argc, char *argv[]) {
                 }
 
                 case ARG_SECURE_BOOT:
-                        r = parse_tristate(optarg, &arg_secure_boot);
+                        r = parse_tristate_argument_with_auto("--secure-boot=", optarg, &arg_secure_boot);
                         if (r < 0)
-                                return log_error_errno(r, "Failed to parse --secure-boot=%s: %m", optarg);
+                                return r;
                         break;
 
                 case ARG_PRIVATE_USERS:
@@ -1915,6 +1915,16 @@ static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
         if (r < 0)
                 return log_error_errno(r, "Failed to find OVMF config: %m");
 
+        if (arg_secure_boot > 0 && !ovmf_config->supports_sb) {
+                assert(arg_firmware);
+
+                return log_error_errno(SYNTHETIC_ERRNO(EMEDIUMTYPE),
+                                       "Secure Boot requested, but supplied OVMF firmware blob doesn't support it.");
+        }
+
+        if (arg_secure_boot < 0)
+                log_debug("Using OVMF firmware %s Secure Boot support.", ovmf_config->supports_sb ? "with" : "without");
+
         _cleanup_(machine_bind_user_context_freep) MachineBindUserContext *bind_user_context = NULL;
         r = machine_bind_user_prepare(
                         /* directory= */ NULL,
@@ -1931,11 +1941,6 @@ static int run_virtual_machine(int kvm_device_fd, int vhost_device_fd) {
         if (r < 0)
                 return r;
 
-        /* only warn if the user hasn't disabled secureboot */
-        if (!ovmf_config->supports_sb && arg_secure_boot)
-                log_warning("Couldn't find OVMF firmware blob with Secure Boot support, "
-                            "falling back to OVMF firmware blobs without Secure Boot support.");
-
         _cleanup_free_ char *machine = NULL;
         const char *shm = arg_directory || arg_runtime_mounts.n_mounts != 0 ? ",memory-backend=mem" : "";
         const char *hpet = ARCHITECTURE_SUPPORTS_HPET ? ",hpet=off" : "";