This function retrieves the name of the OVS bridge that the given netdev is attached to. This separate function is necessary because OVS set the IFLA_MASTER attribute to "ovs-system" for all netdevs that are attached to an OVS bridge, so the standard method of retrieving the master can't be used. Signed-off-by: Laine Stump <laine(a)laine.org&gt; --- src/libvirt_private.syms | 1 + src/util/virnetdevopenvswitch.c | 55 +++++++++++++++++++++++++++++++++++++++++ src/util/virnetdevopenvswitch.h | 6 +++++ 3 files changed, 62 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 98913a577a..386f53eeb9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2364,6 +2364,7 @@ virNetDevMidonetUnbindPort; virNetDevOpenvswitchAddPort; virNetDevOpenvswitchGetMigrateData; virNetDevOpenvswitchGetVhostuserIfname; +virNetDevOpenvswitchInterfaceGetMaster; virNetDevOpenvswitchInterfaceStats; virNetDevOpenvswitchRemovePort; virNetDevOpenvswitchSetMigrateData; diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index f86f698430..af3f2a773d 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -404,6 +404,61 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname, return ret; } + +/** + * virNetDeOpenvswitchGetMaster: + * @ifname: name of interface we're interested in + * @master: used to return a string containing the name of @ifname's "master" + * (this is the bridge or bond device that this device is attached to) + * + * Returns 0 on success, -1 on failure (if @ifname has no master + * @master will be NULL, but return value will still be 0 (success)). + * + * NB: This function is needed because the IFLA_MASTER attribute of an + * interface in a netlink dump (see virNetDevGetMaster()) will always + * return "ovs-system" for any interface that is attached to an OVS + * switch. When that happens, virNetDevOpenvswitchInterfaceGetMaster() + * must be called to get the "real" master of the interface. + */ +int +virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master) +{ + virCommandPtr cmd = NULL; + int ret = -1; + int exitstatus; + + *master = NULL; + cmd = virCommandNew(OVSVSCTL); + virNetDevOpenvswitchAddTimeout(cmd); + virCommandAddArgList(cmd, "iface-to-br", ifname, NULL); + virCommandSetOutputBuffer(cmd, master); + + if (virCommandRun(cmd, &exitstatus) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to run command to get OVS master for " + "interface %s"), ifname); + goto cleanup; + } + + /* non-0 exit code just means that the interface has no master in OVS */ + if (exitstatus != 0) + VIR_FREE(*master); + + if (*master) { + /* truncate at the first newline */ + char *nl = strchr(*master, '\n'); + if (nl) + *nl = '\0'; + } + + VIR_DEBUG("OVS master for %s is %s", ifname, *master ? *master : "(none)"); + + ret = 0; + cleanup: + return ret; +} + + /** * virNetDevOpenvswitchVhostuserGetIfname: * @path: the path of the unix socket diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h index 6f6e620c22..331e483018 100644 --- a/src/util/virnetdevopenvswitch.h +++ b/src/util/virnetdevopenvswitch.h @@ -47,6 +47,9 @@ int virNetDevOpenvswitchAddPort(const char *brname, int virNetDevOpenvswitchRemovePort(const char *brname, const char *ifname) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; @@ -57,6 +60,9 @@ int virNetDevOpenvswitchInterfaceStats(const char *ifname, virDomainInterfaceStatsPtr stats) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; +int virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; + int virNetDevOpenvswitchGetVhostuserIfname(const char *path, char **ifname) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NOINLINE; -- 2.14.4

When libvirtd is restarted, it checks that each guest tap device is still attached to the bridge device that the configuration info says it should be connected to. If not, the tap will be disconnected from [wherever it is] and connected to [wherever it should be]. The previous code that did this did not account for: 1) the IFLA_MASTER attribute in a netdev's ifinfo will be set to "ovs-system" for any tap device connected to an OVS bridge, *not* to the name of the bridge it is attached to. 2) virNetDevRemovePort() only works for devices that are attached to a standard Linux host bridge. If a device is currently attached to an OVS bridge, then virNetDevOpenvswitchRemovePort() must be called instead. The result was an error message like this: error : virNetDevBridgeRemovePort:743 : Unable to remove bridge ovs-system port vnet1: Operation not supported This patch remedies those problems, and adds a couple of information log messages to aid in debugging any future problem. Resolves: https://bugzilla.redhat.com/1596176 Signed-off-by: Laine Stump <laine(a)laine.org&gt; --- src/network/bridge_driver.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index ac849581ec..da3c32e305 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -64,6 +64,7 @@ #include "virnetdev.h" #include "virnetdevip.h" #include "virnetdevbridge.h" +#include "virnetdevopenvswitch.h" #include "virnetdevtap.h" #include "virnetdevvportprofile.h" #include "virpci.h" @@ -4823,19 +4824,35 @@ networkNotifyActualDevice(virDomainDefPtr dom, /* see if we're connected to the correct bridge */ if (netdef->bridge) { + bool useOVS = false; + if (virNetDevGetMaster(iface->ifname, &master) < 0) goto error; + /* IFLA_MASTER for a tap on an OVS switch is always "ovs-system" */ + if (STREQ_NULLABLE(master, "ovs-system")) { + useOVS = true; + VIR_FREE(master); + if (virNetDevOpenvswitchInterfaceGetMaster(iface->ifname, &master) < 0) + goto error; + } + if (STRNEQ_NULLABLE(netdef->bridge, master)) { /* disconnect from current (incorrect) bridge */ - if (master) - ignore_value(virNetDevBridgeRemovePort(master, iface->ifname)); + if (master) { + VIR_INFO("Removing %s from %s", iface->ifname, master); + if (useOVS) + ignore_value(virNetDevOpenvswitchRemovePort(master, iface->ifname)); + else + ignore_value(virNetDevBridgeRemovePort(master, iface->ifname)); + } /* attach/reattach to correct bridge. * NB: we can't notify the guest of any MTU change anyway, * so there is no point in trying to learn the actualMTU * (final arg to virNetDevTapAttachBridge()) */ + VIR_INFO("Attaching %s to %s", iface->ifname, netdef->bridge); if (virNetDevTapAttachBridge(iface->ifname, netdef->bridge, &iface->mac, dom->uuid, virDomainNetGetActualVirtPortProfile(iface), -- 2.14.4