Re: [PATCH] fix invalid frontend path for set_mtu

[James Dingwall]

On 2022-04-27 10:17, Anthony PERARD wrote:

On Tue, Apr 19, 2022 at 01:04:18PM +0100, James Dingwall wrote:

Thank you for your feedback. I've updated the patch as suggested. I've also incorporated two other changes, one is a simple style change for consistency, the other is to change a the test for a valid mtu from > 0 to >= 68. I can

resubmit the original patch if either of these are a problem.

The style change is fine, but I'd rather have the change to the
mtu check in a different patch.

Otherwise, the patch looks better, thanks.

Here is a revised version of the patch that removes the mtu change.

Thanks,
James
commit f6ec92717522e74b4cc3aa4160b8ad6884e0b50c
Author: James Dingwall <ja...@dingwall.me.uk>
Date:   Tue Apr 19 12:45:31 2022 +0100

    The set_mtu() function of xen-network-common.sh currently has this code:
    
            if [ ${type_if} = vif ]
            then
                local dev_=${dev#vif}
                local domid=${dev_%.*}
                local devid=${dev_#*.}
    
                local FRONTEND_PATH="/local/domain/$domid/device/vif/$devid"
    
                xenstore_write "$FRONTEND_PATH/mtu" ${mtu}
            fi
    
    This works fine if the device has its default name but if the xen config
    defines the vifname parameter the FRONTEND_PATH is incorrectly constructed.
    Learn the frontend path by reading the appropriate value from the backend.
    
    Also change use of `...` to $(...) for a consistent style in the script.
    
    Signed-off-by: James Dingwall <ja...@dingwall.me.uk>

diff --git a/tools/hotplug/Linux/xen-network-common.sh b/tools/hotplug/Linux/xen-network-common.sh
index 42fa704e8d..7a63308a9e 100644
--- a/tools/hotplug/Linux/xen-network-common.sh
+++ b/tools/hotplug/Linux/xen-network-common.sh
@@ -171,7 +171,7 @@ set_mtu () {
     local mtu=$(xenstore_read_default "$XENBUS_PATH/mtu" "")
     if [ -z "$mtu" ]
     then
-        mtu="`ip link show dev ${bridge}| awk '/mtu/ { print $5 }'`"
+        mtu="$(ip link show dev ${bridge}| awk '/mtu/ { print $5 }')"
         if [ -n "$mtu" ]
         then
             log debug "$bridge MTU is $mtu"
@@ -184,11 +184,7 @@ set_mtu () {
 
         if [ ${type_if} = vif ]
         then
-            local dev_=${dev#vif}
-            local domid=${dev_%.*}
-            local devid=${dev_#*.}
-
-            local FRONTEND_PATH="/local/domain/$domid/device/vif/$devid"
+            local FRONTEND_PATH="$(xenstore_read "$XENBUS_PATH/frontend")"
 
             xenstore_write "$FRONTEND_PATH/mtu" ${mtu}
         fi