[netcf-devel] [PATCH] rng: Don't fail validation if child interfaces have extraneous info

Laine Stump laine at laine.org
Mon Jun 28 03:16:37 UTC 2010


  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 at 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) {



More information about the netcf-devel mailing list