7:13 a.m.
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(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs