[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Make grub-install check for errors from efibootmgr
From: |
Daniel Kiper |
Subject: |
Re: [PATCH] Make grub-install check for errors from efibootmgr |
Date: |
Mon, 29 Jan 2018 18:57:51 +0100 |
User-agent: |
Mutt/1.3.28i |
On Mon, Jan 29, 2018 at 02:04:54PM +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!
>
> We've been using this patch in Debian now for some time, with no ill
> effects.
>
> Signed-off-by: Steve McIntyre <address@hidden>
> ---
> 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 a3fcfcaca..fdd57a027 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;
s/error/ret/ or s/error/rc/ or s/error/rv/ here and below please...
In that order of preference...
> 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 5910b0c09..0dba8b67f 100644
> --- a/include/grub/util/install.h
> +++ b/include/grub/util/install.h
> @@ -210,7 +210,7 @@ grub_install_create_envblk_file (const char *name);
> const char *
> grub_install_get_default_x86_platform (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 5e4cdfd2b..e783824ba 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -1848,9 +1848,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;
I think that you do not need this initialization.
Daniel