network: merge relevant virtualports rather than choosing one

One of the original ideas behind allowing a <virtualport> in an
interface definition as well as in the <network> definition *and*one
or more <portgroup>s within the network, was that guest-specific
parameteres (like instanceid and interfaceid) could be given in the
interface's virtualport, and more general things (portid, managerid,
etc) could be given in the network and/or portgroup, with all the bits
brought together at guest startup time and combined into a single
virtualport to be used by the guest. This was somehow overlooked in
the implementation, though - it simply picks the "most specific"
virtualport, and uses the entire thing, with no attempt to merge in
details from the others.

This patch uses virNetDevVPortProfileMerge3() to combine the three
possible virtualports into one, then uses
virNetDevVPortProfileCheck*() to verify that the resulting virtualport
type is appropriate for the type of network, and that all the required
attributes for that type are present.

An example of usage is this: assuming a <network> definitions on host
ABC of:

  <network>
    <name>testA</name>
    ...
    <virtualport type='openvswitch'/>
    ...
    <portgroup name='engineering'>
      <virtualport>
        <parameters profileid='eng'/>
      </virtualport>
    </portgroup>
    <portgroup name='sales'>
      <virtualport>
        <parameters profileid='sales'/>
      </virtualport>
    </portgroup>
  </network>

and the same <network> on host DEF of:

  <network>
    <name>testA</name>
    ...
    <virtualport type='802.1Qbg'>
      <parameters typeid="1193047" typeidversion="2"/>
    </virtualport>
    ...
    <portgroup name='engineering'>
      <virtualport>
        <parameters managerid="11"/>
      </virtualport>
    </portgroup>
    <portgroup name='sales'>
      <virtualport>
        <parameters managerid="55"/>
      </virtualport>
    </portgroup>
  </network>

and a guest <interface> definition of:

  <interface type='network'>
    <source network='testA' portgroup='sales'/>
    <virtualport>
      <parameters instanceid="09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f"
                  interfaceid="09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f"\>
    </virtualport>
    ...
  </interface>

If the guest was started on host ABC, the <virtualport> used would be:

  <virtualport type='openvswitch'>
    <parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'
                profileid='sales'/>
  </virtualport>

but if that guest was started on host DEF, the <virtualport> would be:

    <virtualport type='802.1Qbg'>
      <parameters instanceid="09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f"
                  typeid="1193047" typeidversion="2"
                  managerid="55"/>
    </virtualport>

Additionally, if none of the involved <virtualport>s had a specified type
(this includes cases where no virtualport is given at all),

docs/formatdomain.html.in

@@ -2230,11 +2230,40 @@
       the network; one network may have multiple portgroups defined,
       with each portgroup containing slightly different configuration
       information for different classes of network
-      connections. <span class="since">Since 0.9.4</span>). Also,
-      similar to <code>direct</code> network connections (described
-      below), a connection of type <code>network</code> may specify
-      a <code>virtportprofile</code> element, with configuration data
-      to be forwarded to a vepa or 802.1Qbh compliant switch.
+      connections. <span class="since">Since 0.9.4</span>.
+    </p>
+    <p>
+      Also, similar to <code>direct</code> network connections
+      (described below), a connection of type <code>network</code> may
+      specify a <code>virtualport</code> element, with configuration
+      data to be forwarded to a vepa (802.1Qbg) or 802.1Qbh compliant
+      switch (<span class="since">Since 0.8.2</span>), or to an
+      Open vSwitch virtual switch (<span class="since">Since
+      0.9.11</span>).
+    </p>
+    <p>
+      Since the actual type of switch may vary depending on the
+      configuration in the <code>&lt;network&gt;</code> on the host,
+      it is acceptable to omit the virtualport <code>type</code>
+      attribute, and specify attributes from multiple different
+      virtualport types (and also to leave out certain attributes); at
+      domain startup time, a complete <code>&lt;virtualport&gt;</code>
+      element will be constructed by merging together the type and
+      attributes found in the which will be filled in from the network
+      or portgroup <code>&lt;virtualport&gt;</code>)
+      (<span class="since">Since 0.10.0</span>). For example, in order
+      to work properly with both an 802.1Qbh switch and an Open vSwitch
+      switch, you may choose to specify no type, but both
+      an <code>instanceid</code> (in case the switch is 802.1Qbh) and
+      an <code>interfaceid</code> (in case the switch is Open vSwitch)
+      (you may also omit the other attributes, such as managerid,
+      typeid, or profileid, to be filled in from the
+      network's <code>&lt;virtualport&gt;</code>). If you want to
+      limit a guest to connecting only to certain types of switches,
+      you can specify the virtualport type, but still omit some/all of
+      the parameters - in this case if the host's network has a
+      different type of virtualport, connection of the interface will
+      fail.
     </p>
 
 <pre>
@@ -2248,8 +2277,8 @@
       &lt;source network='default' portgroup='engineering'/&gt;
       &lt;target dev='vnet7'/&gt;
       &lt;mac address="00:11:22:33:44:55"/&gt;
-      &lt;virtualport type='802.1Qbg'&gt;
-        &lt;parameters managerid='11' typeid='1193047' typeidversion='2' instanceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/&gt;
+      &lt;virtualport&gt;
+        &lt;parameters instanceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/&gt;
       &lt;/virtualport&gt;
 
     &lt;/interface&gt;
@@ -2261,7 +2290,7 @@
     <p>
       <strong><em>
       This is the recommended config for general guest connectivity on
-      hosts with static wired networking configs
+      hosts with static wired networking configs.
       </em></strong>
     </p>
 
@@ -2276,19 +2305,40 @@
       configuration is whatever is used on the LAN. This provides the guest VM
       full incoming &amp; outgoing net access just like a physical machine.
     </p>
-
+    <p>
+      On Linux systems, the bridge device is normally a standard Linux
+      host bridge. On hosts that support Open vSwitch, it is also
+      possible to connect to an open vSwitch bridge device by adding
+      a <code>&lt;virtualport type='openvswitch'/&gt;</code> to the
+      interface definition.  (<span class="since">Since
+      0.9.11</span>). The Open vSwitch type virtualport accepts two
+      parameters in its <code>&lt;parameters&gt;</code> element -
+      an <code>interfaceid</code> which is a standard uuid used to
+      uniquely identify this particular interface to Open vSwitch (if
+      you do no specify one, a random interfaceid will be generated
+      for you when you first define the interface), and an
+      optional <code>profileid</code> which is sent to Open vSwitch as
+      the interfaces "port-profile".
+    </p>
 <pre>
   ...
   &lt;devices&gt;
+    ...
     &lt;interface type='bridge'&gt;
       &lt;source bridge='br0'/&gt;
     &lt;/interface&gt;
-    ...
     &lt;interface type='bridge'&gt;
-      &lt;source bridge='br0'/&gt;
+      &lt;source bridge='br1'/&gt;
       &lt;target dev='vnet7'/&gt;
       &lt;mac address="00:11:22:33:44:55"/&gt;
     &lt;/interface&gt;
+    &lt;interface type='bridge'&gt;
+      &lt;source bridge='ovsbr'/&gt;
+      &lt;virtualport type='openvswitch'/&gt;
+        &lt;parameters profileid='menial' interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/&gt;
+      &lt;/virtualport&gt;
+    &lt;/interface&gt;
+    ...
   &lt;/devices&gt;
   ...</pre>
 

docs/formatnetwork.html.in

@@ -147,10 +147,17 @@
             This network describes either 1) an existing host bridge
             that was configured outside of libvirt (if
             a <code>&lt;bridge name='xyz'/&gt;</code> element has been
-            specified), or 2) an interface or group of interfaces to
-            be used for a "direct" connection via macvtap using
-            macvtap's "bridge" mode (if the forward element has one or
-            more <code>&lt;interface&gt;</code> subelements)
+            specified, <span class="since">Since 0.9.4</span>), 2) an
+            existing Open vSwitch bridge that was configured outside of
+            libvirt (if both a <code>&lt;bridge name='xyz'/&gt;</code>
+            element <b>and</b> a <code>&lt;virtualport
+            type='openvswitch'/&gt;</code> have been
+            specified <span class="since">Since 0.10.0</span>) 3) an
+            interface or group of interfaces to be used for a "direct"
+            connection via macvtap using macvtap's "bridge" mode (if
+            the forward element has one or
+            more <code>&lt;interface&gt;</code>
+            subelements, <span class="since">Since 0.9.4</span>)
             (see <a href="formatdomain.html#elementsNICSDirect">Direct
             attachment to physical interface</a> for descriptions of
             the various macvtap modes). libvirt doesn't attempt to
@@ -337,9 +344,15 @@
       default portgroup will be used. If no portgroup is given in the
       interface definition, and there is no default portgroup, then
       none will be used. Any <code>&lt;bandwidth&gt;</code>
-      or <code>&lt;virtualport&gt;</code> specified directly in the
-      domain XML will take precedence over any setting in the chosen
-      portgroup.
+
+      specified directly in the domain XML will take precedence over
+      any setting in the chosen portgroup. if
+      a <code>&lt;virtualport&gt;</code> is specified in the portgroup
+      (and/or directly in the network definition), the multiple
+      virtualports will be merged, and any parameter that is specified
+      in more than one virtualport, and is not identical, will be
+      considered an error, and will prevent the interface from
+      starting.
     </p>
 
     <h3><a name="elementsAddress">Addressing</a></h3>

src/network/bridge_driver.c

@@ -61,6 +61,7 @@
 #include "virnetdev.h"
 #include "virnetdevbridge.h"
 #include "virnetdevtap.h"
+#include "virnetdevvportprofile.h"
 
 #define NETWORK_PID_DIR LOCALSTATEDIR "/run/libvirt/network"
 #define NETWORK_STATE_DIR LOCALSTATEDIR "/lib/libvirt/network"
@@ -2746,6 +2747,8 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
     virNetworkObjPtr network;
     virNetworkDefPtr netdef;
     virPortGroupDefPtr portgroup;
+    virNetDevVPortProfilePtr virtport = NULL;
+    virNetworkForwardIfDefPtr dev = NULL;
     unsigned int num_virt_fns = 0;
     char **vfname = NULL;
     int ii;
@@ -2820,11 +2823,33 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
             goto cleanup;
         }
 
+        /* merge virtualports from interface, network, and portgroup to
+         * arrive at actual virtualport to use
+         */
+        if (virNetDevVPortProfileMerge3(&iface->data.network.actual->virtPortProfile,
+                                        iface->virtPortProfile,
+                                        netdef->virtPortProfile,
+                                        portgroup
+                                        ? portgroup->virtPortProfile : NULL) < 0) {
+            goto cleanup;
+        }
+        virtport = iface->data.network.actual->virtPortProfile;
+        if (virtport) {
+            /* only type='openvswitch' is allowed for bridges */
+            if (virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("<virtualport type='%s'> not supported for network "
+                                 "'%s' which uses a bridge device"),
+                               virNetDevVPortTypeToString(virtport->virtPortType),
+                               netdef->name);
+                goto cleanup;
+            }
+        }
+
     } else if ((netdef->forwardType == VIR_NETWORK_FORWARD_BRIDGE) ||
                (netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) ||
                (netdef->forwardType == VIR_NETWORK_FORWARD_VEPA) ||
                (netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH)) {
-        virNetDevVPortProfilePtr virtport = NULL;
 
         /* <forward type='bridge|private|vepa|passthrough'> are all
          * VIR_DOMAIN_NET_TYPE_DIRECT.
@@ -2853,24 +2878,28 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
             break;
         }
 
-        /* Find the most specific virtportprofile and copy it */
-        if (iface->virtPortProfile) {
-            virtport = iface->virtPortProfile;
-        } else {
-            if (portgroup)
-                virtport = portgroup->virtPortProfile;
-            else
-                virtport = netdef->virtPortProfile;
+        /* merge virtualports from interface, network, and portgroup to
+         * arrive at actual virtualport to use
+         */
+        if (virNetDevVPortProfileMerge3(&iface->data.network.actual->virtPortProfile,
+                                        iface->virtPortProfile,
+                                        netdef->virtPortProfile,
+                                        portgroup
+                                        ? portgroup->virtPortProfile : NULL) < 0) {
+            goto cleanup;
         }
+        virtport = iface->data.network.actual->virtPortProfile;
         if (virtport) {
-            if (VIR_ALLOC(iface->data.network.actual->virtPortProfile) < 0) {
-                virReportOOMError();
+            /* make sure type is supported for macvtap connections */
+            if (virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_8021QBG &&
+                virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_8021QBH) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("<virtualport type='%s'> not supported for network "
+                                 "'%s' which uses a macvtap device"),
+                               virNetDevVPortTypeToString(virtport->virtPortType),
+                               netdef->name);
                 goto cleanup;
             }
-            /* There are no pointers in a virtualPortProfile, so a shallow copy
-             * is sufficient
-             */
-            *iface->data.network.actual->virtPortProfile = *virtport;
         }
 
         /* If there is only a single device, just return it (caller will detect
@@ -2883,8 +2912,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
                            netdef->name);
             goto cleanup;
         } else {
-            virNetworkForwardIfDefPtr dev = NULL;
-
             /* pick an interface from the pool */
 
             /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require
@@ -2967,13 +2994,18 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
                 virReportOOMError();
                 goto cleanup;
             }
-            /* we are now assured of success, so mark the allocation */
-            dev->usageCount++;
-            VIR_DEBUG("Using physical device %s, usageCount %d",
-                      dev->dev, dev->usageCount);
         }
     }
 
+    if (virNetDevVPortProfileCheckComplete(virtport, true) < 0)
+        goto cleanup;
+
+    if (dev) {
+        /* we are now assured of success, so mark the allocation */
+        dev->usageCount++;
+        VIR_DEBUG("Using physical device %s, usageCount %d",
+                  dev->dev, dev->usageCount);
+    }
     ret = 0;
 cleanup:
     for (ii = 0; ii < num_virt_fns; ii++)