Libguestfs - Oct 2016 - Re: [PATCH] v2v: bootloaders: search grub config for all distributions

On Thursday, 27 October 2016 20:22:30 CET Pavel Butsykin
wrote:
> This patch improves the search of grub config on EFI partition. This means
that
> the config will be found not only for rhel but also for many other
distributions.
> Tests were performed on the following distributions: centos, fedora,
ubuntu,
> suse. In all cases, the config path was /boot/efi/EFI/*distname*/grub.cfg
> 
> The main purpose of the patch is to improve support for converting of vm
with
> UEFI for most distributions. Unfortunately this patch does not solve the
problem
> for all distributions, for example Debian does not store grub config on the
EFI
> partition, therefore for such distributions another solution is necessary.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> ---
>  v2v/linux_bootloaders.ml | 56
+++++++++++++++++++++++++++++-------------------
>  1 file changed, 34 insertions(+), 22 deletions(-)
> 
> diff --git a/v2v/linux_bootloaders.ml b/v2v/linux_bootloaders.ml
> index e03d22b..210c273 100644
> --- a/v2v/linux_bootloaders.ml
> +++ b/v2v/linux_bootloaders.ml
> @@ -335,32 +335,44 @@ end
>  
>  let detect_bootloader (g : G.guestfs) inspect >    let config_file, typ
> -    let locations = [
> -      "/boot/grub2/grub.cfg", Grub2;
> -      "/boot/grub/grub.cfg", Grub2;
> -      "/boot/grub/menu.lst", Grub1;
> -      "/boot/grub/grub.conf", Grub1;
> +    let grub_configs = [
> +      "grub.cfg", Grub2;
> +      "grub.conf", Grub1;
> +      "menu.lst", Grub1;
>      ] in
> -    let locations > +
> +    let boot_location >        match inspect.i_firmware with
> -      | I_UEFI _ ->
> -        [
> -          "/boot/efi/EFI/redhat/grub.cfg", Grub2;
> -          "/boot/efi/EFI/redhat/grub.conf", Grub1;
> -        ] @ locations
> -      | I_BIOS -> locations in
> -    try
> -      List.find (
> -        fun (config_file, _) -> g#is_file ~followsymlinks:true
config_file
> -      ) locations
> -    with
> -      Not_found ->
> -        error (f_"no bootloader detected") in
> +      | I_BIOS -> "/boot/"
> +      | I_UEFI _ -> "/boot/efi/EFI/"
> +    in
> +
> +    let rec find_grub dirs configs > +      let rec config_check dir
configs > +        match configs with
> +        | [] -> None
> +        | (config, typ) :: configs ->
> +          let cfg_path = boot_location ^ dir ^ "/" ^ config in
> +          if g#is_file ~followsymlinks:true cfg_path then (
> +            Some (cfg_path, typ)
> +          ) else config_check dir configs;
> +      in
> +      match dirs with
> +      | [] -> error (f_"no bootloader detected")
> +      | dir :: dirs ->
> +        let res = config_check dir configs in
> +        match res with
> +        | None -> find_grub dirs configs
> +        | Some (cfg_path, typ) -> cfg_path, typ
> +    in
> +
> +  find_grub (Array.to_list (g#ls boot_location)) grub_configs in
It sounds like this could be simplified by using g#find +
Common_utils.last_part_of (to extract the basename of each path, never
use Filename.basename for paths in the appliance!) + comparison with
the elements in the grub_configs array of this patch.
>    match typ with
>    | Grub1 ->
> -    if config_file = "/boot/efi/EFI/redhat/grub.conf" then
> -      g#aug_transform "grub"
"/boot/efi/EFI/redhat/grub.conf";
> -
> +    (match inspect.i_firmware with
> +    | I_BIOS -> ()
> +    | I_UEFI _ -> g#aug_transform "grub" config_file
> +    );
>      new bootloader_grub1 g inspect config_file
This part looks fine.

Thanks,
-- 
Pino Toscano