Anybody interested in reviewing this?
On Mon, Jan 30, 2017 at 07:04:51PM +0000, Steve McIntyre wrote:
>Code is currently ignoring errors from efibootmgr, giving users
>clearly bogus output like:
>
> Setting up grub-efi-amd64 (2.02~beta3-4) ...
> Installing for x86_64-efi platform.
> Could not delete variable: No space left on device
> Could not prepare Boot variable: No space left on device
> Installation finished. No error reported.
>
>and then potentially unbootable systems. If efibootmgr fails,
>grub-install should know that and report it!
>
>Signed-off-by: Steve McIntyre <93...@debian.org>
>---
> grub-core/osdep/unix/platform.c | 24 +++++++++++++++---------
> include/grub/util/install.h | 2 +-
> util/grub-install.c | 14 +++++++++++---
> 3 files changed, 27 insertions(+), 13 deletions(-)
>
>diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
>index 28cb37e..f9c376c 100644
>--- a/grub-core/osdep/unix/platform.c
>+++ b/grub-core/osdep/unix/platform.c
>@@ -78,19 +78,20 @@ get_ofpathname (const char *dev)
> dev);
> }
>
>-static void
>+static int
> grub_install_remove_efi_entries_by_distributor (const char *efi_distributor)
> {
> int fd;
> pid_t pid = grub_util_exec_pipe ((const char * []){ "efibootmgr", NULL },
> &fd);
> char *line = NULL;
> size_t len = 0;
>+ int error = 0;
>
> if (!pid)
> {
> grub_util_warn (_("Unable to open stream from %s: %s"),
> "efibootmgr", strerror (errno));
>- return;
>+ return errno;
> }
>
> FILE *fp = fdopen (fd, "r");
>@@ -98,7 +99,7 @@ grub_install_remove_efi_entries_by_distributor (const char
>*efi_distributor)
> {
> grub_util_warn (_("Unable to open stream from %s: %s"),
> "efibootmgr", strerror (errno));
>- return;
>+ return errno;
> }
>
> line = xmalloc (80);
>@@ -119,23 +120,25 @@ grub_install_remove_efi_entries_by_distributor (const
>char *efi_distributor)
> bootnum = line + sizeof ("Boot") - 1;
> bootnum[4] = '\0';
> if (!verbosity)
>- grub_util_exec ((const char * []){ "efibootmgr", "-q",
>+ error = grub_util_exec ((const char * []){ "efibootmgr", "-q",
> "-b", bootnum, "-B", NULL });
> else
>- grub_util_exec ((const char * []){ "efibootmgr",
>+ error = grub_util_exec ((const char * []){ "efibootmgr",
> "-b", bootnum, "-B", NULL });
> }
>
> free (line);
>+ return error;
> }
>
>-void
>+int
> grub_install_register_efi (grub_device_t efidir_grub_dev,
> const char *efifile_path,
> const char *efi_distributor)
> {
> const char * efidir_disk;
> int efidir_part;
>+ int error = 0;
> efidir_disk = grub_util_biosdisk_get_osdev (efidir_grub_dev->disk);
> efidir_part = efidir_grub_dev->disk->partition ?
> efidir_grub_dev->disk->partition->number + 1 : 1;
>
>@@ -151,23 +154,26 @@ grub_install_register_efi (grub_device_t efidir_grub_dev,
> grub_util_exec ((const char * []){ "modprobe", "-q", "efivars", NULL });
> #endif
> /* Delete old entries from the same distributor. */
>- grub_install_remove_efi_entries_by_distributor (efi_distributor);
>+ error = grub_install_remove_efi_entries_by_distributor (efi_distributor);
>+ if (error)
>+ return error;
>
> char *efidir_part_str = xasprintf ("%d", efidir_part);
>
> if (!verbosity)
>- grub_util_exec ((const char * []){ "efibootmgr", "-q",
>+ error = grub_util_exec ((const char * []){ "efibootmgr", "-q",
> "-c", "-d", efidir_disk,
> "-p", efidir_part_str, "-w",
> "-L", efi_distributor, "-l",
> efifile_path, NULL });
> else
>- grub_util_exec ((const char * []){ "efibootmgr",
>+ error = grub_util_exec ((const char * []){ "efibootmgr",
> "-c", "-d", efidir_disk,
> "-p", efidir_part_str, "-w",
> "-L", efi_distributor, "-l",
> efifile_path, NULL });
> free (efidir_part_str);
>+ return error;
> }
>
> void
>diff --git a/include/grub/util/install.h b/include/grub/util/install.h
>index 9f517a1..58648e2 100644
>--- a/include/grub/util/install.h
>+++ b/include/grub/util/install.h
>@@ -209,7 +209,7 @@ grub_install_get_default_x86_platform (void);
> const char *
> grub_install_get_default_powerpc_machtype (void);
>
>-void
>+int
> grub_install_register_efi (grub_device_t efidir_grub_dev,
> const char *efifile_path,
> const char *efi_distributor);
>diff --git a/util/grub-install.c b/util/grub-install.c
>index d87d228..7a7e67e 100644
>--- a/util/grub-install.c
>+++ b/util/grub-install.c
>@@ -2002,9 +2002,13 @@ main (int argc, char *argv[])
> if (!removable && update_nvram)
> {
> /* Try to make this image bootable using the EFI Boot Manager, if
> available. */
>- grub_install_register_efi (efidir_grub_dev,
>+ int error = 0;
>+ error = grub_install_register_efi (efidir_grub_dev,
> "\\System\\Library\\CoreServices",
> efi_distributor);
>+ if (error)
>+ grub_util_error (_("efibootmgr failed to register the boot
>entry: %s"),
>+ strerror (error));
> }
>
> grub_device_close (ins_dev);
>@@ -2094,6 +2098,7 @@ main (int argc, char *argv[])
> {
> char * efifile_path;
> char * part;
>+ int error = 0;
>
> /* Try to make this image bootable using the EFI Boot Manager, if
> available. */
> if (!efi_distributor || efi_distributor[0] == '\0')
>@@ -2110,8 +2115,11 @@ main (int argc, char *argv[])
> efidir_grub_dev->disk->name,
> (part ? ",": ""), (part ? : ""));
> grub_free (part);
>- grub_install_register_efi (efidir_grub_dev,
>- efifile_path, efi_distributor);
>+ error = grub_install_register_efi (efidir_grub_dev,
>+ efifile_path, efi_distributor);
>+ if (error)
>+ grub_util_error (_("efibootmgr failed to register the boot entry:
>%s"),
>+ strerror (error));
> }
> break;
>
>--
>2.1.4
>
>
--
Steve McIntyre, Cambridge, UK. st...@einval.com
Is there anybody out there?
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
