Richard W.M. Jones
2014-Oct-24 15:00 UTC
[Libguestfs] [PATCH] daemon: Remove custom Augeas lenses.
Don't carry around Augeas lenses. It is fragile, since if the lens is
added to upstream Augeas but the version number has not changed, then
Augeas won't parse the target file at all. This specifically causes
password adjustments to fail in RHEL 7.1.
In future, if we need an Augeas lens, it must be added to Augeas,
either upstream or as a downstream patch carried around by distros.
---
README | 2 +-
appliance/Makefile.am | 6 +---
appliance/guestfs_lvm_conf.aug | 74 ------------------------------------------
appliance/guestfs_shadow.aug | 72 ----------------------------------------
appliance/hostfiles.in | 1 -
configure.ac | 3 ++
daemon/augeas.c | 21 +-----------
daemon/daemon.h | 11 -------
daemon/lvm-filter.c | 14 +-------
9 files changed, 7 insertions(+), 197 deletions(-)
delete mode 100644 appliance/guestfs_lvm_conf.aug
delete mode 100644 appliance/guestfs_shadow.aug
diff --git a/README b/README
index 30e241a..2e7930b 100644
--- a/README
+++ b/README
@@ -104,7 +104,7 @@ The full requirements are described below.
+--------------+-------------+---+-----------------------------------------+
| libxml2 | | R | Popular XML library. |
+--------------+-------------+---+-----------------------------------------+
-| augeas | 1.0.0 | R | |
+| augeas | 1.2.0 | R | |
+--------------+-------------+---+-----------------------------------------+
| xz | | R | Used to compress disk images. |
| | | | Used by virt-builder for compression. |
diff --git a/appliance/Makefile.am b/appliance/Makefile.am
index 7b30bbe..ebc526e 100644
--- a/appliance/Makefile.am
+++ b/appliance/Makefile.am
@@ -21,8 +21,6 @@ EXTRA_DIST = \
99-guestfs-serial.rules \
excludefiles.in \
guestfsd.suppressions \
- guestfs_lvm_conf.aug \
- guestfs_shadow.aug \
hostfiles.in \
init \
libguestfs-make-fixed-appliance.in \
@@ -76,14 +74,12 @@ packagelist: packagelist.in Makefile
cmp -s $@ $@-t || mv $@-t $@
rm -f $@-t
-supermin.d/daemon.tar.gz: ../daemon/guestfsd guestfsd.suppressions
guestfs_lvm_conf.aug guestfs_shadow.aug
+supermin.d/daemon.tar.gz: ../daemon/guestfsd guestfsd.suppressions
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)/guestfsd.suppressions tmp-d/etc/guestfsd.suppressions
- 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/appliance/hostfiles.in b/appliance/hostfiles.in
index 8ff53b5..01a52ae 100644
--- a/appliance/hostfiles.in
+++ b/appliance/hostfiles.in
@@ -14,4 +14,3 @@ dnl FRUGALWARE=1 For Frugalware.
dnl MAGEIA=1 For Mageia.
/lib/lsb/*
-/usr/share/augeas/lenses/*.aug
diff --git a/configure.ac b/configure.ac
index e2ee946..2682358 100644
--- a/configure.ac
+++ b/configure.ac
@@ -899,6 +899,9 @@ dnl Check for PCRE (required)
PKG_CHECK_MODULES([PCRE], [libpcre])
dnl Check for Augeas >= 1.0.0 (required).
+dnl In reality, various things will fail unless you have Augeas 1.2.0
+dnl some upstream but not released patches. However don't fail to
+dnl compile just because this is not satisfied.
PKG_CHECK_MODULES([AUGEAS],[augeas >= 1.0.0])
dnl libmagic (highly recommended)
diff --git a/daemon/augeas.c b/daemon/augeas.c
index ce49726..9c8bbcc 100644
--- a/daemon/augeas.c
+++ b/daemon/augeas.c
@@ -134,7 +134,7 @@ do_aug_init (const char *root, int flags)
}
/* Pass AUG_NO_ERR_CLOSE so we can display detailed errors. */
- aug = aug_init (buf, "/usr/share/guestfs/", flags |
AUG_NO_ERR_CLOSE);
+ aug = aug_init (buf, NULL, flags | AUG_NO_ERR_CLOSE);
if (!aug) {
reply_with_error ("augeas initialization failed");
@@ -148,25 +148,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 0ccbc9e..95227d6 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -232,17 +232,6 @@ extern void wipe_device_before_mkfs (const char *device);
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 72fe6ac..d119f9e 100644
--- a/daemon/lvm-filter.c
+++ b/daemon/lvm-filter.c
@@ -122,7 +122,7 @@ set_filter (char *const *filters)
* but do that only after having applied the transformation.
*/
const int flags = AUG_NO_ERR_CLOSE | AUG_NO_LOAD;
- aug = aug_init (lvm_system_dir, "/usr/share/guestfs/", flags);
+ aug = aug_init (lvm_system_dir, NULL, flags);
if (!aug) {
reply_with_error ("augeas initialization failed");
return -1;
@@ -133,18 +133,6 @@ set_filter (char *const *filters)
return -1;
}
- r = aug_transform (aug, "guestfs_lvm_conf",
"/lvm/lvm.conf",
- 0 /* = included */);
- if (r == -1) {
- AUGEAS_ERROR ("aug_transform");
- return -1;
- }
-
- if (aug_load (aug) == -1) {
- AUGEAS_ERROR ("aug_load");
- return -1;
- }
-
/* Remove all the old filters ... */
r = aug_rm (aug, "/files/lvm/lvm.conf/devices/dict/filter/list/*");
if (r == -1) {
--
2.0.4
Pino Toscano
2014-Oct-24 15:09 UTC
Re: [Libguestfs] [PATCH] daemon: Remove custom Augeas lenses.
On Friday 24 October 2014 16:00:15 Richard W.M. Jones wrote:> Don't carry around Augeas lenses. It is fragile, since if the lens is > added to upstream Augeas but the version number has not changed, then > Augeas won't parse the target file at all. This specifically causes > password adjustments to fail in RHEL 7.1.I disagree with this patch, in the current situation. The two lenses we carry have not been part of any Augeas release (last is 1.2.0, done months ago), thus this would break the password changing and the user removal from /etc/shadow, and the LVM filtering. What has broken in RHEL 7.1 is the local addition of the lenses, and thus what's needed to fix it is the local disabling of our lenses. Patch attached for it. This patch could be okay for me (modulo change below) once Augeas >= 1.2.1 is released.> diff --git a/daemon/lvm-filter.c b/daemon/lvm-filter.c > index 72fe6ac..d119f9e 100644 > --- a/daemon/lvm-filter.c > +++ b/daemon/lvm-filter.c > @@ -133,18 +133,6 @@ set_filter (char *const *filters) > return -1; > } > > - r = aug_transform (aug, "guestfs_lvm_conf", "/lvm/lvm.conf", > - 0 /* = included */); > - if (r == -1) { > - AUGEAS_ERROR ("aug_transform"); > - return -1; > - } > - > - if (aug_load (aug) == -1) { > - AUGEAS_ERROR ("aug_load"); > - return -1; > - } > - > /* Remove all the old filters ... */ > r = aug_rm (aug, "/files/lvm/lvm.conf/devices/dict/filter/list/*"); > if (r == -1) {Note this part is needed, since within the local lvm config tree the configuration file is /lvm/lvm.conf, thus the transform is needed (see the change in the patch). -- Pino Toscano
Richard W.M. Jones
2014-Oct-24 15:25 UTC
Re: [Libguestfs] [PATCH] daemon: Remove custom Augeas lenses.
On Fri, Oct 24, 2014 at 05:09:19PM +0200, Pino Toscano wrote:> On Friday 24 October 2014 16:00:15 Richard W.M. Jones wrote: > > Don't carry around Augeas lenses. It is fragile, since if the lens is > > added to upstream Augeas but the version number has not changed, then > > Augeas won't parse the target file at all. This specifically causes > > password adjustments to fail in RHEL 7.1. > > I disagree with this patch, in the current situation. > The two lenses we carry have not been part of any Augeas release (last > is 1.2.0, done months ago), thus this would break the password changing > and the user removal from /etc/shadow, and the LVM filtering.Why don't we carry the Augeas lenses downstream in Fedora until Augeas 1.2.<whatever> is out? I'm also sending an email to augeas-devel to encourage them to make an upstream release. Rich.> What has broken in RHEL 7.1 is the local addition of the lenses, and > thus what's needed to fix it is the local disabling of our lenses. Patch > attached for it. > > This patch could be okay for me (modulo change below) once Augeas >= > 1.2.1 is released. > > > diff --git a/daemon/lvm-filter.c b/daemon/lvm-filter.c > > index 72fe6ac..d119f9e 100644 > > --- a/daemon/lvm-filter.c > > +++ b/daemon/lvm-filter.c > > @@ -133,18 +133,6 @@ set_filter (char *const *filters) > > return -1; > > } > > > > - r = aug_transform (aug, "guestfs_lvm_conf", "/lvm/lvm.conf", > > - 0 /* = included */); > > - if (r == -1) { > > - AUGEAS_ERROR ("aug_transform"); > > - return -1; > > - } > > - > > - if (aug_load (aug) == -1) { > > - AUGEAS_ERROR ("aug_load"); > > - return -1; > > - } > > - > > /* Remove all the old filters ... */ > > r = aug_rm (aug, "/files/lvm/lvm.conf/devices/dict/filter/list/*"); > > if (r == -1) { > > Note this part is needed, since within the local lvm config tree the > configuration file is /lvm/lvm.conf, thus the transform is needed (see > the change in the patch). > > -- > Pino Toscano> diff --git a/appliance/Makefile.am b/appliance/Makefile.am > index 7b30bbe..c7eca20 100644 > --- a/appliance/Makefile.am > +++ b/appliance/Makefile.am > @@ -82,8 +82,6 @@ supermin.d/daemon.tar.gz: ../daemon/guestfsd guestfsd.suppressions guestfs_lvm_c > 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)/guestfsd.suppressions tmp-d/etc/guestfsd.suppressions > - 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/daemon/augeas.c b/daemon/augeas.c > index ce49726..7753e67 100644 > --- a/daemon/augeas.c > +++ b/daemon/augeas.c > @@ -134,7 +134,7 @@ do_aug_init (const char *root, int flags) > } > > /* Pass AUG_NO_ERR_CLOSE so we can display detailed errors. */ > - aug = aug_init (buf, "/usr/share/guestfs/", flags | AUG_NO_ERR_CLOSE); > + aug = aug_init (buf, NULL, flags | AUG_NO_ERR_CLOSE); > > if (!aug) { > reply_with_error ("augeas initialization failed"); > @@ -148,6 +148,8 @@ do_aug_init (const char *root, int flags) > return -1; > } > > + /* We already have the needed lenses in RHEL 7 */ > +#if 0 > if (!augeas_is_version (1, 2, 1)) { > int r = aug_transform (aug, "guestfs_shadow", "/etc/shadow", > 0 /* = included */); > @@ -166,6 +168,7 @@ do_aug_init (const char *root, int flags) > } > } > } > +#endif > > return 0; > } > diff --git a/daemon/lvm-filter.c b/daemon/lvm-filter.c > index 72fe6ac..d1e6466 100644 > --- a/daemon/lvm-filter.c > +++ b/daemon/lvm-filter.c > @@ -122,7 +122,7 @@ set_filter (char *const *filters) > * but do that only after having applied the transformation. > */ > const int flags = AUG_NO_ERR_CLOSE | AUG_NO_LOAD; > - aug = aug_init (lvm_system_dir, "/usr/share/guestfs/", flags); > + aug = aug_init (lvm_system_dir, NULL, flags); > if (!aug) { > reply_with_error ("augeas initialization failed"); > return -1; > @@ -133,7 +133,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");> _______________________________________________ > 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 Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW