[Libguestfs] [PATCH v6 1/1] v2v: bootloaders: search grub config for all distributions
Richard W.M. Jones
rjones at redhat.com
Fri Apr 28 18:12:07 UTC 2017
On Fri, Apr 28, 2017 at 07:57:56PM +0300, Pavel Butsykin wrote:
> 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?
Sure, whatever Pino thinks is fine.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
More information about the Libguestfs
mailing list