On 10/07/2015 12:26 PM, Laine Stump wrote:
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1269613
The original augeas search term used by netcf to find, for example, all the ifcfg files associated with device "br1" was:
"/files/etc/sysconfig/network-scripts/*[ " "DEVICE = 'br1' or BRIDGE = 'br1' or MASTER = 'br1' or MASTER = " "../*[BRIDGE = 'br1']/DEVICE ]/DEVICE"
This is *extremely* inefficient - on a test host with 514 host bridges, each with an attached vlan interface, a dumpxml of all toplevel interfaces took 6m40s (*after* installing an augeas that included augeas upstream commits a659f09a, 41e989ca, and 23d5e480 which were all pushed after the augeas-1.4.0 release).
In these two messages:
https://www.redhat.com/archives/augeas-devel/2015-October/msg00003.html https://www.redhat.com/archives/augeas-devel/2015-October/msg00004.html
David Lutterkort suggested changing the search term to:
"(/files/etc/sysconfig/network-scripts/*[(DEVICE|BRIDGE|MASTER) = 'br1']" "|/files/etc/sysconfig/network-scripts/*[MASTER]" "[MASTER = ../*[BRIDGE = 'br1']/DEVICE ])/DEVICE
I think I can see the equivalency between the two expressions, and also why refactoring the conditionals to favor | over [or] could have an efficiency impact. So all that remains is to see if the code change matches the commit message...
+static +int aug_all_related_ifcfgs(struct netcf *ncf, char ***matches, const char *name) {
- int nmatches;
- /* this includes the ifcfg files for:
*
* 1) the named interface itself (DEVICE=$name)
*
* 2) any interface naming $name as a bridge it is attached to
* (BRIDGE=$name)
*
* 3) any interface naming $name as the master of a bond it is
* enslaved to (MASTER=$name)
*
* 4) any interface with a MASTER, where the device named as
* MASTER contains a BRIDGE=$name *and* DEVICE=$itself (thus
* catching ethernet devices that are enslaved to a bond that
* is attached to a bridge).
*/
This comment helps.
- nmatches = aug_fmt_match(ncf, matches,
"(%s[(DEVICE|BRIDGE|MASTER) = '%s']"
"|%s[MASTER][MASTER = ../*[BRIDGE = '%s']/DEVICE "
"])/DEVICE",
ifcfg_path, name, ifcfg_path, name);
...and that indeed matches the new form in the commit message,
- nmatches = aug_fmt_match(ncf, NULL,
"%s[ DEVICE = '%s'"
" or BRIDGE = '%s'"
" or MASTER = '%s'"
" or MASTER = ../*[BRIDGE = '%s']/DEVICE ]/DEVICE",
ifcfg_path, name, name, name, name);
- nmatches = aug_all_related_ifcfgs(ncf, NULL, name);
...replacing the old form.
ACK. Nice speedup! Thanks David for the insights.