From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH] hotplug/Linux: update to new ip command syntax. Date: Thu, 21 Nov 2013 15:08:43 +0000 Message-ID: References: <1376663503-24736-1-git-send-email-ijc@hellion.org.uk> <21010.19305.453261.252793@mariner.uk.xensource.com> <1376931672.9708.91.camel@dagon.hellion.org.uk> <1384863789.30014.70.camel@kazak.uk.xensource.com> <21132.51510.608514.553907@mariner.uk.xensource.com> <1384959187.6071.67.camel@kazak.uk.xensource.com> <1385028110.6071.121.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1385028110.6071.121.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: "xen-devel@lists.xen.org" , Ian Jackson , Mike List-Id: xen-devel@lists.xenproject.org On Thu, Nov 21, 2013 at 10:01 AM, Ian Campbell wrote: > On Wed, 2013-11-20 at 18:50 +0000, George Dunlap wrote: >> On Wed, Nov 20, 2013 at 2:53 PM, Ian Campbell 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 >> > >> > 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 > 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 > Signed-off-by: Mike > Acked-by: Ian Jackson Much better, thanks. Release-acked-by: George Dunlap > --- > 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@lists.xen.org > http://lists.xen.org/xen-devel