grub-devel
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]