[PATCH v6 0/1] v2v bootloaders and augeas
I dare to raise an old topic :) It seemed to me that this patch already applied, but recently discovered that it is not. As far as I understand we can't do a strict requirement to use Augeas >= 1.7.0. But initially, the patch solves another problem. If we still can't do a hard requirement for Augeas, let's use workaround proposed by Pino: let () = (* 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 As a temporary solution looks fine. Pavel Butsykin (1): v2v: bootloaders: search grub config for all distributions v2v/linux_bootloaders.ml | 86 +++++++++++++++++++++++++++++++----------------- 1 file changed, 55 insertions(+), 31 deletions(-) -- 2.11.0
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> Signed-off-by: Richard W.M. Jones <rjones@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 () = + (* 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 = 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; + + (match typ with + | Grub1 -> new bootloader_grub1 + | Grub2 -> new bootloader_grub2) g inspect grub_config -- 2.11.0
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@virtuozzo.com> Signed-off-by: Richard W.M. Jones <rjones@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. Thanks, -- Pino Toscano
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@virtuozzo.com> Signed-off-by: Richard W.M. Jones <rjones@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,
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@virtuozzo.com> Signed-off-by: Richard W.M. Jones <rjones@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/
On Wed, Apr 19, 2017 at 04:58:56PM +0300, Pavel Butsykin wrote:
I dare to raise an old topic :) It seemed to me that this patch already applied, but recently discovered that it is not.
As far as I understand we can't do a strict requirement to use Augeas >= 1.7.0.
I'm not sure I know enough to review this patch, but on the issue of Augeas 1.7.0: As Augeas 1.7.0 was released nearly 6 months ago, I think it would be fine to require it for virt-v2v. If we did that does it help? 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
20.04.2017 10:54, Richard W.M. Jones пишет:
On Wed, Apr 19, 2017 at 04:58:56PM +0300, Pavel Butsykin wrote:
I dare to raise an old topic :) It seemed to me that this patch already applied, but recently discovered that it is not.
As far as I understand we can't do a strict requirement to use Augeas >= 1.7.0. I'm not sure I know enough to review this patch, but on the issue
As I remember the patch was already reviewed, there was only an issue with aug_transform to existing grub config path.
of Augeas 1.7.0: As Augeas 1.7.0 was released nearly 6 months ago, I think it would be fine to require it for virt-v2v. If we did that does it help?
Yes, definitely. In this case, we can just apply your previous patch-set - ( https://www.redhat.com/archives/libguestfs/2016-November/msg00059.html ). After all, the only problem with the bootloader patch is an error when libguestfs attempts to include the existing path to the Augeas lens. With Augeas 1.7.0 we get rid of unnecessary calls to aug_transform.
Rich.
[This sender failed our fraud detection checks and may not be who they appear to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]
20.04.2017 10:54, Richard W.M. Jones пишет:
On Wed, Apr 19, 2017 at 04:58:56PM +0300, Pavel Butsykin wrote:
I dare to raise an old topic :) It seemed to me that this patch already applied, but recently discovered that it is not.
As far as I understand we can't do a strict requirement to use Augeas
= 1.7.0. I'm not sure I know enough to review this patch, but on the issue
As I remember the patch was already reviewed, there was only an issue with aug_transform to existing grub config path.
of Augeas 1.7.0: As Augeas 1.7.0 was released nearly 6 months ago, I think it would be fine to require it for virt-v2v. If we did that does it help?
Yes, definitely. In this case, we can just apply your previous patch-set - ( https://www.redhat.com/archives/libguestfs/2016-November/msg00059.html ). After all, the only problem with the bootloader patch is an error when libguestfs attempts to include the existing path to the Augeas lens. With Augeas 1.7.0 we get rid of unnecessary calls to aug_transform.
On 20.04.2017 12:19, Pavel Butsykin wrote: ping. Can I expect decision on this issue?
Rich.
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
On Fri, Apr 28, 2017 at 05:02:29PM +0300, Pavel Butsykin wrote:
[This sender failed our fraud detection checks and may not be who they appear to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]
20.04.2017 10:54, Richard W.M. Jones пишет:
On Wed, Apr 19, 2017 at 04:58:56PM +0300, Pavel Butsykin wrote:
I dare to raise an old topic :) It seemed to me that this patch already applied, but recently discovered that it is not.
As far as I understand we can't do a strict requirement to use Augeas
= 1.7.0. I'm not sure I know enough to review this patch, but on the issue
As I remember the patch was already reviewed, there was only an issue with aug_transform to existing grub config path.
of Augeas 1.7.0: As Augeas 1.7.0 was released nearly 6 months ago, I think it would be fine to require it for virt-v2v. If we did that does it help?
Yes, definitely. In this case, we can just apply your previous patch-set - ( https://www.redhat.com/archives/libguestfs/2016-November/msg00059.html ). After all, the only problem with the bootloader patch is an error when libguestfs attempts to include the existing path to the Augeas lens. With Augeas 1.7.0 we get rid of unnecessary calls to aug_transform.
On 20.04.2017 12:19, Pavel Butsykin wrote: ping.
Can I expect decision on this issue?
Sorry, I was waiting to see if Pino wanted to say anything abou this since he knows the subject in far more detail than I do. I'm quite happy about moving to Augeas >= 1.7.0. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
On Wednesday, 19 April 2017 15:58:56 CEST Pavel Butsykin wrote:
I dare to raise an old topic :) It seemed to me that this patch already applied, but recently discovered that it is not.
As far as I understand we can't do a strict requirement to use Augeas >= 1.7.0. But initially, the patch solves another problem. If we still can't do a hard requirement for Augeas, let's use workaround proposed by Pino: let () = (* 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
As a temporary solution looks fine.
Better this solution than raising the Augeas requirements, for now. -- Pino Toscano
On 28.04.2017 17:46, Pino Toscano wrote:
On Wednesday, 19 April 2017 15:58:56 CEST Pavel Butsykin wrote:
I dare to raise an old topic :) It seemed to me that this patch already applied, but recently discovered that it is not.
As far as I understand we can't do a strict requirement to use Augeas >= 1.7.0. But initially, the patch solves another problem. If we still can't do a hard requirement for Augeas, let's use workaround proposed by Pino: let () = (* 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
As a temporary solution looks fine.
Better this solution than raising the Augeas requirements, for now.
Rich, What about workaround?
On Fri, Apr 28, 2017 at 07:58:41PM +0300, Pavel Butsykin wrote:
On 28.04.2017 17:46, Pino Toscano wrote:
On Wednesday, 19 April 2017 15:58:56 CEST Pavel Butsykin wrote:
I dare to raise an old topic :) It seemed to me that this patch already applied, but recently discovered that it is not.
As far as I understand we can't do a strict requirement to use Augeas >= 1.7.0. But initially, the patch solves another problem. If we still can't do a hard requirement for Augeas, let's use workaround proposed by Pino: let () = (* 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
As a temporary solution looks fine.
Better this solution than raising the Augeas requirements, for now.
Rich, What about workaround?
I guess. I disagree with Pino on this, but he knows the topic better. Is there a final patch ready to apply now? I think lost the beginning of this thread. 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/
On 28.04.2017 21:11, Richard W.M. Jones wrote:
On Fri, Apr 28, 2017 at 07:58:41PM +0300, Pavel Butsykin wrote:
On 28.04.2017 17:46, Pino Toscano wrote:
On Wednesday, 19 April 2017 15:58:56 CEST Pavel Butsykin wrote:
I dare to raise an old topic :) It seemed to me that this patch already applied, but recently discovered that it is not.
As far as I understand we can't do a strict requirement to use Augeas >= 1.7.0. But initially, the patch solves another problem. If we still can't do a hard requirement for Augeas, let's use workaround proposed by Pino: let () = (* 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
As a temporary solution looks fine.
Better this solution than raising the Augeas requirements, for now.
Rich, What about workaround?
I guess. I disagree with Pino on this, but he knows the topic better.
Is there a final patch ready to apply now? I think lost the beginning of this thread.
I also have little knowledge in this topic and I also wonder why we can't move on Augeas 1.7.0. Are stable version of Linux distros can use the fresh Libguestfs versions? Anyway I sent patch v7 with the latest fixes.
Rich.
participants (3)
-
Pavel Butsykin -
Pino Toscano -
Richard W.M. Jones