From 396e4e0698d9fb542f2eb8b32790a069e1c0df61 Mon Sep 17 00:00:00 2001
From: Laine Stump <laine@laine.org>
Date: Wed, 7 Oct 2015 13:49:45 -0400
Subject: [PATCH] optimize aug_match() query for all ifcfg files related to an
 interface

This resolves:

 https://bugzilla.redhat.com/show_bug.cgi?id=1271341 (Fedora)
 https://bugzilla.redhat.com/show_bug.cgi?id=1269613 (RHEL7)

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

That's what this patch does. Testing shows that it is functionally
equivalent, and reduces the dumpxml time in the previously described
test from 6m40s down to 17 seconds.
---
 src/drv_redhat.c | 44 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/src/drv_redhat.c b/src/drv_redhat.c
index 4935f98..092ef5c 100644
--- a/src/drv_redhat.c
+++ b/src/drv_redhat.c
@@ -88,6 +88,38 @@ static const struct augeas_xfm_table augeas_xfm_common =
     { .size = ARRAY_CARDINALITY(augeas_xfm_common_pv),
       .pv = augeas_xfm_common_pv };
 
+/* aug_all_related_ifcfgs() - return the count of (and optionally a list
+ * of, if matches != NULL) the paths for all ifcfg files that are
+ * related to the interface "name".
+ */
+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).
+     */
+    nmatches = aug_fmt_match(ncf, matches,
+                             "(%s[(DEVICE|BRIDGE|MASTER) = '%s']"
+                             "|%s[MASTER][MASTER = ../*[BRIDGE = '%s']/DEVICE "
+                             "])/DEVICE",
+                             ifcfg_path, name, ifcfg_path, name);
+    return nmatches;
+
+}
+
 /* Entries in a ifcfg file that tell us that the interface
  * is not a toplevel interface
  */
@@ -108,12 +140,7 @@ static int is_slave(struct netcf *ncf, const char *intf) {
 static bool has_ifcfg_file(struct netcf *ncf, const char *name) {
     int nmatches;
 
-    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);
     return nmatches > 0;
 }
 
@@ -588,10 +615,7 @@ static xmlDocPtr aug_get_xml_for_nif(struct netcf_if *nif) {
     int ndevs = 0, nint = 0;
 
     ncf = nif->ncf;
-    ndevs = aug_fmt_match(ncf, &devs,
-              "%s[ DEVICE = '%s' or BRIDGE = '%s' or MASTER = '%s'"
-              "    or MASTER = ../*[BRIDGE = '%s']/DEVICE ]/DEVICE",
-              ifcfg_path, nif->name, nif->name, nif->name, nif->name);
+    ndevs = aug_all_related_ifcfgs(ncf, &devs, nif->name);
     ERR_BAIL(ncf);
 
     nint = uniq_ifcfg_paths(ncf, ndevs, devs, &intf);
-- 
2.4.3