diff mbox series

[v2] tools/hotplug/Linux: don't needlessly use non-standard features in vif-{bridge,route}

Message ID a417690e-539c-b1b0-a372-3aea54d5dabd@suse.com (mailing list archive)
State New, archived
Headers show
Series [v2] tools/hotplug/Linux: don't needlessly use non-standard features in vif-{bridge,route} | expand

Commit Message

Jan Beulich Sept. 2, 2020, 6:09 a.m. UTC 
We're not after any "fall-through" behavior here. Replace the constructs
with ones understood by all conforming shells, including older bash
(problem observed with 3.1.51(1)).

Fixes: b51715f02bf9 ("tools/hotplug/Linux: remove code duplication in vif-bridge")
Fixes: 3683290fc0b0 ("tools/hotplug: only attempt to call 'ip route' if there is valid command")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Also adjust vif-route.

Comments

Paul Durrant Sept. 2, 2020, 8:07 a.m. UTC | #1 
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 02 September 2020 07:09
> To: xen-devel@lists.xenproject.org
> Cc: Ian Jackson <iwj@xenproject.org>; Wei Liu <wl@xen.org>; Paul Durrant <paul@xen.org>
> Subject: [PATCH v2] tools/hotplug/Linux: don't needlessly use non-standard features in vif-
> {bridge,route}
> 
> We're not after any "fall-through" behavior here. Replace the constructs
> with ones understood by all conforming shells, including older bash
> (problem observed with 3.1.51(1)).
> 
> Fixes: b51715f02bf9 ("tools/hotplug/Linux: remove code duplication in vif-bridge")
> Fixes: 3683290fc0b0 ("tools/hotplug: only attempt to call 'ip route' if there is valid command")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Paul Durrant <paul@xen.org>

> ---
> v2: Also adjust vif-route.
> 
> --- a/tools/hotplug/Linux/vif-bridge
> +++ b/tools/hotplug/Linux/vif-bridge
> @@ -77,16 +77,12 @@ then
>  fi
> 
>  case "$command" in
> -    add)
> -        ;&
> -    online)
> +    add|online)
>          setup_virtual_bridge_port "$dev"
>          set_mtu "$bridge" "$dev" "$type_if"
>          add_to_bridge "$bridge" "$dev"
>          ;;
> -    remove)
> -        ;&
> -    offline)
> +    remove|offline)
>          remove_from_bridge "$bridge" "$dev"
>          ;;
>  esac
> --- a/tools/hotplug/Linux/vif-route
> +++ b/tools/hotplug/Linux/vif-route
> @@ -22,17 +22,13 @@ dir=$(dirname "$0")
>  main_ip=$(dom0_ip)
> 
>  case "${command}" in
> -    add)
> -        ;&
> -    online)
> +    add|online)
>          ifconfig ${dev} ${main_ip} netmask 255.255.255.255 up
>          echo 1 >/proc/sys/net/ipv4/conf/${dev}/proxy_arp
>          ipcmd='add'
>          cmdprefix=''
>          ;;
> -    remove)
> -        ;&
> -    offline)
> +    remove|offline)
>          do_without_error ifdown ${dev}
>          ipcmd='del'
>          cmdprefix='do_without_error'
Bertrand Marquis Sept. 4, 2020, 3:02 p.m. UTC | #2 
> On 2 Sep 2020, at 07:09, Jan Beulich <jbeulich@suse.com> wrote:
> 
> We're not after any "fall-through" behavior here. Replace the constructs
> with ones understood by all conforming shells, including older bash
> (problem observed with 3.1.51(1)).
> 
> Fixes: b51715f02bf9 ("tools/hotplug/Linux: remove code duplication in vif-bridge")
> Fixes: 3683290fc0b0 ("tools/hotplug: only attempt to call 'ip route' if there is valid command")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

> ---
> v2: Also adjust vif-route.
> 
> --- a/tools/hotplug/Linux/vif-bridge
> +++ b/tools/hotplug/Linux/vif-bridge
> @@ -77,16 +77,12 @@ then
> fi
> 
> case "$command" in
> -    add)
> -        ;&
> -    online)
> +    add|online)
>         setup_virtual_bridge_port "$dev"
>         set_mtu "$bridge" "$dev" "$type_if"
>         add_to_bridge "$bridge" "$dev"
>         ;;
> -    remove)
> -        ;&
> -    offline)
> +    remove|offline)
>         remove_from_bridge "$bridge" "$dev"
>         ;;
> esac
> --- a/tools/hotplug/Linux/vif-route
> +++ b/tools/hotplug/Linux/vif-route
> @@ -22,17 +22,13 @@ dir=$(dirname "$0")
> main_ip=$(dom0_ip)
> 
> case "${command}" in
> -    add)
> -        ;&
> -    online)
> +    add|online)
>         ifconfig ${dev} ${main_ip} netmask 255.255.255.255 up
>         echo 1 >/proc/sys/net/ipv4/conf/${dev}/proxy_arp
>         ipcmd='add'
>         cmdprefix=''
>         ;;
> -    remove)
> -        ;&
> -    offline)
> +    remove|offline)
>         do_without_error ifdown ${dev}
>         ipcmd='del'
>         cmdprefix='do_without_error'
>
Wei Liu Sept. 7, 2020, 2:28 p.m. UTC | #3 
On Wed, Sep 02, 2020 at 08:09:28AM +0200, Jan Beulich wrote:
> We're not after any "fall-through" behavior here. Replace the constructs
> with ones understood by all conforming shells, including older bash
> (problem observed with 3.1.51(1)).
> 
> Fixes: b51715f02bf9 ("tools/hotplug/Linux: remove code duplication in vif-bridge")
> Fixes: 3683290fc0b0 ("tools/hotplug: only attempt to call 'ip route' if there is valid command")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Wei Liu <wl@xen.org>
diff mbox series

Patch

--- a/tools/hotplug/Linux/vif-bridge
+++ b/tools/hotplug/Linux/vif-bridge
@@ -77,16 +77,12 @@  then
 fi
 
 case "$command" in
-    add)
-        ;&
-    online)
+    add|online)
         setup_virtual_bridge_port "$dev"
         set_mtu "$bridge" "$dev" "$type_if"
         add_to_bridge "$bridge" "$dev"
         ;;
-    remove)
-        ;&
-    offline)
+    remove|offline)
         remove_from_bridge "$bridge" "$dev"
         ;;
 esac
--- a/tools/hotplug/Linux/vif-route
+++ b/tools/hotplug/Linux/vif-route
@@ -22,17 +22,13 @@  dir=$(dirname "$0")
 main_ip=$(dom0_ip)
 
 case "${command}" in
-    add)
-        ;&
-    online)
+    add|online)
         ifconfig ${dev} ${main_ip} netmask 255.255.255.255 up
         echo 1 >/proc/sys/net/ipv4/conf/${dev}/proxy_arp
         ipcmd='add'
         cmdprefix=''
         ;;
-    remove)
-        ;&
-    offline)
+    remove|offline)
         do_without_error ifdown ${dev}
         ipcmd='del'
         cmdprefix='do_without_error'