| Message ID | 20220301093513.GA3187840@dingwall.me.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fix invalid frontend path for set_mtu | expand |
Hi James, On Tue, Mar 01, 2022 at 09:35:13AM +0000, James Dingwall wrote: > 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. The patch looks fine, thanks. It is only missing a line "Signed-off-by: your_name <your_email>" at the end of the description. The meaning of this line is described in the file CONTRIBUTING, section "Developer's Certificate of Origin". Thanks,
Hi Anthony, On Tue, Apr 12, 2022 at 02:03:17PM +0100, Anthony PERARD wrote: > Hi James, > > On Tue, Mar 01, 2022 at 09:35:13AM +0000, James Dingwall wrote: > > 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. > > The patch looks fine, thanks. It is only missing a line > "Signed-off-by: your_name <your_email>" at the end of the description. > The meaning of this line is described in the file CONTRIBUTING, section > "Developer's Certificate of Origin". > 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. Thanks, James
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.
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
On Wed, Apr 27, 2022 at 02:20:53PM +0100, James Dingwall wrote: > commit f6ec92717522e74b4cc3aa4160b8ad6884e0b50c > Author: James Dingwall <james@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 <james@dingwall.me.uk> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> Thanks! > 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
diff --git a/tools/hotplug/Linux/xen-network-common.sh b/tools/hotplug/Linux/xen-network-common.sh index 02e2388600..cd98f0d486 100644 --- a/tools/hotplug/Linux/xen-network-common.sh +++ b/tools/hotplug/Linux/xen-network-common.sh @@ -163,11 +163,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
Hi, 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. Thanks, James