[grub-devel] fatal error message on grub-install for i386-pc (x86-64) with latest repo

[grub-devel] fatal error message on grub-install for i386-pc (x86-64) with latest repo

[Javier Vasquez]

Hi,

With latest git repo, after compiling/installing:

% sudo grub-install --boot-directory=/boot --recheck /dev/sdb
FATAL: Error inserting efivars
(/lib/modules/3.12.5/kernel/drivers/firmware/efi/efivars.ko): No such
device
Installation finished. No error reported.

The installation seems to have succeeded, however that fatal error
message sounds scary.

Notice the efivars.ko does exist:

% ls /lib/modules/3.12.5/kernel/drivers/firmware/efi/efivars.ko
/lib/modules/3.12.5/kernel/drivers/firmware/efi/efivars.ko

Is that OK?

-- 
Javier.

Re: [grub-devel] fatal error message on grub-install for i386-pc (x86-64) with latest repo

[Javier Vasquez]

On Sun, Dec 15, 2013 at 1:24 PM, Javier Vasquez <j.e.vasquez.v@gmail.com> wrote:
> Hi,
>
> With latest git repo, after compiling/installing:
>
> % sudo grub-install --boot-directory=/boot --recheck /dev/sdb
> FATAL: Error inserting efivars
> (/lib/modules/3.12.5/kernel/drivers/firmware/efi/efivars.ko): No such
> device
> Installation finished. No error reported.
>
> The installation seems to have succeeded, however that fatal error
> message sounds scary.
>
> Notice the efivars.ko does exist:
>
> % ls /lib/modules/3.12.5/kernel/drivers/firmware/efi/efivars.ko
> /lib/modules/3.12.5/kernel/drivers/firmware/efi/efivars.ko
>
> Is that OK?


BTW, grub still works as it did before.  A bit weird that a fatal
error message shows up when things still work out well, :-)

-- 
Javier.

Re: [grub-devel] fatal error message on grub-install for i386-pc (x86-64) with latest repo

[Andrey Borzenkov]

В Sun, 15 Dec 2013 13:24:14 -0600
Javier Vasquez <j.e.vasquez.v@gmail.com> пишет:

> Hi,
> 
> With latest git repo, after compiling/installing:
> 
> % sudo grub-install --boot-directory=/boot --recheck /dev/sdb
> FATAL: Error inserting efivars
> (/lib/modules/3.12.5/kernel/drivers/firmware/efi/efivars.ko): No such
> device
> Installation finished. No error reported.
> 

The message comes from modprobe because platform is not EFI. grub tries
to load efivars exactly to verify whether we are on EFI or not. So
while it is harmless it is indeed not nice.

Hmm ... I'm not sure we really need it at all (in
grub_install_get_default_x86_platform). Directory /sys/firmware/efi is
not created by module efivars and we check whether this directory
exists. I think it can safely be removed.

[PATCH 0/2] allow redirect of stderr in grub_util_exec*

[Andrey Borzenkov]

> Hmm ... I'm not sure we really need it at all (in
> grub_install_get_default_x86_platform). Directory /sys/firmware/efi is
> not created by module efivars and we check whether this directory
> exists. I think it can safely be removed.

OK, it was changed relatively recently so we still need modprobe but
just need to silence it like it was before in shell code. What about
the following?

Andrey Borzenkov (2):
  consolidate grub_util_exec code
  use grub_util_exec_redirect_all to silence "moprobe efivars"

 grub-core/osdep/linux/platform.c |   4 +-
 grub-core/osdep/unix/exec.c      | 134 +++++++++++++++++++--------------------
 include/grub/emu/exec.h          |   3 +
 include/grub/osdep/exec_unix.h   |  39 ------------
 4 files changed, 70 insertions(+), 110 deletions(-)
 delete mode 100644 include/grub/osdep/exec_unix.h

-- 
1.8.4

[PATCH 1/2] consolidate grub_util_exec code

[Andrey Borzenkov]

We need to hide "modprobe efivars" error output to avoid confusion. So
consolidate grub_util_exec_* into single function that can optionally redirect
all three standard descriptors and make all other functions compatibility
wrappers.

Also remove include/grub/osdep/exec_unix.h which does not appear to be used
anywhere.
---
 grub-core/osdep/unix/exec.c    | 134 ++++++++++++++++++++---------------------
 include/grub/emu/exec.h        |   3 +
 include/grub/osdep/exec_unix.h |  39 ------------
 3 files changed, 68 insertions(+), 108 deletions(-)
 delete mode 100644 include/grub/osdep/exec_unix.h

diff --git a/grub-core/osdep/unix/exec.c b/grub-core/osdep/unix/exec.c
index d4865f6..935ff12 100644
--- a/grub-core/osdep/unix/exec.c
+++ b/grub-core/osdep/unix/exec.c
@@ -34,7 +34,8 @@
 #include <sys/wait.h>
 
 int
-grub_util_exec (const char *const *argv)
+grub_util_exec_redirect_all (const char *const *argv, const char *stdin_file,
+			     const char *stdout_file, const char *stderr_file)
 {
   pid_t pid;
   int status = -1;
@@ -43,71 +44,39 @@ grub_util_exec (const char *const *argv)
   grub_size_t strl = 0;
   for (ptr = argv; *ptr; ptr++)
     strl += grub_strlen (*ptr) + 1;
+  if (stdin_file)
+    strl += grub_strlen (stdin_file) + 2;
+  if (stdout_file)
+    strl += grub_strlen (stdout_file) + 2;
+  if (stderr_file)
+    strl += grub_strlen (stderr_file) + 3;
+
   pstr = str = xmalloc (strl);
   for (ptr = argv; *ptr; ptr++)
     {
       pstr = grub_stpcpy (pstr, *ptr);
       *pstr++ = ' ';
     }
-  if (pstr > str)
-    pstr--;
-  *pstr = '\0';
-
-  grub_util_info ("executing %s", str);
-  grub_free (str);
-
-  pid = fork ();
-  if (pid < 0)
-    grub_util_error (_("Unable to fork: %s"), strerror (errno));
-  else if (pid == 0)
+  if (stdin_file)
     {
-      /* Child.  */
-
-      /* Close fd's.  */
-#ifdef GRUB_UTIL
-      grub_util_devmapper_cleanup ();
-      grub_diskfilter_fini ();
-#endif
-
-      /* Ensure child is not localised.  */
-      setenv ("LC_ALL", "C", 1);
-
-      execvp ((char *) argv[0], (char **) argv);
-      exit (127);
+      *pstr++ = '<';
+      pstr = grub_stpcpy (pstr, stdin_file);
+      *pstr++ = ' ';
     }
-
-  waitpid (pid, &status, 0);
-  if (!WIFEXITED (status))
-    return -1;
-  return WEXITSTATUS (status);
-}
-
-int
-grub_util_exec_redirect (const char *const *argv, const char *stdin_file,
-			 const char *stdout_file)
-{
-  pid_t pid;
-  int status = -1;
-  char *str, *pstr;
-  const char *const *ptr;
-  grub_size_t strl = 0;
-  for (ptr = argv; *ptr; ptr++)
-    strl += grub_strlen (*ptr) + 1;
-  strl += grub_strlen (stdin_file) + 2;
-  strl += grub_strlen (stdout_file) + 2;
-
-  pstr = str = xmalloc (strl);
-  for (ptr = argv; *ptr; ptr++)
+  if (stdout_file)
     {
-      pstr = grub_stpcpy (pstr, *ptr);
+      *pstr++ = '>';
+      pstr = grub_stpcpy (pstr, stdout_file);
       *pstr++ = ' ';
     }
-  *pstr++ = '<';
-  pstr = grub_stpcpy (pstr, stdin_file);
-  *pstr++ = ' ';
-  *pstr++ = '>';
-  pstr = grub_stpcpy (pstr, stdout_file);
-  *pstr = '\0';
+  if (stderr_file)
+    {
+      *pstr++ = '2';
+      *pstr++ = '>';
+      pstr = grub_stpcpy (pstr, stderr_file);
+      pstr++;
+    }
+  *--pstr = '\0';
 
   grub_util_info ("executing %s", str);
   grub_free (str);
@@ -117,7 +86,7 @@ grub_util_exec_redirect (const char *const *argv, const char *stdin_file,
     grub_util_error (_("Unable to fork: %s"), strerror (errno));
   else if (pid == 0)
     {
-      int in, out;
+      int fd;
       /* Child.  */
       
       /* Close fd's.  */
@@ -126,18 +95,32 @@ grub_util_exec_redirect (const char *const *argv, const char *stdin_file,
       grub_diskfilter_fini ();
 #endif
 
-      in = open (stdin_file, O_RDONLY);
-      if (in < 0)
-	exit (127);
-      dup2 (in, STDIN_FILENO);
-      close (in);
-
-      out = open (stdout_file, O_WRONLY | O_CREAT, 0700);
-      dup2 (out, STDOUT_FILENO);
-      close (out);
-
-      if (out < 0)
-	exit (127);
+      if (stdin_file)
+	{
+	  fd = open (stdin_file, O_RDONLY);
+	  if (fd < 0)
+	    exit (127);
+	  dup2 (fd, STDIN_FILENO);
+	  close (fd);
+	}
+
+      if (stdout_file)
+	{
+	  fd = open (stdout_file, O_WRONLY | O_CREAT, 0700);
+	  if (fd < 0)
+	    exit (127);
+	  dup2 (fd, STDOUT_FILENO);
+	  close (fd);
+	}
+
+      if (stderr_file)
+	{
+	  fd = open (stderr_file, O_WRONLY | O_CREAT, 0700);
+	  if (fd < 0)
+	    exit (127);
+	  dup2 (fd, STDERR_FILENO);
+	  close (fd);
+	}
 
       /* Ensure child is not localised.  */
       setenv ("LC_ALL", "C", 1);
@@ -152,9 +135,22 @@ grub_util_exec_redirect (const char *const *argv, const char *stdin_file,
 }
 
 int
+grub_util_exec (const char *const *argv)
+{
+  return grub_util_exec_redirect_all (argv, NULL, NULL, NULL);
+}
+
+int
+grub_util_exec_redirect (const char *const *argv, const char *stdin_file,
+			 const char *stdout_file)
+{
+  return grub_util_exec_redirect_all (argv, stdin_file, stdout_file, NULL);
+}
+
+int
 grub_util_exec_redirect_null (const char *const *argv)
 {
-  return grub_util_exec_redirect (argv, "/dev/null", "/dev/null");
+  return grub_util_exec_redirect_all (argv, "/dev/null", "/dev/null", NULL);
 }
 
 pid_t
diff --git a/include/grub/emu/exec.h b/include/grub/emu/exec.h
index ecc3adc..d1073ef 100644
--- a/include/grub/emu/exec.h
+++ b/include/grub/emu/exec.h
@@ -29,6 +29,9 @@ pid_t
 grub_util_exec_pipe_stderr (const char *const *argv, int *fd);
 
 int
+grub_util_exec_redirect_all (const char *const *argv, const char *stdin_file,
+			     const char *stdout_file, const char *stderr_file);
+int
 grub_util_exec (const char *const *argv);
 int
 grub_util_exec_redirect (const char *const *argv, const char *stdin_file,
diff --git a/include/grub/osdep/exec_unix.h b/include/grub/osdep/exec_unix.h
deleted file mode 100644
index ecc3adc..0000000
--- a/include/grub/osdep/exec_unix.h
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2013  Free Software Foundation, Inc.
- *
- *  GRUB is free software: you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation, either version 3 of the License, or
- *  (at your option) any later version.
- *
- *  GRUB is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-#ifndef GRUB_EMU_EXEC_H
-#define GRUB_EMU_EXEC_H 1
-
-#include <config.h>
-#include <stdarg.h>
-
-#include <sys/types.h>
-pid_t
-grub_util_exec_pipe (const char *const *argv, int *fd);
-pid_t
-grub_util_exec_pipe_stderr (const char *const *argv, int *fd);
-
-int
-grub_util_exec (const char *const *argv);
-int
-grub_util_exec_redirect (const char *const *argv, const char *stdin_file,
-			 const char *stdout_file);
-int
-grub_util_exec_redirect_null (const char *const *argv);
-
-#endif
-- 
1.8.4

[PATCH 2/2] use grub_util_exec_redirect_all to silence "moprobe efivars"

[Andrey Borzenkov]

---
 grub-core/osdep/linux/platform.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/grub-core/osdep/linux/platform.c b/grub-core/osdep/linux/platform.c
index ec67aad..86561f7 100644
--- a/grub-core/osdep/linux/platform.c
+++ b/grub-core/osdep/linux/platform.c
@@ -69,8 +69,8 @@ grub_install_get_default_x86_platform (void)
      anyway later. So it should be safe to
      try to load it here.
    */
-  grub_util_exec ((const char * []){ "modprobe", "-q",
-	"efivars", NULL });
+  grub_util_exec_redirect_all ((const char * []){ "modprobe", "efivars", NULL },
+			       NULL, NULL, "/dev/null");
   if (is_not_empty_directory ("/sys/firmware/efi"))
     {
       if (is_64_kernel ())
-- 
1.8.4

Re: [PATCH 2/2] use grub_util_exec_redirect_all to silence "moprobe efivars"

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]

Go ahead for both patches.
Could you also add a grub_util_info detailing which directories were
tried and if they were found or not? To provide info about why GRUB
assumed this or that platform.
On 16.12.2013 19:20, Andrey Borzenkov wrote:
> ---
>  grub-core/osdep/linux/platform.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/grub-core/osdep/linux/platform.c b/grub-core/osdep/linux/platform.c
> index ec67aad..86561f7 100644
> --- a/grub-core/osdep/linux/platform.c
> +++ b/grub-core/osdep/linux/platform.c
> @@ -69,8 +69,8 @@ grub_install_get_default_x86_platform (void)
>       anyway later. So it should be safe to
>       try to load it here.
>     */
> -  grub_util_exec ((const char * []){ "modprobe", "-q",
> -	"efivars", NULL });
> +  grub_util_exec_redirect_all ((const char * []){ "modprobe", "efivars", NULL },
> +			       NULL, NULL, "/dev/null");
>    if (is_not_empty_directory ("/sys/firmware/efi"))
>      {
>        if (is_64_kernel ())
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 291 bytes --]