[PATCH] v2v: bootloaders: search grub config for all distributions
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 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 | Grub2 -> new bootloader_grub2 g config_file -- 2.8.3
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
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@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@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
On Thu, Oct 27, 2016 at 08:22:30PM +0300, Pavel Butsykin wrote:
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 | Grub2 -> new bootloader_grub2 g config_file
I don't think this part of the patch is correct. We need to use the Augeas transform only for a particular path which is missing from the Augeas lens. What do you think of the attached version instead? It's smaller and more efficient (only makes 2 libguestfs API calls in the normal case)? I'm still running tests on it. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
On 01.11.2016 13:11, Richard W.M. Jones wrote:
On Thu, Oct 27, 2016 at 08:22:30PM +0300, Pavel Butsykin wrote:
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 | Grub2 -> new bootloader_grub2 g config_file
I don't think this part of the patch is correct. We need to use the Augeas transform only for a particular path which is missing from the Augeas lens.
It is necessary for /EFI/redhat/, but no need for others?
What do you think of the attached version instead? It's smaller and more efficient (only makes 2 libguestfs API calls in the normal case)?
Yeah, I agree. And in this version the code is easier to read :)
I'm still running tests on it.l
Rich.
On Tue, Nov 01, 2016 at 03:10:51PM +0300, Pavel Butsykin wrote:
On 01.11.2016 13:11, Richard W.M. Jones wrote:
On Thu, Oct 27, 2016 at 08:22:30PM +0300, Pavel Butsykin wrote:
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 | Grub2 -> new bootloader_grub2 g config_file
I don't think this part of the patch is correct. We need to use the Augeas transform only for a particular path which is missing from the Augeas lens.
It is necessary for /EFI/redhat/, but no need for others?
I believe so, because the Augeas grub lens has: (* View: filter *) let filter = incl "/boot/grub/grub.conf" . incl "/boot/grub/menu.lst" . incl "/etc/grub.conf" so it wouldn't normally be activated for a grub.conf found under any other path.
What do you think of the attached version instead? It's smaller and more efficient (only makes 2 libguestfs API calls in the normal case)?
Yeah, I agree. And in this version the code is easier to read :)
The slow tests passed, so I'll push this version shortly. Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
On 01.11.2016 17:06, Richard W.M. Jones wrote:
On Tue, Nov 01, 2016 at 03:10:51PM +0300, Pavel Butsykin wrote:
On 01.11.2016 13:11, Richard W.M. Jones wrote:
On Thu, Oct 27, 2016 at 08:22:30PM +0300, Pavel Butsykin wrote:
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 | Grub2 -> new bootloader_grub2 g config_file
I don't think this part of the patch is correct. We need to use the Augeas transform only for a particular path which is missing from the Augeas lens.
It is necessary for /EFI/redhat/, but no need for others?
I believe so, because the Augeas grub lens has:
(* View: filter *) let filter = incl "/boot/grub/grub.conf" . incl "/boot/grub/menu.lst" . incl "/etc/grub.conf"
so it wouldn't normally be activated for a grub.conf found under any other path.
We can have the following paths: /boot/efi/EFI/centos/grub.conf /boot/efi/EFI/fedora/grub.conf /boot/efi/EFI/*/grub.conf which also need to activate. The Augeas grub lens has filters which are used only for the BIOS. So (if I understand correctly) we have to use the Augeas transform for other EFI paths too.
What do you think of the attached version instead? It's smaller and more efficient (only makes 2 libguestfs API calls in the normal case)?
Yeah, I agree. And in this version the code is easier to read :)
The slow tests passed, so I'll push this version shortly.
Thanks,
Rich.
participants (3)
-
Pavel Butsykin -
Pino Toscano -
Richard W.M. Jones