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

Pavel Butsykin pbutsykin at virtuozzo.com
Fri Apr 28 16:57:56 UTC 2017


On 28.04.2017 17:45, Pino Toscano wrote:
> On Wednesday, 19 April 2017 15:58:57 CEST 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>
>> Signed-off-by: Richard W.M. Jones <rjones at redhat.com>
>> ---
>>   v2v/linux_bootloaders.ml | 86 +++++++++++++++++++++++++++++++-----------------
>>   1 file changed, 55 insertions(+), 31 deletions(-)
>>
>> diff --git a/v2v/linux_bootloaders.ml b/v2v/linux_bootloaders.ml
>> index cad72a829..593797f4e 100644
>> --- a/v2v/linux_bootloaders.ml
>> +++ b/v2v/linux_bootloaders.ml
>> @@ -42,6 +42,10 @@ type bootloader_type =
>>     | Grub1
>>     | Grub2
>>
>> +let string_of_bootloader_type = function
>> +  | Grub1 -> "Grub1"
>> +  | Grub2 -> "Grub2"
>> +
>>   (* Helper function for SUSE: remove (hdX,X) prefix from a path. *)
>>   let remove_hd_prefix path =
>>     let rex = Str.regexp "^(hd.*)\\(.*\\)" in
>> @@ -49,6 +53,13 @@ let remove_hd_prefix path =
>>
>>   (* Grub1 (AKA grub-legacy) representation. *)
>>   class bootloader_grub1 (g : G.guestfs) inspect grub_config =
>> +    let () =
>
> Wrong indentation here.
>
>> +    (* Apply the "grub" lens if it is not handling the file
>> +     * already -- Augeas < 1.7.0 will error out otherwise.
>> +     *)
>> +    if g#aug_ls ("/files" ^ grub_config) = [||] then
>> +      g#aug_transform "grub" grub_config in
>> +
>>     (* Grub prefix?  Usually "/boot". *)
>>     let grub_prefix =
>>       let mounts = g#inspect_get_mountpoints inspect.i_root in
>> @@ -191,7 +202,7 @@ type default_kernel_method =
>>     | MethodNone  (** No known way. *)
>>
>>   (* Grub2 representation. *)
>> -class bootloader_grub2 (g : G.guestfs) grub_config =
>> +class bootloader_grub2 (g : G.guestfs) inspect grub_config =
>
> NACK, see below.
>
>>
>>     let grub2_mkconfig_cmd =
>>       let elems = [
>> @@ -335,33 +346,46 @@ object (self)
>>   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;
>> -    ] in
>> -    let locations =
>> -      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
>> -
>> -  match typ with
>> -  | Grub1 ->
>> -    if config_file = "/boot/efi/EFI/redhat/grub.conf" then
>> -      g#aug_transform "grub" "/boot/efi/EFI/redhat/grub.conf";
>> -
>> -    new bootloader_grub1 g inspect config_file
>> -  | Grub2 -> new bootloader_grub2 g config_file
>> +  (* Where to start searching for bootloaders. *)
>> +  let mp =
>> +    match inspect.i_firmware with
>> +    | I_BIOS -> "/boot"
>> +    | I_UEFI _ -> "/boot/efi/EFI" in
>> +
>> +  (* Find all paths below the mountpoint, then filter them to find
>> +   * the grub config file.
>> +   *)
>> +  let paths =
>> +    try List.map ((^) mp) (Array.to_list (g#find mp))
>> +    with G.Error msg ->
>> +      error (f_"could not find bootloader mount point (%s): %s") mp msg in
>> +
>> +  (* We can determine if the bootloader config file is grub 1 or
>> +   * grub 2 just by looking at the filename.
>> +   *)
>> +  let bootloader_type_of_filename path =
>> +    match last_part_of path '/' with
>> +    | Some "grub.cfg" -> Some Grub2
>> +    | Some ("grub.conf" | "menu.lst") -> Some Grub1
>> +    | Some _
>> +    | None -> None
>> +  in
>> +
>> +  let grub_config, typ =
>> +    let rec loop = function
>> +      | [] -> error (f_"no bootloader detected")
>> +      | path :: paths ->
>> +         match bootloader_type_of_filename path with
>> +         | None -> loop paths
>> +         | Some typ ->
>> +            if not (g#is_file ~followsymlinks:true path) then loop paths
>> +            else path, typ
>> +    in
>> +    loop paths in
>> +
>> +  debug "detected bootloader %s at %s"
>> +        (string_of_bootloader_type typ) grub_config;
>
> You don't need string_of_bootloader_type, there's already the #name
> method of the bootloader object.  The only change would be moving this
> message after the creation of the bootloader subclass.
>
> Another option is adding a new virtual method to the bootloader object,
> something like output#as_options, e.g.
>    method debug = "grub1(" ^ grub_config ^ ")"
> so you can change the code to be:
>
>    let bl =
>      match typ with
>      | Grub1 -> new bootloader_grub1 g inspect config_file
>      | Grub2 -> new bootloader_grub2 g config_file in
>    debug "detected bootloader: %s" bl#debug;
>
>> +  (match typ with
>> +   | Grub1 -> new bootloader_grub1
>> +   | Grub2 -> new bootloader_grub2) g inspect grub_config
>
> Just leave the current way, which is more readable.

This is not so important point for me, I'll make the following change
in the next patch:

   let bl =
     (match typ with
      | Grub1 -> new bootloader_grub1 g inspect grub_config
      | Grub2 -> new bootloader_grub2 g grub_config) in
   debug "detected bootloader %s at %s" bl#name grub_config;
   bl

But these changes did Rich, I can submit the next patch if all
satisfied with the above suggested option.

Rich?

> Thanks,
>




More information about the Libguestfs mailing list