Pino Toscano
2016-Aug-15 14:48 UTC
[Libguestfs] [PATCH v2] v2v: factor out bootloader handling
Create an object hierarchy to represent different bootloaders for Linux
guests, moving the separate handling of grub1 and grub2 in different
classes: this isolates the code for each type of bootloader together,
instead of scattering it all around.
This is mostly code refactoring, with no actual behaviour change.
---
po/POTFILES-ml | 1 +
v2v/Makefile.am | 2 +
v2v/bootloaders.ml | 317 +++++++++++++++++++++++++++++++++++++++++++++++++++
v2v/bootloaders.mli | 44 +++++++
v2v/convert_linux.ml | 284 ++-------------------------------------------
5 files changed, 376 insertions(+), 272 deletions(-)
create mode 100644 v2v/bootloaders.ml
create mode 100644 v2v/bootloaders.mli
diff --git a/po/POTFILES-ml b/po/POTFILES-ml
index b7b39fa..35325d9 100644
--- a/po/POTFILES-ml
+++ b/po/POTFILES-ml
@@ -105,6 +105,7 @@ sysprep/sysprep_operation_utmp.ml
sysprep/sysprep_operation_yum_uuid.ml
v2v/DOM.ml
v2v/OVF.ml
+v2v/bootloaders.ml
v2v/changeuid.ml
v2v/cmdline.ml
v2v/convert_linux.ml
diff --git a/v2v/Makefile.am b/v2v/Makefile.am
index 008c5ba..9a997e9 100644
--- a/v2v/Makefile.am
+++ b/v2v/Makefile.am
@@ -27,6 +27,7 @@ EXTRA_DIST = \
CLEANFILES = *~ *.annot *.cmi *.cmo *.cmx *.cmxa *.o virt-v2v
SOURCES_MLI = \
+ bootloaders.mli \
changeuid.mli \
cmdline.mli \
convert_linux.mli \
@@ -79,6 +80,7 @@ SOURCES_ML = \
input_libvirt_xen_ssh.ml \
input_libvirt.ml \
input_ova.ml \
+ bootloaders.ml \
convert_linux.ml \
convert_windows.ml \
output_null.ml \
diff --git a/v2v/bootloaders.ml b/v2v/bootloaders.ml
new file mode 100644
index 0000000..689d7b4
--- /dev/null
+++ b/v2v/bootloaders.ml
@@ -0,0 +1,317 @@
+(* virt-v2v
+ * Copyright (C) 2009-2016 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *)
+
+open Printf
+
+open Common_gettext.Gettext
+open Common_utils
+
+open Types
+open Utils
+
+module G = Guestfs
+
+class virtual bootloader = object
+ method virtual name : string
+ method virtual augeas_device_patterns : string list
+ method virtual list_kernels : unit -> string list
+ method virtual set_default_kernel : string -> unit
+ method set_augeas_configuration () = false
+ method virtual configure_console : unit -> unit
+ method virtual remove_console : unit -> unit
+ method update () = ()
+end
+
+(* Helper type used in detect_bootloader. *)
+type bootloader_type + | Grub1
+ | Grub2
+
+(* Helper function for SUSE: remove (hdX,X) prefix from a path. *)
+let remove_hd_prefix path + let rex = Str.regexp "^(hd.*)\\(.*\\)"
in
+ Str.replace_first rex "\\1" path
+
+(* Grub1 (AKA grub-legacy) representation. *)
+class bootloader_grub1 (g : G.guestfs) inspect grub_config + (* Grub prefix?
Usually "/boot". *)
+ let grub_prefix + let mounts = g#inspect_get_mountpoints inspect.i_root in
+ try
+ List.find (
+ fun path -> List.mem_assoc path mounts
+ ) [ "/boot/grub"; "/boot" ]
+ with Not_found -> "" in
+object
+ inherit bootloader
+
+ method name = "grub1"
+
+ method augeas_device_patterns = [
+ "/files" ^ grub_config ^ "/*/kernel/root";
+ "/files" ^ grub_config ^ "/*/kernel/resume";
+ "/files/boot/grub/device.map/*[label() !=
\"#comment\"]";
+ "/files/etc/sysconfig/grub/boot";
+ ]
+
+ method list_kernels () + let paths + let expr = sprintf
"/files%s/title/kernel" grub_config in
+ let paths = g#aug_match expr in
+ let paths = Array.to_list paths in
+
+ (* Remove duplicates. *)
+ let paths = remove_duplicates paths in
+
+ (* Get the default kernel from grub if it's set. *)
+ let default + let expr = sprintf "/files%s/default"
grub_config in
+ try
+ let idx = g#aug_get expr in
+ let idx = int_of_string idx in
+ (* Grub indices are zero-based, augeas is 1-based. *)
+ let expr + sprintf "/files%s/title[%d]/kernel"
grub_config (idx+1) in
+ Some expr
+ with G.Error msg
+ when String.find msg "aug_get: no matching node" >= 0
->
+ None in
+
+ (* If a default kernel was set, put it at the beginning of the paths
+ * list. If not set, assume the first kernel always boots (?)
+ *)
+ match default with
+ | None -> paths
+ | Some p -> p :: List.filter ((<>) p) paths in
+
+ (* Resolve the Augeas paths to kernel filenames. *)
+ let vmlinuzes = List.map g#aug_get paths in
+
+ (* Make sure kernel does not begin with (hdX,X). *)
+ let vmlinuzes = List.map remove_hd_prefix vmlinuzes in
+
+ (* Prepend grub filesystem. *)
+ List.map ((^) grub_prefix) vmlinuzes
+
+ method set_default_kernel vmlinuz + if not (String.is_prefix vmlinuz
grub_prefix) then
+ error (f_"kernel %s is not under grub tree %s")
+ vmlinuz grub_prefix;
+ let kernel_under_grub_prefix + let prefix_len = String.length
grub_prefix in
+ let kernel_len = String.length vmlinuz in
+ String.sub vmlinuz prefix_len (kernel_len - prefix_len) in
+
+ (* Find the grub entry for the given kernel. *)
+ let paths = g#aug_match (sprintf "/files%s/title/kernel[. =
'%s']"
+ grub_config kernel_under_grub_prefix) in
+ let paths = Array.to_list paths in
+ if paths = [] then
+ error (f_"didn't find grub entry for kernel %s") vmlinuz;
+ let path = List.hd paths in
+ let rex = Str.regexp ".*/title\\[\\([1-9][0-9]*\\)\\]/kernel" in
+ if not (Str.string_match rex path 0) then
+ error (f_"internal error: regular expression did not match
'%s'")
+ path;
+ let index = int_of_string (Str.matched_group 1 path) - 1 in
+ g#aug_set (sprintf "/files%s/default" grub_config) (string_of_int
index);
+ g#aug_save ()
+
+ method set_augeas_configuration () + let incls = g#aug_match
"/augeas/load/Grub/incl" in
+ let incls = Array.to_list incls in
+ let incls_contains_conf + List.exists (fun incl -> g#aug_get incl =
grub_config) incls in
+ if not incls_contains_conf then (
+ g#aug_set "/augeas/load/Grub/incl[last()+1]" grub_config;
+ true;
+ ) else false
+
+ method configure_console () + let rex = Str.regexp
"\\(.*\\)\\b\\([xh]vc0\\)\\b\\(.*\\)" in
+ let expr = sprintf "/files%s/title/kernel/console" grub_config in
+
+ let paths = g#aug_match expr in
+ let paths = Array.to_list paths in
+ List.iter (
+ fun path ->
+ let console = g#aug_get path in
+ if Str.string_match rex console 0 then (
+ let console = Str.global_replace rex "\\1ttyS0\\3" console
in
+ g#aug_set path console
+ )
+ ) paths;
+
+ g#aug_save ()
+
+ method remove_console () + let rex = Str.regexp
"\\(.*\\)\\b\\([xh]vc0\\)\\b\\(.*\\)" in
+ let expr = sprintf "/files%s/title/kernel/console" grub_config in
+
+ let rec loop = function
+ | [] -> ()
+ | path :: paths ->
+ let console = g#aug_get path in
+ if Str.string_match rex console 0 then (
+ ignore (g#aug_rm path);
+ (* All the paths are invalid, restart the loop. *)
+ let paths = g#aug_match expr in
+ let paths = Array.to_list paths in
+ loop paths
+ )
+ else
+ loop paths
+ in
+ let paths = g#aug_match expr in
+ let paths = Array.to_list paths in
+ loop paths;
+
+ g#aug_save ()
+end
+
+(* Grub2 representation. *)
+class bootloader_grub2 (g : G.guestfs) grub_config + let grub2_update_console
~remove + let rex = Str.regexp
"\\(.*\\)\\bconsole=[xh]vc0\\b\\(.*\\)" in
+
+ let paths = [
+ "/files/etc/sysconfig/grub/GRUB_CMDLINE_LINUX";
+ "/files/etc/default/grub/GRUB_CMDLINE_LINUX";
+ "/files/etc/default/grub/GRUB_CMDLINE_LINUX_DEFAULT"
+ ] in
+ let paths = List.map g#aug_match paths in
+ let paths = List.map Array.to_list paths in
+ let paths = List.flatten paths in
+ match paths with
+ | [] ->
+ if not remove then
+ warning (f_"could not add grub2 serial console (ignored)")
+ else
+ warning (f_"could not remove grub2 serial console (ignored)")
+ | path :: _ ->
+ let grub_cmdline = g#aug_get path in
+ if Str.string_match rex grub_cmdline 0 then (
+ let new_grub_cmdline + if not remove then
+ Str.global_replace rex "\\1console=ttyS0\\2" grub_cmdline
+ else
+ Str.global_replace rex "\\1\\2" grub_cmdline in
+ g#aug_set path new_grub_cmdline;
+ g#aug_save ();
+
+ try
+ ignore (g#command [| "grub2-mkconfig"; "-o";
grub_config |])
+ with
+ G.Error msg ->
+ warning (f_"could not rebuild grub2 configuration file (%s).
This may mean that grub output will not be sent to the serial port, but
otherwise should be harmless. Original error message: %s")
+ grub_config msg
+ ) in
+object
+ inherit bootloader
+
+ method name = "grub2"
+
+ method augeas_device_patterns = [
+ "/files/etc/sysconfig/grub/GRUB_CMDLINE_LINUX";
+ "/files/etc/default/grub/GRUB_CMDLINE_LINUX";
+ "/files/etc/default/grub/GRUB_CMDLINE_LINUX_DEFAULT";
+ "/files/boot/grub2/device.map/*[label() !=
\"#comment\"]";
+ ]
+
+ method list_kernels () + let get_default_image () + let cmd +
if g#exists "/sbin/grubby" then
+ [| "grubby"; "--default-kernel" |]
+ else
+ [| "/usr/bin/perl"; "-MBootloader::Tools";
"-e"; "
+ InitLibrary();
+ my $default = Bootloader::Tools::GetDefaultSection();
+ print $default->{image};
+ " |] in
+ match g#command cmd with
+ | "" -> None
+ | k ->
+ let len = String.length k in
+ let k + if len > 0 && k.[len-1] = '\n' then
+ String.sub k 0 (len-1)
+ else k in
+ Some (remove_hd_prefix k)
+ in
+
+ let vmlinuzes + (match get_default_image () with
+ | None -> []
+ | Some k -> [k]) @
+ (* This is how the grub2 config generator enumerates kernels. *)
+ Array.to_list (g#glob_expand "/boot/kernel-*") @
+ Array.to_list (g#glob_expand "/boot/vmlinuz-*") @
+ Array.to_list (g#glob_expand "/vmlinuz-*") in
+ let rex = Str.regexp ".*\\.\\(dpkg-.*|rpmsave|rpmnew\\)$" in
+ let vmlinuzes = List.filter (
+ fun file -> not (Str.string_match rex file 0)
+ ) vmlinuzes in
+ vmlinuzes
+
+ method set_default_kernel vmlinuz + let cmd + if g#exists
"/sbin/grubby" then
+ [| "grubby"; "--set-default"; vmlinuz |]
+ else
+ [| "/usr/bin/perl"; "-MBootloader::Tools";
"-e"; sprintf "
+ InitLibrary();
+ my @sections = GetSectionList(type=>image,
image=>\"%s\");
+ my $section = GetSection(@sections);
+ my $newdefault = $section->{name};
+ SetGlobals(default, \"$newdefault\");
+ " vmlinuz |] in
+ ignore (g#command cmd)
+
+ method configure_console () + grub2_update_console ~remove:false
+
+ method remove_console () + grub2_update_console ~remove:true
+
+ method update () + ignore (g#command [| "grub2-mkconfig";
"-o"; grub_config |])
+end
+
+let detect_bootloader (g : G.guestfs) inspect + let config_file, typ + let
locations = [
+ "/boot/grub2/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) ::
locations
+ | I_BIOS -> locations in
+ try
+ List.find (
+ fun (grub_config, _) -> g#is_file ~followsymlinks:true grub_config
+ ) locations
+ with
+ Not_found ->
+ error (f_"no bootloader detected") in
+
+ match typ with
+ | Grub1 -> new bootloader_grub1 g inspect config_file
+ | Grub2 -> new bootloader_grub2 g config_file
diff --git a/v2v/bootloaders.mli b/v2v/bootloaders.mli
new file mode 100644
index 0000000..477312b
--- /dev/null
+++ b/v2v/bootloaders.mli
@@ -0,0 +1,44 @@
+(* virt-v2v
+ * Copyright (C) 2009-2016 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *)
+
+class virtual bootloader : object
+ method virtual name : string
+ (** The name of the bootloader. *)
+ method virtual augeas_device_patterns : string list
+ (** A list of augeas patterns to search for device names. *)
+ method virtual list_kernels : unit -> string list
+ (** Lists all the kernels configured in the bootloader. *)
+ method virtual set_default_kernel : string -> unit
+ (** Sets the specified vmlinuz path as default bootloader entry. *)
+ method set_augeas_configuration : unit -> bool
+ (** Checks whether augeas is reading the configuration file
+ of the bootloader, and if not then add it.
+
+ Returns whether augeas needs to be reloaded. *)
+ method virtual configure_console : unit -> unit
+ (** Sets up the console for the available kernels. *)
+ method virtual remove_console : unit -> unit
+ (** Removes the console in all the available kernels. *)
+ method update : unit -> unit
+ (** Update the bootloader. *)
+end
+(** Encapsulates a UNIX boot loader as object. *)
+
+val detect_bootloader : Guestfs.guestfs -> Types.inspect -> bootloader
+(** Detects the bootloader on the guest, and creates the object
+ representing it. *)
diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
index f9aa334..77d9d8e 100644
--- a/v2v/convert_linux.ml
+++ b/v2v/convert_linux.ml
@@ -89,36 +89,8 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect
source rcaps let dbfiles = Array.to_list dbfiles in
List.iter g#rm_f dbfiles;
- (* What grub is installed? *)
- let grub_config, grub - let locations = [
- "/boot/grub2/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) ::
locations
- | I_BIOS -> locations in
- try
- List.find (
- fun (grub_config, _) -> g#is_file ~followsymlinks:true grub_config
- ) locations
- with
- Not_found ->
- error (f_"no grub1/grub-legacy or grub2 configuration file was
found") in
-
- (* Grub prefix? Usually "/boot". *)
- let grub_prefix - match grub with
- | `Grub2 -> ""
- | `Grub1 ->
- let mounts = g#inspect_get_mountpoints inspect.i_root in
- try
- List.find (
- fun path -> List.mem_assoc path mounts
- ) [ "/boot/grub"; "/boot" ]
- with Not_found -> "" in
+ (* Detect the installed bootloader. *)
+ let bootloader = Bootloaders.detect_bootloader g inspect in
(* What kernel/kernel-like packages are installed on the current guest? *)
let installed_kernels : kernel_info list @@ -269,88 +241,7 @@ let rec convert
~keep_serial_console (g : G.guestfs) inspect source rcaps * list is the
default booting kernel.
*)
let grub_kernels : kernel_info list - (* Helper function for SUSE: remove
(hdX,X) prefix from a path. *)
- let remove_hd_prefix - let rex = Str.regexp
"^(hd.*)\\(.*\\)" in
- Str.replace_first rex "\\1"
- in
-
- let vmlinuzes - match grub with
- | `Grub1 ->
- let paths - let expr = sprintf
"/files%s/title/kernel" grub_config in
- let paths = g#aug_match expr in
- let paths = Array.to_list paths in
-
- (* Remove duplicates. *)
- let paths = remove_duplicates paths in
-
- (* Get the default kernel from grub if it's set. *)
- let default - let expr = sprintf
"/files%s/default" grub_config in
- try
- let idx = g#aug_get expr in
- let idx = int_of_string idx in
- (* Grub indices are zero-based, augeas is 1-based. *)
- let expr - sprintf
"/files%s/title[%d]/kernel" grub_config (idx+1) in
- Some expr
- with G.Error msg
- when String.find msg "aug_get: no matching node"
>= 0 ->
- None in
-
- (* If a default kernel was set, put it at the beginning of the paths
- * list. If not set, assume the first kernel always boots (?)
- *)
- match default with
- | None -> paths
- | Some p -> p :: List.filter ((<>) p) paths in
-
- (* Resolve the Augeas paths to kernel filenames. *)
- let vmlinuzes = List.map g#aug_get paths in
-
- (* Make sure kernel does not begin with (hdX,X). *)
- let vmlinuzes = List.map remove_hd_prefix vmlinuzes in
-
- (* Prepend grub filesystem. *)
- List.map ((^) grub_prefix) vmlinuzes
-
- | `Grub2 ->
- let get_default_image () - let cmd - if g#exists
"/sbin/grubby" then
- [| "grubby"; "--default-kernel" |]
- else
- [| "/usr/bin/perl"; "-MBootloader::Tools";
"-e"; "
- InitLibrary();
- my $default = Bootloader::Tools::GetDefaultSection();
- print $default->{image};
- " |] in
- match g#command cmd with
- | "" -> None
- | k ->
- let len = String.length k in
- let k - if len > 0 && k.[len-1] =
'\n' then
- String.sub k 0 (len-1)
- else k in
- Some (remove_hd_prefix k)
- in
-
- let vmlinuzes - (match get_default_image () with
- | None -> []
- | Some k -> [k]) @
- (* This is how the grub2 config generator enumerates kernels. *)
- Array.to_list (g#glob_expand "/boot/kernel-*") @
- Array.to_list (g#glob_expand "/boot/vmlinuz-*") @
- Array.to_list (g#glob_expand "/vmlinuz-*") in
- let rex = Str.regexp ".*\\.\\(dpkg-.*|rpmsave|rpmnew\\)$" in
- let vmlinuzes = List.filter (
- fun file -> not (Str.string_match rex file 0)
- ) vmlinuzes in
- vmlinuzes in
+ let vmlinuzes = bootloader#list_kernels () in
(* Map these to installed kernels. *)
filter_map (
@@ -389,21 +280,8 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps (* Conversion step. *)
let rec augeas_grub_configuration () - match grub with
- | `Grub1 ->
- (* Ensure Augeas is reading the grub configuration file, and if not
- * then add it.
- *)
- let incls = g#aug_match "/augeas/load/Grub/incl" in
- let incls = Array.to_list incls in
- let incls_contains_conf - List.exists (fun incl -> g#aug_get
incl = grub_config) incls in
- if not incls_contains_conf then (
- g#aug_set "/augeas/load/Grub/incl[last()+1]" grub_config;
- Linux.augeas_reload g;
- )
-
- | `Grub2 -> () (* Not necessary for grub2. *)
+ if bootloader#set_augeas_configuration () then
+ Linux.augeas_reload g
and unconfigure_xen () (* Remove kmod-xenpv-* (RHEL 3). *)
@@ -733,7 +611,7 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect
source rcaps let kernels = List.rev kernels (* so best is first *) in
List.hd kernels in
if best_kernel <> List.hd grub_kernels then
- grub_set_bootable best_kernel;
+ bootloader#set_default_kernel best_kernel.ki_vmlinuz;
(* Does the best/bootable kernel support virtio? *)
let virtio = best_kernel.ki_supports_virtio in
@@ -751,46 +629,6 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps
best_kernel, virtio
- and grub_set_bootable kernel - match grub with
- | `Grub1 ->
- if not (String.is_prefix kernel.ki_vmlinuz grub_prefix) then
- error (f_"kernel %s is not under grub tree %s")
- kernel.ki_vmlinuz grub_prefix;
- let kernel_under_grub_prefix - let prefix_len = String.length
grub_prefix in
- let kernel_len = String.length kernel.ki_vmlinuz in
- String.sub kernel.ki_vmlinuz prefix_len (kernel_len - prefix_len) in
-
- (* Find the grub entry for the given kernel. *)
- let paths = g#aug_match (sprintf "/files%s/title/kernel[. =
'%s']"
- grub_config kernel_under_grub_prefix) in
- let paths = Array.to_list paths in
- if paths = [] then
- error (f_"didn't find grub entry for kernel %s")
kernel.ki_vmlinuz;
- let path = List.hd paths in
- let rex = Str.regexp ".*/title\\[\\([1-9][0-9]*\\)\\]/kernel"
in
- if not (Str.string_match rex path 0) then
- error (f_"internal error: regular expression did not match
'%s'")
- path;
- let index = int_of_string (Str.matched_group 1 path) - 1 in
- g#aug_set (sprintf "/files%s/default" grub_config)
(string_of_int index);
- g#aug_save ()
-
- | `Grub2 ->
- let cmd - if g#exists "/sbin/grubby" then
- [| "grubby"; "--set-default"; kernel.ki_vmlinuz
|]
- else
- [| "/usr/bin/perl"; "-MBootloader::Tools";
"-e"; sprintf "
- InitLibrary();
- my @sections = GetSectionList(type=>image,
image=>\"%s\");
- my $section = GetSection(@sections);
- my $newdefault = $section->{name};
- SetGlobals(default, \"$newdefault\");
- " kernel.ki_vmlinuz |] in
- ignore (g#command cmd)
-
(* Even though the kernel was already installed (this version of
* virt-v2v does not install new kernels), it could have an
* initrd that does not have support virtio. Therefore rebuild
@@ -956,28 +794,6 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps
g#aug_save ()
- and grub_configure_console () - match grub with
- | `Grub1 ->
- let rex = Str.regexp "\\(.*\\)\\b\\([xh]vc0\\)\\b\\(.*\\)" in
- let expr = sprintf "/files%s/title/kernel/console" grub_config
in
-
- let paths = g#aug_match expr in
- let paths = Array.to_list paths in
- List.iter (
- fun path ->
- let console = g#aug_get path in
- if Str.string_match rex console 0 then (
- let console = Str.global_replace rex "\\1ttyS0\\3"
console in
- g#aug_set path console
- )
- ) paths;
-
- g#aug_save ()
-
- | `Grub2 ->
- grub2_update_console ~remove:false
-
(* If the target doesn't support a serial console, we want to remove
* all references to it instead.
*)
@@ -1006,71 +822,6 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps
g#aug_save ()
- and grub_remove_console () - match grub with
- | `Grub1 ->
- let rex = Str.regexp "\\(.*\\)\\b\\([xh]vc0\\)\\b\\(.*\\)" in
- let expr = sprintf "/files%s/title/kernel/console" grub_config
in
-
- let rec loop = function
- | [] -> ()
- | path :: paths ->
- let console = g#aug_get path in
- if Str.string_match rex console 0 then (
- ignore (g#aug_rm path);
- (* All the paths are invalid, restart the loop. *)
- let paths = g#aug_match expr in
- let paths = Array.to_list paths in
- loop paths
- )
- else
- loop paths
- in
- let paths = g#aug_match expr in
- let paths = Array.to_list paths in
- loop paths;
-
- g#aug_save ()
-
- | `Grub2 ->
- grub2_update_console ~remove:true
-
- and grub2_update_console ~remove - let rex = Str.regexp
"\\(.*\\)\\bconsole=[xh]vc0\\b\\(.*\\)" in
-
- let paths = [
- "/files/etc/sysconfig/grub/GRUB_CMDLINE_LINUX";
- "/files/etc/default/grub/GRUB_CMDLINE_LINUX";
- "/files/etc/default/grub/GRUB_CMDLINE_LINUX_DEFAULT"
- ] in
- let paths = List.map g#aug_match paths in
- let paths = List.map Array.to_list paths in
- let paths = List.flatten paths in
- match paths with
- | [] ->
- if not remove then
- warning (f_"could not add grub2 serial console (ignored)")
- else
- warning (f_"could not remove grub2 serial console (ignored)")
- | path :: _ ->
- let grub_cmdline = g#aug_get path in
- if Str.string_match rex grub_cmdline 0 then (
- let new_grub_cmdline - if not remove then
- Str.global_replace rex "\\1console=ttyS0\\2" grub_cmdline
- else
- Str.global_replace rex "\\1\\2" grub_cmdline in
- g#aug_set path new_grub_cmdline;
- g#aug_save ();
-
- try
- ignore (g#command [| "grub2-mkconfig"; "-o";
grub_config |])
- with
- G.Error msg ->
- warning (f_"could not rebuild grub2 configuration file (%s).
This may mean that grub output will not be sent to the serial port, but
otherwise should be harmless. Original error message: %s")
- grub_config msg
- )
-
and supports_acpi () (* ACPI known to cause RHEL 3 to fail. *)
if family = `RHEL_family && inspect.i_major_version == 3 then
@@ -1296,19 +1047,9 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps let paths = [
(* /etc/fstab *)
"/files/etc/fstab/*/spec";
-
- (* grub-legacy config *)
- "/files" ^ grub_config ^ "/*/kernel/root";
- "/files" ^ grub_config ^ "/*/kernel/resume";
- "/files/boot/grub/device.map/*[label() !=
\"#comment\"]";
- "/files/etc/sysconfig/grub/boot";
-
- (* grub2 config *)
- "/files/etc/sysconfig/grub/GRUB_CMDLINE_LINUX";
- "/files/etc/default/grub/GRUB_CMDLINE_LINUX";
- "/files/etc/default/grub/GRUB_CMDLINE_LINUX_DEFAULT";
- "/files/boot/grub2/device.map/*[label() !=
\"#comment\"]";
] in
+ (* Bootloader config *)
+ let paths = paths @ bootloader#augeas_device_patterns in
(* Which of these paths actually exist? *)
let paths @@ -1386,9 +1127,8 @@ let rec convert ~keep_serial_console (g :
G.guestfs) inspect source rcaps if !changed then (
g#aug_save ();
- (* If it's grub2, we have to regenerate the config files. *)
- if grub = `Grub2 then
- ignore (g#command [| "grub2-mkconfig"; "-o";
grub_config |]);
+ (* Make sure the bootloader is up-to-date. *)
+ bootloader#update ();
Linux.augeas_reload g
);
@@ -1418,10 +1158,10 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps
if keep_serial_console then (
configure_console ();
- grub_configure_console ();
+ bootloader#configure_console ();
) else (
remove_console ();
- grub_remove_console ();
+ bootloader#remove_console ();
);
let acpi = supports_acpi () in
--
2.7.4
Richard W.M. Jones
2016-Aug-15 15:26 UTC
Re: [Libguestfs] [PATCH v2] v2v: factor out bootloader handling
On Mon, Aug 15, 2016 at 04:48:29PM +0200, Pino Toscano wrote:> Create an object hierarchy to represent different bootloaders for Linux > guests, moving the separate handling of grub1 and grub2 in different > classes: this isolates the code for each type of bootloader together, > instead of scattering it all around. > > This is mostly code refactoring, with no actual behaviour change. > --- > po/POTFILES-ml | 1 + > v2v/Makefile.am | 2 + > v2v/bootloaders.ml | 317 +++++++++++++++++++++++++++++++++++++++++++++++++++ > v2v/bootloaders.mli | 44 +++++++Is this really "bootloaders"? It's more like "linux_bootloaders", unless this might one day be extended to include NTLDR.> +class virtual bootloader = object > + method virtual name : string > + method virtual augeas_device_patterns : string list > + method virtual list_kernels : unit -> string list > + method virtual set_default_kernel : string -> unit > + method set_augeas_configuration () = false > + method virtual configure_console : unit -> unit > + method virtual remove_console : unit -> unit > + method update () = () > +endI think you'll also find that you don't need to use inheritance at all. You can just declare bootloader as a class type and declare two (unrelated) classes which implement the class type. They won't be able to inherit the two trivial non-virtual functions, but IMHO that's a feature. I still don't much see the point versus an implementation which used a type Bootloader.t declared as: type t = Grub1 of <any local data grub1 needs> | Grub2 of <any local data grub2 needs> (and not exposed through the mli file). As this is to some extent my personal preference, I'll let it slide. I checked all of the code and it appears to be correct and there is nothing omitted from the current implementation. Rich.> +(* Helper type used in detect_bootloader. *) > +type bootloader_type > + | Grub1 > + | Grub2 > + > +(* Helper function for SUSE: remove (hdX,X) prefix from a path. *) > +let remove_hd_prefix path > + let rex = Str.regexp "^(hd.*)\\(.*\\)" in > + Str.replace_first rex "\\1" path > + > +(* Grub1 (AKA grub-legacy) representation. *) > +class bootloader_grub1 (g : G.guestfs) inspect grub_config > + (* Grub prefix? Usually "/boot". *) > + let grub_prefix > + let mounts = g#inspect_get_mountpoints inspect.i_root in > + try > + List.find ( > + fun path -> List.mem_assoc path mounts > + ) [ "/boot/grub"; "/boot" ] > + with Not_found -> "" in > +object > + inherit bootloader > + > + method name = "grub1" > + > + method augeas_device_patterns = [ > + "/files" ^ grub_config ^ "/*/kernel/root"; > + "/files" ^ grub_config ^ "/*/kernel/resume"; > + "/files/boot/grub/device.map/*[label() != \"#comment\"]"; > + "/files/etc/sysconfig/grub/boot"; > + ] > + > + method list_kernels () > + let paths > + let expr = sprintf "/files%s/title/kernel" grub_config in > + let paths = g#aug_match expr in > + let paths = Array.to_list paths in > + > + (* Remove duplicates. *) > + let paths = remove_duplicates paths in > + > + (* Get the default kernel from grub if it's set. *) > + let default > + let expr = sprintf "/files%s/default" grub_config in > + try > + let idx = g#aug_get expr in > + let idx = int_of_string idx in > + (* Grub indices are zero-based, augeas is 1-based. *) > + let expr > + sprintf "/files%s/title[%d]/kernel" grub_config (idx+1) in > + Some expr > + with G.Error msg > + when String.find msg "aug_get: no matching node" >= 0 -> > + None in > + > + (* If a default kernel was set, put it at the beginning of the paths > + * list. If not set, assume the first kernel always boots (?) > + *) > + match default with > + | None -> paths > + | Some p -> p :: List.filter ((<>) p) paths in > + > + (* Resolve the Augeas paths to kernel filenames. *) > + let vmlinuzes = List.map g#aug_get paths in > + > + (* Make sure kernel does not begin with (hdX,X). *) > + let vmlinuzes = List.map remove_hd_prefix vmlinuzes in > + > + (* Prepend grub filesystem. *) > + List.map ((^) grub_prefix) vmlinuzes > + > + method set_default_kernel vmlinuz > + if not (String.is_prefix vmlinuz grub_prefix) then > + error (f_"kernel %s is not under grub tree %s") > + vmlinuz grub_prefix; > + let kernel_under_grub_prefix > + let prefix_len = String.length grub_prefix in > + let kernel_len = String.length vmlinuz in > + String.sub vmlinuz prefix_len (kernel_len - prefix_len) in > + > + (* Find the grub entry for the given kernel. *) > + let paths = g#aug_match (sprintf "/files%s/title/kernel[. = '%s']" > + grub_config kernel_under_grub_prefix) in > + let paths = Array.to_list paths in > + if paths = [] then > + error (f_"didn't find grub entry for kernel %s") vmlinuz; > + let path = List.hd paths in > + let rex = Str.regexp ".*/title\\[\\([1-9][0-9]*\\)\\]/kernel" in > + if not (Str.string_match rex path 0) then > + error (f_"internal error: regular expression did not match '%s'") > + path; > + let index = int_of_string (Str.matched_group 1 path) - 1 in > + g#aug_set (sprintf "/files%s/default" grub_config) (string_of_int index); > + g#aug_save () > + > + method set_augeas_configuration () > + let incls = g#aug_match "/augeas/load/Grub/incl" in > + let incls = Array.to_list incls in > + let incls_contains_conf > + List.exists (fun incl -> g#aug_get incl = grub_config) incls in > + if not incls_contains_conf then ( > + g#aug_set "/augeas/load/Grub/incl[last()+1]" grub_config; > + true; > + ) else false > + > + method configure_console () > + let rex = Str.regexp "\\(.*\\)\\b\\([xh]vc0\\)\\b\\(.*\\)" in > + let expr = sprintf "/files%s/title/kernel/console" grub_config in > + > + let paths = g#aug_match expr in > + let paths = Array.to_list paths in > + List.iter ( > + fun path -> > + let console = g#aug_get path in > + if Str.string_match rex console 0 then ( > + let console = Str.global_replace rex "\\1ttyS0\\3" console in > + g#aug_set path console > + ) > + ) paths; > + > + g#aug_save () > + > + method remove_console () > + let rex = Str.regexp "\\(.*\\)\\b\\([xh]vc0\\)\\b\\(.*\\)" in > + let expr = sprintf "/files%s/title/kernel/console" grub_config in > + > + let rec loop = function > + | [] -> () > + | path :: paths -> > + let console = g#aug_get path in > + if Str.string_match rex console 0 then ( > + ignore (g#aug_rm path); > + (* All the paths are invalid, restart the loop. *) > + let paths = g#aug_match expr in > + let paths = Array.to_list paths in > + loop paths > + ) > + else > + loop paths > + in > + let paths = g#aug_match expr in > + let paths = Array.to_list paths in > + loop paths; > + > + g#aug_save () > +end > + > +(* Grub2 representation. *) > +class bootloader_grub2 (g : G.guestfs) grub_config > + let grub2_update_console ~remove > + let rex = Str.regexp "\\(.*\\)\\bconsole=[xh]vc0\\b\\(.*\\)" in > + > + let paths = [ > + "/files/etc/sysconfig/grub/GRUB_CMDLINE_LINUX"; > + "/files/etc/default/grub/GRUB_CMDLINE_LINUX"; > + "/files/etc/default/grub/GRUB_CMDLINE_LINUX_DEFAULT" > + ] in > + let paths = List.map g#aug_match paths in > + let paths = List.map Array.to_list paths in > + let paths = List.flatten paths in > + match paths with > + | [] -> > + if not remove then > + warning (f_"could not add grub2 serial console (ignored)") > + else > + warning (f_"could not remove grub2 serial console (ignored)") > + | path :: _ -> > + let grub_cmdline = g#aug_get path in > + if Str.string_match rex grub_cmdline 0 then ( > + let new_grub_cmdline > + if not remove then > + Str.global_replace rex "\\1console=ttyS0\\2" grub_cmdline > + else > + Str.global_replace rex "\\1\\2" grub_cmdline in > + g#aug_set path new_grub_cmdline; > + g#aug_save (); > + > + try > + ignore (g#command [| "grub2-mkconfig"; "-o"; grub_config |]) > + with > + G.Error msg -> > + warning (f_"could not rebuild grub2 configuration file (%s). This may mean that grub output will not be sent to the serial port, but otherwise should be harmless. Original error message: %s") > + grub_config msg > + ) in > +object > + inherit bootloader > + > + method name = "grub2" > + > + method augeas_device_patterns = [ > + "/files/etc/sysconfig/grub/GRUB_CMDLINE_LINUX"; > + "/files/etc/default/grub/GRUB_CMDLINE_LINUX"; > + "/files/etc/default/grub/GRUB_CMDLINE_LINUX_DEFAULT"; > + "/files/boot/grub2/device.map/*[label() != \"#comment\"]"; > + ] > + > + method list_kernels () > + let get_default_image () > + let cmd > + if g#exists "/sbin/grubby" then > + [| "grubby"; "--default-kernel" |] > + else > + [| "/usr/bin/perl"; "-MBootloader::Tools"; "-e"; " > + InitLibrary(); > + my $default = Bootloader::Tools::GetDefaultSection(); > + print $default->{image}; > + " |] in > + match g#command cmd with > + | "" -> None > + | k -> > + let len = String.length k in > + let k > + if len > 0 && k.[len-1] = '\n' then > + String.sub k 0 (len-1) > + else k in > + Some (remove_hd_prefix k) > + in > + > + let vmlinuzes > + (match get_default_image () with > + | None -> [] > + | Some k -> [k]) @ > + (* This is how the grub2 config generator enumerates kernels. *) > + Array.to_list (g#glob_expand "/boot/kernel-*") @ > + Array.to_list (g#glob_expand "/boot/vmlinuz-*") @ > + Array.to_list (g#glob_expand "/vmlinuz-*") in > + let rex = Str.regexp ".*\\.\\(dpkg-.*|rpmsave|rpmnew\\)$" in > + let vmlinuzes = List.filter ( > + fun file -> not (Str.string_match rex file 0) > + ) vmlinuzes in > + vmlinuzes > + > + method set_default_kernel vmlinuz > + let cmd > + if g#exists "/sbin/grubby" then > + [| "grubby"; "--set-default"; vmlinuz |] > + else > + [| "/usr/bin/perl"; "-MBootloader::Tools"; "-e"; sprintf " > + InitLibrary(); > + my @sections = GetSectionList(type=>image, image=>\"%s\"); > + my $section = GetSection(@sections); > + my $newdefault = $section->{name}; > + SetGlobals(default, \"$newdefault\"); > + " vmlinuz |] in > + ignore (g#command cmd) > + > + method configure_console () > + grub2_update_console ~remove:false > + > + method remove_console () > + grub2_update_console ~remove:true > + > + method update () > + ignore (g#command [| "grub2-mkconfig"; "-o"; grub_config |]) > +end > + > +let detect_bootloader (g : G.guestfs) inspect > + let config_file, typ > + let locations = [ > + "/boot/grub2/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) :: locations > + | I_BIOS -> locations in > + try > + List.find ( > + fun (grub_config, _) -> g#is_file ~followsymlinks:true grub_config > + ) locations > + with > + Not_found -> > + error (f_"no bootloader detected") in > + > + match typ with > + | Grub1 -> new bootloader_grub1 g inspect config_file > + | Grub2 -> new bootloader_grub2 g config_file > diff --git a/v2v/bootloaders.mli b/v2v/bootloaders.mli > new file mode 100644 > index 0000000..477312b > --- /dev/null > +++ b/v2v/bootloaders.mli > @@ -0,0 +1,44 @@ > +(* virt-v2v > + * Copyright (C) 2009-2016 Red Hat Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License along > + * with this program; if not, write to the Free Software Foundation, Inc., > + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > + *) > + > +class virtual bootloader : object > + method virtual name : string > + (** The name of the bootloader. *) > + method virtual augeas_device_patterns : string list > + (** A list of augeas patterns to search for device names. *) > + method virtual list_kernels : unit -> string list > + (** Lists all the kernels configured in the bootloader. *) > + method virtual set_default_kernel : string -> unit > + (** Sets the specified vmlinuz path as default bootloader entry. *) > + method set_augeas_configuration : unit -> bool > + (** Checks whether augeas is reading the configuration file > + of the bootloader, and if not then add it. > + > + Returns whether augeas needs to be reloaded. *) > + method virtual configure_console : unit -> unit > + (** Sets up the console for the available kernels. *) > + method virtual remove_console : unit -> unit > + (** Removes the console in all the available kernels. *) > + method update : unit -> unit > + (** Update the bootloader. *) > +end > +(** Encapsulates a UNIX boot loader as object. *) > + > +val detect_bootloader : Guestfs.guestfs -> Types.inspect -> bootloader > +(** Detects the bootloader on the guest, and creates the object > + representing it. *) > diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml > index f9aa334..77d9d8e 100644 > --- a/v2v/convert_linux.ml > +++ b/v2v/convert_linux.ml > @@ -89,36 +89,8 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect source rcaps > let dbfiles = Array.to_list dbfiles in > List.iter g#rm_f dbfiles; > > - (* What grub is installed? *) > - let grub_config, grub > - let locations = [ > - "/boot/grub2/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) :: locations > - | I_BIOS -> locations in > - try > - List.find ( > - fun (grub_config, _) -> g#is_file ~followsymlinks:true grub_config > - ) locations > - with > - Not_found -> > - error (f_"no grub1/grub-legacy or grub2 configuration file was found") in > - > - (* Grub prefix? Usually "/boot". *) > - let grub_prefix > - match grub with > - | `Grub2 -> "" > - | `Grub1 -> > - let mounts = g#inspect_get_mountpoints inspect.i_root in > - try > - List.find ( > - fun path -> List.mem_assoc path mounts > - ) [ "/boot/grub"; "/boot" ] > - with Not_found -> "" in > + (* Detect the installed bootloader. *) > + let bootloader = Bootloaders.detect_bootloader g inspect in > > (* What kernel/kernel-like packages are installed on the current guest? *) > let installed_kernels : kernel_info list > @@ -269,88 +241,7 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect source rcaps > * list is the default booting kernel. > *) > let grub_kernels : kernel_info list > - (* Helper function for SUSE: remove (hdX,X) prefix from a path. *) > - let remove_hd_prefix > - let rex = Str.regexp "^(hd.*)\\(.*\\)" in > - Str.replace_first rex "\\1" > - in > - > - let vmlinuzes > - match grub with > - | `Grub1 -> > - let paths > - let expr = sprintf "/files%s/title/kernel" grub_config in > - let paths = g#aug_match expr in > - let paths = Array.to_list paths in > - > - (* Remove duplicates. *) > - let paths = remove_duplicates paths in > - > - (* Get the default kernel from grub if it's set. *) > - let default > - let expr = sprintf "/files%s/default" grub_config in > - try > - let idx = g#aug_get expr in > - let idx = int_of_string idx in > - (* Grub indices are zero-based, augeas is 1-based. *) > - let expr > - sprintf "/files%s/title[%d]/kernel" grub_config (idx+1) in > - Some expr > - with G.Error msg > - when String.find msg "aug_get: no matching node" >= 0 -> > - None in > - > - (* If a default kernel was set, put it at the beginning of the paths > - * list. If not set, assume the first kernel always boots (?) > - *) > - match default with > - | None -> paths > - | Some p -> p :: List.filter ((<>) p) paths in > - > - (* Resolve the Augeas paths to kernel filenames. *) > - let vmlinuzes = List.map g#aug_get paths in > - > - (* Make sure kernel does not begin with (hdX,X). *) > - let vmlinuzes = List.map remove_hd_prefix vmlinuzes in > - > - (* Prepend grub filesystem. *) > - List.map ((^) grub_prefix) vmlinuzes > - > - | `Grub2 -> > - let get_default_image () > - let cmd > - if g#exists "/sbin/grubby" then > - [| "grubby"; "--default-kernel" |] > - else > - [| "/usr/bin/perl"; "-MBootloader::Tools"; "-e"; " > - InitLibrary(); > - my $default = Bootloader::Tools::GetDefaultSection(); > - print $default->{image}; > - " |] in > - match g#command cmd with > - | "" -> None > - | k -> > - let len = String.length k in > - let k > - if len > 0 && k.[len-1] = '\n' then > - String.sub k 0 (len-1) > - else k in > - Some (remove_hd_prefix k) > - in > - > - let vmlinuzes > - (match get_default_image () with > - | None -> [] > - | Some k -> [k]) @ > - (* This is how the grub2 config generator enumerates kernels. *) > - Array.to_list (g#glob_expand "/boot/kernel-*") @ > - Array.to_list (g#glob_expand "/boot/vmlinuz-*") @ > - Array.to_list (g#glob_expand "/vmlinuz-*") in > - let rex = Str.regexp ".*\\.\\(dpkg-.*|rpmsave|rpmnew\\)$" in > - let vmlinuzes = List.filter ( > - fun file -> not (Str.string_match rex file 0) > - ) vmlinuzes in > - vmlinuzes in > + let vmlinuzes = bootloader#list_kernels () in > > (* Map these to installed kernels. *) > filter_map ( > @@ -389,21 +280,8 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect source rcaps > (* Conversion step. *) > > let rec augeas_grub_configuration () > - match grub with > - | `Grub1 -> > - (* Ensure Augeas is reading the grub configuration file, and if not > - * then add it. > - *) > - let incls = g#aug_match "/augeas/load/Grub/incl" in > - let incls = Array.to_list incls in > - let incls_contains_conf > - List.exists (fun incl -> g#aug_get incl = grub_config) incls in > - if not incls_contains_conf then ( > - g#aug_set "/augeas/load/Grub/incl[last()+1]" grub_config; > - Linux.augeas_reload g; > - ) > - > - | `Grub2 -> () (* Not necessary for grub2. *) > + if bootloader#set_augeas_configuration () then > + Linux.augeas_reload g > > and unconfigure_xen () > (* Remove kmod-xenpv-* (RHEL 3). *) > @@ -733,7 +611,7 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect source rcaps > let kernels = List.rev kernels (* so best is first *) in > List.hd kernels in > if best_kernel <> List.hd grub_kernels then > - grub_set_bootable best_kernel; > + bootloader#set_default_kernel best_kernel.ki_vmlinuz; > > (* Does the best/bootable kernel support virtio? *) > let virtio = best_kernel.ki_supports_virtio in > @@ -751,46 +629,6 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect source rcaps > > best_kernel, virtio > > - and grub_set_bootable kernel > - match grub with > - | `Grub1 -> > - if not (String.is_prefix kernel.ki_vmlinuz grub_prefix) then > - error (f_"kernel %s is not under grub tree %s") > - kernel.ki_vmlinuz grub_prefix; > - let kernel_under_grub_prefix > - let prefix_len = String.length grub_prefix in > - let kernel_len = String.length kernel.ki_vmlinuz in > - String.sub kernel.ki_vmlinuz prefix_len (kernel_len - prefix_len) in > - > - (* Find the grub entry for the given kernel. *) > - let paths = g#aug_match (sprintf "/files%s/title/kernel[. = '%s']" > - grub_config kernel_under_grub_prefix) in > - let paths = Array.to_list paths in > - if paths = [] then > - error (f_"didn't find grub entry for kernel %s") kernel.ki_vmlinuz; > - let path = List.hd paths in > - let rex = Str.regexp ".*/title\\[\\([1-9][0-9]*\\)\\]/kernel" in > - if not (Str.string_match rex path 0) then > - error (f_"internal error: regular expression did not match '%s'") > - path; > - let index = int_of_string (Str.matched_group 1 path) - 1 in > - g#aug_set (sprintf "/files%s/default" grub_config) (string_of_int index); > - g#aug_save () > - > - | `Grub2 -> > - let cmd > - if g#exists "/sbin/grubby" then > - [| "grubby"; "--set-default"; kernel.ki_vmlinuz |] > - else > - [| "/usr/bin/perl"; "-MBootloader::Tools"; "-e"; sprintf " > - InitLibrary(); > - my @sections = GetSectionList(type=>image, image=>\"%s\"); > - my $section = GetSection(@sections); > - my $newdefault = $section->{name}; > - SetGlobals(default, \"$newdefault\"); > - " kernel.ki_vmlinuz |] in > - ignore (g#command cmd) > - > (* Even though the kernel was already installed (this version of > * virt-v2v does not install new kernels), it could have an > * initrd that does not have support virtio. Therefore rebuild > @@ -956,28 +794,6 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect source rcaps > > g#aug_save () > > - and grub_configure_console () > - match grub with > - | `Grub1 -> > - let rex = Str.regexp "\\(.*\\)\\b\\([xh]vc0\\)\\b\\(.*\\)" in > - let expr = sprintf "/files%s/title/kernel/console" grub_config in > - > - let paths = g#aug_match expr in > - let paths = Array.to_list paths in > - List.iter ( > - fun path -> > - let console = g#aug_get path in > - if Str.string_match rex console 0 then ( > - let console = Str.global_replace rex "\\1ttyS0\\3" console in > - g#aug_set path console > - ) > - ) paths; > - > - g#aug_save () > - > - | `Grub2 -> > - grub2_update_console ~remove:false > - > (* If the target doesn't support a serial console, we want to remove > * all references to it instead. > *) > @@ -1006,71 +822,6 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect source rcaps > > g#aug_save () > > - and grub_remove_console () > - match grub with > - | `Grub1 -> > - let rex = Str.regexp "\\(.*\\)\\b\\([xh]vc0\\)\\b\\(.*\\)" in > - let expr = sprintf "/files%s/title/kernel/console" grub_config in > - > - let rec loop = function > - | [] -> () > - | path :: paths -> > - let console = g#aug_get path in > - if Str.string_match rex console 0 then ( > - ignore (g#aug_rm path); > - (* All the paths are invalid, restart the loop. *) > - let paths = g#aug_match expr in > - let paths = Array.to_list paths in > - loop paths > - ) > - else > - loop paths > - in > - let paths = g#aug_match expr in > - let paths = Array.to_list paths in > - loop paths; > - > - g#aug_save () > - > - | `Grub2 -> > - grub2_update_console ~remove:true > - > - and grub2_update_console ~remove > - let rex = Str.regexp "\\(.*\\)\\bconsole=[xh]vc0\\b\\(.*\\)" in > - > - let paths = [ > - "/files/etc/sysconfig/grub/GRUB_CMDLINE_LINUX"; > - "/files/etc/default/grub/GRUB_CMDLINE_LINUX"; > - "/files/etc/default/grub/GRUB_CMDLINE_LINUX_DEFAULT" > - ] in > - let paths = List.map g#aug_match paths in > - let paths = List.map Array.to_list paths in > - let paths = List.flatten paths in > - match paths with > - | [] -> > - if not remove then > - warning (f_"could not add grub2 serial console (ignored)") > - else > - warning (f_"could not remove grub2 serial console (ignored)") > - | path :: _ -> > - let grub_cmdline = g#aug_get path in > - if Str.string_match rex grub_cmdline 0 then ( > - let new_grub_cmdline > - if not remove then > - Str.global_replace rex "\\1console=ttyS0\\2" grub_cmdline > - else > - Str.global_replace rex "\\1\\2" grub_cmdline in > - g#aug_set path new_grub_cmdline; > - g#aug_save (); > - > - try > - ignore (g#command [| "grub2-mkconfig"; "-o"; grub_config |]) > - with > - G.Error msg -> > - warning (f_"could not rebuild grub2 configuration file (%s). This may mean that grub output will not be sent to the serial port, but otherwise should be harmless. Original error message: %s") > - grub_config msg > - ) > - > and supports_acpi () > (* ACPI known to cause RHEL 3 to fail. *) > if family = `RHEL_family && inspect.i_major_version == 3 then > @@ -1296,19 +1047,9 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect source rcaps > let paths = [ > (* /etc/fstab *) > "/files/etc/fstab/*/spec"; > - > - (* grub-legacy config *) > - "/files" ^ grub_config ^ "/*/kernel/root"; > - "/files" ^ grub_config ^ "/*/kernel/resume"; > - "/files/boot/grub/device.map/*[label() != \"#comment\"]"; > - "/files/etc/sysconfig/grub/boot"; > - > - (* grub2 config *) > - "/files/etc/sysconfig/grub/GRUB_CMDLINE_LINUX"; > - "/files/etc/default/grub/GRUB_CMDLINE_LINUX"; > - "/files/etc/default/grub/GRUB_CMDLINE_LINUX_DEFAULT"; > - "/files/boot/grub2/device.map/*[label() != \"#comment\"]"; > ] in > + (* Bootloader config *) > + let paths = paths @ bootloader#augeas_device_patterns in > > (* Which of these paths actually exist? *) > let paths > @@ -1386,9 +1127,8 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect source rcaps > if !changed then ( > g#aug_save (); > > - (* If it's grub2, we have to regenerate the config files. *) > - if grub = `Grub2 then > - ignore (g#command [| "grub2-mkconfig"; "-o"; grub_config |]); > + (* Make sure the bootloader is up-to-date. *) > + bootloader#update (); > > Linux.augeas_reload g > ); > @@ -1418,10 +1158,10 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect source rcaps > > if keep_serial_console then ( > configure_console (); > - grub_configure_console (); > + bootloader#configure_console (); > ) else ( > remove_console (); > - grub_remove_console (); > + bootloader#remove_console (); > ); > > let acpi = supports_acpi () in > -- > 2.7.4 > > _______________________________________________ > Libguestfs mailing list > Libguestfs@redhat.com > https://www.redhat.com/mailman/listinfo/libguestfs-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Pino Toscano
2016-Aug-15 16:44 UTC
Re: [Libguestfs] [PATCH v2] v2v: factor out bootloader handling
On Monday, 15 August 2016 16:26:51 CEST Richard W.M. Jones wrote:> On Mon, Aug 15, 2016 at 04:48:29PM +0200, Pino Toscano wrote: > > Create an object hierarchy to represent different bootloaders for Linux > > guests, moving the separate handling of grub1 and grub2 in different > > classes: this isolates the code for each type of bootloader together, > > instead of scattering it all around. > > > > This is mostly code refactoring, with no actual behaviour change. > > --- > > po/POTFILES-ml | 1 + > > v2v/Makefile.am | 2 + > > v2v/bootloaders.ml | 317 +++++++++++++++++++++++++++++++++++++++++++++++++++ > > v2v/bootloaders.mli | 44 +++++++ > > Is this really "bootloaders"? It's more like "linux_bootloaders", > unless this might one day be extended to include NTLDR.OK, make sense -- I will rename it.> > +class virtual bootloader = object > > + method virtual name : string > > + method virtual augeas_device_patterns : string list > > + method virtual list_kernels : unit -> string list > > + method virtual set_default_kernel : string -> unit > > + method set_augeas_configuration () = false > > + method virtual configure_console : unit -> unit > > + method virtual remove_console : unit -> unit > > + method update () = () > > +end > > I think you'll also find that you don't need to use inheritance at > all. You can just declare bootloader as a class type and declare two > (unrelated) classes which implement the class type. They won't be > able to inherit the two trivial non-virtual functions, but IMHO that's > a feature.Using inheritance helps here to spot typos in methods, and also when implementing subclasses it'll be spotted if a method is not implemented.> I still don't much see the point versus an implementation which used a > type Bootloader.t declared as: > > type t = Grub1 of <any local data grub1 needs> > | Grub2 of <any local data grub2 needs> > > (and not exposed through the mli file). As this is to some extent my > personal preference, I'll let it slide.I did this approach as well, and it turns out that: * every method is basically a giant "match t with Grub1 _ | Grub2 _", which makes it ugly to read: this is also because the code shared between one bootloader implementation and another is not really much, and currently only the "remove_hd_prefix" function is * the implementation of a bootloader is scattered all around, so looking at what is done for a bootloader means looking everywhere * passing a data type when calling every method is basically like what is done in C to mimim OOP; hence if I have to do something that resembles OOP but not actually doing it, then it will be a suboptimal implementation than OO (IMHO at least) Thanks, -- Pino Toscano
Seemingly Similar Threads
- [PATCH v2] v2v: factor out bootloader handling
- Re: [PATCH v2] v2v: factor out bootloader handling
- [PATCH v2] v2v: factor out bootloader handling
- [PATCH] v2v: factor out bootloader handling
- [PATCH] v2v: Use unitless methods for methods which don't change the internal state.