[Libguestfs] [PATCH] v2v: bootloaders: search grub config for all distributions

[Pavel Butsykin]

On 31.10.2016 13:00, Pino Toscano wrote:
> 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 at 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.

Maybe I don't understand something? But it will be almost the same,
except that instead of g#is_file, we will use last_part_of + string
comparison. But here in addition, I would not like to iterate
subdirectories.

If g#find had parameters to specify the pattern and level of nesting,
then it would be possible to simplify the patch, but since it doesn't
have it.

>>     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,
>
>
>
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
>