Re: [Xen-devel] [PATCH] hotplug/Linux: update to new ip command syntax.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-11-20 at 18:50 +0000, George Dunlap wrote:
> On Wed, Nov 20, 2013 at 2:53 PM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> > On Wed, 2013-11-20 at 14:37 +0000, Ian Jackson wrote:
> >> Ian Campbell writes ("Re: [Xen-devel] [PATCH] hotplug/Linux: update to new 
> >> ip command syntax."):
> >> > On Mon, 2013-08-19 at 18:01 +0100, Ian Campbell wrote:
> >> > > On Mon, 2013-08-19 at 17:44 +0100, Ian Jackson wrote:
> >> > > > Do we know whether this new syntax is backwards-compatible ?
> >> > >
> >> > > Good question. Squeeze's ip(8) doesn't mention dev for "ip link show" 
> >> > > in
> >> > > the synopsis by virtue of not defining DEVICE anywhere but it does show
> >> > > the use of dev later on and the tool accepts it. Likewise for "ip link
> >> > > set". Given this, as Mike says, makes it impossible to name a device
> >> > > "dev" it seems likely to be a documentation bug.
> >> > >
> >> > > "ip addr flush" is correctly documented as needing the dev (by saying
> >> > > "dev STRING" and not "DEVICE" , I didn't try without but I assume it
> >> > > works since our scripts do that right now.
> >> > >
> >> > > I wasn't easily able to come up with an older machine.
> >> >
> >> > Ian, was that a satisfactory explanation?
> >>
> >> Yes, it was.  Sorry.
> >>
> >> Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> >
> > Thanks.
> >
> > Now, WRT 4.4...
> >
> > With reference to bugs.debian.org/cgi-bin/bugreport.cgi?bug=705659 the
> > patch allows the creation of a vif named "dev". Which although I'm sure
> > useful it is easily worked around. But on the other hand we are early in
> > the freeze and this could easily be considered a bug fix.
> >
> > At this stage I'm leaning towards say lets take it.
> 
> Should I say 'no' once just to let people know who's in charge? :-)
> 
> I might be inclined to do so actually, just on principle; except that
> the best way to get this actually tested will be the upcoming test
> days, when (hopefully) it will be tested on all the major distros.  If
> we put it off until 4.5, I doubt it will get much more testing than it
> would right now.
> 
> One thing I'm not happy with the patch though -- it doesn't have a
> proper description of the problem and what the patch is actually doing
> in the changeset itself.  Links to supplemental information are fine,
> I think, but we have to assume that 1) people will be browsing the
> source code offline, and 2) links will disappear; so the critical
> information needs to be included.

Ack.

8>----------------------

From aed6743365d17c58561481ce1d74613f9a731746 Mon Sep 17 00:00:00 2001
From: Mike <debian@xxxxxxxxxxxxxxxxxxxxx>
Date: Fri, 16 Aug 2013 15:31:43 +0100
Subject: [PATCH] hotplug/Linux: update to new ip command syntax.

The current usage prevents naming a vif "dev". Although the current syntax is
documented in Squeeze's ip(8) it appears that this was a documentation bug.
Newer versions of the man page describe the new syntax used here and Squeeze's
implementation accepts it as well.

This is Debian bug #705659.
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=705659

Mike provided the initial patch to tools/hotplug/Linux/xen-network-common.sh
and Ian extended it to catch some cases in vif-* too.

Signed-off-by: Ian Campbell <ijc@xxxxxxxxxxxxxx>
Signed-off-by: Mike <debian@xxxxxxxxxxxxxxxxxxxxx>
Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
---
v2: Updated commit message.
---
 tools/hotplug/Linux/vif-bridge            |    2 +-
 tools/hotplug/Linux/vif-nat               |    2 +-
 tools/hotplug/Linux/xen-network-common.sh |   14 +++++++-------
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/tools/hotplug/Linux/vif-bridge b/tools/hotplug/Linux/vif-bridge
index 678262d..b7dcbd6 100644
--- a/tools/hotplug/Linux/vif-bridge
+++ b/tools/hotplug/Linux/vif-bridge
@@ -72,7 +72,7 @@ else
 fi
 
 RET=0
-ip link show $bridge 1>/dev/null 2>&1 || RET=1
+ip link show dev $bridge 1>/dev/null 2>&1 || RET=1
 if [ "$RET" -eq 1 ]
 then
     fatal "Could not find bridge device $bridge"
diff --git a/tools/hotplug/Linux/vif-nat b/tools/hotplug/Linux/vif-nat
index 8d29fb6..0b900d5 100644
--- a/tools/hotplug/Linux/vif-nat
+++ b/tools/hotplug/Linux/vif-nat
@@ -170,7 +170,7 @@ case "$command" in
           exit 0
         fi
 
-        do_or_die ip link set "${dev}" up arp on
+        do_or_die ip link set dev "${dev}" up arp on
         do_or_die ip addr add "$router_ip" dev "${dev}"
         do_or_die ip route add "$vif_ip" dev "${dev}" src "$router_ip"
         echo 1 >/proc/sys/net/ipv4/conf/${dev}/proxy_arp
diff --git a/tools/hotplug/Linux/xen-network-common.sh 
b/tools/hotplug/Linux/xen-network-common.sh
index 50b8711..3c63c55 100644
--- a/tools/hotplug/Linux/xen-network-common.sh
+++ b/tools/hotplug/Linux/xen-network-common.sh
@@ -85,18 +85,18 @@ _setup_bridge_port() {
     local virtual="$2"
 
     # take interface down ...
-    ip link set ${dev} down
+    ip link set dev ${dev} down
 
     if [ $virtual -ne 0 ] ; then
         # Initialise a dummy MAC address. We choose the numerically
         # largest non-broadcast address to prevent the address getting
         # stolen by an Ethernet bridge for STP purposes.
         # (FE:FF:FF:FF:FF:FF)
-        ip link set ${dev} address fe:ff:ff:ff:ff:ff || true
+        ip link set dev ${dev} address fe:ff:ff:ff:ff:ff || true
     fi
 
     # ... and configure it
-    ip addr flush ${dev}
+    ip address flush dev ${dev}
 }
 
 setup_physical_bridge_port() {
@@ -125,20 +125,20 @@ add_to_bridge () {
 
     # Don't add $dev to $bridge if it's already on a bridge.
     if [ -e "/sys/class/net/${bridge}/brif/${dev}" ]; then
-       ip link set ${dev} up || true
+       ip link set dev ${dev} up || true
        return
     fi
     brctl addif ${bridge} ${dev}
-    ip link set ${dev} up
+    ip link set dev ${dev} up
 }
 
 # Usage: set_mtu bridge dev
 set_mtu () {
     local bridge=$1
     local dev=$2
-    mtu="`ip link show ${bridge}| awk '/mtu/ { print $5 }'`"
+    mtu="`ip link show dev ${bridge}| awk '/mtu/ { print $5 }'`"
     if [ -n "$mtu" ] && [ "$mtu" -gt 0 ]
     then
-            ip link set ${dev} mtu $mtu || :
+            ip link set dev ${dev} mtu $mtu || :
     fi
 }
-- 
1.7.10.4




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel