The interface xml schema was written with strict rules about the ordering of the elements. This was never intentional, but just due to omission of <interleave> in the appropriate places. This patch just adds in <interleave> wherever there is more than one element, and re-indents everything else appropriately. --- docs/schemas/interface.rng | 290 ++++++++++++++++++++++++--------------------- 1 file changed, 156 insertions(+), 134 deletions(-) diff --git a/docs/schemas/interface.rng b/docs/schemas/interface.rng index 8e2218d..27610a5 100644 --- a/docs/schemas/interface.rng +++ b/docs/schemas/interface.rng @@ -29,35 +29,41 @@ Ethernet adapter --> <define name="basic-ethernet-content"> - <attribute name="type"> - <value>ethernet</value> - </attribute> - <ref name="name-attr"/> - <!-- If no MAC is given when the interface is defined, it is determined - by using the device name. - FIXME: What if device name and MAC don't specify the same NIC ? --> - <optional> - <element name="mac"> - <attribute name="address"><ref name="macAddr"/></attribute> - </element> - </optional> - <ref name="link-speed-state"/> - <!-- FIXME: Allow (some) ethtool options --> + <interleave> + <attribute name="type"> + <value>ethernet</value> + </attribute> + <ref name="name-attr"/> + <!-- If no MAC is given when the interface is defined, it is determined + by using the device name. + FIXME: What if device name and MAC don't specify the same NIC ? --> + <optional> + <element name="mac"> + <attribute name="address"><ref name="macAddr"/></attribute> + </element> + </optional> + <ref name="link-speed-state"/> + <!-- FIXME: Allow (some) ethtool options --> + </interleave> </define> <!-- Ethernet adapter without IP addressing, e.g. for a bridge --> <define name="bare-ethernet-interface"> <element name="interface"> - <ref name="basic-ethernet-content"/> + <interleave> + <ref name="basic-ethernet-content"/> + </interleave> </element> </define> <define name="ethernet-interface"> <element name="interface"> - <ref name="startmode"/> - <ref name="basic-ethernet-content"/> - <ref name="mtu"/> - <ref name="interface-addressing"/> + <interleave> + <ref name="startmode"/> + <ref name="basic-ethernet-content"/> + <ref name="mtu"/> + <ref name="interface-addressing"/> + </interleave> </element> </define> @@ -85,18 +91,22 @@ <define name="bare-vlan-interface"> <element name="interface"> - <ref name="vlan-interface-common"/> - <ref name="vlan-device"/> + <interleave> + <ref name="vlan-interface-common"/> + <ref name="vlan-device"/> + </interleave> </element> </define> <define name="vlan-interface"> <element name="interface"> - <ref name="vlan-interface-common"/> - <ref name="startmode"/> - <ref name="mtu"/> - <ref name="interface-addressing"/> - <ref name="vlan-device"/> + <interleave> + <ref name="vlan-interface-common"/> + <ref name="startmode"/> + <ref name="mtu"/> + <ref name="interface-addressing"/> + <ref name="vlan-device"/> + </interleave> </element> </define> @@ -105,31 +115,33 @@ --> <define name="bridge-interface"> <element name="interface"> - <attribute name="type"> - <value>bridge</value> - </attribute> - <ref name="name-attr"/> - <ref name="startmode"/> - <ref name="mtu"/> - <ref name="interface-addressing"/> - <element name="bridge"> - <optional> - <attribute name="stp"> - <ref name="on-or-off"/> - </attribute> - </optional> - <!-- Bridge forward delay (see 'brctl setfd') --> - <optional v:since="2"> - <attribute name="delay"><ref name="timeval"/></attribute> - </optional> - <zeroOrMore> - <choice> - <ref name="bare-ethernet-interface"/> - <ref name="bare-vlan-interface"/> - <ref v:since="2" name="bare-bond-interface"/> - </choice> - </zeroOrMore> - </element> + <interleave> + <attribute name="type"> + <value>bridge</value> + </attribute> + <ref name="name-attr"/> + <ref name="startmode"/> + <ref name="mtu"/> + <ref name="interface-addressing"/> + <element name="bridge"> + <optional> + <attribute name="stp"> + <ref name="on-or-off"/> + </attribute> + </optional> + <!-- Bridge forward delay (see 'brctl setfd') --> + <optional v:since="2"> + <attribute name="delay"><ref name="timeval"/></attribute> + </optional> + <zeroOrMore> + <choice> + <ref name="bare-ethernet-interface"/> + <ref name="bare-vlan-interface"/> + <ref v:since="2" name="bare-bond-interface"/> + </choice> + </zeroOrMore> + </element> + </interleave> </element> </define> <!-- Jim Fehlig would like support for other bridge attributes, in @@ -180,67 +192,73 @@ xmit_hash_policy (since 2.6.3/3.2.2) --> - <optional> - <choice> - <element name="miimon"> - <!-- miimon frequency in ms --> - <attribute name="freq"><ref name="unsignedInt"/></attribute> - <optional> - <attribute name="downdelay"><ref name="unsignedInt"/></attribute> - </optional> - <optional> - <attribute name="updelay"><ref name="unsignedInt"/></attribute> - </optional> - <optional> - <!-- use_carrier --> - <attribute name="carrier"> - <choice> - <!-- use MII/ETHTOOL ioctl --> - <value>ioctl</value> - <!-- use netif_carrier_ok() --> - <value>netif</value> - </choice> - </attribute> - </optional> - </element> - <element name="arpmon"> - <attribute name="interval"><ref name="unsignedInt"/></attribute> - <attribute name="target"><ref name="ipv4Addr"/></attribute> - <optional> - <attribute name="validate"> - <choice> - <value>none</value> - <value>active</value> - <value>backup</value> - <value>all</value> - </choice> - </attribute> - </optional> - </element> - </choice> - </optional> + <interleave> + <optional> + <choice> + <element name="miimon"> + <!-- miimon frequency in ms --> + <attribute name="freq"><ref name="unsignedInt"/></attribute> + <optional> + <attribute name="downdelay"><ref name="unsignedInt"/></attribute> + </optional> + <optional> + <attribute name="updelay"><ref name="unsignedInt"/></attribute> + </optional> + <optional> + <!-- use_carrier --> + <attribute name="carrier"> + <choice> + <!-- use MII/ETHTOOL ioctl --> + <value>ioctl</value> + <!-- use netif_carrier_ok() --> + <value>netif</value> + </choice> + </attribute> + </optional> + </element> + <element name="arpmon"> + <attribute name="interval"><ref name="unsignedInt"/></attribute> + <attribute name="target"><ref name="ipv4Addr"/></attribute> + <optional> + <attribute name="validate"> + <choice> + <value>none</value> + <value>active</value> + <value>backup</value> + <value>all</value> + </choice> + </attribute> + </optional> + </element> + </choice> + </optional> - <oneOrMore> - <!-- The slave interfaces --> - <ref name="bare-ethernet-interface"/> - </oneOrMore> + <oneOrMore> + <!-- The slave interfaces --> + <ref name="bare-ethernet-interface"/> + </oneOrMore> + </interleave> </element> </define> <define name="bare-bond-interface"> <element name="interface"> - <ref name="bond-interface-common"/> - <ref name="bond-element"/> + <interleave> + <ref name="bond-interface-common"/> + <ref name="bond-element"/> + </interleave> </element> </define> <define name="bond-interface"> <element name="interface"> - <ref name="bond-interface-common"/> - <ref name="startmode"/> - <ref name="mtu"/> - <ref name="interface-addressing"/> - <ref name="bond-element"/> + <interleave> + <ref name="bond-interface-common"/> + <ref name="startmode"/> + <ref name="mtu"/> + <ref name="interface-addressing"/> + <ref name="bond-element"/> + </interleave> </element> </define> @@ -302,22 +320,24 @@ <attribute name="family"> <value>ipv4</value> </attribute> - <choice> - <ref name="dhcp-element"/> - <group> - <element name="ip"> - <attribute name="address"><ref name="ipv4Addr"/></attribute> + <interleave> + <choice> + <ref name="dhcp-element"/> + <group> + <element name="ip"> + <attribute name="address"><ref name="ipv4Addr"/></attribute> + <optional> + <attribute name="prefix"><ref name="ipv4Prefix"/></attribute> + </optional> + </element> <optional> - <attribute name="prefix"><ref name="ipv4Prefix"/></attribute> + <element name="route"> + <attribute name="gateway"><ref name="ipv4Addr"/></attribute> + </element> </optional> - </element> - <optional> - <element name="route"> - <attribute name="gateway"><ref name="ipv4Addr"/></attribute> - </element> - </optional> - </group> - </choice> + </group> + </choice> + </interleave> </element> </define> @@ -326,25 +346,27 @@ <attribute name="family"> <value>ipv6</value> </attribute> - <optional> - <element name="autoconf"><empty/></element> - </optional> - <optional> - <ref name="dhcp-element"/> - </optional> - <zeroOrMore> - <element name="ip"> - <attribute name="address"><ref name="ipv6Addr"/></attribute> - <optional> - <attribute name="prefix"><ref name="ipv6Prefix"/></attribute> - </optional> - </element> - </zeroOrMore> - <optional> - <element name="route"> - <attribute name="gateway"><ref name="ipv6Addr"/></attribute> - </element> - </optional> + <interleave> + <optional> + <element name="autoconf"><empty/></element> + </optional> + <optional> + <ref name="dhcp-element"/> + </optional> + <zeroOrMore> + <element name="ip"> + <attribute name="address"><ref name="ipv6Addr"/></attribute> + <optional> + <attribute name="prefix"><ref name="ipv6Prefix"/></attribute> + </optional> + </element> + </zeroOrMore> + <optional> + <element name="route"> + <attribute name="gateway"><ref name="ipv6Addr"/></attribute> + </element> + </optional> + </interleave> </element> </define> -- 1.9.3

The interface state for bonds and vlans does seem to reflect the state of the underlying physical devices, at least in some cases, so it makes sense to allow reporting it (netcf now does). The link state/speed for bridge devices is meaningless though, so we don't even look for it. --- docs/schemas/interface.rng | 2 ++ src/conf/interface_conf.c | 18 ++++++++++++------ tests/interfaceschemadata/bond.xml | 1 + tests/interfaceschemadata/vlan.xml | 1 + 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/docs/schemas/interface.rng b/docs/schemas/interface.rng index 27610a5..80962d4 100644 --- a/docs/schemas/interface.rng +++ b/docs/schemas/interface.rng @@ -78,6 +78,7 @@ of the form DEVICE.VLAN --> <optional><ref name="name-attr"/></optional> + <ref name="link-speed-state"/> </define> <define name="vlan-device"> @@ -156,6 +157,7 @@ <value>bond</value> </attribute> <ref name="name-attr"/> + <ref name="link-speed-state"/> </define> <define name="bond-element"> diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 2b3f699..397920b 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -669,6 +669,8 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt, int parentIfType) int type; char *tmp; xmlNodePtr cur = ctxt->node; + xmlNodePtr lnk; + /* check @type */ tmp = virXPathString("string(./@type)", ctxt); @@ -704,20 +706,22 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt, int parentIfType) goto error; } def->type = type; + + if (type != VIR_INTERFACE_TYPE_BRIDGE) { + /* link status makes no sense for a bridge */ + lnk = virXPathNode("./link", ctxt); + if (lnk && virInterfaceLinkParseXML(lnk, &def->lnk) < 0) + goto error; + } + switch (type) { case VIR_INTERFACE_TYPE_ETHERNET: { - xmlNodePtr lnk; - if (virInterfaceDefParseName(def, ctxt) < 0) goto error; tmp = virXPathString("string(./mac/@address)", ctxt); if (tmp != NULL) def->mac = tmp; - lnk = virXPathNode("./link", ctxt); - if (lnk && virInterfaceLinkParseXML(lnk, &def->lnk) < 0) - goto error; - if (parentIfType == VIR_INTERFACE_TYPE_LAST) { /* only recognize these in toplevel bond interfaces */ if (virInterfaceDefParseStartMode(def, ctxt) < 0) @@ -1110,6 +1114,7 @@ virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDef *def) break; case VIR_INTERFACE_TYPE_BOND: virInterfaceStartmodeDefFormat(buf, def->startmode); + virInterfaceLinkFormat(buf, &def->lnk); if (def->mtu != 0) virBufferAsprintf(buf, "<mtu size='%d'/>\n", def->mtu); virInterfaceProtocolDefFormat(buf, def); @@ -1119,6 +1124,7 @@ virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDef *def) virInterfaceStartmodeDefFormat(buf, def->startmode); if (def->mac != NULL) virBufferAsprintf(buf, "<mac address='%s'/>\n", def->mac); + virInterfaceLinkFormat(buf, &def->lnk); if (def->mtu != 0) virBufferAsprintf(buf, "<mtu size='%d'/>\n", def->mtu); virInterfaceProtocolDefFormat(buf, def); diff --git a/tests/interfaceschemadata/bond.xml b/tests/interfaceschemadata/bond.xml index c4e6d40..cbc1dfa 100644 --- a/tests/interfaceschemadata/bond.xml +++ b/tests/interfaceschemadata/bond.xml @@ -1,5 +1,6 @@ <interface type='bond' name='bond0'> <start mode='none'/> + <link speed='1000' state='up'/> <protocol family='ipv4'> <ip address='192.168.50.7' prefix='24'/> <route gateway='192.168.50.1'/> diff --git a/tests/interfaceschemadata/vlan.xml b/tests/interfaceschemadata/vlan.xml index a9570e3..6432b96 100644 --- a/tests/interfaceschemadata/vlan.xml +++ b/tests/interfaceschemadata/vlan.xml @@ -1,5 +1,6 @@ <interface type='vlan' name='eth0.42'> <start mode='onboot'/> + <link state='lowerlayerdown'/> <protocol family='ipv4'> <dhcp peerdns='no'/> </protocol> -- 1.9.3

For some reason the bridge stp mode and delay were put directly into the "bridge" case of the switch in virInterfaceDefParseXML(), although they are inside the <bridge> element, and so should be parsed in the function created for that purpose - virInterfaceBridgeDefFormat(). --- src/conf/interface_conf.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 397920b..c1a089a 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -487,11 +487,27 @@ virInterfaceDefParseBridge(virInterfaceDefPtr def, xmlNodePtr *interfaces = NULL; xmlNodePtr bridge; virInterfaceDefPtr itf; + char *tmp = NULL; int nbItf; size_t i; int ret = 0; bridge = ctxt->node; + def->data.bridge.stp = -1; + if ((tmp = virXMLPropString(bridge, "stp"))) { + if (STREQ(tmp, "on")) { + def->data.bridge.stp = 1; + } else if (STREQ(tmp, "off")) { + def->data.bridge.stp = 0; + } else { + virReportError(VIR_ERR_XML_ERROR, + _("bridge interface stp should be on or off got %s"), + tmp); + goto error; + } + } + def->data.bridge.delay = virXMLPropString(bridge, "delay"); + nbItf = virXPathNodeSet("./interface", ctxt, &interfaces); if (nbItf < 0) { ret = -1; @@ -517,6 +533,7 @@ virInterfaceDefParseBridge(virInterfaceDefPtr def, } error: + VIR_FREE(tmp); VIR_FREE(interfaces); ctxt->node = bridge; return ret; @@ -751,23 +768,6 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt, int parentIfType) "%s", _("bridge interface misses the bridge element")); goto error; } - tmp = virXMLPropString(bridge, "stp"); - def->data.bridge.stp = -1; - if (tmp != NULL) { - if (STREQ(tmp, "on")) { - def->data.bridge.stp = 1; - } else if (STREQ(tmp, "off")) { - def->data.bridge.stp = 0; - } else { - virReportError(VIR_ERR_XML_ERROR, - _("bridge interface stp should be on or off got %s"), - tmp); - VIR_FREE(tmp); - goto error; - } - VIR_FREE(tmp); - } - def->data.bridge.delay = virXMLPropString(bridge, "delay"); ctxt->node = bridge; if (virInterfaceDefParseBridge(def, ctxt) < 0) goto error; -- 1.9.3

the switch cases for the 4 different interface types had repetitive code which has now been pulled out as common. While touching those lines, some extra usage of "!= NULL" etc has been eliminated to make things more compact and inline with current coding practices. NB: parentIfType == VIR_INTERFACE_TYPE_LAST means that this is a toplevel interface (not a subordinate of a bridge or bond). Only toplevel interfaces can have a start mode, mtu, or IP address element. --- src/conf/interface_conf.c | 68 +++++++++++++---------------------------------- 1 file changed, 18 insertions(+), 50 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index c1a089a..883053f 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -724,6 +724,19 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt, int parentIfType) } def->type = type; + if (virInterfaceDefParseName(def, ctxt) < 0) + goto error; + + if (parentIfType == VIR_INTERFACE_TYPE_LAST) { + /* only recognize these in toplevel bond interfaces */ + if (virInterfaceDefParseStartMode(def, ctxt) < 0) + goto error; + if (virInterfaceDefParseMtu(def, ctxt) < 0) + goto error; + if (virInterfaceDefParseIfAdressing(def, ctxt) < 0) + goto error; + } + if (type != VIR_INTERFACE_TYPE_BRIDGE) { /* link status makes no sense for a bridge */ lnk = virXPathNode("./link", ctxt); @@ -732,38 +745,14 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt, int parentIfType) } switch (type) { - case VIR_INTERFACE_TYPE_ETHERNET: { - if (virInterfaceDefParseName(def, ctxt) < 0) - goto error; - tmp = virXPathString("string(./mac/@address)", ctxt); - if (tmp != NULL) + case VIR_INTERFACE_TYPE_ETHERNET: + if ((tmp = virXPathString("string(./mac/@address)", ctxt))) def->mac = tmp; - - if (parentIfType == VIR_INTERFACE_TYPE_LAST) { - /* only recognize these in toplevel bond interfaces */ - if (virInterfaceDefParseStartMode(def, ctxt) < 0) - goto error; - if (virInterfaceDefParseMtu(def, ctxt) < 0) - goto error; - if (virInterfaceDefParseIfAdressing(def, ctxt) < 0) - goto error; - } break; - } case VIR_INTERFACE_TYPE_BRIDGE: { xmlNodePtr bridge; - if (virInterfaceDefParseName(def, ctxt) < 0) - goto error; - if (virInterfaceDefParseStartMode(def, ctxt) < 0) - goto error; - if (virInterfaceDefParseMtu(def, ctxt) < 0) - goto error; - if (virInterfaceDefParseIfAdressing(def, ctxt) < 0) - goto error; - - bridge = virXPathNode("./bridge[1]", ctxt); - if (bridge == NULL) { + if (!(bridge = virXPathNode("./bridge[1]", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("bridge interface misses the bridge element")); goto error; @@ -776,20 +765,7 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt, int parentIfType) case VIR_INTERFACE_TYPE_BOND: { xmlNodePtr bond; - if (virInterfaceDefParseName(def, ctxt) < 0) - goto error; - if (parentIfType == VIR_INTERFACE_TYPE_LAST) { - /* only recognize these in toplevel bond interfaces */ - if (virInterfaceDefParseStartMode(def, ctxt) < 0) - goto error; - if (virInterfaceDefParseMtu(def, ctxt) < 0) - goto error; - if (virInterfaceDefParseIfAdressing(def, ctxt) < 0) - goto error; - } - - bond = virXPathNode("./bond[1]", ctxt); - if (bond == NULL) { + if (!(bond = virXPathNode("./bond[1]", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("bond interface misses the bond element")); goto error; @@ -802,15 +778,7 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt, int parentIfType) case VIR_INTERFACE_TYPE_VLAN: { xmlNodePtr vlan; - tmp = virXPathString("string(./@name)", ctxt); - if (tmp != NULL) - def->name = tmp; - if (virInterfaceDefParseStartMode(def, ctxt) < 0) - goto error; - if (virInterfaceDefParseIfAdressing(def, ctxt) < 0) - goto error; - vlan = virXPathNode("./vlan[1]", ctxt); - if (vlan == NULL) { + if (!(vlan = virXPathNode("./vlan[1]", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("vlan interface misses the vlan element")); goto error; -- 1.9.3

This modifies the formatting function of virInterface to be a proper mirror of the parse function, including the addition of a "parentIfType" arg so that we can decide whether or not it is appropriate to emit the elements that are only in toplevel interfaces, as well as the <link> element (which isn't allowed for bridge interfaces). Since the restructuring of the code necessarily changes the order of some of the elements, some test case data had to be updated. --- src/conf/interface_conf.c | 52 ++++++++++++------------- tests/interfaceschemadata/bond.xml | 2 +- tests/interfaceschemadata/bridge-no-address.xml | 2 +- tests/interfaceschemadata/bridge.xml | 2 +- tests/interfaceschemadata/ethernet-dhcp.xml | 4 +- tests/interfaceschemadata/vlan.xml | 2 +- 6 files changed, 31 insertions(+), 33 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 883053f..141b4a2 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -41,7 +41,8 @@ VIR_ENUM_IMPL(virInterface, static virInterfaceDefPtr virInterfaceDefParseXML(xmlXPathContextPtr ctxt, int parentIfType); static int -virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDef *def); +virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDef *def, + virInterfaceType parentIfType); static void virInterfaceIpDefFree(virInterfaceIpDefPtr def) @@ -870,7 +871,8 @@ virInterfaceBridgeDefFormat(virBufferPtr buf, const virInterfaceDef *def) virBufferAdjustIndent(buf, 2); for (i = 0; i < def->data.bridge.nbItf; i++) { - if (virInterfaceDefDevFormat(buf, def->data.bridge.itf[i]) < 0) + if (virInterfaceDefDevFormat(buf, def->data.bridge.itf[i], + VIR_INTERFACE_TYPE_BRIDGE) < 0) ret = -1; } @@ -932,7 +934,8 @@ virInterfaceBondDefFormat(virBufferPtr buf, const virInterfaceDef *def) virBufferAddLit(buf, "/>\n"); } for (i = 0; i < def->data.bond.nbItf; i++) { - if (virInterfaceDefDevFormat(buf, def->data.bond.itf[i]) < 0) + if (virInterfaceDefDevFormat(buf, def->data.bond.itf[i], + VIR_INTERFACE_TYPE_BOND) < 0) ret = -1; } @@ -1035,7 +1038,8 @@ virInterfaceStartmodeDefFormat(virBufferPtr buf, } static int -virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDef *def) +virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDef *def, + virInterfaceType parentIfType) { const char *type = NULL; @@ -1063,39 +1067,33 @@ virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDef *def) virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); + if (parentIfType == VIR_INTERFACE_TYPE_LAST) { + /* these elements are only valid on top-level interfaces - IP + * address info ("protocol") only makes sense for the + * top-level, and subordinate interfaces inherit the toplevel + * setting for mtu and start mode, which cannot be overridden. + */ + virInterfaceStartmodeDefFormat(buf, def->startmode); + if (def->mtu) + virBufferAsprintf(buf, "<mtu size='%d'/>\n", def->mtu); + virInterfaceProtocolDefFormat(buf, def); + } + + if (def->type != VIR_INTERFACE_TYPE_BRIDGE) { + virInterfaceLinkFormat(buf, &def->lnk); + } switch (def->type) { case VIR_INTERFACE_TYPE_ETHERNET: - virInterfaceStartmodeDefFormat(buf, def->startmode); - if (def->mac != NULL) + if (def->mac) virBufferAsprintf(buf, "<mac address='%s'/>\n", def->mac); - virInterfaceLinkFormat(buf, &def->lnk); - if (def->mtu != 0) - virBufferAsprintf(buf, "<mtu size='%d'/>\n", def->mtu); - virInterfaceProtocolDefFormat(buf, def); break; case VIR_INTERFACE_TYPE_BRIDGE: - virInterfaceStartmodeDefFormat(buf, def->startmode); - if (def->mtu != 0) - virBufferAsprintf(buf, "<mtu size='%d'/>\n", def->mtu); - virInterfaceProtocolDefFormat(buf, def); virInterfaceBridgeDefFormat(buf, def); break; case VIR_INTERFACE_TYPE_BOND: - virInterfaceStartmodeDefFormat(buf, def->startmode); - virInterfaceLinkFormat(buf, &def->lnk); - if (def->mtu != 0) - virBufferAsprintf(buf, "<mtu size='%d'/>\n", def->mtu); - virInterfaceProtocolDefFormat(buf, def); virInterfaceBondDefFormat(buf, def); break; case VIR_INTERFACE_TYPE_VLAN: - virInterfaceStartmodeDefFormat(buf, def->startmode); - if (def->mac != NULL) - virBufferAsprintf(buf, "<mac address='%s'/>\n", def->mac); - virInterfaceLinkFormat(buf, &def->lnk); - if (def->mtu != 0) - virBufferAsprintf(buf, "<mtu size='%d'/>\n", def->mtu); - virInterfaceProtocolDefFormat(buf, def); virInterfaceVlanDefFormat(buf, def); break; } @@ -1116,7 +1114,7 @@ char *virInterfaceDefFormat(const virInterfaceDef *def) { virBuffer buf = VIR_BUFFER_INITIALIZER; - if (virInterfaceDefDevFormat(&buf, def) < 0) { + if (virInterfaceDefDevFormat(&buf, def, VIR_INTERFACE_TYPE_LAST) < 0) { virBufferFreeAndReset(&buf); return NULL; } diff --git a/tests/interfaceschemadata/bond.xml b/tests/interfaceschemadata/bond.xml index cbc1dfa..3d24bea 100644 --- a/tests/interfaceschemadata/bond.xml +++ b/tests/interfaceschemadata/bond.xml @@ -1,10 +1,10 @@ <interface type='bond' name='bond0'> <start mode='none'/> - <link speed='1000' state='up'/> <protocol family='ipv4'> <ip address='192.168.50.7' prefix='24'/> <route gateway='192.168.50.1'/> </protocol> + <link speed='1000' state='up'/> <bond mode='active-backup'> <miimon freq='100' updelay='10' carrier='ioctl'/> <interface type='ethernet' name='eth1'> diff --git a/tests/interfaceschemadata/bridge-no-address.xml b/tests/interfaceschemadata/bridge-no-address.xml index 68b8c94..fd6c1a7 100644 --- a/tests/interfaceschemadata/bridge-no-address.xml +++ b/tests/interfaceschemadata/bridge-no-address.xml @@ -3,8 +3,8 @@ <mtu size='1500'/> <bridge stp='off'> <interface type='ethernet' name='eth0'> - <mac address='ab:bb:cc:dd:ee:ff'/> <link speed='1000' state='up'/> + <mac address='ab:bb:cc:dd:ee:ff'/> </interface> <interface type='ethernet' name='eth1'> </interface> diff --git a/tests/interfaceschemadata/bridge.xml b/tests/interfaceschemadata/bridge.xml index c865116..ece087e 100644 --- a/tests/interfaceschemadata/bridge.xml +++ b/tests/interfaceschemadata/bridge.xml @@ -6,8 +6,8 @@ </protocol> <bridge stp='off' delay='0.01'> <interface type='ethernet' name='eth0'> - <mac address='ab:bb:cc:dd:ee:ff'/> <link speed='10'/> + <mac address='ab:bb:cc:dd:ee:ff'/> </interface> <interface type='ethernet' name='eth1'> </interface> diff --git a/tests/interfaceschemadata/ethernet-dhcp.xml b/tests/interfaceschemadata/ethernet-dhcp.xml index c124372..9a8a160 100644 --- a/tests/interfaceschemadata/ethernet-dhcp.xml +++ b/tests/interfaceschemadata/ethernet-dhcp.xml @@ -1,9 +1,9 @@ <interface type='ethernet' name='eth0'> <start mode='none'/> - <mac address='aa:bb:cc:dd:ee:ff'/> - <link state='down'/> <mtu size='1492'/> <protocol family='ipv4'> <dhcp peerdns='no'/> </protocol> + <link state='down'/> + <mac address='aa:bb:cc:dd:ee:ff'/> </interface> diff --git a/tests/interfaceschemadata/vlan.xml b/tests/interfaceschemadata/vlan.xml index 6432b96..3013777 100644 --- a/tests/interfaceschemadata/vlan.xml +++ b/tests/interfaceschemadata/vlan.xml @@ -1,9 +1,9 @@ <interface type='vlan' name='eth0.42'> <start mode='onboot'/> - <link state='lowerlayerdown'/> <protocol family='ipv4'> <dhcp peerdns='no'/> </protocol> + <link state='lowerlayerdown'/> <vlan tag='42'> <interface name='eth0'/> </vlan> -- 1.9.3