Prior to defining a new interface, drv_define() would previously call rm_all_interfaces(), which would look for all nodes called "interface" at every level, and remove the config for any existing interface with the same name. This is necessary when defining a bridge or a bond (since those interfaces will now be slaves of the toplevel bridge/bond, and their config will be created anew along with the toplevel), but is the wrong thing to do for vlans - not only does the physical interface associated with a vlan have an existence of its own, multiple vlans can be associated with the same physical interface, and the act of defining a vlan *does not* re-create the physical interface config. The result of this was dismal failure when attempting to define a vlan interface - the associated physical interface would disappear.
To solve this problem, rm_all_interfaces() has been replaced with rm_interface_and_slaves(), which removes the config for the toplevel interface, the checks to see if it is a bridge or a bond; if so, it also removes the config for any interfaces slaved to this bridge/bond.
Because a bridge may enslave a bond, which in turn may enslave physical interfaces, this function was made recursive. --- src/drv_initscripts.c | 100 ++++++++++++++++++++++++++++++++++++++----------- 1 files changed, 78 insertions(+), 22 deletions(-)
diff --git a/src/drv_initscripts.c b/src/drv_initscripts.c index d5bbd44..389dbb6 100644 --- a/src/drv_initscripts.c +++ b/src/drv_initscripts.c @@ -936,34 +936,90 @@ static void rm_interface(struct netcf *ncf, const char *name) { FREE(path); }
-/* Remove all interfaces and their slaves mentioned in NCF_XML. We need to - * remove interfaces one by one when we define an interface, since what - * will become a subinterface may not be related to the new toplevel - * interface, and calling RM_INTERFACE on the toplevel interface is - * therefore not enough. + +/* rm_interface_and_slaves_helper() + * + * Given a context pointing to an "interface" node, delete the config + * for the interface with the name of this node's "name" property. + * Then, if this node's type is "bridge" or "bond", cycle through + * all "type"/interface subnodes and call this function to delete + * their config as well. */ -static void rm_all_interfaces(struct netcf *ncf, xmlDocPtr ncf_xml) { - xmlXPathContextPtr context = NULL; - xmlXPathObjectPtr obj = NULL; +static void rm_interface_and_slaves_helper(struct netcf *ncf, + xmlDocPtr ncf_xml, + xmlXPathContextPtr ctxt) { + xmlNodePtr save_node = ctxt->node; + xmlXPathObjectPtr obj = NULL; + xmlChar *name = NULL, *type = NULL; + char *if_node_exp = NULL; + + xmlNodeSetPtr ns; + + /* remove the top-level interface */ + name = xmlGetProp(ctxt->node, BAD_CAST "name"); + ERR_NOMEM(name == NULL, ncf); + rm_interface(ncf, (char *) name); + ERR_BAIL(ncf);
- context = xmlXPathNewContext(ncf_xml); - ERR_NOMEM(context == NULL, ncf); + type = xmlGetProp(ctxt->node, BAD_CAST "type"); + ERR_NOMEM(name == NULL, ncf);
- obj = xmlXPathEvalExpression(BAD_CAST "//interface", context); - ERR_NOMEM(obj == NULL, ncf); + if (xmlStrEqual(type, BAD_CAST "bridge") + || xmlStrEqual(type, BAD_CAST "bond")) {
+ /* get a list of all interface subnodes under + * this interface's "type" node. + * eg "./bond/interface" + */ + + int r = xasprintf(&if_node_exp, "./%s/interface", (char *)type); + ERR_NOMEM(r < 0, ncf); + + obj = xmlXPathEvalExpression(BAD_CAST if_node_exp, ctxt); + ERR_NOMEM(obj == NULL, ncf); + ns = obj->nodesetval; + ERR_NOMEM(ns == NULL, ncf); + + /* iterate through them all, removing them as we go */ + for (int ii=0; ii < ns->nodeNr; ii++) { + + ERR_NOMEM(ns->nodeTab[ii] == NULL, ncf); + + /* make this interface the toplevel node & recurse */ + ctxt->node = ns->nodeTab[ii]; + rm_interface_and_slaves_helper(ncf, ncf_xml, ctxt); + ERR_BAIL(ncf); + } + }
- xmlNodeSetPtr ns = obj->nodesetval; - for (int i=0; i < ns->nodeNr; i++) { - xmlChar *name = xmlGetProp(ns->nodeTab[i], BAD_CAST "name"); - ERR_NOMEM(name == NULL, ncf); - rm_interface(ncf, (char *) name); - xmlFree(name); - ERR_BAIL(ncf); - } error: xmlXPathFreeObject(obj); - xmlXPathFreeContext(context); + FREE(if_node_exp); + xmlFree(name); + xmlFree(type); + ctxt->node = save_node; +} + +/* Remove the toplevel interface, and all slaves recursively. We need to + * remove interfaces one by one when we define an interface, since what + * will become a subinterface may not be related to the new toplevel + * interface, and calling RM_INTERFACE on the toplevel interface is + * therefore not enough. + * + * This function just sets up the initial context and calls the helper + * function above, which does the real work. + */ +static void rm_interface_and_slaves(struct netcf *ncf, xmlDocPtr ncf_xml) { + xmlXPathContextPtr ctxt; + + ctxt = xmlXPathNewContext(ncf_xml); + ERR_NOMEM(ctxt == NULL, ncf); + + ctxt->node = xmlDocGetRootElement(ncf_xml); + if (xmlStrEqual(ctxt->node->name, BAD_CAST "interface")) + rm_interface_and_slaves_helper(ncf, ncf_xml, ctxt); +error: + xmlXPathFreeContext(ctxt); }
/* Dig through interface NAME and all its subinterfaces for bonds @@ -1017,7 +1073,7 @@ struct netcf_if *drv_define(struct netcf *ncf, const char *xml_str) { name = device_name_from_xml(ncf, ncf_xml); ERR_COND_BAIL(name == NULL, ncf, EINTERNAL);
- rm_all_interfaces(ncf, ncf_xml); + rm_interface_and_slaves(ncf, ncf_xml); ERR_BAIL(ncf);
aug_xml = apply_stylesheet(ncf, ncf->driver->get, ncf_xml);
This is a much simpler way of solving the problem, suggested by David Lutterkort.
Prior to defining a new interface, drv_define() would previously call rm_all_interfaces(), which would look for all nodes called "interface" at every level, and remove the config for any existing interface with the same name. This is necessary when defining a bridge or a bond (since those interfaces will now be slaves of the toplevel bridge/bond, and their config will be created anew along with the toplevel), but is the wrong thing to do for vlans - not only does the physical interface associated with a vlan have an existence of its own, multiple vlans can be associated with the same physical interface, and the act of defining a vlan *does not* re-create the physical interface config. The result of this was dismal failure when attempting to define a vlan interface - the associated physical interface would disappear.
The solution is to beef up the expression used when finding all interfaces inside the XML to exclude any whose parent is a "vlan" node. --- src/drv_initscripts.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/src/drv_initscripts.c b/src/drv_initscripts.c index d5bbd44..c0bcfd5 100644 --- a/src/drv_initscripts.c +++ b/src/drv_initscripts.c @@ -949,7 +949,9 @@ static void rm_all_interfaces(struct netcf *ncf, xmlDocPtr ncf_xml) { context = xmlXPathNewContext(ncf_xml); ERR_NOMEM(context == NULL, ncf);
- obj = xmlXPathEvalExpression(BAD_CAST "//interface", context); + obj = xmlXPathEvalExpression(BAD_CAST + "//interface[count(parent::vlan) = 0]", + context); ERR_NOMEM(obj == NULL, ncf);
On Tue, 2010-07-27 at 14:43 -0400, Laine Stump wrote:
This is a much simpler way of solving the problem, suggested by David Lutterkort.
Prior to defining a new interface, drv_define() would previously call rm_all_interfaces(), which would look for all nodes called "interface" at every level, and remove the config for any existing interface with the same name. This is necessary when defining a bridge or a bond (since those interfaces will now be slaves of the toplevel bridge/bond, and their config will be created anew along with the toplevel), but is the wrong thing to do for vlans - not only does the physical interface associated with a vlan have an existence of its own, multiple vlans can be associated with the same physical interface, and the act of defining a vlan *does not* re-create the physical interface config. The result of this was dismal failure when attempting to define a vlan interface - the associated physical interface would disappear.
The solution is to beef up the expression used when finding all interfaces inside the XML to exclude any whose parent is a "vlan" node.
ACK
David
netcf-devel@lists.fedorahosted.org
-
David Lutterkort -
Laine Stump