Re: [libvirt] [PATCHv2 9/9] 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 | 72 ++++++++++++++++++++++++++++++++++++------- docs/formatnetwork.html.in | 27 ++++++++++++----- src/network/bridge_driver.c | 74 ++++++++++++++++++++++++++++++++------------- 3 files changed, 134 insertions(+), 39 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f97c630..f3b3fa8 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. <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 + OpenvSwitch 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 OpenvSwitch

+ 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 OpenvSwitch)

+ (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 OpenvSwitch, it is also

+ possible to connect to an openvswitch 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 OpenvSwitch 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 OpenvSwitch (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 OpenvSwitch 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> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 7e8e991..0ba4d2d 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 <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 OpenvSwitch 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> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index d9646fb..ec99e4d 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, + _("<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++) -- 1.7.11.2 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list