[libvirt] Re: [augeas-devel] PATCH: Add a augeas lens for libvirtd.conf

[Daniel P. Berrange]

On Tue, Aug 26, 2008 at 10:37:02PM +0200, Rapha?l Pinson wrote:
> On Tue, Aug 26, 2008 at 10:05 PM, Daniel P. Berrange <berrange at redhat.com>wrote:
> > THis patch is intended to be committed to libvirt, so the config file rules
> > are distributed alongside libvirt. I'm CC'ing augeas-devel for feedback on
> > the lens itself.
> >
> 
> Very nice idea. This might have to be thought about for the future since so
> far we're only adding lenses to the Augeas repository.

My motivation is that we add more config file entries in each release,
so its more sustainable to distribute lens with the corresponding 
upstream tar.gz, than in augeas. IMHO its only worth putting stuff
in augeas if upstream is not willing to accept the lens.

> > +   let str_entry       (kw:string) = [ prespace . key kw . value_sep .
> > str_val . eol ]
> > +   let bool_entry      (kw:string) = [ prespace . key kw . value_sep .
> > bool_val . eol ]
> > +   let str_array_entry (kw:string) = [ prespace . key kw . value_sep .
> > str_array_val . eol ]
> > +
> > +   let network_entry = bool_entry "listen_tls"
> > +                     | bool_entry "listen_tcp"
> > +                     | str_entry "tls_port"
> > +                     | str_entry "tcp_port"
> > +                     | str_entry "listen_addr"
> > +                     | bool_entry "mdns_adv"
> > +                     | str_entry "mdns_name"
> > +
> 
> 
> While I can see why it is useful to gather these entries logically, it's not
> very optimised for the parser. Eventually, all this will be compiled into a
> huge regexp, so it's more efficient to regroup entries by type and feed the
> functions with regexps instead of strings, like

Is performance really that much of a problem ?  The libvirt config file
only has 20 or so different settings, and while we'll add more I can't
see it getting very much larger.  Changing these settings is also not
something that would be done on a frequent basis / performance critical
path.  I find it more readable to group them by functional area unless
there is a serious real world performance issue.


> +   let comment = [ label "comment" . del /#[ \t]*/ "# " .  store /([^
> > \t\n][^\n]*)?/ . del /\n/ "\n" ]
> >
> 
> Some time ago, we thought that it would be good to map comments in
> "#comment" nodes to avoid confusion with possible "comment" nodes,
> especially when the lens allows a wide range of labels. Util.comment would
> fit your need here, and the new version of it would provide a "#comment"
> node.

Thanks, will try that.

> 
> 
> +   let empty = [ label "empty" . del /[ \t]*\n/ "" ]
> 
> 
> Util.empty provides a definition of empty lines, too. Do you really need to
> map them with a label? I thought about doing that some time ago, but mapping
> them in "#empty" for the same reason that we map comments in "#comment". The
> useful application to this is to allow users to control empty lines in
> conffile from the augeas API.

Will switch to Util.empty too.

> > +
> > +   let lns = ( entry | comment | empty ) +
> >
> 
> Is an empty file not valid?

It is - typo left over from testing.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|