On 06/14/2010 05:25 PM, Cole Robinson wrote:
Currently, child interfaces for bonds and bridges are expected to be
specified in a slimmed down 'bare' style, only supplying the required
information like name, MAC, etc. This makes the lives of API users
difficult for a couple reasons:
1) The existing interface XML can not be reused wholesale as a child
interface, it must be processed to remove unneeded information (which
netcf already does during the XSL transform).
2) Any future changes to the allowed values in the 'bare' definition will
require applications to be updated.
This patch changes the RNG to optionally accept unneeded interface
XML for child interfaces, and a few test files to validate the changes.
XSL stylesheets seem to already handle the hard work for us of ignoring
the pieces it does not need.
Now that I've finally gotten the time to check this, I've found that
libvirt actually already strips out the extraneous stuff in the
definition, so this patch is not needed for the purposes of libvirt and
virt-manager (the miimon/arpmon patch you had sent along with this one
seems to make everything work for me)..
Beyond virt-manager and libvirt, netcf itself does not have the
tradition of ignoring extra stuff that gets sent in the xml, so I think
we should leave this it as it is, for consistency's sake.
For that reason, NAK on this patch.
Signed-off-by: Cole Robinson<crobinso(a)redhat.com>
---
data/xml/interface.rng | 30 +++++++++++++++++++++++++
tests/initscripts/bridge-nonbare.xml | 40 ++++++++++++++++++++++++++++++++++
tests/interface/bridge-nonbare.xml | 40 ++++++++++++++++++++++++++++++++++
tests/test-initscripts.c | 15 +++++++++++-
4 files changed, 123 insertions(+), 2 deletions(-)
create mode 100644 tests/initscripts/bridge-nonbare.xml
create mode 100644 tests/interface/bridge-nonbare.xml
diff --git a/data/xml/interface.rng b/data/xml/interface.rng
index 80d686e..aabfc77 100644
--- a/data/xml/interface.rng
+++ b/data/xml/interface.rng
@@ -45,10 +45,20 @@
<!-- Ethernet adapter without IP addressing, e.g. for a bridge -->
<define name="bare-ethernet-interface">
<element name="interface">
+<optional>
+<ref name="startmode"/>
+</optional>
<ref name="basic-ethernet-content"/>
+<optional>
+<ref name="mtu"/>
+</optional>
+<optional>
+<ref name="interface-addressing"/>
+</optional>
</element>
</define>
+<!-- Any change here must update bare-ethernet-interface accordingly -->
<define name="ethernet-interface">
<element name="interface">
<ref name="startmode"/>
@@ -83,10 +93,20 @@
<define name="bare-vlan-interface">
<element name="interface">
<ref name="vlan-interface-common"/>
+<optional>
+<ref name="startmode"/>
+</optional>
+<optional>
+<ref name="mtu"/>
+</optional>
+<optional>
+<ref name="interface-addressing"/>
+</optional>
<ref name="vlan-device"/>
</element>
</define>
+<!-- Any change here must update bare-vlan-interface accordingly -->
<define name="vlan-interface">
<element name="interface">
<ref name="vlan-interface-common"/>
@@ -227,10 +247,20 @@
<define name="bare-bond-interface">
<element name="interface">
<ref name="bond-interface-common"/>
+<optional>
+<ref name="startmode"/>
+</optional>
+<optional>
+<ref name="mtu"/>
+</optional>
+<optional>
+<ref name="interface-addressing"/>
+</optional>
<ref name="bond-element"/>
</element>
</define>
+<!-- Any change here must update bare-bond-interface accordingly -->
<define name="bond-interface">
<element name="interface">
<ref name="bond-interface-common"/>
diff --git a/tests/initscripts/bridge-nonbare.xml b/tests/initscripts/bridge-nonbare.xml
new file mode 100644
index 0000000..67bb89c
--- /dev/null
+++ b/tests/initscripts/bridge-nonbare.xml
@@ -0,0 +1,40 @@
+<?xml version="1.0"?>
+<forest>
+<tree path="/files/etc/sysconfig/network-scripts/ifcfg-br0">
+<node label="DEVICE" value="br0"/>
+<node label="ONBOOT" value="yes"/>
+<node label="TYPE" value="Bridge"/>
+<node label="BOOTPROTO" value="dhcp"/>
+<node label="STP" value="off"/>
+</tree>
+<tree path="/files/etc/sysconfig/network-scripts/ifcfg-eth0.42">
+<node label="DEVICE" value="eth0.42"/>
+<node label="VLAN" value="yes"/>
+<node label="ONBOOT" value="yes"/>
+<node label="BRIDGE" value="br0"/>
+</tree>
+<tree path="/files/etc/sysconfig/network-scripts/ifcfg-bond0">
+<node label="DEVICE" value="bond0"/>
+<node label="ONBOOT" value="yes"/>
+<node label="BONDING_OPTS" value="'mode=active-backup
primary=bondeth0 miimon=100 updelay=10 use_carrier=0'"/>
+<node label="BRIDGE" value="br0"/>
+</tree>
+<tree path="/files/etc/sysconfig/network-scripts/ifcfg-bondeth0">
+<node label="DEVICE" value="bondeth0"/>
+<node label="ONBOOT" value="yes"/>
+<node label="MASTER" value="bond0"/>
+<node label="SLAVE" value="yes"/>
+</tree>
+<tree path="/files/etc/sysconfig/network-scripts/ifcfg-bondeth1">
+<node label="DEVICE" value="bondeth1"/>
+<node label="ONBOOT" value="yes"/>
+<node label="MASTER" value="bond0"/>
+<node label="SLAVE" value="yes"/>
+</tree>
+<tree path="/files/etc/sysconfig/network-scripts/ifcfg-eth0">
+<node label="DEVICE" value="eth0"/>
+<node label="HWADDR" value="aa:bb:cc:dd:ee:ff"/>
+<node label="ONBOOT" value="yes"/>
+<node label="BRIDGE" value="br0"/>
+</tree>
+</forest>
diff --git a/tests/interface/bridge-nonbare.xml b/tests/interface/bridge-nonbare.xml
new file mode 100644
index 0000000..8029b34
--- /dev/null
+++ b/tests/interface/bridge-nonbare.xml
@@ -0,0 +1,40 @@
+<interface type="bridge" name="br0">
+<start mode="onboot"/>
+<protocol family="ipv4">
+<dhcp/>
+</protocol>
+<bridge stp="off">
+<interface type="vlan" name="eth0.42">
+<start mode="onboot"/>
+<protocol family="ipv4">
+<dhcp peerdns="no"/>
+</protocol>
+<vlan tag="42">
+<interface name="eth0"/>
+</vlan>
+</interface>
+
+<interface type="bond" name="bond0">
+<start mode="none"/>
+<protocol family="ipv4">
+<ip address="192.168.50.7" prefix="24"/>
+<route gateway="192.168.50.1"/>
+</protocol>
+<bond mode="active-backup">
+<miimon freq="100" updelay="10" carrier="ioctl"/>
+<interface type="ethernet" name="bondeth0"/>
+<interface type="ethernet" name="bondeth1"/>
+</bond>
+</interface>
+
+<interface type="ethernet" name="eth0">
+<start mode="none"/>
+<mac address="aa:bb:cc:dd:ee:ff"/>
+<mtu size="1492"/>
+<protocol family="ipv4">
+<dhcp peerdns="no"/>
+</protocol>
+</interface>
+
+</bridge>
+</interface>
diff --git a/tests/test-initscripts.c b/tests/test-initscripts.c
index 404ec0a..e557b82 100644
--- a/tests/test-initscripts.c
+++ b/tests/test-initscripts.c
@@ -144,7 +144,8 @@ static void testDefineUndefine(CuTest *tc) {
CuAssertPtrEquals(tc, NULL, nif);
}
-static void assert_transforms(CuTest *tc, const char *base) {
+static void assert_transforms_flags(CuTest *tc, const char *base,
+ unsigned int skip_ncf) {
char *aug_fname = NULL, *ncf_fname = NULL;
char *aug_xml_exp = NULL, *ncf_xml_exp = NULL;
char *aug_xml_act = NULL, *ncf_xml_act = NULL;
@@ -162,7 +163,9 @@ static void assert_transforms(CuTest *tc, const char *base) {
r = ncf_put_aug(ncf, aug_xml_exp,&ncf_xml_act);
CuAssertIntEquals(tc, 0, r);
- assert_xml_equals(tc, ncf_fname, ncf_xml_exp, ncf_xml_act);
+ if (!skip_ncf) {
+ assert_xml_equals(tc, ncf_fname, ncf_xml_exp, ncf_xml_act);
+ }
assert_xml_equals(tc, aug_fname, aug_xml_exp, aug_xml_act);
free(ncf_xml_exp);
@@ -171,6 +174,10 @@ static void assert_transforms(CuTest *tc, const char *base) {
free(aug_xml_act);
}
+static void assert_transforms(CuTest *tc, const char *base) {
+ assert_transforms_flags(tc, base, 0);
+}
+
static void testTransforms(CuTest *tc) {
assert_transforms(tc, "bond");
assert_transforms(tc, "bond-arp");
@@ -191,6 +198,10 @@ static void testTransforms(CuTest *tc) {
assert_transforms(tc, "ipv6-autoconf");
assert_transforms(tc, "ipv6-autoconf-dhcp");
assert_transforms(tc, "ipv6-static-multi");
+
+ /* This test discards unneeded XML elements, so netcf roundtrip won't
+ * match. Skip the netcf check */
+ assert_transforms_flags(tc, "bridge-nonbare", 1);
}
static void testCorruptedSetup(CuTest *tc) {