[libvirt] [PATCH v2] RFC Libvirt + Openvswitch
Ansis Atteka
aatteka at nicira.com
Wed Feb 15 17:00:44 UTC 2012
On Mon, Feb 13, 2012 at 9:56 PM, Laine Stump <laine at laine.org> wrote:
> On 02/10/2012 04:09 PM, Ansis Atteka wrote:
> > This patch allows libvirt to add interfaces to already
> > existing Open vSwitch bridges. The following syntax in
> > domain XML file can be used:
> >
> > <interface type='bridge'>
> > <mac address='52:54:00:d0:3f:f2'/>
> > <source bridge='ovsbr'/>
> > <virtualport type='openvswitch'>
> > <parameters interfaceid='921a80cd-e6de-5a2e-db9c-ab27f15a6e1d'/>
> > </virtualport>
> > <address type='pci' domain='0x0000' bus='0x00' slot='0x03'
> function='0x0'/>
> > </interface>
> >
> > or if libvirt should auto-generate the interfaceid use
> > following syntax:
> >
> > <interface type='bridge'>
> > <mac address='52:54:00:d0:3f:f2'/>
> > <source bridge='ovsbr'/>
> > <virtualport type='openvswitch'>
> > </virtualport>
>
> Everything in this patch looks really good - it could be pushed with
> just a few nits fixed (detailed later on).
>
> This bit right above which demonstrates a case where interfaceid will be
> autogenerated is the only concern I have left - as I mentioned before,
> although you're doing the auto-generate of the interfaceid exactly the
> same as is done for instanceid (and also uuid and macaddr), this is
> going to cause a problem when we get to defining networks for
> openvswitch. In that case, to specify that a network uses openvswitch,
> we'll want to do this:
>
> <network>
> <name>ovs-network</name>
> <forward mode='bridge'/>
> <bridge name='ovbr0'/>
> <virtualport type='openvswitch'>
> </virtualport>
> </network>
>
I haven't put a lot of thought into this, but I guess once we get to
networks, we might consider to use following syntax in the network
XML:
<network>
<name>ovs-network</name>
<forward mode='bridge'/>
<bridge name='ovbr0' type='[openvswitch|linux|default]' />
</network>
This would ensure that switch from Linux to OVS bridge networks
would not require any changes to existing XML config.
Anyway this is something that would require more careful thinking.
>
> But because an interfaceid is auto-generated by the parser when one
> isn't given, and a common parser function for virtualport is used by the
> domain and network parsers, this will result in an interfaceid being
> filled in when the network is defined, which makes no sense for a
> <network> definition, since each interface that uses this network is
> supposed to have its own unique interfaceid. However, the code decides
> that the bridge is an openvswitch by looking for the presence of a
> virtualport of type='openvswitch'.
>
> I can see a couple ways out of this:
>
> 1) we could add an arg to virNetDevVPortProfileParse:
>
> virNetDevVPortProfilePtr
> virNetDevVPortProfileParse(xmlNodePtr node, unsigned int flags);
>
> with a possible flag being VIR_NETDEV_VPORT_AUTOGEN
>
> If that flag is given, interfaceid and instanceid will be auto-generated
> if necessary during parse, otherwise they will be left blank.
>
> 2) Rather than deciding which type of bridge we're dealing with by
> looking at the config, we could try to do it by direct examination of
> the system.
>
The first thing that comes into my mind would be to use SIOCETHTOOL
ioctl call with cmd==ETHTOOL_GDRVINFO. Then the bridge's driver name
would be either "linux" or "openvswitch".
>
> Something along the lines of (2) may be a good idea anyway, especially
> if we think about the idea of a guest that might migrate between a host
> that has openvswitch and one that doesn't, with the restriction that we
> want a persistent interfaceid for the times it's connected to an
> openvswitch. In that case, the interface definition would look like:
>
> <interface type='network'>
> <source network='bridged-network'>
> <mac address='00:11:22:33:44:55'/>
> <virtualport type='openvswitch'/>
> <parameters interfaceid='921a80cd-e6de-5a2e-db9c-ab27f15a6e1d'
> profileid='test-profile'/>
> </virtualport>
> </interface>
>
> (you would need to specify the virtualport in the <interface> so that it
> would always be the same). Of course if you migrated to a machine where
> bridged-network happened to describe an openvswitch, this would
> coincidentally work correctly. But if bridged-network described a
> standard linux bridge, we would be trying to do openvswitch commands on
> a device that wasn't an openvswitch!
>
> So, I'm wondering if there's a simple way to confirm that a device
> *isn't* an openvswitch without relying on calling any utilities that are
> part of the ovs package.
>
> I'm thinking it may be okay to push this code more or less as-is though,
> since it works properly for the case of straight <interface
> type='bridge'/>, and the changes I'm talking about are refinements that
> are backward compatible, and are only necessary when we add support for
> <network>s that use openvswitch. Any other libvirt regulars have an
> opinion on this?
>
>
> > <address type='pci' domain='0x0000' bus='0x00' slot='0x03'
> function='0x0'/>
> > </interface>
> >
> > It is also possible to pass an optional profileid. To do that
> > use following syntax:
> >
> > <interface type='bridge'>
> > <source bridge='ovsbr'/>
> > <mac address='00:55:1a:65:a2:8d'/>
> > <virtualport type='openvswitch'>
> > <parameters interfaceid='921a80cd-e6de-5a2e-db9c-ab27f15a6e1d'
> profileid='test-profile'/>
> > </virtualport>
> > </interface>
> >
> > To create Open vSwitch bridge install Open vSwitch and
> > run the following command:
> >
> > ovs-vsctl add-br ovsbr
> > ---
> > configure.ac | 5 +
> > src/Makefile.am | 1 +
> > src/conf/domain_conf.c | 51 +++++++++++--
> > src/conf/domain_conf.h | 5 +-
> > src/conf/netdev_vport_profile_conf.c | 47 +++++++++++-
> > src/libvirt_private.syms | 7 ++-
> > src/lxc/lxc_driver.c | 37 +++++++--
> > src/network/bridge_driver.c | 3 +-
> > src/qemu/qemu_command.c | 5 +-
> > src/qemu/qemu_hotplug.c | 17 ++++-
> > src/qemu/qemu_migration.c | 4 +-
> > src/qemu/qemu_process.c | 10 ++-
> > src/uml/uml_conf.c | 3 +-
> > src/util/virnetdevopenvswitch.c | 135
> ++++++++++++++++++++++++++++++++++
> > src/util/virnetdevopenvswitch.h | 42 +++++++++++
> > src/util/virnetdevtap.c | 14 +++-
> > src/util/virnetdevtap.h | 5 +-
> > src/util/virnetdevvportprofile.c | 2 +
> > src/util/virnetdevvportprofile.h | 7 ++-
> > 19 files changed, 365 insertions(+), 35 deletions(-)
> > create mode 100644 src/util/virnetdevopenvswitch.c
> > create mode 100644 src/util/virnetdevopenvswitch.h
> >
> > diff --git a/configure.ac b/configure.ac
> > index 9fb7bfc..dca178f 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -213,6 +213,8 @@ AC_PATH_PROG([UDEVSETTLE], [udevsettle], [],
> > [/sbin:/usr/sbin:/usr/local/sbin:$PATH])
> > AC_PATH_PROG([MODPROBE], [modprobe], [],
> > [/sbin:/usr/sbin:/usr/local/sbin:$PATH])
> > +AC_PATH_PROG([OVSVSCTL], [ovs-vsctl], [ovs-vsctl],
> > + [/sbin:/usr/sbin:/usr/local/sbin:$PATH])
> >
> > AC_DEFINE_UNQUOTED([DNSMASQ],["$DNSMASQ"],
> > [Location or name of the dnsmasq program])
> > @@ -220,6 +222,9 @@ AC_DEFINE_UNQUOTED([RADVD],["$RADVD"],
> > [Location or name of the radvd program])
> > AC_DEFINE_UNQUOTED([TC],["$TC"],
> > [Location or name of the tc profram (see iproute2)])
> > +AC_DEFINE_UNQUOTED([OVSVSCTL],["$OVSVSCTL"],
> > + [Location or name of the ovs-vsctl program])
> > +
> > if test -n "$UDEVADM"; then
> > AC_DEFINE_UNQUOTED([UDEVADM],["$UDEVADM"],
> > [Location or name of the udevadm program])
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index a3dd847..d5f52a0 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -97,6 +97,7 @@ UTIL_SOURCES =
> \
> > util/virnetdevbandwidth.h util/virnetdevbandwidth.c \
> > util/virnetdevbridge.h util/virnetdevbridge.c \
> > util/virnetdevmacvlan.c util/virnetdevmacvlan.h \
> > + util/virnetdevopenvswitch.h util/virnetdevopenvswitch.c \
> > util/virnetdevtap.h util/virnetdevtap.c \
> > util/virnetdevveth.h util/virnetdevveth.c \
> > util/virnetdevvportprofile.h util/virnetdevvportprofile.c \
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index a09a506..5b5db43 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -31,6 +31,7 @@
> > #include <sys/time.h>
> > #include <strings.h>
> >
> > +#include "internal.h"
> > #include "virterror_internal.h"
> > #include "datatypes.h"
> > #include "domain_conf.h"
> > @@ -951,6 +952,7 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr
> def)
> > switch (def->type) {
> > case VIR_DOMAIN_NET_TYPE_BRIDGE:
> > VIR_FREE(def->data.bridge.brname);
> > + VIR_FREE(def->data.bridge.ovsPort);
>
> We should change all occurrences of ovsPort to virtPortProfile for
> consistency, and to allow for future use of the portprofile here for
> other non-ovs scenarios.
>
> > break;
> > case VIR_DOMAIN_NET_TYPE_DIRECT:
> > VIR_FREE(def->data.direct.linkdev);
> > @@ -994,6 +996,7 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
> > case VIR_DOMAIN_NET_TYPE_BRIDGE:
> > VIR_FREE(def->data.bridge.brname);
> > VIR_FREE(def->data.bridge.ipaddr);
> > + VIR_FREE(def->data.bridge.ovsPort);
> > break;
> >
> > case VIR_DOMAIN_NET_TYPE_INTERNAL:
> > @@ -3732,7 +3735,11 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
> > }
> >
> > if (actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
> > - actual->data.bridge.brname =
> virXPathString("string(./source[1]/@bridge)", ctxt);
> > + xmlNodePtr virtPortNode = virXPathNode("./virtualport", ctxt);
> > + if (virtPortNode &&
> > + (!(actual->data.bridge.ovsPort =
> > + virNetDevVPortProfileParse(virtPortNode))))
> > + goto error;
> > } else if (actual->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
> > xmlNodePtr virtPortNode;
> >
> > @@ -3861,7 +3868,8 @@ virDomainNetDefParseXML(virCapsPtr caps,
> > mode = virXMLPropString(cur, "mode");
> > } else if ((virtPort == NULL) &&
> > ((def->type == VIR_DOMAIN_NET_TYPE_DIRECT) ||
> > - (def->type == VIR_DOMAIN_NET_TYPE_NETWORK)) &&
> > + (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) ||
> > + (def->type == VIR_DOMAIN_NET_TYPE_BRIDGE)) &&
> > xmlStrEqual(cur->name, BAD_CAST "virtualport")) {
> > if (!(virtPort = virNetDevVPortProfileParse(cur)))
> > goto error;
> > @@ -4000,6 +4008,8 @@ virDomainNetDefParseXML(virCapsPtr caps,
> > def->data.bridge.ipaddr = address;
> > address = NULL;
> > }
> > + def->data.bridge.ovsPort = virtPort;
> > + virtPort = NULL;
> > break;
> >
> > case VIR_DOMAIN_NET_TYPE_CLIENT:
> > @@ -10429,6 +10439,12 @@ virDomainActualNetDefFormat(virBufferPtr buf,
> > case VIR_DOMAIN_NET_TYPE_BRIDGE:
> > virBufferEscapeString(buf, " <source bridge='%s'/>\n",
> > def->data.bridge.brname);
> > + if (def->data.bridge.ovsPort) {
> > + virBufferAdjustIndent(buf, 6);
> > + if (virNetDevVPortProfileFormat(def->data.bridge.ovsPort,
> buf) < 0)
> > + return -1;
> > + virBufferAdjustIndent(buf, -6);
> > + }
> > break;
> >
> > case VIR_DOMAIN_NET_TYPE_DIRECT:
> > @@ -10516,6 +10532,12 @@ virDomainNetDefFormat(virBufferPtr buf,
> > if (def->data.bridge.ipaddr)
> > virBufferAsprintf(buf, " <ip address='%s'/>\n",
> > def->data.bridge.ipaddr);
> > + if (def->data.bridge.ovsPort) {
> > + virBufferAdjustIndent(buf, 6);
> > + if (virNetDevVPortProfileFormat(def->data.bridge.ovsPort,
> buf) < 0)
> > + return -1;
> > + virBufferAdjustIndent(buf, -6);
> > + }
> > break;
> >
> > case VIR_DOMAIN_NET_TYPE_SERVER:
> > @@ -13832,15 +13854,27 @@
> virDomainNetGetActualDirectMode(virDomainNetDefPtr iface)
> > }
> >
> > virNetDevVPortProfilePtr
> > -virDomainNetGetActualDirectVirtPortProfile(virDomainNetDefPtr iface)
> > +virDomainNetGetActualVirtPortProfile(virDomainNetDefPtr iface)
>
>
> In hindsight, renaming this function would have been better done in a
> separate pre-requisite patch, just to avoid clutter in this patch.
>
>
> > {
> > - if (iface->type == VIR_DOMAIN_NET_TYPE_DIRECT)
> > + switch (iface->type) {
> > + case VIR_DOMAIN_NET_TYPE_DIRECT:
> > return iface->data.direct.virtPortProfile;
> > - if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
> > - return NULL;
> > - if (!iface->data.network.actual)
> > + case VIR_DOMAIN_NET_TYPE_BRIDGE:
> > + return iface->data.bridge.ovsPort;
> > + case VIR_DOMAIN_NET_TYPE_NETWORK:
> > + if (!iface->data.network.actual)
> > + return NULL;
> > + switch (iface->data.network.actual->type) {
> > + case VIR_DOMAIN_NET_TYPE_DIRECT:
> > + return
> iface->data.network.actual->data.direct.virtPortProfile;
> > + case VIR_DOMAIN_NET_TYPE_BRIDGE:
> > + return iface->data.network.actual->data.bridge.ovsPort;
> > + default:
> > + return NULL;
> > + }
> > + default:
> > return NULL;
> > - return iface->data.network.actual->data.direct.virtPortProfile;
> > + }
> > }
> >
> > virNetDevBandwidthPtr
> > @@ -13853,7 +13887,6 @@
> virDomainNetGetActualBandwidth(virDomainNetDefPtr iface)
> > return iface->bandwidth;
> > }
> >
> > -
>
> stray extra whitespace change...
>
> > /* Return listens[ii] from the appropriate union for the graphics
> > * type, or NULL if this is an unsuitable type, or the index is out of
> > * bounds. If force0 is TRUE, ii == 0, and there is no listen array,
> > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> > index 8bb21cf..c4c3551 100644
> > --- a/src/conf/domain_conf.h
> > +++ b/src/conf/domain_conf.h
> > @@ -41,6 +41,7 @@
> > # include "virnetdevmacvlan.h"
> > # include "sysinfo.h"
> > # include "virnetdevvportprofile.h"
> > +# include "virnetdevopenvswitch.h"
> > # include "virnetdevbandwidth.h"
> >
> > /* Different types of hypervisor */
> > @@ -596,6 +597,7 @@ struct _virDomainActualNetDef {
> > union {
> > struct {
> > char *brname;
> > + virNetDevVPortProfilePtr ovsPort;
>
>
> These should be named the same as they are in the existing cases
> (virtPortProfile), since there may be other uses for a virtPortProfile
> on a bridge aside from openvswitch.
>
>
> > } bridge;
> > struct {
> > char *linkdev;
> > @@ -647,6 +649,7 @@ struct _virDomainNetDef {
> > struct {
> > char *brname;
> > char *ipaddr;
> > + virNetDevVPortProfilePtr ovsPort;
>
> Likewise.
>
> > } bridge;
> > struct {
> > char *name;
> > @@ -1875,7 +1878,7 @@ const char
> *virDomainNetGetActualBridgeName(virDomainNetDefPtr iface);
> > const char *virDomainNetGetActualDirectDev(virDomainNetDefPtr iface);
> > int virDomainNetGetActualDirectMode(virDomainNetDefPtr iface);
> > virNetDevVPortProfilePtr
> > -virDomainNetGetActualDirectVirtPortProfile(virDomainNetDefPtr iface);
> > +virDomainNetGetActualVirtPortProfile(virDomainNetDefPtr iface);
> > virNetDevBandwidthPtr
> > virDomainNetGetActualBandwidth(virDomainNetDefPtr iface);
> >
> > diff --git a/src/conf/netdev_vport_profile_conf.c
> b/src/conf/netdev_vport_profile_conf.c
> > index b48b2cb..205d674 100644
> > --- a/src/conf/netdev_vport_profile_conf.c
> > +++ b/src/conf/netdev_vport_profile_conf.c
> > @@ -35,7 +35,8 @@
> > VIR_ENUM_IMPL(virNetDevVPort, VIR_NETDEV_VPORT_PROFILE_LAST,
> > "none",
> > "802.1Qbg",
> > - "802.1Qbh")
> > + "802.1Qbh",
> > + "openvswitch")
> >
> >
> > virNetDevVPortProfilePtr
> > @@ -47,6 +48,7 @@ virNetDevVPortProfileParse(xmlNodePtr node)
> > char *virtPortTypeIDVersion = NULL;
> > char *virtPortInstanceID = NULL;
> > char *virtPortProfileID = NULL;
> > + char *virtPortInterfaceID = NULL;
> > virNetDevVPortProfilePtr virtPort = NULL;
> > xmlNodePtr cur = node->children;
> >
> > @@ -76,7 +78,7 @@ virNetDevVPortProfileParse(xmlNodePtr node)
> > virtPortTypeIDVersion = virXMLPropString(cur,
> "typeidversion");
> > virtPortInstanceID = virXMLPropString(cur, "instanceid");
> > virtPortProfileID = virXMLPropString(cur, "profileid");
> > -
> > + virtPortInterfaceID = virXMLPropString(cur, "interfaceid");
> > break;
> > }
> >
> > @@ -171,6 +173,33 @@ virNetDevVPortProfileParse(xmlNodePtr node)
> > goto error;
> > }
> > break;
> > + case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
> > + if (virtPortInterfaceID != NULL) {
> > + if (virUUIDParse(virtPortInterfaceID,
> > + virtPort->u.openvswitch.interfaceID)) {
> > + virNetDevError(VIR_ERR_XML_ERROR, "%s",
> > + _("cannot parse interfaceid
> parameter as a uuid"));
> > + goto error;
> > + }
> > + } else {
> > + if (virUUIDGenerate(virtPort->u.openvswitch.interfaceID)) {
> > + virNetDevError(VIR_ERR_XML_ERROR, "%s",
> > + _("cannot generate a random uuid
> for interfaceid"));
> > + goto error;
> > + }
> > + }
> > + /* profileid is not mandatory for Open vSwitch */
> > + if (virtPortProfileID != NULL) {
> > + if (virStrcpyStatic(virtPort->u.openvswitch.profileID,
> > + virtPortProfileID) == NULL) {
> > + virNetDevError(VIR_ERR_XML_ERROR, "%s",
> > + _("profileid parameter too long"));
>
>
> I'm not sure why the existing profileID is a fixed length. If that's
> something that is limiting to openvswitch, we should take care of that
> limitation. But that shouldn't hole up this patch going in (since you're
> only propagating an existing practice).
>
> > + goto error;
> > + }
> > + } else {
> > + virtPort->u.openvswitch.profileID[0] = '\0';
> > + }
> > + break;
> >
> > default:
> > virNetDevError(VIR_ERR_XML_ERROR,
> > @@ -225,6 +254,20 @@
> virNetDevVPortProfileFormat(virNetDevVPortProfilePtr virtPort,
> > virtPort->u.virtPort8021Qbh.profileID);
> > break;
> >
> > + case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
> > + virUUIDFormat(virtPort->u.openvswitch.interfaceID,
> > + uuidstr);
> > + if (virtPort->u.openvswitch.profileID[0] == '\0') {
> > + virBufferAsprintf(buf, " <parameters interfaceid='%s'/>\n",
> > + uuidstr);
> > + } else {
> > + virBufferAsprintf(buf, " <parameters interfaceid='%s' "
> > + "profileid='%s'/>\n", uuidstr,
> > + virtPort->u.openvswitch.profileID);
> > + }
> > +
> > + break;
> > +
> > default:
> > virNetDevError(VIR_ERR_XML_ERROR,
> > _("unexpected virtualport type %d"),
> virtPort->virtPortType);
> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> > index 0c22dec..ad5c7d3 100644
> > --- a/src/libvirt_private.syms
> > +++ b/src/libvirt_private.syms
> > @@ -378,7 +378,7 @@ virDomainNetGetActualBandwidth;
> > virDomainNetGetActualBridgeName;
> > virDomainNetGetActualDirectDev;
> > virDomainNetGetActualDirectMode;
> > -virDomainNetGetActualDirectVirtPortProfile;
> > +virDomainNetGetActualVirtPortProfile;
>
> We like to keep these in alphabetic order, so this one will have to move
> down.
>
>
> > virDomainNetGetActualType;
> > virDomainNetIndexByMac;
> > virDomainNetInsert;
> > @@ -1237,6 +1237,11 @@ virNetDevMacVLanCreateWithVPortProfile;
> > virNetDevMacVLanDeleteWithVPortProfile;
> >
> >
> > +# virnetdevopenvswitch.h
> > +virNetDevOpenvswitchAddPort;
> > +virNetDevOpenvswitchRemovePort;
> > +
> > +
> > # virnetdevtap.h
> > virNetDevTapCreate;
> > virNetDevTapCreateInBridgePort;
> > diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> > index b712da4..17e8312 100644
> > --- a/src/lxc/lxc_driver.c
> > +++ b/src/lxc/lxc_driver.c
> > @@ -56,6 +56,7 @@
> > #include "domain_nwfilter.h"
> > #include "network/bridge_driver.h"
> > #include "virnetdev.h"
> > +#include "virnetdevtap.h"
> > #include "virnodesuspend.h"
> > #include "virtime.h"
> > #include "virtypedparam.h"
> > @@ -1105,6 +1106,7 @@ static void lxcVmCleanup(lxc_driver_t *driver,
> > virCgroupPtr cgroup;
> > int i;
> > lxcDomainObjPrivatePtr priv = vm->privateData;
> > + virNetDevVPortProfilePtr vport = NULL;
> >
> > /* now that we know it's stopped call the hook if present */
> > if (virHookPresent(VIR_HOOK_DRIVER_LXC)) {
> > @@ -1132,10 +1134,15 @@ static void lxcVmCleanup(lxc_driver_t *driver,
> > priv->monitorWatch = -1;
> >
> > for (i = 0 ; i < vm->def->nnets ; i++) {
> > - ignore_value(virNetDevSetOnline(vm->def->nets[i]->ifname,
> false));
> > - ignore_value(virNetDevVethDelete(vm->def->nets[i]->ifname));
> > -
> > - networkReleaseActualDevice(vm->def->nets[i]);
> > + virDomainNetDefPtr iface = vm->def->nets[i];
> > + vport = virDomainNetGetActualVirtPortProfile(iface);
> > + ignore_value(virNetDevSetOnline(iface->ifname, false));
> > + if (vport && vport->virtPortType ==
> VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH)
> > + ignore_value(virNetDevOpenvswitchRemovePort(
> > +
> virDomainNetGetActualBridgeName(iface),
> > + iface->ifname));
> > + ignore_value(virNetDevVethDelete(iface->ifname));
> > + networkReleaseActualDevice(iface);
> > }
> >
> > virDomainConfVMNWFilterTeardown(vm);
> > @@ -1165,6 +1172,7 @@ static int lxcSetupInterfaceBridged(virConnectPtr
> conn,
> > int ret = -1;
> > char *parentVeth;
> > char *containerVeth = NULL;
> > + const virNetDevVPortProfilePtr vport =
> virDomainNetGetActualVirtPortProfile(net);
> >
> > VIR_DEBUG("calling vethCreate()");
> > parentVeth = net->ifname;
> > @@ -1186,7 +1194,11 @@ static int lxcSetupInterfaceBridged(virConnectPtr
> conn,
> > if (virNetDevSetMAC(containerVeth, net->mac) < 0)
> > goto cleanup;
> >
> > - if (virNetDevBridgeAddPort(brname, parentVeth) < 0)
> > + if (vport && vport->virtPortType ==
> VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH)
> > + ret = virNetDevOpenvswitchAddPort(brname, parentVeth, net->mac,
> vport);
> > + else
> > + ret = virNetDevBridgeAddPort(brname, parentVeth);
> > + if (ret < 0)
> > goto cleanup;
> >
> > if (virNetDevSetOnline(parentVeth, true) < 0)
> > @@ -1242,7 +1254,7 @@ static int lxcSetupInterfaceDirect(virConnectPtr
> conn,
> > * and automagically dies when the container dies. So
> > * we have no dev to perform disassociation with.
> > */
> > - prof = virDomainNetGetActualDirectVirtPortProfile(net);
> > + prof = virDomainNetGetActualVirtPortProfile(net);
> > if (prof) {
> > lxcError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > _("Unable to set port profile on direct interfaces"));
> > @@ -1260,7 +1272,7 @@ static int lxcSetupInterfaceDirect(virConnectPtr
> conn,
> > virDomainNetGetActualDirectDev(net),
> > virDomainNetGetActualDirectMode(net),
> > false, false, def->uuid,
> > - virDomainNetGetActualDirectVirtPortProfile(net),
> > + virDomainNetGetActualVirtPortProfile(net),
> > &res_ifname,
> > VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
> > driver->stateDir,
> > @@ -1377,8 +1389,15 @@ static int lxcSetupInterfaces(virConnectPtr conn,
> >
> > cleanup:
> > if (ret != 0) {
> > - for (i = 0 ; i < def->nnets ; i++)
> > - networkReleaseActualDevice(def->nets[i]);
> > + for (i = 0 ; i < def->nnets ; i++) {
> > + virDomainNetDefPtr iface = def->nets[i];
> > + virNetDevVPortProfilePtr vport =
> virDomainNetGetActualVirtPortProfile(iface);
> > + if (vport && vport->virtPortType ==
> VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH)
> > + ignore_value(virNetDevOpenvswitchRemovePort(
> > +
> virDomainNetGetActualBridgeName(iface),
> > + iface->ifname));
> > + networkReleaseActualDevice(iface);
> > + }
>
> While going through this, I noticed what seemed to be some
> inconsistencies in the way lxc cleans up network interfaces (prior to
> your changes), but some of the code also led me to believe I don't
> understand everything that's going on, so I'm going to say your patches
> here are okay, as they don't change behavior for non-ovs bridge devices.
>
> > }
> > return ret;
> > }
> > diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> > index 57ebb9f..1423d3f 100644
> > --- a/src/network/bridge_driver.c
> > +++ b/src/network/bridge_driver.c
> > @@ -1765,7 +1765,8 @@ networkStartNetworkVirtual(struct network_driver
> *driver,
> > goto err0;
> > }
> > if (virNetDevTapCreateInBridgePort(network->def->bridge,
> > - &macTapIfName,
> network->def->mac, 0, false, NULL) < 0) {
> > + &macTapIfName, network->def->mac, 0,
> > + false, NULL, NULL) < 0) {
>
> There's an indentation problem here.
>
>
> > VIR_FREE(macTapIfName);
> > goto err0;
> > }
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 1b92aa4..591e371 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -154,7 +154,7 @@ qemuPhysIfaceConnect(virDomainDefPtr def,
> > virDomainNetGetActualDirectDev(net),
> > virDomainNetGetActualDirectMode(net),
> > true, vnet_hdr, def->uuid,
> > - virDomainNetGetActualDirectVirtPortProfile(net),
> > + virDomainNetGetActualVirtPortProfile(net),
> > &res_ifname,
> > vmop, driver->stateDir,
> > virDomainNetGetActualBandwidth(net));
> > @@ -247,7 +247,8 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
> > memcpy(tapmac, net->mac, VIR_MAC_BUFLEN);
> > tapmac[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */
> > err = virNetDevTapCreateInBridgePort(brname, &net->ifname, tapmac,
> > - vnet_hdr, true, &tapfd);
> > + vnet_hdr, true, &tapfd,
> > + virDomainNetGetActualVirtPortProfile(net));
> > virDomainAuditNetDevice(def, net, "/dev/net/tun", tapfd >= 0);
> > if (err < 0) {
> > if (template_ifname)
> > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> > index b4870be..3b1dbbf 100644
> > --- a/src/qemu/qemu_hotplug.c
> > +++ b/src/qemu/qemu_hotplug.c
> > @@ -40,6 +40,7 @@
> > #include "qemu_cgroup.h"
> > #include "locking/domain_lock.h"
> > #include "network/bridge_driver.h"
> > +#include "virnetdevtap.h"
> >
> > #define VIR_FROM_THIS VIR_FROM_QEMU
> >
> > @@ -652,6 +653,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
> > int vhostfd = -1;
> > char *nicstr = NULL;
> > char *netstr = NULL;
> > + virNetDevVPortProfilePtr vport = NULL;
> > int ret = -1;
> > virDomainDevicePCIAddress guestAddr;
> > int vlan;
> > @@ -838,6 +840,12 @@ cleanup:
> > if (iface_connected)
> > virDomainConfNWFilterTeardown(net);
> >
> > + vport = virDomainNetGetActualVirtPortProfile(net);
> > + if (vport && vport->virtPortType ==
> VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH)
> > + ignore_value(virNetDevOpenvswitchRemovePort(
> > +
> virDomainNetGetActualBridgeName(net),
> > + net->ifname));
>
> Again a small indentation problem.
>
> > +
> > networkReleaseActualDevice(net);
> > }
> >
> > @@ -1833,6 +1841,7 @@ int qemuDomainDetachNetDevice(struct qemud_driver
> *driver,
> > qemuDomainObjPrivatePtr priv = vm->privateData;
> > int vlan;
> > char *hostnet_name = NULL;
> > + virNetDevVPortProfilePtr vport = NULL;
> >
> > for (i = 0 ; i < vm->def->nnets ; i++) {
> > virDomainNetDefPtr net = vm->def->nets[i];
> > @@ -1923,7 +1932,7 @@ int qemuDomainDetachNetDevice(struct qemud_driver
> *driver,
> > detach->ifname, detach->mac,
> > virDomainNetGetActualDirectDev(detach),
> > virDomainNetGetActualDirectMode(detach),
> > -
> virDomainNetGetActualDirectVirtPortProfile(detach),
> > + virDomainNetGetActualVirtPortProfile(detach),
> > driver->stateDir));
> > VIR_FREE(detach->ifname);
> > }
> > @@ -1938,6 +1947,12 @@ int qemuDomainDetachNetDevice(struct qemud_driver
> *driver,
> > }
> > }
> >
> > + vport = virDomainNetGetActualVirtPortProfile(detach);
> > + if (vport && vport->virtPortType ==
> VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH)
> > + ignore_value(virNetDevOpenvswitchRemovePort(
> > +
> virDomainNetGetActualBridgeName(detach),
> > + detach->ifname));
> > +
> > networkReleaseActualDevice(detach);
> > if (vm->def->nnets > 1) {
> > memmove(vm->def->nets + i,
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 12cfbde..a513b3c 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
> > @@ -2563,7 +2563,7 @@
> qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) {
> > net = def->nets[i];
> > if (virDomainNetGetActualType(net) ==
> VIR_DOMAIN_NET_TYPE_DIRECT) {
> > if (virNetDevVPortProfileAssociate(net->ifname,
> > -
> virDomainNetGetActualDirectVirtPortProfile(net),
> > +
> virDomainNetGetActualVirtPortProfile(net),
> > net->mac,
> >
> virDomainNetGetActualDirectDev(net),
> > def->uuid,
> > @@ -2580,7 +2580,7 @@ err_exit:
> > net = def->nets[i];
> > if (virDomainNetGetActualType(net) ==
> VIR_DOMAIN_NET_TYPE_DIRECT) {
> > ignore_value(virNetDevVPortProfileDisassociate(net->ifname,
> > -
> virDomainNetGetActualDirectVirtPortProfile(net),
> > +
> virDomainNetGetActualVirtPortProfile(net),
> > net->mac,
> >
> virDomainNetGetActualDirectDev(net),
> >
> VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH));
> > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> > index 2d92d66..9bc5436 100644
> > --- a/src/qemu/qemu_process.c
> > +++ b/src/qemu/qemu_process.c
> > @@ -62,6 +62,7 @@
> > #include "network/bridge_driver.h"
> > #include "uuid.h"
> > #include "virtime.h"
> > +#include "virnetdevtap.h"
> >
> > #define VIR_FROM_THIS VIR_FROM_QEMU
> >
> > @@ -3604,6 +3605,7 @@ void qemuProcessStop(struct qemud_driver *driver,
> > qemuDomainObjPrivatePtr priv = vm->privateData;
> > virErrorPtr orig_err;
> > virDomainDefPtr def;
> > + virNetDevVPortProfilePtr vport = NULL;
> > int i;
> > int logfile = -1;
> > char *timestamp;
> > @@ -3724,13 +3726,19 @@ void qemuProcessStop(struct qemud_driver *driver,
> > net->ifname, net->mac,
> > virDomainNetGetActualDirectDev(net),
> > virDomainNetGetActualDirectMode(net),
> > -
> virDomainNetGetActualDirectVirtPortProfile(net),
> > + virDomainNetGetActualVirtPortProfile(net),
> > driver->stateDir));
> > VIR_FREE(net->ifname);
> > }
> > /* release the physical device (or any other resources used by
> > * this interface in the network driver
> > */
> > + vport = virDomainNetGetActualVirtPortProfile(net);
> > + if (vport && vport->virtPortType ==
> VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH)
> > + ignore_value(virNetDevOpenvswitchRemovePort(
> > +
> virDomainNetGetActualBridgeName(net),
> > + net->ifname));
> > +
> > networkReleaseActualDevice(net);
> > }
> >
> > diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
> > index 316d558..8c17578 100644
> > --- a/src/uml/uml_conf.c
> > +++ b/src/uml/uml_conf.c
> > @@ -142,7 +142,8 @@ umlConnectTapDevice(virConnectPtr conn,
> > memcpy(tapmac, net->mac, VIR_MAC_BUFLEN);
> > tapmac[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */
> > if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, tapmac,
> > - 0, true, NULL) < 0) {
> > + 0, true, NULL,
> > + virDomainNetGetActualVirtPortProfile(net)) < 0) {
> > if (template_ifname)
> > VIR_FREE(net->ifname);
> > goto error;
> > diff --git a/src/util/virnetdevopenvswitch.c
> b/src/util/virnetdevopenvswitch.c
> > new file mode 100644
> > index 0000000..24a335a
> > --- /dev/null
> > +++ b/src/util/virnetdevopenvswitch.c
> > @@ -0,0 +1,135 @@
> > +/*
> > + * Copyright (C) 2012 Nicira, Inc.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, write to the Free Software
> > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> USA
> > + *
> > + * Authors:
> > + * Dan Wendlandt <dan at nicira.com>
> > + * Kyle Mestery <kmestery at cisco.com>
> > + * Ansis Atteka <aatteka at nicira.com>
> > + */
> > +
> > +#include <config.h>
> > +
> > +#include "virnetdevopenvswitch.h"
> > +#include "command.h"
> > +#include "memory.h"
> > +#include "virterror_internal.h"
> > +#include "ignore-value.h"
> > +#include "virmacaddr.h"
> > +
> > +#define VIR_FROM_THIS VIR_FROM_NONE
> > +
> > +/**
> > + * virNetDevOpenvswitchAddPort:
> > + * @brname: the bridge name
> > + * @ifname: the network interface name
> > + * @macaddr: the mac address of the virtual interface
> > + * @ovsport: the ovs specific fields
> > + *
> > + * Add an interface to the OVS bridge
> > + *
> > + * Returns 0 in case of success or -1 in case of failure.
> > + */
> > +int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
> > + const unsigned char *macaddr,
> > + virNetDevVPortProfilePtr ovsport)
> > +{
> > + int ret = -1;
> > + virCommandPtr cmd = NULL;
> > + char macaddrstr[VIR_MAC_STRING_BUFLEN];
> > + char uuidstr[VIR_UUID_STRING_BUFLEN];
> > + char *attachedmac_ex_id = NULL;
> > + char *ifaceid_ex_id = NULL;
> > + char *profile_ex_id = NULL;
> > +
> > + virMacAddrFormat(macaddr, macaddrstr);
> > + virUUIDFormat(ovsport->u.openvswitch.interfaceID, uuidstr);
> > +
> > + if (virAsprintf(&attachedmac_ex_id,
> "external-ids:attached-mac=\"%s\"",
> > + macaddrstr) < 0)
>
> If the quotes aren't necessary, there is a "virCommandAddArgPair" that
> will do this for you once you've created the command:
>
> virCommandAddArgPair(cmd, "external-ids:attached-mac", macaddrstr);
>
> (I wonder if it would be worthwhile adding a "quote the arg" (and/or
> "quote the arg if necessary") flag to that function (or maybe creating a
> new function virCommandAddArgPairQuoted).
>
> > + goto cleanup;
> > + if (virAsprintf(&ifaceid_ex_id, "external-ids:iface-id=\"%s\"",
> > + uuidstr) < 0)
> > + goto cleanup;
> > + if (ovsport->u.openvswitch.profileID[0] != '\0') {
> > + if (virAsprintf(&profile_ex_id,
> "external-ids:port-profile=\"%s\"",
> > + ovsport->u.openvswitch.profileID) < 0)
> > + goto cleanup;
> > + }
> > +
> > + cmd = virCommandNew(OVSVSCTL);
> > + if (ovsport->u.openvswitch.profileID[0] == '\0') {
> > + virCommandAddArgList(cmd, "--", "--may-exist", "add-port",
> > + brname, ifname,
> > + "--", "set", "Interface", ifname,
> attachedmac_ex_id,
> > + "--", "set", "Interface", ifname, ifaceid_ex_id,
> > + "--", "set", "Interface", ifname,
> > + "external-ids:iface-status=active",
> > + NULL);
> > + } else {
> > + virCommandAddArgList(cmd, "--", "--may-exist", "add-port",
> > + brname, ifname,
> > + "--", "set", "Interface", ifname,
> attachedmac_ex_id,
> > + "--", "set", "Interface", ifname, ifaceid_ex_id,
> > + "--", "set", "Interface", ifname, profile_ex_id,
> > + "--", "set", "Interface", ifname,
> > + "external-ids:iface-status=active",
> > + NULL);
> > + }
> > +
> > + if (virCommandRun(cmd, NULL) < 0) {
> > + virReportSystemError(VIR_ERR_INTERNAL_ERROR,
> > + _("Unable to add port %s to OVS bridge
> %s"),
> > + ifname, brname);
> > + goto cleanup;
> > + }
> > + ret = 0;
> > +
> > + cleanup:
> > + VIR_FREE(attachedmac_ex_id);
> > + VIR_FREE(ifaceid_ex_id);
> > + VIR_FREE(profile_ex_id);
> > + virCommandFree(cmd);
> > + return ret;
> > +}
> > +
> > +/**
> > + * virNetDevOpenvswitchRemovePort:
> > + * @ifname: the network interface name
> > + *
> > + * Deletes an interface from a OVS bridge
> > + *
> > + * Returns 0 in case of success or -1 in case of failure.
> > + */
> > +int virNetDevOpenvswitchRemovePort(const char *brname ATTRIBUTE_UNUSED,
> const char *ifname)
> > +{
> > + int ret = -1;
> > + virCommandPtr cmd = NULL;
> > +
> > + cmd = virCommandNew(OVSVSCTL);
> > + virCommandAddArgList(cmd, "--", "--if-exists", "del-port", ifname,
> NULL);
> > +
> > + if (virCommandRun(cmd, NULL) < 0) {
> > + virReportSystemError(VIR_ERR_INTERNAL_ERROR,
> > + _("Unable to delete port %s from OVS"),
> ifname);
> > + goto cleanup;
> > + }
> > + ret = 0;
> > +
> > + cleanup:
> > + virCommandFree(cmd);
> > + return ret;
> > +}
> > diff --git a/src/util/virnetdevopenvswitch.h
> b/src/util/virnetdevopenvswitch.h
> > new file mode 100644
> > index 0000000..bca4c3e
> > --- /dev/null
> > +++ b/src/util/virnetdevopenvswitch.h
> > @@ -0,0 +1,42 @@
> > +/*
> > + * Copyright (C) 2012 Nicira, Inc.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, write to the Free Software
> > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> USA
> > + *
> > + * Authors:
> > + * Dan Wendlandt <dan at nicira.com>
> > + * Kyle Mestery <kmestery at cisco.com>
> > + * Ansis Atteka <aatteka at nicira.com>
> > + */
> > +
> > +#ifndef __VIR_NETDEV_OPENVSWITCH_H__
> > +# define __VIR_NETDEV_OPENVSWITCH_H__
> > +
> > +# include "internal.h"
> > +# include "util.h"
> > +# include "virnetdevvportprofile.h"
> > +
> > +
> > +int virNetDevOpenvswitchAddPort(const char *brname,
> > + const char *ifname,
> > + const unsigned char *macaddr,
> > + virNetDevVPortProfilePtr ovsport)
> > + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
> > + ATTRIBUTE_RETURN_CHECK;
> > +
> > +int virNetDevOpenvswitchRemovePort(const char *brname, const char
> *ifname)
> > + ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
> > +
> > +#endif /* __VIR_NETDEV_OPENVSWITCH_H__ */
> > diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
> > index 2ed53c6..cd1d4c1 100644
> > --- a/src/util/virnetdevtap.c
> > +++ b/src/util/virnetdevtap.c
> > @@ -25,6 +25,7 @@
> > #include "virnetdevtap.h"
> > #include "virnetdev.h"
> > #include "virnetdevbridge.h"
> > +#include "virnetdevopenvswitch.h"
> > #include "virterror_internal.h"
> > #include "virfile.h"
> > #include "virterror_internal.h"
> > @@ -249,6 +250,7 @@ int virNetDevTapDelete(const char *ifname
> ATTRIBUTE_UNUSED)
> > * @macaddr: desired MAC address (VIR_MAC_BUFLEN long)
> > * @vnet_hdr: whether to try enabling IFF_VNET_HDR
> > * @tapfd: file descriptor return value for the new tap device
> > + * @ovsport: Open vSwitch specific configuration
> > *
> > * This function creates a new tap device on a bridge. @ifname can be
> either
> > * a fixed name or a name template with '%d' for dynamic name
> allocation.
> > @@ -265,7 +267,8 @@ int virNetDevTapCreateInBridgePort(const char
> *brname,
> > const unsigned char *macaddr,
> > int vnet_hdr,
> > bool up,
> > - int *tapfd)
> > + int *tapfd,
> > + virNetDevVPortProfilePtr ovsport)
>
> This arg should be named virtPortProfile for consistency (although I
> think that name is too long, maybe a global replace is in order
> sometime...)
>
> > {
> > if (virNetDevTapCreate(ifname, vnet_hdr, tapfd) < 0)
> > return -1;
> > @@ -286,8 +289,13 @@ int virNetDevTapCreateInBridgePort(const char
> *brname,
> > if (virNetDevSetMTUFromDevice(*ifname, brname) < 0)
> > goto error;
> >
> > - if (virNetDevBridgeAddPort(brname, *ifname) < 0)
> > - goto error;
> > + if (ovsport) {
> > + if (virNetDevOpenvswitchAddPort(brname, *ifname, macaddr,
> ovsport) < 0)
> > + goto error;
> > + } else {
> > + if (virNetDevBridgeAddPort(brname, *ifname) < 0)
> > + goto error;
> > + }
> >
> > if (virNetDevSetOnline(*ifname, up) < 0)
> > goto error;
> > diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h
> > index fb35ac5..ee159f7 100644
> > --- a/src/util/virnetdevtap.h
> > +++ b/src/util/virnetdevtap.h
> > @@ -24,6 +24,7 @@
> > # define __VIR_NETDEV_TAP_H__
> >
> > # include "internal.h"
> > +# include "virnetdevvportprofile.h"
> >
> > int virNetDevTapCreate(char **ifname,
> > int vnet_hdr,
> > @@ -38,8 +39,10 @@ int virNetDevTapCreateInBridgePort(const char *brname,
> > const unsigned char *macaddr,
> > int vnet_hdr,
> > bool up,
> > - int *tapfd)
> > + int *tapfd,
> > + virNetDevVPortProfilePtr ovsport)
> > ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
> > ATTRIBUTE_RETURN_CHECK;
> >
> > +
> > #endif /* __VIR_NETDEV_TAP_H__ */
> > diff --git a/src/util/virnetdevvportprofile.c
> b/src/util/virnetdevvportprofile.c
> > index faadc3a..67fd7bc 100644
> > --- a/src/util/virnetdevvportprofile.c
> > +++ b/src/util/virnetdevvportprofile.c
> > @@ -968,6 +968,7 @@ virNetDevVPortProfileAssociate(const char
> *macvtap_ifname,
> >
> > switch (virtPort->virtPortType) {
> > case VIR_NETDEV_VPORT_PROFILE_NONE:
> > + case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
> > case VIR_NETDEV_VPORT_PROFILE_LAST:
> > break;
> >
> > @@ -1027,6 +1028,7 @@ virNetDevVPortProfileDisassociate(const char
> *macvtap_ifname,
> >
> > switch (virtPort->virtPortType) {
> > case VIR_NETDEV_VPORT_PROFILE_NONE:
> > + case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
> > case VIR_NETDEV_VPORT_PROFILE_LAST:
> > break;
> >
> > diff --git a/src/util/virnetdevvportprofile.h
> b/src/util/virnetdevvportprofile.h
> > index 7a8d81f..ec2d06d 100644
> > --- a/src/util/virnetdevvportprofile.h
> > +++ b/src/util/virnetdevvportprofile.h
> > @@ -35,6 +35,7 @@ enum virNetDevVPortProfile {
> > VIR_NETDEV_VPORT_PROFILE_NONE,
> > VIR_NETDEV_VPORT_PROFILE_8021QBG,
> > VIR_NETDEV_VPORT_PROFILE_8021QBH,
> > + VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH,
> >
> > VIR_NETDEV_VPORT_PROFILE_LAST,
> > };
> > @@ -54,7 +55,7 @@ enum virNetDevVPortProfileOp {
> > };
> > VIR_ENUM_DECL(virNetDevVPortProfileOp)
> >
> > -/* profile data for macvtap (VEPA) */
> > +/* profile data for macvtap (VEPA) and openvswitch */
> > typedef struct _virNetDevVPortProfile virNetDevVPortProfile;
> > typedef virNetDevVPortProfile *virNetDevVPortProfilePtr;
> > struct _virNetDevVPortProfile {
> > @@ -69,6 +70,10 @@ struct _virNetDevVPortProfile {
> > struct {
> > char profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX];
> > } virtPort8021Qbh;
> > + struct {
> > + unsigned char interfaceID[VIR_UUID_BUFLEN];
> > + char profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX];
> > + } openvswitch;
> > } u;
> > };
> >
>
> I didn't see any major problems in this version, so I'm inclined to ACK
> it conditioned on the nits being fixed (they're simple enough I could
> just take care of them while pushing it). The one thing I would like to
> get other opinions of before pushing is problem with auto-generating the
> interfaceid (just in case there's a better way to fix it that would
> required making a change now).
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120215/7d82bb15/attachment-0001.htm>
More information about the libvir-list
mailing list