Re: [PATCH v3 06/36] util: add helper method for re-attaching a tap device to a bridge

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/21/19 9:14 PM, Cole Robinson wrote:
On 3/19/19 8:46 AM, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
---
  src/libvirt_private.syms |  1 +
  src/util/virnetdevtap.c  | 69 ++++++++++++++++++++++++++++++++++++++++
  src/util/virnetdevtap.h  | 12 +++++++
  3 files changed, 82 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d9494a04bb..6f5a734fdb 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2461,6 +2461,7 @@ virNetDevTapDelete;
  virNetDevTapGetName;
  virNetDevTapGetRealDeviceName;
  virNetDevTapInterfaceStats;
+virNetDevTapReattachBridge;
# util/virnetdevveth.h
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 972f3405aa..0484c7c5a4 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -553,6 +553,75 @@ virNetDevTapAttachBridge(const char *tapname,
  }
+/**
+ * virNetDevTapReattachBridge:
+ * @tapname: the tap interface name (or name template)
+ * @brname: the bridge name
+ * @macaddr: desired MAC address
+ * @virtPortProfile: bridge/port specific configuration
+ * @virtVlan: vlan tag info
+ * @mtu: requested MTU for port (or 0 for "default")
+ * @actualMTU: MTU actually set for port (after accounting for bridge's MTU)
+ *
+ * Ensures that the tap device (@tapname) is connected to the bridge
+ * (@brname), potentially removing it from any existing bridge that
+ * does not match.
+ *
+ * Returns 0 in case of success or -1 on failure
+ */
+int
+virNetDevTapReattachBridge(const char *tapname,
+                           const char *brname,
+                           const virMacAddr *macaddr,
+                           const unsigned char *vmuuid,
+                           virNetDevVPortProfilePtr virtPortProfile,
+                           virNetDevVlanPtr virtVlan,
+                           unsigned int mtu,
+                           unsigned int *actualMTU)
+{
+    bool useOVS = false;
+    char *master = NULL;
+
Could use VIR_AUTOFREE to simplify things a bit

+    if (virNetDevGetMaster(tapname, &master) < 0)
+        return -1;
+
+    /* 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(tapname, &master) < 0)
+            return -1;
+    }
+
+    /* Nothing more todo if we're on the right bridge already */
+    if (STREQ_NULLABLE(brname, master))
+        return 0;
+
+    /* disconnect from current (incorrect) bridge, if any  */
+    if (master) {
+        int ret;
+        VIR_INFO("Removing %s from %s", tapname, master);
+        if (useOVS)
+            ret = virNetDevOpenvswitchRemovePort(master, tapname);
+        else
+            ret = virNetDevBridgeRemovePort(master, tapname);
+        VIR_FREE(master);
+        if (ret < 0)
+            return -1;
+    }
+
These were non-fatal in the original code. Is it okay to change?


I don't remember why I made those non-fatal in the original code. Probably wanted to give as high a chance as possible of muddling through even in the face of errors from ovs. But looking at the code again, I agree that it's just as well to fail hard instead of ignoring the errors.



Provided there's an answer:

Reviewed-by: Cole Robinson <crobinso@xxxxxxxxxx>

- Cole

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux