Connecting a tap device to an Open vSwitch is done by adding a "port" to the switch with the ovs-vsctl "add-port" command. The port will have the same name as the tap device, but it is a separate entity, and can survive beyond the destruction of the tap device (although under normal circumstances the port will be deleted around the same time the tap device is deleted). This makes it possible for a port of a particular name to already exist at the time libvirt calls ovs-vsctl to add that port. The original commit of Open vSwitch support (commit df81004632, libvirt 0.9.10, Feb. 2012) used the "--may-exist" option to the add-port command to indicate that a port of the desired name might already exist, and that it was okay to simply re-use this port (rather than failing with an error message). Then in commit 33445ce8446d9 (libvirt 1.2.7, April 2014) the command was changed to use "--if-exists del-port blah" instead of "--may-exist". The reason given was that there was a bug in OVS where a stale port would be unusable even though it still existed; the workaround was to forcibly delete any existing port prior to adding the new port (of the same name). This is the ovs-vsctl command still in use by libvirt today. It recently came up in the discussion of a bug concerning guest packet loss during OpenStack upgrades (https://bugzilla.redhat.com/1963164) that the bug in OVS that necessitated the del-port workaround was fixed quite a long time ago (August 2015): https://github.com/openvswitch/ovs/commit/e21c6643a02c6b446d2fbdfde366ea303b4c2730 thus rendering the workaround in libvirt unnecessary. The assertion in that discussion is that this workaround is now the cause of the packet loss being experienced during OpenStack upgrades. I'm not convinced this is the case, but it does appear that there is no reason to carry this workaround in libvirt any longer, so this patch reverts the code back to the original behavior (using "--may-exist" instead of "--if-exists del-port"). Signed-off-by: Laine Stump <laine@xxxxxxxxxx> --- In an attempt to sabotage my own patch, I will point out that this makes it a possibility that libvirt could end up using an OVS port created by "someone else" who may have set it up in some way that renders the guest network connection unusable, e.g. if the port had been created with flow filters or something. On the other hand 1) the "someone else" in this scenario would need to have all the same privileges that would also permit them to mess up the connection after it had been created (so there is no *extra* security risk), and 2) this could possibly be *desired* (although difficult for libvirt to support) behavior in the case someone needed libvirt-managed guest network connections to use extra capabilities not directly supported in libvirt's config; additionally pre-creating the OVS port could provide a method of getting the network connection into a usable state (i.e. forwarding traffic) much sooner. src/util/virnetdevopenvswitch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 21ee4bdd42..eac68d9556 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -155,8 +155,8 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, } cmd = virNetDevOpenvswitchCreateCmd(); - virCommandAddArgList(cmd, "--", "--if-exists", "del-port", - ifname, "--", "add-port", brname, ifname, NULL); + virCommandAddArgList(cmd, "--", "--may-exist", + "add-port", brname, ifname, NULL); virNetDevOpenvswitchConstructVlans(cmd, virtVlan); -- 2.31.1
