virsh: add iface-bridge and iface-unbridge commands

One of the top questions by libvirt users is how to create a host
bridge device so that guests can be directly on the physical
network. There are several example documents that explain how to do
this manually, but following them often results in confusion and
failure. virt-manager does a good job of creating a bridge based on an
existing network device, but not everyone wants to use virt-manager.

This patch adds a new command, iface-bridge that makes it just about
as simple as possible to create a new bridge device based on an
existing ethernet/vlan/bond device (including associating IP
configuration with the bridge rather than the now-attached device),
and start that new bridge up ready for action, eg:

    virsh iface-bridge eth0 br0

For symmetry's sake, it also adds a command to remove a device from a
bridge, restoring the IP config to the now-unattached device:

    virsh iface-unbridge br0

(I had a short debate about whether to do "iface-unbridge eth0"
instead, but that would involve searching through all bridge devices
for the one that contained eth0, which seems like a bit too much
trouble).

NOTE: These two commands require that the netcf library be available
on the host. Hopefully this will provide some extra incentive for
people using suse, debian, ubuntu, and other similar systems to polish
up (and push downstream) the ports to those distros recently pushed to
the upstream netcf repo by Dan Berrange. Anyone interested in helping
with that effort in any way should join the netcf-devel mailing list
(subscription info at
https://fedorahosted.org/mailman/listinfo/netcf-devel)

During creation of the bridge, it's possible to specify whether or not
the STP protocol should be started up on the bridge and, if so, how
many seconds the bridge should squelch traffic from newly added
devices while learning new topology (defaults are stp='on' and
delay='0', which seems to usually work best for bridges used in the
context of libvirt guests).

There is also an option to not immediately start the bridge (and a
similar option to not immediately start the un-attached device after
destroying the bridge. Default is to start the new device, because in
the case of iface-unbridge not starting is strongly discouraged as it
will leave the system with no network connectivity on that interface
(because it's necessary to destroy/undefine the bridge device before
the unattached device can be defined), and it seemed better to make
the option for iface-bridge behave consistently.

NOTE TO THOSE TRYING THESE COMMANDS FOR THE FIRST TIME: to guard
against any "unexpected" change to configuration, it is advisable to
issue an "virsh iface-begin" command before starting any interface
config changes, and "virsh iface-commit" only after you've verified
that everything is working as you expect. If something goes wrong,
you can always run "virsh iface-rollback" or reboot the system (which
should automatically do iface-rollback).

Aside from adding the code for these two functions, and the two
entries into the command table, the only other change to virsh.c was
to add the option name to vshCommandOptInterfaceBy(), because the
iface-unbridge command names its interface option as "bridge".

virsh.pod has also been updated with short descriptions of these two
new commands.

tools/virsh.c

@@ -330,11 +330,12 @@ static virNWFilterPtr vshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd
                             VSH_BYUUID|VSH_BYNAME)
 
 static virInterfacePtr vshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd,
+                                                const char *optname,
                                                 const char **name, int flag);
 
 /* default is lookup by Name and MAC */
 #define vshCommandOptInterface(_ctl, _cmd, _name)                    \
-    vshCommandOptInterfaceBy(_ctl, _cmd, _name,                      \
+    vshCommandOptInterfaceBy(_ctl, _cmd, NULL, _name,                \
                              VSH_BYMAC|VSH_BYNAME)
 
 static virStoragePoolPtr vshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd,
@@ -6807,7 +6808,7 @@ cmdInterfaceName(vshControl *ctl, const vshCmd *cmd)
 
     if (!vshConnectionUsability(ctl, ctl->conn))
         return false;
-    if (!(iface = vshCommandOptInterfaceBy(ctl, cmd, NULL,
+    if (!(iface = vshCommandOptInterfaceBy(ctl, cmd, NULL, NULL,
                                            VSH_BYMAC)))
         return false;
 
@@ -6837,7 +6838,7 @@ cmdInterfaceMAC(vshControl *ctl, const vshCmd *cmd)
 
     if (!vshConnectionUsability(ctl, ctl->conn))
         return false;
-    if (!(iface = vshCommandOptInterfaceBy(ctl, cmd, NULL,
+    if (!(iface = vshCommandOptInterfaceBy(ctl, cmd, NULL, NULL,
                                            VSH_BYNAME)))
         return false;
 
@@ -7136,6 +7137,419 @@ cmdInterfaceRollback(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
     return true;
 }
 
+/*
+ * "iface-bridge" command
+ */
+static const vshCmdInfo info_interface_bridge[] = {
+    {"help", N_("create a bridge device and attach an existing network device to it")},
+    {"desc", N_("bridge an existing network device")},
+    {NULL, NULL}
+};
+
+static const vshCmdOptDef opts_interface_bridge[] = {
+    {"interface", VSH_OT_DATA, VSH_OFLAG_REQ, N_("existing interface name")},
+    {"bridge", VSH_OT_DATA, VSH_OFLAG_REQ, N_("new bridge device name")},
+    {"no-stp", VSH_OT_BOOL, 0, N_("do not enable STP for this bridge")},
+    {"delay", VSH_OT_INT, 0,
+     N_("number of seconds to squelch traffic on newly connected ports")},
+    {"no-start", VSH_OT_BOOL, 0, N_("don't start the bridge immediately")},
+    {NULL, 0, 0, NULL}
+};
+
+static bool
+cmdInterfaceBridge(vshControl *ctl, const vshCmd *cmd)
+{
+    bool ret = false;
+    virInterfacePtr if_handle = NULL, br_handle = NULL;
+    const char *if_name, *br_name;
+    char *if_type = NULL, *if2_name = NULL, *delay_str = NULL;
+    bool stp = false, nostart = false;
+    unsigned int delay = 0;
+    char *if_xml = NULL;
+    xmlChar *br_xml = NULL;
+    int br_xml_size;
+    xmlDocPtr xml_doc = NULL;
+    xmlXPathContextPtr ctxt = NULL;
+    xmlNodePtr top_node, br_node, if_node, cur;
+
+    if (!vshConnectionUsability(ctl, ctl->conn))
+        goto cleanup;
+
+    /* Get a handle to the original device */
+    if (!(if_handle = vshCommandOptInterfaceBy(ctl, cmd, "interface",
+                                               &if_name, VSH_BYNAME))) {
+        goto cleanup;
+    }
+
+    /* Name for new bridge device */
+    if (vshCommandOptString(cmd, "bridge", &br_name) <= 0) {
+        vshError(ctl, "%s", _("Missing bridge device name in command"));
+        goto cleanup;
+    }
+
+    /* make sure "new" device doesn't already exist */
+    if ((br_handle = virInterfaceLookupByName(ctl->conn, br_name))) {
+        vshError(ctl, _("Network device %s already exists"), br_name);
+        goto cleanup;
+    }
+
+    /* use "no-stp" because we want "stp" to default true */
+    stp = !vshCommandOptBool(cmd, "no-stp");
+
+    if (vshCommandOptUInt(cmd, "delay", &delay) < 0) {
+        vshError(ctl, "%s", _("Unable to parse delay parameter"));
+        goto cleanup;
+    }
+
+    nostart = vshCommandOptBool(cmd, "no-start");
+
+    /* Get the original interface into an xmlDoc */
+    if (!(if_xml = virInterfaceGetXMLDesc(if_handle, VIR_INTERFACE_XML_INACTIVE)))
+        goto cleanup;
+    if (!(xml_doc = virXMLParseStringCtxt(if_xml,
+                                          _("(interface definition)"), &ctxt))) {
+        vshError(ctl, _("Failed to parse configuration of %s"), if_name);
+        goto cleanup;
+    }
+    top_node = ctxt->node;
+
+    /* Verify that the original device isn't already a bridge. */
+    if (!(if_type = virXMLPropString(top_node, "type"))) {
+        vshError(ctl, _("Existing device %s has no type"), if_name);
+        goto cleanup;
+    }
+
+    if (STREQ(if_type, "bridge")) {
+        vshError(ctl, _("Existing device %s is already a bridge"), if_name);
+        goto cleanup;
+    }
+
+    /* verify the name in the XML matches the device name */
+    if (!(if2_name = virXMLPropString(top_node, "name")) ||
+        STRNEQ(if2_name, if_name)) {
+        vshError(ctl, _("Interface name from config %s doesn't match given supplied name %s"),
+                 if2_name, if_name);
+        goto cleanup;
+    }
+
+    /* Create a <bridge> node under <interface>. */
+    if (!(br_node = xmlNewChild(top_node, NULL, BAD_CAST "bridge", NULL))) {
+        vshError(ctl, "%s", _("Failed to create bridge node in xml document"));
+        goto cleanup;
+    }
+
+    /* Set stp and delay attributes in <bridge> according to the
+     * commandline options.
+     */
+    if (!xmlSetProp(br_node, BAD_CAST "stp", BAD_CAST (stp ? "on" : "off"))) {
+        vshError(ctl, "%s", _("Failed to set stp attribute in xml document"));
+        goto cleanup;
+    }
+
+    if ((delay || stp) &&
+        ((virAsprintf(&delay_str, "%d", delay) < 0) ||
+         !xmlSetProp(br_node, BAD_CAST "delay", BAD_CAST delay_str))) {
+        vshError(ctl, _("Failed to set bridge delay %d in xml document"), delay);
+        goto cleanup;
+    }
+
+    /* Change the type of the outer/master interface to "bridge" and the
+     * name to the provided bridge name.
+     */
+    if (!xmlSetProp(top_node, BAD_CAST "type", BAD_CAST "bridge")) {
+        vshError(ctl, "%s", _("Failed to set bridge interface type to 'bridge' in xml document"));
+        goto cleanup;
+    }
+
+    if (!xmlSetProp(top_node, BAD_CAST "name", BAD_CAST br_name)) {
+        vshError(ctl, _("Failed to set master bridge interface name to '%s' in xml document"),
+            br_name);
+        goto cleanup;
+    }
+
+    /* Create an <interface> node under <bridge> that uses the
+     * original interface's type and name.
+     */
+    if (!(if_node = xmlNewChild(br_node, NULL, BAD_CAST "interface", NULL))) {
+        vshError(ctl, "%s", _("Failed to create interface node under bridge node in xml document"));
+        goto cleanup;
+    }
+
+    /* set the type of the inner/slave interface to the original
+     * if_type, and the name to the original if_name.
+     */
+    if (!xmlSetProp(if_node, BAD_CAST "type", BAD_CAST if_type)) {
+        vshError(ctl, _("Failed to set new slave interface type to '%s' in xml document"),
+                 if_name);
+        goto cleanup;
+    }
+
+    if (!xmlSetProp(if_node, BAD_CAST "name", BAD_CAST if_name)) {
+        vshError(ctl, _("Failed to set new slave interface name to '%s' in xml document"),
+            br_name);
+        goto cleanup;
+    }
+
+    /* Cycle through all the nodes under the original <interface>,
+     * moving all <mac>, <bond> and <vlan> nodes down into the new
+     * lower level <interface>.
+     */
+    cur = top_node->children;
+    while (cur) {
+        xmlNodePtr old = cur;
+
+        cur = cur->next;
+        if ((old->type == XML_ELEMENT_NODE) &&
+            (xmlStrEqual(old->name, BAD_CAST "mac") ||  /* ethernet stuff to move down */
+             xmlStrEqual(old->name, BAD_CAST "bond") || /* bond stuff to move down */
+             xmlStrEqual(old->name, BAD_CAST "vlan"))) { /* vlan stuff to move down */
+            xmlUnlinkNode(old);
+            if (!xmlAddChild(if_node, old)) {
+                vshError(ctl, _("Failed to move '%s' element in xml document"), old->name);
+                xmlFreeNode(old);
+                goto cleanup;
+            }
+        }
+    }
+
+    /* The document should now be fully converted; write it out to a string. */
+    xmlDocDumpMemory(xml_doc, &br_xml, &br_xml_size);
+
+    if (!br_xml || br_xml_size <= 0) {
+        vshError(ctl, _("Failed to format new xml document for bridge %s"), br_name);
+        goto cleanup;
+    }
+
+
+    /* br_xml is the new interface to define. It will automatically undefine the
+     * independent original interface.
+     */
+    if (!(br_handle = virInterfaceDefineXML(ctl->conn, (char *) br_xml, 0))) {
+        vshError(ctl, _("Failed to define new bridge interface %s"),
+                 br_name);
+        goto cleanup;
+    }
+
+    vshPrint(ctl, _("Created bridge %s with attached device %s\n"),
+             if_name, br_name);
+
+    /* start it up unless requested not to */
+    if (!nostart) {
+        if (virInterfaceCreate(br_handle, 0) < 0) {
+            vshError(ctl, _("Failed to start bridge interface %s"), br_name);
+            goto cleanup;
+        }
+        vshPrint(ctl, _("Bridge interface %s started\n"), br_name);
+    }
+
+    ret = true;
+ cleanup:
+    if (if_handle)
+       virInterfaceFree(if_handle);
+    if (br_handle)
+       virInterfaceFree(br_handle);
+    VIR_FREE(if_xml);
+    VIR_FREE(br_xml);
+    VIR_FREE(if_type);
+    VIR_FREE(if2_name);
+    VIR_FREE(delay_str);
+    xmlXPathFreeContext(ctxt);
+    xmlFreeDoc(xml_doc);
+    return ret;
+}
+
+/*
+ * "iface-unbridge" command
+ */
+static const vshCmdInfo info_interface_unbridge[] = {
+    {"help", N_("undefine a bridge device after detaching its slave device")},
+    {"desc", N_("unbridge a network device")},
+    {NULL, NULL}
+};
+
+static const vshCmdOptDef opts_interface_unbridge[] = {
+    {"bridge", VSH_OT_DATA, VSH_OFLAG_REQ, N_("current bridge device name")},
+    {"no-start", VSH_OT_BOOL, 0,
+     N_("don't start the un-slaved interface immediately (not recommended)")},
+    {NULL, 0, 0, NULL}
+};
+
+static bool
+cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
+{
+    bool ret = false;
+    virInterfacePtr if_handle = NULL, br_handle = NULL;
+    const char *br_name;
+    char *if_type = NULL, *if_name = NULL;
+    bool nostart = false;
+    char *br_xml = NULL;
+    xmlChar *if_xml = NULL;
+    int if_xml_size;
+    xmlDocPtr xml_doc = NULL;
+    xmlXPathContextPtr ctxt = NULL;
+    xmlNodePtr top_node, br_node, if_node, cur;
+
+    if (!vshConnectionUsability(ctl, ctl->conn))
+        goto cleanup;
+
+    /* Get a handle to the original device */
+    if (!(br_handle = vshCommandOptInterfaceBy(ctl, cmd, "bridge",
+                                               &br_name, VSH_BYNAME))) {
+        goto cleanup;
+    }
+
+    nostart = vshCommandOptBool(cmd, "no-start");
+
+    /* Get the bridge xml into an xmlDoc */
+    if (!(br_xml = virInterfaceGetXMLDesc(br_handle, VIR_INTERFACE_XML_INACTIVE)))
+        goto cleanup;
+    if (!(xml_doc = virXMLParseStringCtxt(br_xml,
+                                          _("(bridge interface definition)"),
+                                          &ctxt))) {
+        vshError(ctl, _("Failed to parse configuration of %s"), br_name);
+        goto cleanup;
+    }
+    top_node = ctxt->node;
+
+    /* Verify that the device really is a bridge. */
+    if (!(if_type = virXMLPropString(top_node, "type"))) {
+        vshError(ctl, _("Existing device %s has no type"), br_name);
+        goto cleanup;
+    }
+
+    if (STRNEQ(if_type, "bridge")) {
+        vshError(ctl, _("Device %s is not a bridge"), br_name);
+        goto cleanup;
+    }
+    VIR_FREE(if_type);
+
+    /* verify the name in the XML matches the device name */
+    if (!(if_name = virXMLPropString(top_node, "name")) ||
+        STRNEQ(if_name, br_name)) {
+        vshError(ctl, _("Interface name from config %s doesn't match given supplied name %s"),
+                 if_name, br_name);
+        goto cleanup;
+    }
+    VIR_FREE(if_name);
+
+    /* Find the <bridge> node under <interface>. */
+    if (!(br_node = virXPathNode("./bridge", ctxt))) {
+        vshError(ctl, "%s", _("No bridge node in xml document"));
+        goto cleanup;
+    }
+
+    if ((if_node = virXPathNode("./bridge/interface[2]", ctxt))) {
+        vshError(ctl, "%s", _("Multiple interfaecs attached to bridge"));
+        goto cleanup;
+    }
+
+    if (!(if_node = virXPathNode("./bridge/interface", ctxt))) {
+        vshError(ctl, "%s", _("No interface attached to bridge"));
+        goto cleanup;
+    }
+
+    /* Change the type and name of the outer/master interface to
+     * the type/name of the attached slave interface.
+     */
+    if (!(if_name = virXMLPropString(if_node, "name"))) {
+        vshError(ctl, _("Device attached to bridge %s has no name"), br_name);
+        goto cleanup;
+    }
+
+    if (!(if_type = virXMLPropString(if_node, "type"))) {
+        vshError(ctl, _("Attached device %s has no type"), if_name);
+        goto cleanup;
+    }
+
+    if (!xmlSetProp(top_node, BAD_CAST "type", BAD_CAST if_type)) {
+        vshError(ctl, _("Failed to set interface type to '%s' in xml document"),
+                 if_type);
+        goto cleanup;
+    }
+
+    if (!xmlSetProp(top_node, BAD_CAST "name", BAD_CAST if_name)) {
+        vshError(ctl, _("Failed to set interface name to '%s' in xml document"),
+                 if_name);
+        goto cleanup;
+    }
+
+    /* Cycle through all the nodes under the attached <interface>,
+     * moving all <mac>, <bond> and <vlan> nodes up into the toplevel
+     * <interface>.
+     */
+    cur = if_node->children;
+    while (cur) {
+        xmlNodePtr old = cur;
+
+        cur = cur->next;
+        if ((old->type == XML_ELEMENT_NODE) &&
+            (xmlStrEqual(old->name, BAD_CAST "mac") ||  /* ethernet stuff to move down */
+             xmlStrEqual(old->name, BAD_CAST "bond") || /* bond stuff to move down */
+             xmlStrEqual(old->name, BAD_CAST "vlan"))) { /* vlan stuff to move down */
+            xmlUnlinkNode(old);
+            if (!xmlAddChild(top_node, old)) {
+                vshError(ctl, _("Failed to move '%s' element in xml document"), old->name);
+                xmlFreeNode(old);
+                goto cleanup;
+            }
+        }
+    }
+
+    /* The document should now be fully converted; write it out to a string. */
+    xmlDocDumpMemory(xml_doc, &if_xml, &if_xml_size);
+
+    if (!if_xml || if_xml_size <= 0) {
+        vshError(ctl, _("Failed to format new xml document for un-enslaved interface %s"),
+                 if_name);
+        goto cleanup;
+    }
+
+    /* Destroy and Undefine the bridge device, since we otherwise
+     * can't safely define the unattached device.
+     */
+    if (virInterfaceDestroy(br_handle, 0) < 0) {
+        vshError(ctl, _("Failed to destroy bridge interface %s"), br_name);
+        goto cleanup;
+    }
+    if (virInterfaceUndefine(br_handle) < 0) {
+        vshError(ctl, _("Failed to undefine bridge interface %s"), br_name);
+        goto cleanup;
+    }
+
+    /* if_xml is the new interface to define.
+     */
+    if (!(if_handle = virInterfaceDefineXML(ctl->conn, (char *) if_xml, 0))) {
+        vshError(ctl, _("Failed to define new interface %s"), if_name);
+        goto cleanup;
+    }
+
+    vshPrint(ctl, _("Device %s un-attached from bridge %s\n"),
+             if_name, br_name);
+
+    /* unless requested otherwise, undefine the bridge device */
+    if (!nostart) {
+        if (virInterfaceCreate(if_handle, 0) < 0) {
+            vshError(ctl, _("Failed to start interface %s"), if_name);
+            goto cleanup;
+        }
+        vshPrint(ctl, _("Interface %s started\n"), if_name);
+    }
+
+    ret = true;
+ cleanup:
+    if (if_handle)
+       virInterfaceFree(if_handle);
+    if (br_handle)
+       virInterfaceFree(br_handle);
+    VIR_FREE(if_xml);
+    VIR_FREE(br_xml);
+    VIR_FREE(if_type);
+    VIR_FREE(if_name);
+    xmlXPathFreeContext(ctxt);
+    xmlFreeDoc(xml_doc);
+    return ret;
+}
+
 /*
  * "nwfilter-define" command
  */
@@ -14199,6 +14613,8 @@ static const vshCmdDef nodedevCmds[] = {
 static const vshCmdDef ifaceCmds[] = {
     {"iface-begin", cmdInterfaceBegin, opts_interface_begin,
      info_interface_begin, 0},
+    {"iface-bridge", cmdInterfaceBridge, opts_interface_bridge,
+     info_interface_bridge, 0},
     {"iface-commit", cmdInterfaceCommit, opts_interface_commit,
      info_interface_commit, 0},
     {"iface-define", cmdInterfaceDefine, opts_interface_define,
@@ -14219,6 +14635,8 @@ static const vshCmdDef ifaceCmds[] = {
      info_interface_rollback, 0},
     {"iface-start", cmdInterfaceStart, opts_interface_start,
      info_interface_start, 0},
+    {"iface-unbridge", cmdInterfaceUnbridge, opts_interface_unbridge,
+     info_interface_unbridge, 0},
     {"iface-undefine", cmdInterfaceUndefine, opts_interface_undefine,
      info_interface_undefine, 0},
     {NULL, NULL, NULL, NULL, 0}
@@ -15101,11 +15519,14 @@ vshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd,
 
 static virInterfacePtr
 vshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd,
+                         const char *optname,
                          const char **name, int flag)
 {
     virInterfacePtr iface = NULL;
     const char *n = NULL;
-    const char *optname = "interface";
+
+    if (!optname)
+       optname = "interface";
     if (!cmd_has_option (ctl, cmd, optname))
         return NULL;
 

tools/virsh.pod

@@ -1465,6 +1465,18 @@ resort to a name instead).
 
 =over 4
 
+=item B<iface-bridge> I<interface> I<bridge> [I<--no-stp>] [I<delay>]
+[I<--no-start>]
+
+Create a bridge device named I<bridge>, and attach the existing
+network device I<interface> to the new bridge.  The new bridge
+defaults to starting immediately, with STP enabled and a delay of 0;
+these settings can be altered with I<--no-stp>, I<--no-start>, and an
+integer number of seconds for I<delay>. All IP address configuration
+of I<interface> will be moved to the new bridge device.
+
+See also B<iface-unbridge> for undoing this operation.
+
 =item B<iface-define> I<file>
 
 Define a host interface from an XML I<file>, the interface is just defined but
@@ -1519,6 +1531,16 @@ I<interface> specifies the interface name.
 
 Start a (previously defined) host interface, such as by running "if-up".
 
+=item B<iface-unbridge> I<bridge> [I<--no-start>]
+
+Tear down a bridge device named I<bridge>, releasing its underlying
+interface back to normal usage, and moving all IP address
+configuration from the bridge device to the underlying device.  The
+underlying interface is restarted unless I<--no-start> is present;
+this flag is present for symmetry, but generally not recommended.
+
+See also B<iface-bridge> for creating a bridge.
+
 =item B<iface-undefine> I<interface>
 
 Undefine the configuration for an inactive host interface.