[Libguestfs] [PATCH] daemon: augeas: filter out AUG_NO_STDINC from aug-init (RHBZ#1144927)

[Pino Toscano]

On Monday 22 September 2014 14:09:54 Richard W.M. Jones wrote:
> On Mon, Sep 22, 2014 at 02:56:18PM +0200, Pino Toscano wrote:
> > On Monday 22 September 2014 13:50:18 Richard W.M. Jones wrote:
> > > On Mon, Sep 22, 2014 at 10:49:42AM +0200, Pino Toscano wrote:
> > > > The lenses in our custom path need the system lens for base
> > > > definitions. Disabling the system path was worthless anyway,
> > > > since
> > > > our API does not allow user-specified custom paths.
> > > > 
> > > > The only possible use for AUG_NO_STDINC to aug-init could have
> > > > been
> > > > to not load the lenses right at init time loading them later;
> > > > however, this is what the AUG_NO_LOAD flag (= 32) does already.
> > > > ---
> > > > 
> > > >  daemon/augeas.c | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > > 
> > > > diff --git a/daemon/augeas.c b/daemon/augeas.c
> > > > index ce49726..4f1d9a8 100644
> > > > --- a/daemon/augeas.c
> > > > +++ b/daemon/augeas.c
> > > > @@ -133,6 +133,11 @@ do_aug_init (const char *root, int flags)
> > > > 
> > > >      return -1;
> > > >    
> > > >    }
> > > > 
> > > > +  /* Filter out AUG_NO_STDINC, since the lenses in our custom
> > > > path
> > > > +   * need the lenses from the system path.
> > > > +   */
> > > > +  flags &= ~AUG_NO_STDINC;
> > > 
> > > This is wrong.  It should be possible to specify this, plus it is
> > > specified in the API.
> > 
> > As I wrote it above, specifying that flag serves no purpose at all.
> > I
> > don't see how describing it in API means it will actually be useful,
> > especially with the current aug_* API.

> It's not really clear to me whether setting an entry under
> /augeas/load could be used.

No, it doesn't.

On the other hand, I stand corrected, as we do have a way (although not 
officially recommended) to set environment variables in the daemon, and 
thus set AUGEAS_LENS_LIB. I just doubt anyone actually made use of it, 
though...

> >  In any case ...
> 
> > > The right solution is to get all the required Augeas lenses into
> > > Fedora & RHEL 7.1 (backporting the upstream patches if necessary),
> > > then remove the hacks from libguestfs.
> > 
> > That would imply being incompatible with upstream Augeas, since at
> > least the local LVM lens has changes which are not even accepted
> > upstream (they have been sent though, nobody looked at them yet).
> 
> ... the bigger problem is that Augeas lenses are fragile since they
> depend on essentially internal details of the standard library.  This
> isn't the first time a lens has broken.
> 
> We shouldn't be in the business of shipping lenses with libguestfs.
> Augeas can ship them and deal with the problems of consistency between
> lenses.

Well, we are not the only one shipping lenses outside of Augeas -- at 
least on my system I see abrt and libvirt shipping lenses. So if they 
decide to break third-party lenses, it's just bad for them

> If they don't review patches fast enough we should encourage
> them to review faster.
> 
> What Augeas patch(es) still need review?  I'll see if Dominic or David
> can expedite the process for us.

- guestfs_lvm_conf.aug: https://github.com/hercules-team/augeas/pull/155
- guestfs_shadow.aug: already upstream, not been any upstream release
  after its inclusion

-- 
Pino Toscano