diff mbox series

[v2,2/4] tools/hotplug: combine add/online and remove/offline in vif-bridge...

Message ID 20200803124931.2678-3-paul@xen.org (mailing list archive)
State New, archived
Headers show
Series tools: propagate bridge MTU to vif frontends | expand

Commit Message

Paul Durrant Aug. 3, 2020, 12:49 p.m. UTC 
From: Paul Durrant <pdurrant@amazon.com>

... as they are in vif-route.

The script is invoked with online/offline for vifs and add/remove for taps.
The operations that are necessary, however, are the same in both cases. This
patch therefore combines the cases.

The open-coded bridge removal code is also replaced with call to
remove_from_bridge().

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wl@xen.org>
---
 tools/hotplug/Linux/vif-bridge | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

Comments

Ian Jackson Aug. 4, 2020, 11:08 a.m. UTC | #1 
Paul Durrant writes ("[PATCH v2 2/4] tools/hotplug: combine add/online and remove/offline in vif-bridge..."):
> From: Paul Durrant <pdurrant@amazon.com>
> 
> ... as they are in vif-route.
> 
> The script is invoked with online/offline for vifs and add/remove for taps.
> The operations that are necessary, however, are the same in both cases. This
> patch therefore combines the cases.

This seems to newly add a "remove" case.  Previously "remove" was a
no-op here.  Is that right ?  If so it needs to be discussed in the
commit message.  We're not talking about a simple refactoring here!

Perhaps it would be best to move this bit

> +    remove)
> +        ;&
>      offline)

which I think is the relevant change, into its own commit ?

Ian.
diff mbox series

Patch

diff --git a/tools/hotplug/Linux/vif-bridge b/tools/hotplug/Linux/vif-bridge
index e722090ca8..e1d7c49788 100644
--- a/tools/hotplug/Linux/vif-bridge
+++ b/tools/hotplug/Linux/vif-bridge
@@ -77,25 +77,17 @@  then
 fi
 
 case "$command" in
+    add)
+        ;&
     online)
         setup_virtual_bridge_port "$dev"
         set_mtu "$bridge" "$dev"
         add_to_bridge "$bridge" "$dev"
         ;;
-
+    remove)
+        ;&
     offline)
-        if which brctl >&/dev/null; then
-            do_without_error brctl delif "$bridge" "$dev"
-        else
-            do_without_error ip link set "$dev" nomaster
-        fi
-        do_without_error ifconfig "$dev" down
-        ;;
-
-    add)
-        setup_virtual_bridge_port "$dev"
-        set_mtu "$bridge" "$dev"
-        add_to_bridge "$bridge" "$dev"
+        remove_from_bridge "$bridge" "$dev"
         ;;
 esac