Ian Campbell
2013-Aug-16 14:31 UTC
[PATCH] hotplug/Linux: update to new ip command syntax.
From: Mike <debian@good-with-numbers.com> 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 I extended it to catch some cases in vif-* too. Signed-off-by: Ian Campbell <ijc@hellion.org.uk> Cc: Mike <debian@good-with-numbers.com> --- Mike, could you offer a S-o-b for your bits please, this certifies the change is under the DCO which is described at http://wiki.xen.org/wiki/Submitting_Xen_Patches#Signing_off_a_patch --- tools/hotplug/Linux/vif-bridge | 6 +++--- tools/hotplug/Linux/vif-nat | 2 +- tools/hotplug/Linux/xen-network-common.sh | 10 +++++----- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/hotplug/Linux/vif-bridge b/tools/hotplug/Linux/vif-bridge index f489519..55f8b99 100644 --- a/tools/hotplug/Linux/vif-bridge +++ b/tools/hotplug/Linux/vif-bridge @@ -73,7 +73,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" @@ -82,10 +82,10 @@ fi case "$command" in online) setup_virtual_bridge_port "$dev" - 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 add_to_bridge "$bridge" "$dev" ;; 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 8cff156..db030d8 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,10 +125,10 @@ 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 } -- 1.8.3.2
Signed-off-by: Ian Campbell <ijc@hellion.org.uk> Signed-off-by: Mike <debian@good-with-numbers.com> --- tools/hotplug/Linux/vif-bridge | 6 +++--- tools/hotplug/Linux/vif-nat | 2 +- tools/hotplug/Linux/xen-network-common.sh | 10 +++++----- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/hotplug/Linux/vif-bridge b/tools/hotplug/Linux/vif-bridge index f489519..55f8b99 100644 --- a/tools/hotplug/Linux/vif-bridge +++ b/tools/hotplug/Linux/vif-bridge @@ -73,7 +73,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" @@ -82,10 +82,10 @@ fi case "$command" in online) setup_virtual_bridge_port "$dev" - 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 add_to_bridge "$bridge" "$dev" ;; 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 8cff156..db030d8 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,10 +125,10 @@ 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 } -- 1.8.3.2
Ian Jackson
2013-Aug-19 16:44 UTC
Re: [PATCH] hotplug/Linux: update to new ip command syntax.
Ian Campbell writes ("[PATCH] hotplug/Linux: update to new ip command syntax."):> From: Mike <debian@good-with-numbers.com> > > 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 I extended it to catch some cases in vif-* too. > > Signed-off-by: Ian Campbell <ijc@hellion.org.uk> > Cc: Mike <debian@good-with-numbers.com>Do we know whether this new syntax is backwards-compatible ? Ian.
Ian Campbell
2013-Aug-19 17:01 UTC
Re: [PATCH] hotplug/Linux: update to new ip command syntax.
On Mon, 2013-08-19 at 17:44 +0100, Ian Jackson wrote:> Ian Campbell writes ("[PATCH] hotplug/Linux: update to new ip command syntax."): > > From: Mike <debian@good-with-numbers.com> > > > > 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 I extended it to catch some cases in vif-* too. > > > > Signed-off-by: Ian Campbell <ijc@hellion.org.uk> > > Cc: Mike <debian@good-with-numbers.com> > > 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.
Ian Campbell
2013-Nov-19 12:23 UTC
Re: [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: > > Ian Campbell writes ("[PATCH] hotplug/Linux: update to new ip command syntax."): > > > From: Mike <debian@good-with-numbers.com> > > > > > > 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 I extended it to catch some cases in vif-* too. > > > > > > Signed-off-by: Ian Campbell <ijc@hellion.org.uk> > > > Cc: Mike <debian@good-with-numbers.com> > > > > 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? Ian.
Ian Jackson
2013-Nov-20 14:37 UTC
Re: [PATCH] hotplug/Linux: update to new ip command syntax.
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@eu.citrix.com> Ian.
Ian Campbell
2013-Nov-20 14:53 UTC
Re: [PATCH] hotplug/Linux: update to new ip command syntax.
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@eu.citrix.com>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. Ian.
George Dunlap
2013-Nov-20 18:50 UTC
Re: [PATCH] hotplug/Linux: update to new ip command syntax.
On Wed, Nov 20, 2013 at 2:53 PM, Ian Campbell <Ian.Campbell@citrix.com> 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@eu.citrix.com> > > 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. -George
Ian Campbell
2013-Nov-21 10:01 UTC
Re: [PATCH] hotplug/Linux: update to new ip command syntax.
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@citrix.com> 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@eu.citrix.com> > > > > 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@good-with-numbers.com> 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@hellion.org.uk> Signed-off-by: Mike <debian@good-with-numbers.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> --- 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
George Dunlap
2013-Nov-21 15:08 UTC
Re: [PATCH] hotplug/Linux: update to new ip command syntax.
On Thu, Nov 21, 2013 at 10:01 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:> 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@citrix.com> 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@eu.citrix.com> >> > >> > 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@good-with-numbers.com> > 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@hellion.org.uk> > Signed-off-by: Mike <debian@good-with-numbers.com> > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>Much better, thanks. Release-acked-by: George Dunlap <george.dunlap@eu.citrix.com>> --- > 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
Ian Jackson
2013-Nov-21 18:44 UTC
Re: [PATCH] hotplug/Linux: update to new ip command syntax.
George Dunlap writes ("Re: [Xen-devel] [PATCH] hotplug/Linux: update to new ip command syntax."):> On Thu, Nov 21, 2013 at 10:01 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > From: Mike <debian@good-with-numbers.com> > > Date: Fri, 16 Aug 2013 15:31:43 +0100 > > Subject: [PATCH] hotplug/Linux: update to new ip command syntax....> > Signed-off-by: Ian Campbell <ijc@hellion.org.uk> > > Signed-off-by: Mike <debian@good-with-numbers.com> > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> > > Much better, thanks. > > Release-acked-by: George Dunlap <george.dunlap@eu.citrix.com>Applied, thanks. Ian.