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@xxxxxxxxxx>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 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list