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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxx> 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

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]