From 0be286b3062fc2ff8718cbbc914eb596506d9fac Mon Sep 17 00:00:00 2001

From: Laine Stump <laine@laine.org>

Date: Wed, 7 Oct 2015 13:49:45 -0400

Subject: [PATCH 2/2] optimize aug_match() query for all ifcfg files related to

 an interface

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

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.

(cherry picked from commit 396e4e0698d9fb542f2eb8b32790a069e1c0df61)

---

 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;;

-- 

1.8.3.1