[libvirt] [PATCH 3/5] ip link needs 'name' in 3.16 to create the veth pair

[Martin Kletzander]

On Tue, Nov 25, 2014 at 04:19:48PM +0100, Richard Weinberger wrote:
>On Tue, Nov 25, 2014 at 9:21 AM, Cedric Bosdonnat <cbosdonnat at suse.com> wrote:
>> On Tue, 2014-11-25 at 08:42 +0100, Martin Kletzander wrote:
>>> On Mon, Nov 24, 2014 at 09:54:44PM +0100, Cédric Bosdonnat wrote:
>>> >Due to a change (or bug?) in ip link implementation, the command
>>> >    'ip link add vnet0...'
>>> >is forced into
>>> >    'ip link add name vnet0...'
>>> >The changed command also works on older versions of iproute2, just the
>>> >'name' parameter has been made mandatory.
>>> >---
>>> > src/util/virnetdevveth.c | 4 ++--
>>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>>> >
>>> >diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c
>>> >index e9d6f9c..ad30e1d 100644
>>> >--- a/src/util/virnetdevveth.c
>>> >+++ b/src/util/virnetdevveth.c
>>> >@@ -89,7 +89,7 @@ static int virNetDevVethGetFreeNum(int startDev)
>>> >  * @veth2: pointer to return name for container end of veth pair
>>> >  *
>>> >  * Creates a veth device pair using the ip command:
>>> >- * ip link add veth1 type veth peer name veth2
>>> >+ * ip link add name veth1 type veth peer name veth2
>>> >  * If veth1 points to NULL on entry, it will be a valid interface on
>>> >  * return.  veth2 should point to NULL on entry.
>>> >  *
>>> >@@ -146,7 +146,7 @@ int virNetDevVethCreate(char** veth1, char** veth2)
>>> >         }
>>> >
>>> >         cmd = virCommandNew("ip");
>>> >-        virCommandAddArgList(cmd, "link", "add",
>>> >+        virCommandAddArgList(cmd, "link", "add", "name",
>>> >                              *veth1 ? *veth1 : veth1auto,
>>> >                              "type", "veth", "peer", "name",
>>> >                              *veth2 ? *veth2 : veth2auto,
>>> >--
>>> >2.1.2
>>> >
>>>
>>> I agree, the 'name' was always there, just optional.  But what version
>>> of iproute2 do you have that requires it?  I checked the current HEAD
>>> and it's still optional.  This must be a bug in that particular
>>> implementation.
>>>
>>> ACK if you can argue with the version or platform this is required
>>> on.
>>
>> At least the 3.16 shipped on openSUSE 13.2 has that problem... though I
>> think it's just a side effect of another change in iproute2. It worked
>> fine with version 3.12.
>
>Instead of papering over the issue in libvirt better ship a non-broken iproute2
>in openSUSE 13.2.
>real fix: https://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id=f1b66ff
>

Oh, thank you for finding that, I should've done my homework!  Since
it really is just a bug on iproute2 side in openSUSE, I'd rather keep
it in its original state.  And since the patch is already pushed, I'm
inclining to reverting it.

Other opinions?

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141126/0c2a7441/attachment-0001.sig>