Augeas 1.7.0 was released a couple of days ago. By encouraging everyone to upgrade to this we can drop several calls to aug_transform and also our custom copies of two lenses, and a lot of related code. Rich.
Richard W.M. Jones
2016-Nov-10 14:59 UTC
[Libguestfs] [PATCH v5 1/3] v2v: bootloaders: search grub config for all distributions
From: Pavel Butsykin <pbutsykin@virtuozzo.com>
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 | 83 ++++++++++++++++++++++++++++++------------------
1 file changed, 52 insertions(+), 31 deletions(-)
diff --git a/v2v/linux_bootloaders.ml b/v2v/linux_bootloaders.ml
index e03d22b..9160dcc 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,10 @@ let remove_hd_prefix path
(* Grub1 (AKA grub-legacy) representation. *)
class bootloader_grub1 (g : G.guestfs) inspect grub_config + let () + if
grub_config = "/boot/efi/EFI/redhat/grub.conf" then
+ g#aug_transform "grub"
"/boot/efi/EFI/redhat/grub.conf" in
+
(* Grub prefix? Usually "/boot". *)
let grub_prefix let mounts = g#inspect_get_mountpoints inspect.i_root in
@@ -191,7 +199,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 = [
@@ -334,33 +342,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.9.3
Richard W.M. Jones
2016-Nov-10 15:00 UTC
[Libguestfs] [PATCH v5 2/3] v2v: bootloaders: Remove call to g#aug_transform.
Let's fix this in Augeas upstream instead of hacking around it in libguestfs. --- docs/guestfs-building.pod | 3 +++ v2v/linux_bootloaders.ml | 4 ---- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/guestfs-building.pod b/docs/guestfs-building.pod index 0593acb..0e81f70 100644 --- a/docs/guestfs-building.pod +++ b/docs/guestfs-building.pod @@ -157,6 +157,9 @@ I<Required>. I<Required>. +Augeas E<ge> 1.7.0 may be necessary to correctly convert certain +guests using virt-v2v. + =item xz I<Required>. diff --git a/v2v/linux_bootloaders.ml b/v2v/linux_bootloaders.ml index 9160dcc..ec32715 100644 --- a/v2v/linux_bootloaders.ml +++ b/v2v/linux_bootloaders.ml @@ -53,10 +53,6 @@ let remove_hd_prefix path (* Grub1 (AKA grub-legacy) representation. *) class bootloader_grub1 (g : G.guestfs) inspect grub_config - let () - if grub_config = "/boot/efi/EFI/redhat/grub.conf" then - g#aug_transform "grub" "/boot/efi/EFI/redhat/grub.conf" in - (* Grub prefix? Usually "/boot". *) let grub_prefix let mounts = g#inspect_get_mountpoints inspect.i_root in -- 2.9.3
Richard W.M. Jones
2016-Nov-10 15:00 UTC
[Libguestfs] [PATCH v5 3/3] augeas: Remove our copies of lvm.aug and shadow.aug lenses.
Assume everyone is using Augeas 1.7.0.
---
appliance/Makefile.am | 6 +---
appliance/guestfs_lvm_conf.aug | 74 --------------------------------------
appliance/guestfs_shadow.aug | 72 -------------------------------------
daemon/augeas.c | 80 ------------------------------------------
daemon/daemon.h | 12 -------
daemon/lvm-filter.c | 2 +-
docs/guestfs-building.pod | 5 +--
7 files changed, 3 insertions(+), 248 deletions(-)
delete mode 100644 appliance/guestfs_lvm_conf.aug
delete mode 100644 appliance/guestfs_shadow.aug
diff --git a/appliance/Makefile.am b/appliance/Makefile.am
index 08a7f4c..9f56d06 100644
--- a/appliance/Makefile.am
+++ b/appliance/Makefile.am
@@ -32,8 +32,6 @@ include $(top_srcdir)/subdir-rules.mk
EXTRA_DIST = \
99-guestfs-serial.rules \
excludefiles.in \
- guestfs_lvm_conf.aug \
- guestfs_shadow.aug \
hostfiles.in \
init \
libguestfs-make-fixed-appliance.in \
@@ -87,13 +85,11 @@ packagelist: packagelist.in Makefile
cmp -s $@ $@-t || mv $@-t $@
rm -f $@-t
-supermin.d/daemon.tar.gz: ../daemon/guestfsd guestfs_lvm_conf.aug
guestfs_shadow.aug
+supermin.d/daemon.tar.gz: ../daemon/guestfsd
rm -f $@ $@-t
rm -rf tmp-d
mkdir -p tmp-d$(DAEMON_SUPERMIN_DIR) tmp-d/etc tmp-d/usr/share/guestfs
ln ../daemon/guestfsd tmp-d$(DAEMON_SUPERMIN_DIR)/guestfsd
- ln $(srcdir)/guestfs_lvm_conf.aug tmp-d/usr/share/guestfs/guestfs_lvm_conf.aug
- ln $(srcdir)/guestfs_shadow.aug tmp-d/usr/share/guestfs/guestfs_shadow.aug
( cd tmp-d && tar zcf - * ) > $@-t
rm -r tmp-d
mv $@-t $@
diff --git a/appliance/guestfs_lvm_conf.aug b/appliance/guestfs_lvm_conf.aug
deleted file mode 100644
index ffa5b01..0000000
--- a/appliance/guestfs_lvm_conf.aug
+++ /dev/null
@@ -1,74 +0,0 @@
-(*
-Module: LVM
- Parses LVM metadata.
-
-Author: Gabriel de Perthuis <g2p.code+augeas@gmail.com>
-
-About: License
- This file is licensed under the LGPL v2+.
-
-About: Configuration files
- This lens applies to files in /etc/lvm/backup and /etc/lvm/archive.
-
-About: Examples
- The <Test_LVM> file contains various examples and tests.
-*)
-
-module Guestfs_LVM_conf - autoload xfm
-
- (* See lvm2/libdm/libdm-config.c for tokenisation;
- * libdm uses a blacklist but I prefer the safer whitelist approach. *)
- (* View: identifier
- * The left hand side of a definition *)
- let identifier = /[a-zA-Z0-9_-]+/
-
- (* strings can contain backslash-escaped dquotes, but I don't know
- * how to get the message across to augeas *)
- let str = [label "str". Quote.do_dquote (store
/([^\"]|\\\\.)*/)]
- let int = [label "int". store Rx.relinteger]
- (* View: flat_literal
- * A literal without structure *)
- let flat_literal = int|str
-
- (* allow multiline and mixed int/str, used for raids and stripes *)
- (* View: list
- * A list containing flat literals *)
- let list = [
- label "list" . counter "list"
- . del /\[[ \t\n]*/ "["
- .([seq "list". flat_literal . del /,[ \t\n]*/ ", "]*
- . [seq "list". flat_literal . del /[ \t\n]*/ ""])?
- . Util.del_str "]"]
-
- (* View: val
- * Any value that appears on the right hand side of an assignment *)
- let val = flat_literal | list
-
- (* View: nondef
- * A line that doesn't contain a statement *)
- let nondef - Util.empty
- | Util.comment
-
- (* Build.block couldn't be reused, because of recursion and
- * a different philosophy of whitespace handling. *)
- (* View: def
- * An assignment, or a block containing definitions *)
- let rec def = [
- Util.indent . key identifier . (
- del /[ \t]*\{\n/ " {\n"
- .[label "dict".(nondef | def)*]
- . Util.indent . Util.del_str "}\n"
- |Sep.space_equal . val . Util.comment_or_eol)]
-
- (* View: lns
- * The main lens *)
- let lns = (nondef | def)*
-
- let filter - incl "/etc/lvm/archive/*.vg"
- . incl "/etc/lvm/backup/*"
- . Util.stdexcl
-
- let xfm = transform lns filter
diff --git a/appliance/guestfs_shadow.aug b/appliance/guestfs_shadow.aug
deleted file mode 100644
index 2fbf455..0000000
--- a/appliance/guestfs_shadow.aug
+++ /dev/null
@@ -1,72 +0,0 @@
-(*
- Module: Shadow
- Parses /etc/shadow
-
- Author: Lorenzo M. Catucci <catucci@ccd.uniroma2.it>
-
- Original Author: Free Ekanayaka <free@64studio.com>
-
- About: Reference
-
- - man 5 shadow
- - man 3 getspnam
-
- About: License
- This file is licensed under the LGPL v2+, like the rest of Augeas.
-
- About:
-
- Each line in the shadow files represents the additional shadow-defined
attributes
- for the corresponding user, as defined in the passwd file.
-
-*)
-
-module Guestfs_Shadow -
- autoload xfm
-
-(************************************************************************
- * USEFUL PRIMITIVES
- *************************************************************************)
-
-let eol = Util.eol
-let comment = Util.comment
-let empty = Util.empty
-let dels = Util.del_str
-
-let colon = Sep.colon
-
-let word = Rx.word
-let integer = Rx.integer
-
-let sto_to_col = Passwd.sto_to_col
-let sto_to_eol = Passwd.sto_to_eol
-
-(************************************************************************
- * Group: ENTRIES
- *************************************************************************)
-
-(* View: entry *)
-let entry = [ key word
- . colon
- . [ label "password" . sto_to_col? .
colon ]
- . [ label "lastchange_date" . store integer? .
colon ]
- . [ label "minage_days" . store integer? .
colon ]
- . [ label "maxage_days" . store integer? .
colon ]
- . [ label "warn_days" . store integer? .
colon ]
- . [ label "inactive_days" . store integer? .
colon ]
- . [ label "expire_date" . store integer? .
colon ]
- . [ label "flag" . store integer? ]
- . eol ]
-
-(************************************************************************
- * LENS
- *************************************************************************)
-
-let lns = (comment|empty|entry) *
-
-let filter
- = incl "/shadow"
- . Util.stdexcl
-
-let xfm = transform lns filter
diff --git a/daemon/augeas.c b/daemon/augeas.c
index 5adc959..d423efb 100644
--- a/daemon/augeas.c
+++ b/daemon/augeas.c
@@ -29,73 +29,12 @@
#include "actions.h"
#include "optgroups.h"
-#define FPRINTF_AUGEAS_ERROR(aug,fs,...) \
- do { \
- const int code = aug_error (aug); \
- if (code == AUG_ENOMEM) \
- reply_with_error (fs ": augeas out of memory", ##__VA_ARGS__);
\
- else { \
- const char *aug_err_message = aug_error_message (aug); \
- const char *aug_err_minor = aug_error_minor_message (aug); \
- const char *aug_err_details = aug_error_details (aug); \
- fprintf (stderr, fs ": %s%s%s%s%s", ##__VA_ARGS__,
\
- aug_err_message, \
- aug_err_minor ? ": " : "", aug_err_minor ?
aug_err_minor : "", \
- aug_err_details ? ": " : "", aug_err_details ?
aug_err_details : ""); \
- } \
- } while (0)
-
-int augeas_version;
-
/* The Augeas handle. We maintain a single handle per daemon, which
* is all that is necessary and reduces the complexity of the API
* considerably.
*/
static augeas *aug = NULL;
-void
-aug_read_version (void)
-{
- CLEANUP_AUG_CLOSE augeas *ah = NULL;
- int r;
- const char *str;
- int major = 0, minor = 0, patch = 0;
-
- if (augeas_version != 0)
- return;
-
- /* Optimization: do not load the files nor the lenses, since we are
- * only interested in the version.
- */
- ah = aug_init ("/", NULL, AUG_NO_ERR_CLOSE | AUG_NO_LOAD |
AUG_NO_STDINC);
- if (!ah) {
- FPRINTF_AUGEAS_ERROR (ah, "augeas initialization failed");
- return;
- }
-
- if (aug_error (ah) != AUG_NOERROR) {
- FPRINTF_AUGEAS_ERROR (ah, "aug_init");
- return;
- }
-
- r = aug_get (ah, "/augeas/version", &str);
- if (r != 1) {
- FPRINTF_AUGEAS_ERROR (ah, "aug_get");
- return;
- }
-
- r = sscanf (str, "%d.%d.%d", &major, &minor, &patch);
- if (r != 2 && r != 3) {
- fprintf (stderr, "cannot match the version string in
'%s'\n", str);
- return;
- }
-
- if (verbose)
- fprintf (stderr, "augeas version: %d.%d.%d\n", major, minor,
patch);
-
- augeas_version = (major << 16) | (minor << 8) | patch;
-}
-
/* Clean up the augeas handle on daemon exit. */
void aug_finalize (void) __attribute__((destructor));
void
@@ -148,25 +87,6 @@ do_aug_init (const char *root, int flags)
return -1;
}
- if (!augeas_is_version (1, 2, 1)) {
- int r = aug_transform (aug, "guestfs_shadow",
"/etc/shadow",
- 0 /* = included */);
- if (r == -1) {
- AUGEAS_ERROR ("aug_transform");
- aug_close (aug);
- aug = NULL;
- return -1;
- }
-
- /* If aug_load was implicitly called, reload the handle. */
- if ((flags & AUG_NO_LOAD) == 0) {
- if (aug_load (aug) == -1) {
- AUGEAS_ERROR ("aug_load");
- return -1;
- }
- }
- }
-
return 0;
}
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 79a5288..2a99af4 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -203,20 +203,8 @@ extern void start_lvmetad (void);
extern void wipe_device_before_mkfs (const char *device);
/*-- in augeas.c --*/
-extern void aug_read_version (void);
extern void aug_finalize (void);
-/* The version of augeas, saved as:
- * (MAJOR << 16) | (MINOR << 8) | PATCH
- */
-extern int augeas_version;
-static inline int
-augeas_is_version (int major, int minor, int patch)
-{
- aug_read_version (); /* Lazy version reading. */
- return augeas_version >= ((major << 16) | (minor << 8) |
patch);
-}
-
/*-- hivex.c, journal.c --*/
extern void hivex_finalize (void);
extern void journal_finalize (void);
diff --git a/daemon/lvm-filter.c b/daemon/lvm-filter.c
index 5c9ce18..309f08d 100644
--- a/daemon/lvm-filter.c
+++ b/daemon/lvm-filter.c
@@ -151,7 +151,7 @@ set_filter (char *const *filters)
return -1;
}
- r = aug_transform (aug, "guestfs_lvm_conf",
"/lvm/lvm.conf",
+ r = aug_transform (aug, "lvm", "/lvm/lvm.conf",
0 /* = included */);
if (r == -1) {
AUGEAS_ERROR ("aug_transform");
diff --git a/docs/guestfs-building.pod b/docs/guestfs-building.pod
index 0e81f70..22545c3 100644
--- a/docs/guestfs-building.pod
+++ b/docs/guestfs-building.pod
@@ -153,13 +153,10 @@ I<Required>.
I<Required>.
-=item augeas E<ge> 1.0.0
+=item augeas E<ge> 1.7.0
I<Required>.
-Augeas E<ge> 1.7.0 may be necessary to correctly convert certain
-guests using virt-v2v.
-
=item xz
I<Required>.
--
2.9.3
On Thursday, 10 November 2016 14:59:58 CET Richard W.M. Jones wrote:> Augeas 1.7.0 was released a couple of days ago. By encouraging > everyone to upgrade to this we can drop several calls to aug_transform > and also our custom copies of two lenses, and a lot of related code.The problem is that raising the requirement to a version released few days ago makes the development more difficult, and it won't build OOTB on any stable version of Linux distros. Encourage: yes -- hard requirement -- not for a while. IMHO. -- Pino Toscano
Pino Toscano
2016-Nov-11 13:49 UTC
Re: [Libguestfs] [PATCH v5 1/3] v2v: bootloaders: search grub config for all distributions
On Thursday, 10 November 2016 14:59:59 CET Richard W.M. Jones wrote:> From: Pavel Butsykin <pbutsykin@virtuozzo.com> > > 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> > ---Mostly LGTM, just one thing it can be improved.> class bootloader_grub1 (g : G.guestfs) inspect grub_config > + let () > + if grub_config = "/boot/efi/EFI/redhat/grub.conf" then > + g#aug_transform "grub" "/boot/efi/EFI/redhat/grub.conf" inAugeas < 1.7.0 does not like adding a transformation for a file with the same lens it is handled already [1]. As workaround, we can first try to list the configuration file: if there are no results, then the transformation is applied. Something like: 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 grub_config = [||] then g#aug_transform "grub" grub_config in This way it will: - pick configuration files for any distro, not just redhat-based ones (fixing what Pavel reported) - work fine (by doing nothing) with Augeas >= 1.7.0 [1] https://www.redhat.com/archives/augeas-devel/2016-November/thread.html -- Pino Toscano
Possibly Parallel Threads
- Re: [PATCH v6 1/1] v2v: bootloaders: search grub config for all distributions
- Re: [PATCH v6 1/1] v2v: bootloaders: search grub config for all distributions
- [PATCH v4 1/2] v2v: bootloaders: search grub config for all distributions
- Re: [PATCH v3] v2v: bootloaders: search grub config for all distributions
- [PATCH] v2v: bootloaders: search grub config for all distributions