Libguestfs - Aug 2016 - Re: [PATCH 2/2] v2v: ilnux: detect name of grub2-mkconfig

On Debian family of OSes Grub2 tools are prefixed with 'grub-', not with
'grub2-'. We have to detect the correct name of the tool to use it.

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 v2v/convert_linux.ml | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
index 103728b..1f5f12c 100644
--- a/v2v/convert_linux.ml
+++ b/v2v/convert_linux.ml
@@ -109,6 +109,23 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps        Not_found ->
         error (f_"no grub1/grub-legacy or grub2 configuration file was
found") in
 
+  let grub2_mkconfig_cmd +    if grub = `Grub2 then
+      try
+        (* Red Hat and Suse families *)
+        ignore (g#command [| "grub2-mkconfig"; "--version"
|]);
+        "grub2-mkconfig"
+      with G.Error _ ->
+        try
+          (* Debian family *)
+          ignore (g#command [| "grub-mkconfig"; "--version"
|]);
+          "grub-mkconfig"
+        with G.Error _ ->
+          error (f_"failed to find grub2-mkconfig binary (but Grub2 was
detected on guest)")
+    else
+      ""
+  in
+
   (* Grub prefix?  Usually "/boot". *)
   let grub_prefix      match grub with
@@ -1109,7 +1126,7 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps          g#aug_save ();
 
         try
-          ignore (g#command [| "grub2-mkconfig"; "-o";
grub_config |])
+          ignore (g#command [| grub2_mkconfig_cmd; "-o"; grub_config
|])
         with
           G.Error msg ->
             warning (f_"could not rebuild grub2 configuration file (%s). 
This may mean that grub output will not be sent to the serial port, but
otherwise should be harmless.  Original error message: %s")
@@ -1431,7 +1448,7 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps
       (* If it's grub2, we have to regenerate the config files. *)
       if grub = `Grub2 then
-        ignore (g#command [| "grub2-mkconfig"; "-o";
grub_config |]);
+        ignore (g#command [| grub2_mkconfig_cmd; "-o"; grub_config
|]);
 
       Linux.augeas_reload g
     );
-- 
2.9.2

On Thu, 11 Aug 2016 00:35:55 +0200
Tomáš Golembiovský <tgolembi@redhat.com> wrote:
> On Debian family of OSes Grub2 tools are prefixed with 'grub-', not
with
> 'grub2-'. We have to detect the correct name of the tool to use it.
> 
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> ---
>  v2v/convert_linux.ml | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
> index 103728b..1f5f12c 100644
> --- a/v2v/convert_linux.ml
> +++ b/v2v/convert_linux.ml
> @@ -109,6 +109,23 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps >        Not_found ->
>          error (f_"no grub1/grub-legacy or grub2 configuration file
was found") in
>  
> +  let grub2_mkconfig_cmd > +    if grub = `Grub2 then
We could base the detection on the Debian vs. other family, but this is
more generic.
> +      try
> +        (* Red Hat and Suse families *)
> +        ignore (g#command [| "grub2-mkconfig";
"--version" |]);
> +        "grub2-mkconfig"
> +      with G.Error _ ->
> +        try
> +          (* Debian family *)
> +          ignore (g#command [| "grub-mkconfig";
"--version" |]);
> +          "grub-mkconfig"
> +        with G.Error _ ->
> +          error (f_"failed to find grub2-mkconfig binary (but Grub2
was detected on guest)")
> +    else
> +      ""
> +  in
> +
>    (* Grub prefix?  Usually "/boot". *)
>    let grub_prefix >      match grub with
> @@ -1109,7 +1126,7 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps >          g#aug_save ();
>  
>          try
> -          ignore (g#command [| "grub2-mkconfig"; "-o";
grub_config |])
> +          ignore (g#command [| grub2_mkconfig_cmd; "-o";
grub_config |])
>          with
>            G.Error msg ->
>              warning (f_"could not rebuild grub2 configuration file
(%s).  This may mean that grub output will not be sent to the serial port, but
otherwise should be harmless.  Original error message: %s")
> @@ -1431,7 +1448,7 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps >
>        (* If it's grub2, we have to regenerate the config files. *)
>        if grub = `Grub2 then
> -        ignore (g#command [| "grub2-mkconfig"; "-o";
grub_config |]);
> +        ignore (g#command [| grub2_mkconfig_cmd; "-o";
grub_config |]);
>  
>        Linux.augeas_reload g
>      );
> -- 
> 2.9.2
> 
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Tomáš Golembiovský <tgolembi@redhat.com>

On Thursday, 11 August 2016 00:35:55 CEST Tomáš Golembiovský
wrote:
> On Debian family of OSes Grub2 tools are prefixed with 'grub-', not
with
> 'grub2-'. We have to detect the correct name of the tool to use it.
> 
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> ---
>  v2v/convert_linux.ml | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
> index 103728b..1f5f12c 100644
> --- a/v2v/convert_linux.ml
> +++ b/v2v/convert_linux.ml
> @@ -109,6 +109,23 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps >        Not_found ->
>          error (f_"no grub1/grub-legacy or grub2 configuration file
was found") in
>  
> +  let grub2_mkconfig_cmd > +    if grub = `Grub2 then
> +      try
> +        (* Red Hat and Suse families *)
> +        ignore (g#command [| "grub2-mkconfig";
"--version" |]);
> +        "grub2-mkconfig"
> +      with G.Error _ ->
> +        try
> +          (* Debian family *)
> +          ignore (g#command [| "grub-mkconfig";
"--version" |]);
> +          "grub-mkconfig"
> +        with G.Error _ ->
> +          error (f_"failed to find grub2-mkconfig binary (but Grub2
was detected on guest)")
> +    else
> +      ""
> +  in
Maybe it would be worth to put the elements in a list, and iterate
using List.find:

  let elems = [ "grub2-mkconfig"; "grub-mkconfig" ] in
  let elem     try
      List.find (
        let e ->
          try ignore (g#command [| e; "--version" |]); true
          with G.Error _ -> false
      ) elems
    with Not_found ->
      error "not found" in

Also, what I've seen usually done is checking for the existance of the
executable, eg: if g#file_exists "/usr/sbin/grub2-mkconfig" then ...
this way, even if requiring to specify the exact locations of the tools,
would avoid ignoring a tool just because it cannot run for some reason,
while we need to fail because of that.

-- 
Pino Toscano

On Thu, 11 Aug 2016 11:51:20 +0200
Pino Toscano <ptoscano@redhat.com> wrote:
> On Thursday, 11 August 2016 00:35:55 CEST Tomáš Golembiovský wrote:
> > On Debian family of OSes Grub2 tools are prefixed with
'grub-', not with
> > 'grub2-'. We have to detect the correct name of the tool to
use it.
> > 
> > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> > ---
> >  v2v/convert_linux.ml | 21 +++++++++++++++++++--
> >  1 file changed, 19 insertions(+), 2 deletions(-)
> > 
> > diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
> > index 103728b..1f5f12c 100644
> > --- a/v2v/convert_linux.ml
> > +++ b/v2v/convert_linux.ml
> > @@ -109,6 +109,23 @@ let rec convert ~keep_serial_console (g :
G.guestfs) inspect source rcaps > >        Not_found ->
> >          error (f_"no grub1/grub-legacy or grub2 configuration
file was found") in
> >  
> > +  let grub2_mkconfig_cmd > > +    if grub = `Grub2 then
> > +      try
> > +        (* Red Hat and Suse families *)
> > +        ignore (g#command [| "grub2-mkconfig";
"--version" |]);
> > +        "grub2-mkconfig"
> > +      with G.Error _ ->
> > +        try
> > +          (* Debian family *)
> > +          ignore (g#command [| "grub-mkconfig";
"--version" |]);
> > +          "grub-mkconfig"
> > +        with G.Error _ ->
> > +          error (f_"failed to find grub2-mkconfig binary (but
Grub2 was detected on guest)")
> > +    else
> > +      ""
> > +  in  
> 
> Maybe it would be worth to put the elements in a list, and iterate
> using List.find:
> 
>   let elems = [ "grub2-mkconfig"; "grub-mkconfig" ] in
>   let elem >     try
>       List.find (
>         let e ->
>           try ignore (g#command [| e; "--version" |]); true
>           with G.Error _ -> false
>       ) elems
>     with Not_found ->
>       error "not found" in
Yes, we can do it that way.
> 
> Also, what I've seen usually done is checking for the existance of the
> executable, eg: if g#file_exists "/usr/sbin/grub2-mkconfig" then
...
I tried to avoid that, because I wasn't sure if the tool is always in
/usr/sbin (vs. e.g. in /sbin) on all supported guests. If you can
confirm it is always in /usr/sbin we can use g#file_exists. Also, even
if we cannot confirm that, we can enumerate multiple paths if we use
your method with list above.
> this way, even if requiring to specify the exact locations of the tools,
> would avoid ignoring a tool just because it cannot run for some reason,
> while we need to fail because of that.
I'm not sure I understand. I doubt you can find multiple versions of the
tool on single host. That means if the tool is not working the check
will fail, because it won't find any usable tool.


-- 
Tomáš Golembiovský <tgolembi@redhat.com>