bootctl: tweak "bootctl update" to be a NOP when boot loader is already current and...

Previously, the "bootctl update" logic would refrain from downrgading a
boot loader, but if the boot loader that is installed already matched
the version we could install we'd install it anyway, under the
assumption this was effectively without effect. This behaviour was handy
while developing boot loaders, since installing a modified boot loader
didn't require a version bump.

However, outside of the systems of boot loader developers I don't think
this behaviour makes much sense: we should always emphasize doing
minimal changes to the ESP, hence when an update is supposedly not
necessary, then don't do it. Only update if it really makes sense, to
minimize writes to the ESP. Updating the boot loader is a good thing
after all, but doing so redundantly is not.

Also, downgrade the message about this to LOG_NOTICE, given this
shouldn't be a reason to log.

Finally, exit cleanly in this cases (or if another boot loader is
detected)

diff --git a/man/bootctl.xml b/man/bootctl.xml
index d05c3f34d07d827a825b412f6782ff1b018dc7a0..a958cde7df84d0d8f1285ced7e87fed182bb661d 100644 (file)
--- a/man/bootctl.xml
+++ b/man/bootctl.xml
@@ -233,8 +233,9 @@
 
       <varlistentry>
         <term><option>--graceful</option></term>
-        <listitem><para>Ignore failure when the EFI System Partition cannot be found, or when EFI variables
-        cannot be written. Currently only applies to random seed operations.</para></listitem>
+        <listitem><para>Ignore failure when the EFI System Partition cannot be found, when EFI variables
+        cannot be written, or a different or newer boot loader is already installed. Currently only applies
+        to random seed and update operations.</para></listitem>
       </varlistentry>
 
       <varlistentry>

diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c
index fa8c6003218d2e436e939d3b5bef23a2e9508e33..5d126f4bea3bfa9af82f876f835b6f28fcb0ab0c 100644 (file)
--- a/src/boot/bootctl.c
+++ b/src/boot/bootctl.c
@@ -65,6 +65,7 @@ static const char *arg_dollar_boot_path(void) {
 
 static int acquire_esp(
                 bool unprivileged_mode,
+                bool graceful,
                 uint32_t *ret_part,
                 uint64_t *ret_pstart,
                 uint64_t *ret_psize,
@@ -80,10 +81,14 @@ static int acquire_esp(
          * this). */
 
         r = find_esp_and_warn(arg_esp_path, unprivileged_mode, &np, ret_part, ret_pstart, ret_psize, ret_uuid);
-        if (r == -ENOKEY)
+        if (r == -ENOKEY) {
+                if (graceful)
+                        return log_info_errno(r, "Couldn't find EFI system partition, skipping.");
+
                 return log_error_errno(r,
                                        "Couldn't find EFI system partition. It is recommended to mount it to /boot or /efi.\n"
                                        "Alternatively, use --esp-path= to specify path to mount point.");
+        }
         if (r < 0)
                 return r;
 
@@ -500,7 +505,7 @@ static int version_check(int fd_from, const char *from, int fd_to, const char *t
         if (r < 0)
                 return r;
         if (r == 0)
-                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
+                return log_notice_errno(SYNTHETIC_ERRNO(EREMOTE),
                                        "Source file \"%s\" does not carry version information!",
                                        from);
 
@@ -508,12 +513,15 @@ static int version_check(int fd_from, const char *from, int fd_to, const char *t
         if (r < 0)
                 return r;
         if (r == 0 || compare_product(a, b) != 0)
-                return log_notice_errno(SYNTHETIC_ERRNO(EEXIST),
+                return log_notice_errno(SYNTHETIC_ERRNO(EREMOTE),
                                         "Skipping \"%s\", since it's owned by another boot loader.",
                                         to);
 
-        if (compare_version(a, b) < 0)
-                return log_warning_errno(SYNTHETIC_ERRNO(ESTALE), "Skipping \"%s\", since a newer boot loader version exists already.", to);
+        r = compare_version(a, b);
+        if (r < 0)
+                return log_warning_errno(SYNTHETIC_ERRNO(ESTALE), "Skipping \"%s\", since newer boot loader version in place already.", to);
+        else if (r == 0)
+                return log_info_errno(SYNTHETIC_ERRNO(ESTALE), "Skipping \"%s\", since same boot loader version in place already.", to);
 
         return 0;
 }
@@ -665,6 +673,10 @@ static int install_binaries(const char *esp_path, bool force) {
                         continue;
 
                 k = copy_one_file(esp_path, de->d_name, force);
+                /* Don't propagate an error code if no update necessary, installed version already equal or
+                 * newer version, or other boot loader in place. */
+                if (arg_graceful && IN_SET(k, -ESTALE, -EREMOTE))
+                        continue;
                 if (k < 0 && r == 0)
                         r = k;
         }
@@ -1243,7 +1255,7 @@ static int verb_status(int argc, char *argv[], void *userdata) {
         sd_id128_t esp_uuid = SD_ID128_NULL, xbootldr_uuid = SD_ID128_NULL;
         int r, k;
 
-        r = acquire_esp(geteuid() != 0, NULL, NULL, NULL, &esp_uuid);
+        r = acquire_esp(/* unprivileged_mode= */ geteuid() != 0, /* graceful= */ false, NULL, NULL, NULL, &esp_uuid);
         if (arg_print_esp_path) {
                 if (r == -EACCES) /* If we couldn't acquire the ESP path, log about access errors (which is the only
                                    * error the find_esp_and_warn() won't log on its own) */
@@ -1254,7 +1266,7 @@ static int verb_status(int argc, char *argv[], void *userdata) {
                 puts(arg_esp_path);
         }
 
-        r = acquire_xbootldr(geteuid() != 0, &xbootldr_uuid);
+        r = acquire_xbootldr(/* unprivileged_mode= */ geteuid() != 0, &xbootldr_uuid);
         if (arg_print_dollar_boot_path) {
                 if (r == -EACCES)
                         return log_error_errno(r, "Failed to determine XBOOTLDR location: %m");
@@ -1402,13 +1414,13 @@ static int verb_list(int argc, char *argv[], void *userdata) {
          * off logging about access errors and turn off potentially privileged device probing. Here we're interested in
          * the latter but not the former, hence request the mode, and log about EACCES. */
 
-        r = acquire_esp(geteuid() != 0, NULL, NULL, NULL, NULL);
+        r = acquire_esp(/* unprivileged_mode= */ geteuid() != 0, /* graceful= */ false, NULL, NULL, NULL, NULL);
         if (r == -EACCES) /* We really need the ESP path for this call, hence also log about access errors */
                 return log_error_errno(r, "Failed to determine ESP: %m");
         if (r < 0)
                 return r;
 
-        r = acquire_xbootldr(geteuid() != 0, NULL);
+        r = acquire_xbootldr(/* unprivileged_mode= */ geteuid() != 0, NULL);
         if (r == -EACCES)
                 return log_error_errno(r, "Failed to determine XBOOTLDR partition: %m");
         if (r < 0)
@@ -1600,21 +1612,24 @@ static int verb_install(int argc, char *argv[], void *userdata) {
         sd_id128_t uuid = SD_ID128_NULL;
         uint64_t pstart = 0, psize = 0;
         uint32_t part = 0;
-        bool install;
+        bool install, graceful;
         int r;
 
-        r = acquire_esp(false, &part, &pstart, &psize, &uuid);
+        install = streq(argv[0], "install");
+        graceful = !install && arg_graceful; /* support graceful mode for updates */
+
+        r = acquire_esp(/* unprivileged_mode= */ false, graceful, &part, &pstart, &psize, &uuid);
+        if (graceful && r == -ENOKEY)
+                return 0; /* If --graceful is specified and we can't find an ESP, handle this cleanly */
         if (r < 0)
                 return r;
 
-        r = acquire_xbootldr(false, NULL);
+        r = acquire_xbootldr(/* unprivileged_mode= */ false, NULL);
         if (r < 0)
                 return r;
 
         settle_make_machine_id_directory();
 
-        install = streq(argv[0], "install");
-
         RUN_WITH_UMASK(0002) {
                 if (install) {
                         /* Don't create any of these directories when we are just updating. When we update
@@ -1663,11 +1678,11 @@ static int verb_remove(int argc, char *argv[], void *userdata) {
         sd_id128_t uuid = SD_ID128_NULL;
         int r, q;
 
-        r = acquire_esp(false, NULL, NULL, NULL, &uuid);
+        r = acquire_esp(/* unprivileged_mode= */ false, /* graceful= */ false, NULL, NULL, NULL, &uuid);
         if (r < 0)
                 return r;
 
-        r = acquire_xbootldr(false, NULL);
+        r = acquire_xbootldr(/* unprivileged_mode= */ false, NULL);
         if (r < 0)
                 return r;
 
@@ -1726,7 +1741,7 @@ static int verb_is_installed(int argc, char *argv[], void *userdata) {
         _cleanup_free_ char *p = NULL;
         int r;
 
-        r = acquire_esp(false, NULL, NULL, NULL, NULL);
+        r = acquire_esp(/* privileged_mode= */ false, /* graceful= */ false, NULL, NULL, NULL, NULL);
         if (r < 0)
                 return r;