| Message ID | 20180131214936.19655-1-steve@einval.com |
|---|---|
| State | Accepted |
| Commit | 6400613ad0b463abc93362086a491cd2a5e99b0d |
| Headers | show |
| Series | Make grub-install check for errors from efibootmgr | expand |
On Wed, Jan 31, 2018 at 09:49:36PM +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 s/this/similar/? If you are OK I will change that during commit. > effects. > > Signed-off-by: Steve McIntyre <93sam@debian.org> If there are no objections I will commit this in a week or so. Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Apologies for the delayed response - just found this in the archives but for some reason I never got it in my inbox. Weird... :-/ On Tue, Feb 06, 2018 at 04:29:23PM +0100, Daniel Kiper wrote: >On Wed, Jan 31, 2018 at 09:49:36PM +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 > >s/this/similar/? If you are OK I will change that during commit. Yup, perfect. :-) >If there are no objections I will commit this in a week or so. Please... -- Steve McIntyre, Cambridge, UK. steve@einval.com We don't need no education. We don't need no thought control. _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
On Wed, Feb 14, 2018 at 03:30:48PM +0000, Steve McIntyre wrote: > Apologies for the delayed response - just found this in the archives > but for some reason I never got it in my inbox. Weird... :-/ > > On Tue, Feb 06, 2018 at 04:29:23PM +0100, Daniel Kiper wrote: > >On Wed, Jan 31, 2018 at 09:49:36PM +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 > > > >s/this/similar/? If you are OK I will change that during commit. > > Yup, perfect. :-) > > >If there are no objections I will commit this in a week or so. > > Please... Applied! > -- > Steve McIntyre, Cambridge, UK. steve@einval.com > We don't need no education. > We don't need no thought control. Ha! :-))) Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c index a3fcfcaca..ca448bc11 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 rc; 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", + rc = grub_util_exec ((const char * []){ "efibootmgr", "-q", "-b", bootnum, "-B", NULL }); else - grub_util_exec ((const char * []){ "efibootmgr", + rc = grub_util_exec ((const char * []){ "efibootmgr", "-b", bootnum, "-B", NULL }); } free (line); + return rc; } -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 ret; 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); + ret = grub_install_remove_efi_entries_by_distributor (efi_distributor); + if (ret) + return ret; char *efidir_part_str = xasprintf ("%d", efidir_part); if (!verbosity) - grub_util_exec ((const char * []){ "efibootmgr", "-q", + ret = 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", + ret = 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 ret; } 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..690f180c5 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, - "\\System\\Library\\CoreServices", - efi_distributor); + int ret; + ret = grub_install_register_efi (efidir_grub_dev, + "\\System\\Library\\CoreServices", + efi_distributor); + if (ret) + grub_util_error (_("efibootmgr failed to register the boot entry: %s"), + strerror (ret)); } grub_device_close (ins_dev); @@ -1871,6 +1875,7 @@ main (int argc, char *argv[]) { char * efifile_path; char * part; + int ret; /* Try to make this image bootable using the EFI Boot Manager, if available. */ if (!efi_distributor || efi_distributor[0] == '\0') @@ -1887,8 +1892,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); + ret = grub_install_register_efi (efidir_grub_dev, + efifile_path, efi_distributor); + if (ret) + grub_util_error (_("efibootmgr failed to register the boot entry: %s"), + strerror (ret)); } break;
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 <93sam@debian.org> --- grub-core/osdep/unix/platform.c | 24 +++++++++++++++--------- include/grub/util/install.h | 2 +- util/grub-install.c | 18 +++++++++++++----- 3 files changed, 29 insertions(+), 15 deletions(-) -- 2.11.0 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel