bootctl: replace --no-variables by --variables=BOOL

I think the current behaviour of not doing EFI variables when we are run
in a container makes a ton of sense, but in some cases it's useful to
do EFI var setup even when a set of namespaces is set up for us, for
example to recover a hosed installation from a rescue disk.

While we are at it, let's remove some duplicate checks, and
systematically output information why we skip various operations.

Fixes: #36174 #35005

diff --git a/man/bootctl.xml b/man/bootctl.xml
index 89ae019ad79047098e7231190603e9cb0a8b8ccc..91d572e6430232781919369b08b334707c583bf9 100644 (file)
--- a/man/bootctl.xml
+++ b/man/bootctl.xml
@@ -398,10 +398,12 @@
       </varlistentry>
 
       <varlistentry>
-        <term><option>--no-variables</option></term>
-        <listitem><para>Do not touch the firmware's boot loader list stored in EFI variables.</para>
+        <term><option>--variables=yes|no</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
+        detected, yes otherwise.</para>
 
-        <xi:include href="version-info.xml" xpointer="v220"/></listitem>
+        <xi:include href="version-info.xml" xpointer="v258"/></listitem>
       </varlistentry>
 
       <varlistentry>

diff --git a/src/bootctl/bootctl-install.c b/src/bootctl/bootctl-install.c
index 61e172a037e27301147aae449e7d696a08e9fbd0..16ae5de99f6e5c9e2524396e812553fffc3578e5 100644 (file)
--- a/src/bootctl/bootctl-install.c
+++ b/src/bootctl/bootctl-install.c
@@ -865,17 +865,6 @@ static int install_variables(
         uint16_t slot;
         int r;
 
-        if (arg_root) {
-                log_info("Acting on %s, skipping EFI variable setup.",
-                         arg_image ? "image" : "root directory");
-                return 0;
-        }
-
-        if (!is_efi_boot()) {
-                log_warning("Not booted with EFI, skipping EFI variable setup.");
-                return 0;
-        }
-
         r = chase_and_access(path, esp_path, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, F_OK, NULL);
         if (r == -ENOENT)
                 return 0;
@@ -1075,7 +1064,7 @@ int verb_install(int argc, char *argv[], void *userdata) {
 
         (void) sync_everything();
 
-        if (!arg_touch_variables)
+        if (!touch_variables())
                 return 0;
 
         if (arg_arch_all) {
@@ -1206,9 +1195,6 @@ static int remove_variables(sd_id128_t uuid, const char *path, bool in_order) {
         uint16_t slot;
         int r;
 
-        if (arg_root || !is_efi_boot())
-                return 0;
-
         r = find_slot(uuid, path, &slot);
         if (r != 1)
                 return 0;
@@ -1327,7 +1313,7 @@ int verb_remove(int argc, char *argv[], void *userdata) {
 
         (void) sync_everything();
 
-        if (!arg_touch_variables)
+        if (!touch_variables())
                 return r;
 
         if (arg_arch_all) {

diff --git a/src/bootctl/bootctl-random-seed.c b/src/bootctl/bootctl-random-seed.c
index 2c81ee32b7a0043689cf8232964d31f9074a7c1a..0336cb3153ff0103a2da4476a33fef4ab65147cc 100644 (file)
--- a/src/bootctl/bootctl-random-seed.c
+++ b/src/bootctl/bootctl-random-seed.c
@@ -58,20 +58,9 @@ static int set_system_token(void) {
         size_t token_size;
         int r;
 
-        if (!arg_touch_variables)
+        if (!touch_variables())
                 return 0;
 
-        if (arg_root) {
-                log_warning("Acting on %s, skipping EFI variable setup.",
-                             arg_image ? "image" : "root directory");
-                return 0;
-        }
-
-        if (!is_efi_boot()) {
-                log_notice("Not booted with EFI, skipping EFI variable setup.");
-                return 0;
-        }
-
         r = getenv_bool("SYSTEMD_WRITE_SYSTEM_TOKEN");
         if (r < 0) {
                 if (r != -ENXIO)

diff --git a/src/bootctl/bootctl-set-efivar.c b/src/bootctl/bootctl-set-efivar.c
index f23fcc713e66a3f7cf026a9219fa790e3d1d95e4..fe55462ffd501991504068ce85367cdf77cf0dc3 100644 (file)
--- a/src/bootctl/bootctl-set-efivar.c
+++ b/src/bootctl/bootctl-set-efivar.c
@@ -105,32 +105,36 @@ static int parse_loader_entry_target_arg(const char *arg1, char16_t **ret_target
 int verb_set_efivar(int argc, char *argv[], void *userdata) {
         int r;
 
-        if (arg_root)
-                return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP),
-                                       "Acting on %s, skipping EFI variable setup.",
-                                       arg_image ? "image" : "root directory");
-
-        if (!is_efi_boot())
-                return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP),
-                                       "Not booted with UEFI.");
-
-        if (access(EFIVAR_PATH(EFI_LOADER_VARIABLE_STR("LoaderInfo")), F_OK) < 0) {
-                if (errno == ENOENT) {
-                        log_error_errno(errno, "Not booted with a supported boot loader.");
-                        return -EOPNOTSUPP;
+        /* Note: changing EFI variables is the primary purpose of these verbs, hence unlike in the other
+         * verbs that might touch EFI variables where we skip things gracefully, here we fail loudly if we
+         * are not run on EFI or EFI variable modifications were turned off. */
+
+        if (arg_touch_variables < 0) {
+                if (arg_root)
+                        return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP),
+                                               "Acting on %s, refusing EFI variable setup.",
+                                               arg_image ? "image" : "root directory");
+
+                if (detect_container() > 0)
+                        return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP),
+                                               "'%s' operation not supported in a container.",
+                                               argv[0]);
+                if (!is_efi_boot())
+                        return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP),
+                                               "Not booted with UEFI.");
+
+                if (access(EFIVAR_PATH(EFI_LOADER_VARIABLE_STR("LoaderInfo")), F_OK) < 0) {
+                        if (errno == ENOENT) {
+                                log_error_errno(errno, "Not booted with a supported boot loader.");
+                                return -EOPNOTSUPP;
+                        }
+
+                        return log_error_errno(errno, "Failed to detect whether boot loader supports '%s' operation: %m", argv[0]);
                 }
 
-                return log_error_errno(errno, "Failed to detect whether boot loader supports '%s' operation: %m", argv[0]);
-        }
-
-        if (detect_container() > 0)
-                return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP),
-                                       "'%s' operation not supported in a container.",
-                                       argv[0]);
-
-        if (!arg_touch_variables)
+        } else if (!arg_touch_variables)
                 return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                       "'%s' operation cannot be combined with --no-variables.",
+                                       "'%s' operation cannot be combined with --variables=no.",
                                        argv[0]);
 
         const char *variable;

diff --git a/src/bootctl/bootctl.c b/src/bootctl/bootctl.c
index 0a7893224c74ca3dc00259a175af9d22ecfed35c..0c1ee867d7a4ce0380e32c6326eb8a0e4c0c96b2 100644 (file)
--- a/src/bootctl/bootctl.c
+++ b/src/bootctl/bootctl.c
@@ -43,7 +43,7 @@ bool arg_print_dollar_boot_path = false;
 bool arg_print_loader_path = false;
 bool arg_print_stub_path = false;
 unsigned arg_print_root_device = 0;
-bool arg_touch_variables = true;
+int arg_touch_variables = -1;
 bool arg_install_random_seed = true;
 PagerFlags arg_pager_flags = 0;
 bool arg_graceful = false;
@@ -213,6 +213,29 @@ static int print_loader_or_stub_path(void) {
         return 0;
 }
 
+bool touch_variables(void) {
+        /* If we run in a container or on a non-EFI system, automatically turn off EFI file system access,
+         * unless explicitly overriden. */
+
+        if (arg_touch_variables >= 0)
+                return arg_touch_variables;
+
+        if (arg_root) {
+                log_once(LOG_NOTICE,
+                         "Operating on %s, skipping EFI variable modifications.",
+                         arg_image ? "image" : "root directory");
+                return false;
+        }
+
+        if (!is_efi_boot()) { /* NB: this internally checks if we run in a container */
+                log_once(LOG_NOTICE,
+                         "Not booted with EFI or running in a container, skipping EFI variable modifications.");
+                return false;
+        }
+
+        return true;
+}
+
 static int help(int argc, char *argv[], void *userdata) {
         _cleanup_free_ char *link = NULL;
         int r;
@@ -271,7 +294,8 @@ static int help(int argc, char *argv[], void *userdata) {
                "                       Specify disk image dissection policy\n"
                "     --install-source=auto|image|host\n"
                "                       Where to pick files when using --root=/--image=\n"
-               "     --no-variables    Don't touch EFI variables\n"
+               "     --variables=yes|no\n"
+               "                       Whether to modify EFI variables\n"
                "     --random-seed=yes|no\n"
                "                       Whether to create random-seed file during install\n"
                "     --no-pager        Do not pipe output into a pager\n"
@@ -327,6 +351,7 @@ static int parse_argv(int argc, char *argv[]) {
                 ARG_IMAGE_POLICY,
                 ARG_INSTALL_SOURCE,
                 ARG_VERSION,
+                ARG_VARIABLES,
                 ARG_NO_VARIABLES,
                 ARG_RANDOM_SEED,
                 ARG_NO_PAGER,
@@ -362,7 +387,8 @@ static int parse_argv(int argc, char *argv[]) {
                 { "print-loader-path",           no_argument,       NULL, ARG_PRINT_LOADER_PATH           },
                 { "print-stub-path",             no_argument,       NULL, ARG_PRINT_STUB_PATH             },
                 { "print-root-device",           no_argument,       NULL, 'R'                             },
-                { "no-variables",                no_argument,       NULL, ARG_NO_VARIABLES                },
+                { "variables",                   required_argument, NULL, ARG_VARIABLES                   },
+                { "no-variables",                no_argument,       NULL, ARG_NO_VARIABLES                }, /* Compability */
                 { "random-seed",                 required_argument, NULL, ARG_RANDOM_SEED                 },
                 { "no-pager",                    no_argument,       NULL, ARG_NO_PAGER                    },
                 { "graceful",                    no_argument,       NULL, ARG_GRACEFUL                    },
@@ -460,6 +486,12 @@ static int parse_argv(int argc, char *argv[]) {
                         arg_print_root_device++;
                         break;
 
+                case ARG_VARIABLES:
+                        r = parse_tristate_argument("--variables=", optarg, &arg_touch_variables);
+                        if (r < 0)
+                                return r;
+                        break;
+
                 case ARG_NO_VARIABLES:
                         arg_touch_variables = false;
                         break;
@@ -643,10 +675,6 @@ static int run(int argc, char *argv[]) {
 
         log_setup();
 
-        /* If we run in a container, automatically turn off EFI file system access */
-        if (detect_container() > 0)
-                arg_touch_variables = false;
-
         r = parse_argv(argc, argv);
         if (r <= 0)
                 return r;

diff --git a/src/bootctl/bootctl.h b/src/bootctl/bootctl.h
index 6d0dfec47f0dcebe1d2add3922753c053059cf72..191a970cd7e38b8f43b451b89c3a40e8af75afdb 100644 (file)
--- a/src/bootctl/bootctl.h
+++ b/src/bootctl/bootctl.h
@@ -20,7 +20,7 @@ extern char *arg_xbootldr_path;
 extern bool arg_print_esp_path;
 extern bool arg_print_dollar_boot_path;
 extern unsigned arg_print_root_device;
-extern bool arg_touch_variables;
+extern int arg_touch_variables;
 extern bool arg_install_random_seed;
 extern PagerFlags arg_pager_flags;
 extern bool arg_graceful;
@@ -54,3 +54,5 @@ static inline const char* arg_dollar_boot_path(void) {
 
 int acquire_esp(int unprivileged_mode, bool graceful, uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, sd_id128_t *ret_uuid, dev_t *ret_devid);
 int acquire_xbootldr(int unprivileged_mode, sd_id128_t *ret_uuid, dev_t *ret_devid);
+
+bool touch_variables(void);