From 6a3691b7436505835989e7464a50c8c8bd542d75 Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Thu, 2 Aug 2012 14:10:00 -0400 Subject: [PATCH] network: merge relevant virtualports rather than choosing one One of the original ideas behind allowing a in an interface definition as well as in the definition *and*one or more 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 definitions on host ABC of: testA ... ... and the same on host DEF of: testA ... ... and a guest definition of: ... If the guest was started on host ABC, the used would be: but if that guest was started on host DEF, the would be: Additionally, if none of the involved s had a specified type (this includes cases where no virtualport is given at all), --- docs/formatdomain.html.in | 74 +++++++++++++++++++++++++++++++------ docs/formatnetwork.html.in | 27 ++++++++++---- src/network/bridge_driver.c | 74 ++++++++++++++++++++++++++----------- 3 files changed, 135 insertions(+), 40 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f97c630bed..ec160bd2ad 100644 --- a/docs/formatdomain.html.in +++ b/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. Since 0.9.4). Also, - similar to direct network connections (described - below), a connection of type network may specify - a virtportprofile element, with configuration data - to be forwarded to a vepa or 802.1Qbh compliant switch. + connections. Since 0.9.4. +

+

+ Also, similar to direct network connections + (described below), a connection of type network may + specify a virtualport element, with configuration + data to be forwarded to a vepa (802.1Qbg) or 802.1Qbh compliant + switch (Since 0.8.2), or to an + Open vSwitch virtual switch (Since + 0.9.11). +

+

+ Since the actual type of switch may vary depending on the + configuration in the <network> on the host, + it is acceptable to omit the virtualport type + attribute, and specify attributes from multiple different + virtualport types (and also to leave out certain attributes); at + domain startup time, a complete <virtualport> + element will be constructed by merging together the type and + attributes found in the which will be filled in from the network + or portgroup <virtualport>) + (Since 0.10.0). 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 instanceid (in case the switch is 802.1Qbh) and + an interfaceid (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 <virtualport>). 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. 

@@ -2248,8 +2277,8 @@
       <source network='default' portgroup='engineering'/>
       <target dev='vnet7'/>
       <mac address="00:11:22:33:44:55"/>
-      <virtualport type='802.1Qbg'>
-        <parameters managerid='11' typeid='1193047' typeidversion='2' instanceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/>
+      <virtualport>
+        <parameters instanceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/>
       </virtualport>
 
     </interface>
@@ -2261,7 +2290,7 @@
     

       
       This is the recommended config for general guest connectivity on
-      hosts with static wired networking configs
+      hosts with static wired networking configs.
       
     

 
@@ -2276,19 +2305,40 @@
       configuration is whatever is used on the LAN. This provides the guest VM
       full incoming & outgoing net access just like a physical machine.
     

-
+    

+      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 <virtualport type='openvswitch'/> to the
+      interface definition.  (Since
+      0.9.11). The Open vSwitch type virtualport accepts two
+      parameters in its <parameters> element -
+      an interfaceid 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 profileid which is sent to Open vSwitch as
+      the interfaces "port-profile".
+    

 
   ...
   <devices>
-    <interface type='bridge'>
-      <source bridge='br0'/>
-    </interface>
     ...
     <interface type='bridge'>
       <source bridge='br0'/>
+    </interface>
+    <interface type='bridge'>
+      <source bridge='br1'/>
       <target dev='vnet7'/>
       <mac address="00:11:22:33:44:55"/>
     </interface>
+    <interface type='bridge'>
+      <source bridge='ovsbr'/>
+      <virtualport type='openvswitch'/>
+        <parameters profileid='menial' interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/>
+      </virtualport>
+    </interface>
+    ...
   </devices>
   ...

 
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 7e8e991b4b..daa6524f0f 100644
--- a/docs/formatnetwork.html.in
+++ b/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 <bridge name='xyz'/> 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 <interface> subelements)
+            specified, Since 0.9.4), 2) an
+            existing Open vSwitch bridge that was configured outside of
+            libvirt (if both a <bridge name='xyz'/>
+            element and a <virtualport
+            type='openvswitch'/> have been
+            specified Since 0.10.0) 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 <interface>
+            subelements, Since 0.9.4)
             (see Direct
             attachment to physical interface 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 <bandwidth>
-      or <virtualport> 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 <virtualport> 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.
     

 
     
Addressing

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index d9646fb5d9..ec99e4dceb 100644
--- a/src/network/bridge_driver.c
+++ b/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,
+                               _(" 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;
 
         /*  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,
+                               _(" 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++)