[PATCH] Properly classify bond devices with no slaves
by Laine Stump
Although initscripts only considers an interface to be a bond if it
has slaves, there are times when setting up a bond, or testing, when a
bond may not have any slaves (yet) but does have a BONDING_OPTS
attribute. Previously in those situations netcf would identify the
interface as a plain ethernet. This patch makes the check more
inclusive - now any interface with slaves *or* with a BONDING_OPTS
attribute is considered to be a bond.
This patch was inspired by an earlier patch sent by Lubomir Rintel
which looked for BONDING_OPTS *instead of* looking for slaves.
---
After asking a couple questions about Lubomir's original patch last
May, I forgot about this until today; I took the time to figure out
how to combine the old and new expressions, and it seems to work well
with example bond definitions having all combinations of with/without
BONDING_OPTS/slaves.
data/xml/redhat-put.xsl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/data/xml/redhat-put.xsl b/data/xml/redhat-put.xsl
index ed56c66..89145e5 100644
--- a/data/xml/redhat-put.xsl
+++ b/data/xml/redhat-put.xsl
@@ -135,7 +135,7 @@
</xsl:template>
<xsl:template name="bond-interface"
- match="tree[node[@label = 'DEVICE'][@value = //tree/node[@label = 'MASTER']/@value]][count(node[@label = 'BRIDGE']) = 0]">
+ match="tree[count(node[@label = 'BONDING_OPTS']) or (node[@label = 'DEVICE'][@value = //tree/node[@label = 'MASTER']/@value])][count(node[@label = 'BRIDGE']) = 0]">
<interface type="bond">
<xsl:call-template name="name-attr"/>
<xsl:call-template name="startmode"/>
--
2.4.3
8 years, 6 months
[PATCH] optimize aug_match() query for all ifcfg files related to an interface
by Laine Stump
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.
---
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
8 years, 6 months