[patch] xen udev rule interfering with openvpn

[patch] xen udev rule interfering with openvpn

[M A Young]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 751 bytes --]

There is a Fedora bug report 
https://bugzilla.redhat.com/show_bug.cgi?id=812421 reporting that openvpn 
is having problems because of the line
SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
in /etc/udev/rules.d/xen-backend.rules which is causing the xen script to 
run when openvpn tries to use a tap device, causing it to fail. I have 
used the attached patch to solve this problem, by matching the form of the 
tap device that xen uses more exactly to avoid to openvpn case. A better 
long-term solution (suggested in one of the comments in the bug) might be 
to use a more specific name instead of "tap" so we have less chance of 
interfering with another application.

 	Michael Young

[-- Attachment #2: Type: TEXT/PLAIN, Size: 806 bytes --]

The udev tap xen backend rule interferes with openvpn. Make the match condition 
more specific to xen so it doesn't match the openvpn tap device.

Signed-off-by: Michael Young <m.a.young@durham.ac.uk>

--- xen-4.1.2/tools/hotplug/Linux/xen-backend.rules.orig	2011-10-20 18:05:42.000000000 +0100
+++ xen-4.1.2/tools/hotplug/Linux/xen-backend.rules	2012-04-15 17:08:24.774955932 +0100
@@ -13,4 +13,4 @@
 KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
 KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
 KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
-SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
+SUBSYSTEM=="net", KERNEL=="tap[0-9]*.[0-9]*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: [patch] xen udev rule interfering with openvpn

[Ian Campbell]

On Mon, 2012-04-16 at 20:03 +0100, M A Young wrote:
> There is a Fedora bug report 
> https://bugzilla.redhat.com/show_bug.cgi?id=812421 reporting that openvpn 
> is having problems because of the line
> SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
> in /etc/udev/rules.d/xen-backend.rules which is causing the xen script to 
> run when openvpn tries to use a tap device, causing it to fail. I have 
> used the attached patch to solve this problem, by matching the form of the 
> tap device that xen uses more exactly to avoid to openvpn case. A better 
> long-term solution (suggested in one of the comments in the bug) might be 
> to use a more specific name instead of "tap" so we have less chance of 
> interfering with another application.

This is a good start, I think we should do this for 4.2.

Changing the name might be pretty simple though e.g. the following.
Works for me with xl but I didn't try xend (seems "obviously correct"?)

I noticed that when vifname is set xend prepends "tap-" (presumably to
distinguish it from the vif device) whereas libxl does not, so I suspect
named vifs for HVM guests don't work so well, I fixed that while I was
there...

Also at least for the libxl case we will likely not be running these
hotplug scripts via udev any more in 4.2, however I don't think there is
any harm in making this change first (iff we decide it is suitable for
4.2).

Ian.

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1334658366 -3600
# Node ID de3e65d804cceab7291e2accc18d50ae8b816433
# Parent  8d92d1f34921c8675d85c74aa36e319c9451f68f
libxl/xend: name tap devices with a xentap prefix

This prevents the udev scripts from operating on other tap devices (e.g.
openvpn etc)

Also add "xentap-" prefix to the tap device when an explicit name is given to
avoid a conflict with the vif device, which would otherwise have the same name.
Likewise correct the documentation for this option which suggested it applied
to HVM tap devices only.

Reported by Michael Young.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 8d92d1f34921 -r de3e65d804cc docs/misc/xl-network-configuration.markdown
--- a/docs/misc/xl-network-configuration.markdown	Mon Apr 16 17:57:00 2012 +0100
+++ b/docs/misc/xl-network-configuration.markdown	Tue Apr 17 11:26:06 2012 +0100
@@ -93,11 +93,14 @@ are:
 
 ### vifname
 
-This keyword is valid for HVM guest devices with `type=ioemu` only.
+Specifies the backend device name for the virtual device.
 
-Specifies the backend device name for an emulated device. The default
-is `tapDOMID.DEVID` where `DOMID` is the guest domain ID and `DEVID`
-is the device number.
+If the domain is an HVM domain then the associated emulated (tap)
+device will have a "xentap-" prefix added.
+
+The default name for the virtual device is `vifDOMID.DEVID` where
+`DOMID` is the guest domain ID and `DEVID` is the device
+number. Likewise the default tap name is `xentapDOMID.DEVID`.
 
 ### script
 
diff -r 8d92d1f34921 -r de3e65d804cc tools/hotplug/Linux/vif-common.sh
--- a/tools/hotplug/Linux/vif-common.sh	Mon Apr 16 17:57:00 2012 +0100
+++ b/tools/hotplug/Linux/vif-common.sh	Tue Apr 17 11:26:06 2012 +0100
@@ -85,8 +85,8 @@ elif [ "$type_if" = tap ]; then
     : ${INTERFACE:?}
 
     # Get xenbus_path from device name.
-    # The name is built like that: "tap${domid}.${devid}".
-    dev_=${dev#tap}
+    # The name is built like that: "xentap${domid}.${devid}".
+    dev_=${dev#xentap}
     domid=${dev_%.*}
     devid=${dev_#*.}
 
diff -r 8d92d1f34921 -r de3e65d804cc tools/hotplug/Linux/xen-backend.rules
--- a/tools/hotplug/Linux/xen-backend.rules	Mon Apr 16 17:57:00 2012 +0100
+++ b/tools/hotplug/Linux/xen-backend.rules	Tue Apr 17 11:26:06 2012 +0100
@@ -13,4 +13,4 @@ KERNEL=="blktap-control", NAME="xen/blkt
 KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
 KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
 KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
-SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
+SUBSYSTEM=="net", KERNEL=="xentap*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
diff -r 8d92d1f34921 -r de3e65d804cc tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Mon Apr 16 17:57:00 2012 +0100
+++ b/tools/libxl/libxl_dm.c	Tue Apr 17 11:26:06 2012 +0100
@@ -212,9 +212,9 @@ static char ** libxl__build_device_model
                 char *ifname;
                 if (!vifs[i].ifname)
                     ifname = libxl__sprintf(gc,
-                                            "tap%d.%d", domid, vifs[i].devid);
+                                            "xentap%d.%d", domid, vifs[i].devid);
                 else
-                    ifname = vifs[i].ifname;
+                    ifname = libxl__sprintf(gc, "xentap-%s", vifs[i].ifname);
                 flexarray_vappend(dm_args,
                                 "-net", libxl__sprintf(gc, "nic,vlan=%d,macaddr=%s,model=%s",
                                                        vifs[i].devid, smac, vifs[i].model),
@@ -451,10 +451,10 @@ static char ** libxl__build_device_model
                                 LIBXL_MAC_FMT, LIBXL_MAC_BYTES(vifs[i].mac));
                 char *ifname;
                 if (!vifs[i].ifname) {
-                    ifname = libxl__sprintf(gc, "tap%d.%d",
+                    ifname = libxl__sprintf(gc, "xentap%d.%d",
                                             guest_domid, vifs[i].devid);
                 } else {
-                    ifname = vifs[i].ifname;
+                    ifname = libxl__sprintf(gc, "xentap-%s", vifs[i].ifname);
                 }
                 flexarray_append(dm_args, "-device");
                 flexarray_append(dm_args,
diff -r 8d92d1f34921 -r de3e65d804cc tools/python/xen/xend/image.py
--- a/tools/python/xen/xend/image.py	Mon Apr 16 17:57:00 2012 +0100
+++ b/tools/python/xen/xend/image.py	Tue Apr 17 11:26:06 2012 +0100
@@ -921,7 +921,7 @@ class HVMImageHandler(ImageHandler):
             if vifname:
                 vifname = "tap-" + vifname
             else:
-                vifname = "tap%d.%d" % (self.vm.getDomid(), nics-1)
+                vifname = "xentap%d.%d" % (self.vm.getDomid(), nics-1)
             ret.append("-net")
             ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
                        (nics, vifname, bridge))

Re: [patch] xen udev rule interfering with openvpn

[Roger Pau Monne]

El 17/04/2012, a las 11:26, Ian Campbell escribió:
> On Mon, 2012-04-16 at 20:03 +0100, M A Young wrote:
>> There is a Fedora bug report 
>> https://bugzilla.redhat.com/show_bug.cgi?id=812421 reporting that openvpn 
>> is having problems because of the line
>> SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
>> in /etc/udev/rules.d/xen-backend.rules which is causing the xen script to 
>> run when openvpn tries to use a tap device, causing it to fail. I have 
>> used the attached patch to solve this problem, by matching the form of the 
>> tap device that xen uses more exactly to avoid to openvpn case. A better 
>> long-term solution (suggested in one of the comments in the bug) might be 
>> to use a more specific name instead of "tap" so we have less chance of 
>> interfering with another application.
> 
> This is a good start, I think we should do this for 4.2.
> 
> Changing the name might be pretty simple though e.g. the following.
> Works for me with xl but I didn't try xend (seems "obviously correct"?)
> 
> I noticed that when vifname is set xend prepends "tap-" (presumably to
> distinguish it from the vif device) whereas libxl does not, so I suspect
> named vifs for HVM guests don't work so well, I fixed that while I was
> there...
> 
> Also at least for the libxl case we will likely not be running these
> hotplug scripts via udev any more in 4.2, however I don't think there is
> any harm in making this change first (iff we decide it is suitable for
> 4.2).
> 
> Ian.
> 
> # HG changeset patch
> # User Ian Campbell <ian.campbell@citrix.com>
> # Date 1334658366 -3600
> # Node ID de3e65d804cceab7291e2accc18d50ae8b816433
> # Parent  8d92d1f34921c8675d85c74aa36e319c9451f68f
> libxl/xend: name tap devices with a xentap prefix
> 
> This prevents the udev scripts from operating on other tap devices (e.g.
> openvpn etc)
> 
> Also add "xentap-" prefix to the tap device when an explicit name is given to
> avoid a conflict with the vif device, which would otherwise have the same name.
> Likewise correct the documentation for this option which suggested it applied
> to HVM tap devices only.
> 
> Reported by Michael Young.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Acked-by: Roger Pau Monne <roger.pau@citrix.com>

I've already changed my hotplug series to match this change in the udev rules, so this has to go in before mine.

> 
> diff -r 8d92d1f34921 -r de3e65d804cc docs/misc/xl-network-configuration.markdown
> --- a/docs/misc/xl-network-configuration.markdown	Mon Apr 16 17:57:00 2012 +0100
> +++ b/docs/misc/xl-network-configuration.markdown	Tue Apr 17 11:26:06 2012 +0100
> @@ -93,11 +93,14 @@ are:
> 
> ### vifname
> 
> -This keyword is valid for HVM guest devices with `type=ioemu` only.
> +Specifies the backend device name for the virtual device.
> 
> -Specifies the backend device name for an emulated device. The default
> -is `tapDOMID.DEVID` where `DOMID` is the guest domain ID and `DEVID`
> -is the device number.
> +If the domain is an HVM domain then the associated emulated (tap)
> +device will have a "xentap-" prefix added.
> +
> +The default name for the virtual device is `vifDOMID.DEVID` where
> +`DOMID` is the guest domain ID and `DEVID` is the device
> +number. Likewise the default tap name is `xentapDOMID.DEVID`.
> 
> ### script
> 
> diff -r 8d92d1f34921 -r de3e65d804cc tools/hotplug/Linux/vif-common.sh
> --- a/tools/hotplug/Linux/vif-common.sh	Mon Apr 16 17:57:00 2012 +0100
> +++ b/tools/hotplug/Linux/vif-common.sh	Tue Apr 17 11:26:06 2012 +0100
> @@ -85,8 +85,8 @@ elif [ "$type_if" = tap ]; then
>     : ${INTERFACE:?}
> 
>     # Get xenbus_path from device name.
> -    # The name is built like that: "tap${domid}.${devid}".
> -    dev_=${dev#tap}
> +    # The name is built like that: "xentap${domid}.${devid}".
> +    dev_=${dev#xentap}
>     domid=${dev_%.*}
>     devid=${dev_#*.}
> 
> diff -r 8d92d1f34921 -r de3e65d804cc tools/hotplug/Linux/xen-backend.rules
> --- a/tools/hotplug/Linux/xen-backend.rules	Mon Apr 16 17:57:00 2012 +0100
> +++ b/tools/hotplug/Linux/xen-backend.rules	Tue Apr 17 11:26:06 2012 +0100
> @@ -13,4 +13,4 @@ KERNEL=="blktap-control", NAME="xen/blkt
> KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
> KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
> KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
> -SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
> +SUBSYSTEM=="net", KERNEL=="xentap*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
> diff -r 8d92d1f34921 -r de3e65d804cc tools/libxl/libxl_dm.c
> --- a/tools/libxl/libxl_dm.c	Mon Apr 16 17:57:00 2012 +0100
> +++ b/tools/libxl/libxl_dm.c	Tue Apr 17 11:26:06 2012 +0100
> @@ -212,9 +212,9 @@ static char ** libxl__build_device_model
>                 char *ifname;
>                 if (!vifs[i].ifname)
>                     ifname = libxl__sprintf(gc,
> -                                            "tap%d.%d", domid, vifs[i].devid);
> +                                            "xentap%d.%d", domid, vifs[i].devid);
>                 else
> -                    ifname = vifs[i].ifname;
> +                    ifname = libxl__sprintf(gc, "xentap-%s", vifs[i].ifname);
>                 flexarray_vappend(dm_args,
>                                 "-net", libxl__sprintf(gc, "nic,vlan=%d,macaddr=%s,model=%s",
>                                                        vifs[i].devid, smac, vifs[i].model),
> @@ -451,10 +451,10 @@ static char ** libxl__build_device_model
>                                 LIBXL_MAC_FMT, LIBXL_MAC_BYTES(vifs[i].mac));
>                 char *ifname;
>                 if (!vifs[i].ifname) {
> -                    ifname = libxl__sprintf(gc, "tap%d.%d",
> +                    ifname = libxl__sprintf(gc, "xentap%d.%d",
>                                             guest_domid, vifs[i].devid);
>                 } else {
> -                    ifname = vifs[i].ifname;
> +                    ifname = libxl__sprintf(gc, "xentap-%s", vifs[i].ifname);
>                 }
>                 flexarray_append(dm_args, "-device");
>                 flexarray_append(dm_args,
> diff -r 8d92d1f34921 -r de3e65d804cc tools/python/xen/xend/image.py
> --- a/tools/python/xen/xend/image.py	Mon Apr 16 17:57:00 2012 +0100
> +++ b/tools/python/xen/xend/image.py	Tue Apr 17 11:26:06 2012 +0100
> @@ -921,7 +921,7 @@ class HVMImageHandler(ImageHandler):
>             if vifname:
>                 vifname = "tap-" + vifname
>             else:
> -                vifname = "tap%d.%d" % (self.vm.getDomid(), nics-1)
> +                vifname = "xentap%d.%d" % (self.vm.getDomid(), nics-1)
>             ret.append("-net")
>             ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
>                        (nics, vifname, bridge))
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [patch] xen udev rule interfering with openvpn

[Teck Choon Giam]

SNAP

> +++ b/tools/libxl/libxl_dm.c    Tue Apr 17 11:26:06 2012 +0100
> @@ -212,9 +212,9 @@ static char ** libxl__build_device_model
>                 char *ifname;
>                 if (!vifs[i].ifname)
>                     ifname = libxl__sprintf(gc,
> -                                            "tap%d.%d", domid, vifs[i].devid);
> +                                            "xentap%d.%d", domid, vifs[i].devid);
>                 else
> -                    ifname = vifs[i].ifname;
> +                    ifname = libxl__sprintf(gc, "xentap-%s", vifs[i].ifname);

To my understanding, you set ifname to prefix xentap instead of tap
for type LIBXL_NIC_TYPE_IOEMU which is for hvmdomain.  So please read
my comments below related to tools/python/xen/xend/image.py

>                 flexarray_vappend(dm_args,
>                                 "-net", libxl__sprintf(gc, "nic,vlan=%d,macaddr=%s,model=%s",
>                                                        vifs[i].devid, smac, vifs[i].model),
> @@ -451,10 +451,10 @@ static char ** libxl__build_device_model
>                                 LIBXL_MAC_FMT, LIBXL_MAC_BYTES(vifs[i].mac));
>                 char *ifname;
>                 if (!vifs[i].ifname) {
> -                    ifname = libxl__sprintf(gc, "tap%d.%d",
> +                    ifname = libxl__sprintf(gc, "xentap%d.%d",
>                                             guest_domid, vifs[i].devid);
>                 } else {
> -                    ifname = vifs[i].ifname;
> +                    ifname = libxl__sprintf(gc, "xentap-%s", vifs[i].ifname);
>                 }
>                 flexarray_append(dm_args, "-device");
>                 flexarray_append(dm_args,
> diff -r 8d92d1f34921 -r de3e65d804cc tools/python/xen/xend/image.py
> --- a/tools/python/xen/xend/image.py    Mon Apr 16 17:57:00 2012 +0100
> +++ b/tools/python/xen/xend/image.py    Tue Apr 17 11:26:06 2012 +0100
> @@ -921,7 +921,7 @@ class HVMImageHandler(ImageHandler):
>             if vifname:
>                 vifname = "tap-" + vifname

The above shouldn't it be:

vifname = "xentap-" + vifname

For your libxl related is:

ifname = libxl__sprintf(gc, "xentap-%s", vifs[i].ifname);

Sorry if my thinking is wrong please correct me.

Thanks.

Kindest regards,
Giam Teck Choon


>             else:
> -                vifname = "tap%d.%d" % (self.vm.getDomid(), nics-1)
> +                vifname = "xentap%d.%d" % (self.vm.getDomid(), nics-1)
>             ret.append("-net")
>             ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
>                        (nics, vifname, bridge))
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [patch] xen udev rule interfering with openvpn

[Ian Campbell]

> > diff -r 8d92d1f34921 -r de3e65d804cc tools/python/xen/xend/image.py
> > --- a/tools/python/xen/xend/image.py    Mon Apr 16 17:57:00 2012 +0100
> > +++ b/tools/python/xen/xend/image.py    Tue Apr 17 11:26:06 2012 +0100
> > @@ -921,7 +921,7 @@ class HVMImageHandler(ImageHandler):
> >             if vifname:
> >                 vifname = "tap-" + vifname
> 
> The above shouldn't it be:
> 
> vifname = "xentap-" + vifname

You are absolutely right, not sure how I missed that!

I'm out-of-office today but I'll resend later.

Ian.

Re: [patch] xen udev rule interfering with openvpn

[Ian Campbell]

On Thu, 2012-04-19 at 07:39 +0100, Ian Campbell wrote:
> > > diff -r 8d92d1f34921 -r de3e65d804cc tools/python/xen/xend/image.py
> > > --- a/tools/python/xen/xend/image.py    Mon Apr 16 17:57:00 2012 +0100
> > > +++ b/tools/python/xen/xend/image.py    Tue Apr 17 11:26:06 2012 +0100
> > > @@ -921,7 +921,7 @@ class HVMImageHandler(ImageHandler):
> > >             if vifname:
> > >                 vifname = "tap-" + vifname
> > 
> > The above shouldn't it be:
> > 
> > vifname = "xentap-" + vifname
> 
> You are absolutely right, not sure how I missed that!
> 
> I'm out-of-office today but I'll resend later.

8<--------------------

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1334912375 -3600
# Node ID fbc0dc31b6d71bed654a05ea85ec56c34ac2ef21
# Parent  f28eea0efe1223277400ff5408b1e85df5efdeac
libxl/xend: name tap devices with a xentap prefix

This prevents the udev scripts from operating on other tap devices (e.g.
openvpn etc)

Also add "xentap-" prefix to the tap device when an explicit name is given to
avoid a conflict with the vif device, which would otherwise have the same name.
Likewise correct the documentation for this option which suggested it applied
to HVM tap devices only.

Reported by Michael Young.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r f28eea0efe12 -r fbc0dc31b6d7 docs/misc/xl-network-configuration.markdown
--- a/docs/misc/xl-network-configuration.markdown	Fri Apr 20 09:59:12 2012 +0100
+++ b/docs/misc/xl-network-configuration.markdown	Fri Apr 20 09:59:35 2012 +0100
@@ -93,11 +93,14 @@ are:
 
 ### vifname
 
-This keyword is valid for HVM guest devices with `type=ioemu` only.
+Specifies the backend device name for the virtual device.
 
-Specifies the backend device name for an emulated device. The default
-is `tapDOMID.DEVID` where `DOMID` is the guest domain ID and `DEVID`
-is the device number.
+If the domain is an HVM domain then the associated emulated (tap)
+device will have a "xentap-" prefix added.
+
+The default name for the virtual device is `vifDOMID.DEVID` where
+`DOMID` is the guest domain ID and `DEVID` is the device
+number. Likewise the default tap name is `xentapDOMID.DEVID`.
 
 ### script
 
diff -r f28eea0efe12 -r fbc0dc31b6d7 tools/hotplug/Linux/vif-common.sh
--- a/tools/hotplug/Linux/vif-common.sh	Fri Apr 20 09:59:12 2012 +0100
+++ b/tools/hotplug/Linux/vif-common.sh	Fri Apr 20 09:59:35 2012 +0100
@@ -85,8 +85,8 @@ elif [ "$type_if" = tap ]; then
     : ${INTERFACE:?}
 
     # Get xenbus_path from device name.
-    # The name is built like that: "tap${domid}.${devid}".
-    dev_=${dev#tap}
+    # The name is built like that: "xentap${domid}.${devid}".
+    dev_=${dev#xentap}
     domid=${dev_%.*}
     devid=${dev_#*.}
 
diff -r f28eea0efe12 -r fbc0dc31b6d7 tools/hotplug/Linux/xen-backend.rules
--- a/tools/hotplug/Linux/xen-backend.rules	Fri Apr 20 09:59:12 2012 +0100
+++ b/tools/hotplug/Linux/xen-backend.rules	Fri Apr 20 09:59:35 2012 +0100
@@ -13,4 +13,4 @@ KERNEL=="blktap-control", NAME="xen/blkt
 KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
 KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
 KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
-SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
+SUBSYSTEM=="net", KERNEL=="xentap*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
diff -r f28eea0efe12 -r fbc0dc31b6d7 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Fri Apr 20 09:59:12 2012 +0100
+++ b/tools/libxl/libxl_dm.c	Fri Apr 20 09:59:35 2012 +0100
@@ -212,9 +212,9 @@ static char ** libxl__build_device_model
                 char *ifname;
                 if (!vifs[i].ifname)
                     ifname = libxl__sprintf(gc,
-                                            "tap%d.%d", domid, vifs[i].devid);
+                                            "xentap%d.%d", domid, vifs[i].devid);
                 else
-                    ifname = vifs[i].ifname;
+                    ifname = libxl__sprintf(gc, "xentap-%s", vifs[i].ifname);
                 flexarray_vappend(dm_args,
                                 "-net", libxl__sprintf(gc, "nic,vlan=%d,macaddr=%s,model=%s",
                                                        vifs[i].devid, smac, vifs[i].model),
@@ -451,10 +451,10 @@ static char ** libxl__build_device_model
                                 LIBXL_MAC_FMT, LIBXL_MAC_BYTES(vifs[i].mac));
                 char *ifname;
                 if (!vifs[i].ifname) {
-                    ifname = libxl__sprintf(gc, "tap%d.%d",
+                    ifname = libxl__sprintf(gc, "xentap%d.%d",
                                             guest_domid, vifs[i].devid);
                 } else {
-                    ifname = vifs[i].ifname;
+                    ifname = libxl__sprintf(gc, "xentap-%s", vifs[i].ifname);
                 }
                 flexarray_append(dm_args, "-device");
                 flexarray_append(dm_args,
diff -r f28eea0efe12 -r fbc0dc31b6d7 tools/python/xen/xend/image.py
--- a/tools/python/xen/xend/image.py	Fri Apr 20 09:59:12 2012 +0100
+++ b/tools/python/xen/xend/image.py	Fri Apr 20 09:59:35 2012 +0100
@@ -919,9 +919,9 @@ class HVMImageHandler(ImageHandler):
                        (nics, mac, model))
             vifname = devinfo.get('vifname')
             if vifname:
-                vifname = "tap-" + vifname
+                vifname = "xentap-" + vifname
             else:
-                vifname = "tap%d.%d" % (self.vm.getDomid(), nics-1)
+                vifname = "xentap%d.%d" % (self.vm.getDomid(), nics-1)
             ret.append("-net")
             ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
                        (nics, vifname, bridge))

Re: [patch] xen udev rule interfering with openvpn

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
> libxl/xend: name tap devices with a xentap prefix
...
> Also add "xentap-" prefix to the tap device when an explicit name is
> given to avoid a conflict with the vif device, which would otherwise
> have the same name.  Likewise correct the documentation for this
> option which suggested it applied to HVM tap devices only.

This sounds like a good idea but of course the names of interfaces
on Linux have a fairly short (16 character) iirc length limit.  Are we
sure that this isn't going to break some existing setup where the name
is already using most of the available length ?

Ian.

Re: [patch] xen udev rule interfering with openvpn

[Ian Campbell]

On Fri, 2012-04-20 at 11:38 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
> > libxl/xend: name tap devices with a xentap prefix
> ...
> > Also add "xentap-" prefix to the tap device when an explicit name is
> > given to avoid a conflict with the vif device, which would otherwise
> > have the same name.  Likewise correct the documentation for this
> > option which suggested it applied to HVM tap devices only.
> 
> This sounds like a good idea but of course the names of interfaces
> on Linux have a fairly short (16 character) iirc length limit.

Oh, right, I'd forgotten that.

>   Are we
> sure that this isn't going to break some existing setup where the name
> is already using most of the available length ?

On the contrary I'm fairly sure it will break them... How common they
would be I can't say (other than my gut feeling being "not very").

Not sure what our options are here. Perhaps in the grant Unix tradition
of dropping vowels and random consonants "xtp-". Or perhaps "emu-" (for
emulated)?

This still adds a new 4 char prefix in the xl case but xend already had
that behaviour so hopefully people haven't come to rely on it in the
meantime.

Ian.

Re: [patch] xen udev rule interfering with openvpn

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
> On Fri, 2012-04-20 at 11:38 +0100, Ian Jackson wrote:
> >   Are we
> > sure that this isn't going to break some existing setup where the name
> > is already using most of the available length ?
> 
> On the contrary I'm fairly sure it will break them... How common they
> would be I can't say (other than my gut feeling being "not very").
> 
> Not sure what our options are here. Perhaps in the grant Unix tradition
> of dropping vowels and random consonants "xtp-". Or perhaps "emu-" (for
> emulated)?

I'm not quite up to speed with all the context here but is the reason
that you're not suggesting "xen-" is that that's already used for
something else ?

> This still adds a new 4 char prefix in the xl case but xend already had
> that behaviour so hopefully people haven't come to rely on it in the
> meantime.

I think that's a fair enough assumption.

Ian.

Re: [patch] xen udev rule interfering with openvpn

[Ian Campbell]

On Fri, 2012-04-20 at 11:55 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
> > On Fri, 2012-04-20 at 11:38 +0100, Ian Jackson wrote:
> > >   Are we
> > > sure that this isn't going to break some existing setup where the name
> > > is already using most of the available length ?
> > 
> > On the contrary I'm fairly sure it will break them... How common they
> > would be I can't say (other than my gut feeling being "not very").
> > 
> > Not sure what our options are here. Perhaps in the grant Unix tradition
> > of dropping vowels and random consonants "xtp-". Or perhaps "emu-" (for
> > emulated)?
> 
> I'm not quite up to speed with all the context here but is the reason
> that you're not suggesting "xen-" is that that's already used for
> something else ?

This is to distinguish the vif device from the associated tap device,
which would otherwise both be called whatever the user gave as "vifname"
in their config, so for vifname=foo you would get foo (the PV one) and
xen-foo (the EMU one) which does the job but doesn't really distinguish
them.

Ian.

> 
> > This still adds a new 4 char prefix in the xl case but xend already had
> > that behaviour so hopefully people haven't come to rely on it in the
> > meantime.
> 
> I think that's a fair enough assumption.
> 
> Ian.

Re: [patch] xen udev rule interfering with openvpn

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
> On Fri, 2012-04-20 at 11:55 +0100, Ian Jackson wrote:
> > I'm not quite up to speed with all the context here but is the reason
> > that you're not suggesting "xen-" is that that's already used for
> > something else ?
> 
> This is to distinguish the vif device from the associated tap device,
> which would otherwise both be called whatever the user gave as "vifname"
> in their config, so for vifname=foo you would get foo (the PV one) and
> xen-foo (the EMU one) which does the job but doesn't really distinguish
> them.

Ah, I see.  This sounds like more a job for a suffix than a prefix so
if we can spare 4 chars I would suggest foo-emu.

Ian.

Re: [patch] xen udev rule interfering with openvpn

[Ian Campbell]

On Fri, 2012-04-20 at 12:04 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
> > On Fri, 2012-04-20 at 11:55 +0100, Ian Jackson wrote:
> > > I'm not quite up to speed with all the context here but is the reason
> > > that you're not suggesting "xen-" is that that's already used for
> > > something else ?
> > 
> > This is to distinguish the vif device from the associated tap device,
> > which would otherwise both be called whatever the user gave as "vifname"
> > in their config, so for vifname=foo you would get foo (the PV one) and
> > xen-foo (the EMU one) which does the job but doesn't really distinguish
> > them.
> 
> Ah, I see.  This sounds like more a job for a suffix than a prefix so
> if we can spare 4 chars I would suggest foo-emu.

I agree.

This patch interacts a bit with Roger's hotplug series, I'll rebase on
top of his with this change when he reposts it.

Ian.

Re: [patch] xen udev rule interfering with openvpn

[Teck Choon Giam]

On Fri, Apr 20, 2012 at 9:21 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Fri, 2012-04-20 at 12:04 +0100, Ian Jackson wrote:
>> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
>> > On Fri, 2012-04-20 at 11:55 +0100, Ian Jackson wrote:
>> > > I'm not quite up to speed with all the context here but is the reason
>> > > that you're not suggesting "xen-" is that that's already used for
>> > > something else ?
>> >
>> > This is to distinguish the vif device from the associated tap device,
>> > which would otherwise both be called whatever the user gave as "vifname"
>> > in their config, so for vifname=foo you would get foo (the PV one) and
>> > xen-foo (the EMU one) which does the job but doesn't really distinguish
>> > them.
>>
>> Ah, I see.  This sounds like more a job for a suffix than a prefix so
>> if we can spare 4 chars I would suggest foo-emu.

So what is the final prefix/suffix here?  Is it "emu-"?  Sorry, need
to counter check :p

Question... vifname is limited to 16 characters?  If so, does the
configuration for xm/xl check for its allowable length?  I mean if a
user set vifname more than its allowable length in the domU
configuration, would xm/xl show error?

Sorry for asking as it isn't clear in manual... ...
http://xenbits.xen.org/docs/unstable/misc/xl-network-configuration.html
================================
vifname

This keyword is valid for HVM guest devices with type=ioemu only.

Specifies the backend device name for an emulated device. The default
is tapDOMID.DEVID where DOMID is the guest domain ID and DEVID is the
device number.
================================

>
> I agree.
>
> This patch interacts a bit with Roger's hotplug series, I'll rebase on
> top of his with this change when he reposts it.

Looking forward for your reports ;)

>
> Ian.
>

Thanks.

Kindest regards,
Giam Teck Choon

Re: [patch] xen udev rule interfering with openvpn

[Ian Campbell]

On Fri, 2012-04-20 at 16:26 +0100, Teck Choon Giam wrote:
> On Fri, Apr 20, 2012 at 9:21 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Fri, 2012-04-20 at 12:04 +0100, Ian Jackson wrote:
> >> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
> >> > On Fri, 2012-04-20 at 11:55 +0100, Ian Jackson wrote:
> >> > > I'm not quite up to speed with all the context here but is the reason
> >> > > that you're not suggesting "xen-" is that that's already used for
> >> > > something else ?
> >> >
> >> > This is to distinguish the vif device from the associated tap device,
> >> > which would otherwise both be called whatever the user gave as "vifname"
> >> > in their config, so for vifname=foo you would get foo (the PV one) and
> >> > xen-foo (the EMU one) which does the job but doesn't really distinguish
> >> > them.
> >>
> >> Ah, I see.  This sounds like more a job for a suffix than a prefix so
> >> if we can spare 4 chars I would suggest foo-emu.
> 
> So what is the final prefix/suffix here?  Is it "emu-"?  Sorry, need
> to counter check :p

I think we have agreed on a "-emu" suffix.

> Question... vifname is limited to 16 characters?  If so, does the
> configuration for xm/xl check for its allowable length?  I mean if a
> user set vifname more than its allowable length in the domU
> configuration, would xm/xl show error?

I can't see any existing check for the length of the vif name.

It's a bit hard for us to do, consider a network driver domain running a
kernel which has a different, or no, limit here. libxl might not have
any idea what that name limit should be. We could pick an arbitrary
limit which is the smallest we know of, e.g. 12 chars (to allow +4), I
suppose.

BTW, the failure if your name is too long will be that the vif hotplug
script will fail to rename the device.

This limit has long existed and I don't recall ever having seen a report
about it, maybe the failure case is so obvious that people just fix it
and move on. I also expect that setting such a long name is rare...

> 
> Sorry for asking as it isn't clear in manual... ...
> http://xenbits.xen.org/docs/unstable/misc/xl-network-configuration.html
> ================================
> vifname
> 
> This keyword is valid for HVM guest devices with type=ioemu only.
> 
> Specifies the backend device name for an emulated device. The default
> is tapDOMID.DEVID where DOMID is the guest domain ID and DEVID is the
> device number.
> ================================
> 
> >
> > I agree.
> >
> > This patch interacts a bit with Roger's hotplug series, I'll rebase on
> > top of his with this change when he reposts it.
> 
> Looking forward for your reports ;)
> 
> >
> > Ian.
> >
> 
> Thanks.
> 
> Kindest regards,
> Giam Teck Choon

Re: [patch] xen udev rule interfering with openvpn

[Teck Choon Giam]

On Fri, Apr 20, 2012 at 11:38 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Fri, 2012-04-20 at 16:26 +0100, Teck Choon Giam wrote:
>> On Fri, Apr 20, 2012 at 9:21 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> > On Fri, 2012-04-20 at 12:04 +0100, Ian Jackson wrote:
>> >> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
>> >> > On Fri, 2012-04-20 at 11:55 +0100, Ian Jackson wrote:
>> >> > > I'm not quite up to speed with all the context here but is the reason
>> >> > > that you're not suggesting "xen-" is that that's already used for
>> >> > > something else ?
>> >> >
>> >> > This is to distinguish the vif device from the associated tap device,
>> >> > which would otherwise both be called whatever the user gave as "vifname"
>> >> > in their config, so for vifname=foo you would get foo (the PV one) and
>> >> > xen-foo (the EMU one) which does the job but doesn't really distinguish
>> >> > them.
>> >>
>> >> Ah, I see.  This sounds like more a job for a suffix than a prefix so
>> >> if we can spare 4 chars I would suggest foo-emu.
>>
>> So what is the final prefix/suffix here?  Is it "emu-"?  Sorry, need
>> to counter check :p
>
> I think we have agreed on a "-emu" suffix.

Thanks and I will update my backport to reflect this "-emu" instead of
"xentap-" as well then test ;)

>
>> Question... vifname is limited to 16 characters?  If so, does the
>> configuration for xm/xl check for its allowable length?  I mean if a
>> user set vifname more than its allowable length in the domU
>> configuration, would xm/xl show error?
>
> I can't see any existing check for the length of the vif name.
>
> It's a bit hard for us to do, consider a network driver domain running a
> kernel which has a different, or no, limit here. libxl might not have
> any idea what that name limit should be. We could pick an arbitrary
> limit which is the smallest we know of, e.g. 12 chars (to allow +4), I
> suppose.
>
> BTW, the failure if your name is too long will be that the vif hotplug
> script will fail to rename the device.

Ok, noted.

>
> This limit has long existed and I don't recall ever having seen a report
> about it, maybe the failure case is so obvious that people just fix it
> and move on. I also expect that setting such a long name is rare...

Yes, rare I agree but you know some _rare_ users... ... As long as the
error is obvious then I think this shouldn't be an issue.

Thanks for taking time to reply.

Kindest regards,
Giam Teck Choon


>
>>
>> Sorry for asking as it isn't clear in manual... ...
>> http://xenbits.xen.org/docs/unstable/misc/xl-network-configuration.html
>> ================================
>> vifname
>>
>> This keyword is valid for HVM guest devices with type=ioemu only.
>>
>> Specifies the backend device name for an emulated device. The default
>> is tapDOMID.DEVID where DOMID is the guest domain ID and DEVID is the
>> device number.
>> ================================
>>
>> >
>> > I agree.
>> >
>> > This patch interacts a bit with Roger's hotplug series, I'll rebase on
>> > top of his with this change when he reposts it.
>>
>> Looking forward for your reports ;)
>>
>> >
>> > Ian.
>> >
>>
>> Thanks.
>>
>> Kindest regards,
>> Giam Teck Choon
>
>

Re: [patch] xen udev rule interfering with openvpn

[Ian Campbell]

On Fri, 2012-04-20 at 12:04 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
> > On Fri, 2012-04-20 at 11:55 +0100, Ian Jackson wrote:
> > > I'm not quite up to speed with all the context here but is the reason
> > > that you're not suggesting "xen-" is that that's already used for
> > > something else ?
> > 
> > This is to distinguish the vif device from the associated tap device,
> > which would otherwise both be called whatever the user gave as "vifname"
> > in their config, so for vifname=foo you would get foo (the PV one) and
> > xen-foo (the EMU one) which does the job but doesn't really distinguish
> > them.
> 
> Ah, I see.  This sounds like more a job for a suffix than a prefix so
> if we can spare 4 chars I would suggest foo-emu.

So for vifname="foo" it is a no-brainer to call the vif "foo" and the
emulated tap device "foo-emu".

But what about the case where no vifname is given, in that case vif is
named "vif<DOM>.<DEV>". But what to call the tap? Previously I was
changing the name from tap<DOM>.<DEV> to xentap<DOM>.<DEV> but perhaps
now "vif<DOM>.<DEV>-emu" makes more sense/is more consistent?

Ian.

Re: [patch] xen udev rule interfering with openvpn

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
> So for vifname="foo" it is a no-brainer to call the vif "foo" and the
> emulated tap device "foo-emu".
> 
> But what about the case where no vifname is given, in that case vif is
> named "vif<DOM>.<DEV>". But what to call the tap? Previously I was
> changing the name from tap<DOM>.<DEV> to xentap<DOM>.<DEV> but perhaps
> now "vif<DOM>.<DEV>-emu" makes more sense/is more consistent?

I think either is fine.  While we're changing it it probably makes
sense to use "vif..." as indeed it is more consistent.

Ian.

Re: [patch] xen udev rule interfering with openvpn

[Ian Campbell]

On Wed, 2012-04-25 at 11:11 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
> > So for vifname="foo" it is a no-brainer to call the vif "foo" and the
> > emulated tap device "foo-emu".
> > 
> > But what about the case where no vifname is given, in that case vif is
> > named "vif<DOM>.<DEV>". But what to call the tap? Previously I was
> > changing the name from tap<DOM>.<DEV> to xentap<DOM>.<DEV> but perhaps
> > now "vif<DOM>.<DEV>-emu" makes more sense/is more consistent?
> 
> I think either is fine.  While we're changing it it probably makes
> sense to use "vif..." as indeed it is more consistent.

OK, vifX.Y and vifX.Y-emu it is...

Ian.

Re: [patch] xen udev rule interfering with openvpn

[Ian Campbell]

On Wed, 2012-04-25 at 11:14 +0100, Ian Campbell wrote:
> On Wed, 2012-04-25 at 11:11 +0100, Ian Jackson wrote:
> > Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
> > > So for vifname="foo" it is a no-brainer to call the vif "foo" and the
> > > emulated tap device "foo-emu".
> > > 
> > > But what about the case where no vifname is given, in that case vif is
> > > named "vif<DOM>.<DEV>". But what to call the tap? Previously I was
> > > changing the name from tap<DOM>.<DEV> to xentap<DOM>.<DEV> but perhaps
> > > now "vif<DOM>.<DEV>-emu" makes more sense/is more consistent?
> > 
> > I think either is fine.  While we're changing it it probably makes
> > sense to use "vif..." as indeed it is more consistent.
> 
> OK, vifX.Y and vifX.Y-emu it is...

This turned out to be a bit more complex than I was expecting, mostly
because the use of "vifname" with tap devices was already broken. I
think this does the right things with each type of device.

Roger, not sure what knock on effect this has on your series. The main
thing is likely to be that you needn't find the vifname since you
actually want the standard name not the user supplied once on most
occasions.

Ian.

8<---------------------------------------------------------

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1335354957 -3600
# Node ID 44fc76485f797bdd726fed4a2a7c4b06363fdb88
# Parent  a14b1dd0feeee300c37c44564381f4e8ea837c45
libxl/xend: name tap devices vifX.Y-emu

This prevents the udev scripts from operating on other tap devices (e.g.
openvpn etc)

Correct the documentation for the "vifname" option which suggested it applied
to HVM tap devices only, which is not the case.

Reported by Michael Young.

Also fix the use of vifname with emulated devices. This is slightly complex.
The current hotplug scripts rely on being able to parse the "tapX.Y" (now
"vifX.Y-emu") name in order to locate the xenstore backend dir relating to the
corresponding vif. This is because we cannot inject our own environment vars
into the tap hotplug events. However this means that if the tap is initially
named with a user specified name (which will not match the expected scheme) we
fail to do anything useful with the device. So now we create the initial tap
device with the standard "vifX.Y-emu" name and the hotplug script will handle
the rename to the desired name. This is also how PV vif devices work -- they
are always created by netback with the name vifX.Y and renamed in the script.

Lastly also move libxl__device_* to a better place in the header, otherwise the
comment about evgen stuff isn't next to the associated functions (noticed jsut
because I was going to add nic_devname near to the setdefault functions)

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r a14b1dd0feee -r 44fc76485f79 docs/misc/xl-network-configuration.markdown
--- a/docs/misc/xl-network-configuration.markdown	Wed Apr 25 10:53:52 2012 +0100
+++ b/docs/misc/xl-network-configuration.markdown	Wed Apr 25 12:55:57 2012 +0100
@@ -93,11 +93,14 @@ are:
 
 ### vifname
 
-This keyword is valid for HVM guest devices with `type=ioemu` only.
+Specifies the backend device name for the virtual device.
 
-Specifies the backend device name for an emulated device. The default
-is `tapDOMID.DEVID` where `DOMID` is the guest domain ID and `DEVID`
-is the device number.
+If the domain is an HVM domain then the associated emulated (tap)
+device will have a "-emu" suffice added.
+
+The default name for the virtual device is `vifDOMID.DEVID` where
+`DOMID` is the guest domain ID and `DEVID` is the device
+number. Likewise the default tap name is `vifDOMID.DEVID-emu`.
 
 ### script
 
diff -r a14b1dd0feee -r 44fc76485f79 tools/hotplug/Linux/vif-common.sh
--- a/tools/hotplug/Linux/vif-common.sh	Wed Apr 25 10:53:52 2012 +0100
+++ b/tools/hotplug/Linux/vif-common.sh	Wed Apr 25 12:55:57 2012 +0100
@@ -85,12 +85,23 @@ elif [ "$type_if" = tap ]; then
     : ${INTERFACE:?}
 
     # Get xenbus_path from device name.
-    # The name is built like that: "tap${domid}.${devid}".
-    dev_=${dev#tap}
+    # The name is built like that: "vif${domid}.${devid}-emu".
+    dev_=${dev#vif}
+    dev_=${dev_%-emu}
     domid=${dev_%.*}
     devid=${dev_#*.}
 
     XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid"
+    vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
+    if [ "$vifname" ]
+    then
+        vifname="${vifname}-emu"
+        if [ "$command" == "add" ] && ! ip link show "$vifname" >&/dev/null
+        then
+            do_or_die ip link set "$dev" name "$vifname"
+        fi
+        dev="$vifname"
+    fi
 fi
 
 ip=${ip:-}
diff -r a14b1dd0feee -r 44fc76485f79 tools/hotplug/Linux/xen-backend.rules
--- a/tools/hotplug/Linux/xen-backend.rules	Wed Apr 25 10:53:52 2012 +0100
+++ b/tools/hotplug/Linux/xen-backend.rules	Wed Apr 25 12:55:57 2012 +0100
@@ -13,4 +13,4 @@ KERNEL=="blktap-control", NAME="xen/blkt
 KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
 KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
 KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
-SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
+SUBSYSTEM=="net", KERNEL=="vif*-emu", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
diff -r a14b1dd0feee -r 44fc76485f79 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Wed Apr 25 10:53:52 2012 +0100
+++ b/tools/libxl/libxl.c	Wed Apr 25 12:55:57 2012 +0100
@@ -2095,6 +2095,21 @@ int libxl_device_nic_getinfo(libxl_ctx *
     return 0;
 }
 
+const char *libxl__device_nic_devname(libxl__gc *gc,
+                                      uint32_t domid,
+                                      uint32_t devid,
+                                      libxl_nic_type type)
+{
+    switch (type) {
+    case LIBXL_NIC_TYPE_VIF:
+        return GCSPRINTF("vif%u.%d", domid, devid);
+    case LIBXL_NIC_TYPE_IOEMU:
+        return GCSPRINTF("vif%u.%d" TAP_DEVICE_SUFFIX, domid, devid);
+    default:
+        abort();
+    }
+}
+
 /******************************************************************************/
 int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
                               libxl__device_console *console,
diff -r a14b1dd0feee -r 44fc76485f79 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Wed Apr 25 10:53:52 2012 +0100
+++ b/tools/libxl/libxl_dm.c	Wed Apr 25 12:55:57 2012 +0100
@@ -209,12 +209,9 @@ static char ** libxl__build_device_model
             if (vifs[i].nictype == LIBXL_NIC_TYPE_IOEMU) {
                 char *smac = libxl__sprintf(gc,
                                    LIBXL_MAC_FMT, LIBXL_MAC_BYTES(vifs[i].mac));
-                char *ifname;
-                if (!vifs[i].ifname)
-                    ifname = libxl__sprintf(gc,
-                                            "tap%d.%d", domid, vifs[i].devid);
-                else
-                    ifname = vifs[i].ifname;
+                const char *ifname = libxl__device_nic_devname(gc,
+                                                domid, vifs[i].devid,
+                                                LIBXL_NIC_TYPE_IOEMU);
                 flexarray_vappend(dm_args,
                                 "-net", libxl__sprintf(gc, "nic,vlan=%d,macaddr=%s,model=%s",
                                                        vifs[i].devid, smac, vifs[i].model),
@@ -449,13 +446,9 @@ static char ** libxl__build_device_model
             if (vifs[i].nictype == LIBXL_NIC_TYPE_IOEMU) {
                 char *smac = libxl__sprintf(gc,
                                 LIBXL_MAC_FMT, LIBXL_MAC_BYTES(vifs[i].mac));
-                char *ifname;
-                if (!vifs[i].ifname) {
-                    ifname = libxl__sprintf(gc, "tap%d.%d",
-                                            guest_domid, vifs[i].devid);
-                } else {
-                    ifname = vifs[i].ifname;
-                }
+                const char *ifname = libxl__device_nic_devname(gc,
+                                                guest_domid, vifs[i].devid,
+                                                LIBXL_NIC_TYPE_IOEMU);
                 flexarray_append(dm_args, "-device");
                 flexarray_append(dm_args,
                    libxl__sprintf(gc, "%s,id=nic%d,netdev=net%d,mac=%s",
diff -r a14b1dd0feee -r 44fc76485f79 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Wed Apr 25 10:53:52 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Wed Apr 25 12:55:57 2012 +0100
@@ -83,6 +83,7 @@
 #define STUBDOM_CONSOLE_RESTORE 2
 #define STUBDOM_CONSOLE_SERIAL 3
 #define STUBDOM_SPECIAL_CONSOLES 3
+#define TAP_DEVICE_SUFFIX "-emu"
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
 
@@ -196,17 +197,6 @@ _hidden libxl__ev_xswatch *libxl__watch_
  * version of the _evdisable_FOO function; the internal one is
  * used during cleanup.
  */
-_hidden int libxl__domain_create_info_setdefault(libxl__gc *gc,
-                                        libxl_domain_create_info *c_info);
-_hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
-                                        libxl_domain_build_info *b_info);
-_hidden int libxl__device_disk_setdefault(libxl__gc *gc,
-                                          libxl_device_disk *disk);
-_hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic);
-_hidden int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb);
-_hidden int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb);
-_hidden int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci);
-
 struct libxl__evgen_domain_death {
     uint32_t domid;
     unsigned shutdown_reported:1, death_reported:1;
@@ -704,6 +694,21 @@ _hidden int libxl__wait_for_backend(libx
  *     All libxl API functions are expected to have arranged for this
  *     to be called before using any values within these structures.
  */
+_hidden int libxl__domain_create_info_setdefault(libxl__gc *gc,
+                                        libxl_domain_create_info *c_info);
+_hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
+                                        libxl_domain_build_info *b_info);
+_hidden int libxl__device_disk_setdefault(libxl__gc *gc,
+                                          libxl_device_disk *disk);
+_hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic);
+_hidden int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb);
+_hidden int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb);
+_hidden int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci);
+
+_hidden const char *libxl__device_nic_devname(libxl__gc *gc,
+                                              uint32_t domid,
+                                              uint32_t devid,
+                                              libxl_nic_type type);
 
 /* Arranges that dev will be removed from its guest.  When
  * this is done, the ao will be completed.  An error
diff -r a14b1dd0feee -r 44fc76485f79 tools/python/xen/xend/image.py
--- a/tools/python/xen/xend/image.py	Wed Apr 25 10:53:52 2012 +0100
+++ b/tools/python/xen/xend/image.py	Wed Apr 25 12:55:57 2012 +0100
@@ -917,11 +917,7 @@ class HVMImageHandler(ImageHandler):
             ret.append("-net")
             ret.append("nic,vlan=%d,macaddr=%s,model=%s" %
                        (nics, mac, model))
-            vifname = devinfo.get('vifname')
-            if vifname:
-                vifname = "tap-" + vifname
-            else:
-                vifname = "tap%d.%d" % (self.vm.getDomid(), nics-1)
+            vifname = "vif%d.%d-emu" % (self.vm.getDomid(), nics-1)
             ret.append("-net")
             ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
                        (nics, vifname, bridge))

Re: [patch] xen udev rule interfering with openvpn

[Roger Pau Monne]

Ian Campbell escribió:
> On Wed, 2012-04-25 at 11:14 +0100, Ian Campbell wrote:
>> On Wed, 2012-04-25 at 11:11 +0100, Ian Jackson wrote:
>>> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
>>>> So for vifname="foo" it is a no-brainer to call the vif "foo" and the
>>>> emulated tap device "foo-emu".
>>>>
>>>> But what about the case where no vifname is given, in that case vif is
>>>> named "vif<DOM>.<DEV>". But what to call the tap? Previously I was
>>>> changing the name from tap<DOM>.<DEV>  to xentap<DOM>.<DEV>  but perhaps
>>>> now "vif<DOM>.<DEV>-emu" makes more sense/is more consistent?
>>> I think either is fine.  While we're changing it it probably makes
>>> sense to use "vif..." as indeed it is more consistent.
>> OK, vifX.Y and vifX.Y-emu it is...
>
> This turned out to be a bit more complex than I was expecting, mostly
> because the use of "vifname" with tap devices was already broken. I
> think this does the right things with each type of device.
>
> Roger, not sure what knock on effect this has on your series. The main
> thing is likely to be that you needn't find the vifname since you
> actually want the standard name not the user supplied once on most
> occasions.

 From what I see, I have to pass the name returned by 
libxl__device_nic_devname to the hotplug scripts, is that right?

Thanks for doing this!

> Ian.
>
> 8<---------------------------------------------------------
>
> # HG changeset patch
> # User Ian Campbell<ian.campbell@citrix.com>
> # Date 1335354957 -3600
> # Node ID 44fc76485f797bdd726fed4a2a7c4b06363fdb88
> # Parent  a14b1dd0feeee300c37c44564381f4e8ea837c45
> libxl/xend: name tap devices vifX.Y-emu
>
> This prevents the udev scripts from operating on other tap devices (e.g.
> openvpn etc)
>
> Correct the documentation for the "vifname" option which suggested it applied
> to HVM tap devices only, which is not the case.
>
> Reported by Michael Young.
>
> Also fix the use of vifname with emulated devices. This is slightly complex.
> The current hotplug scripts rely on being able to parse the "tapX.Y" (now
> "vifX.Y-emu") name in order to locate the xenstore backend dir relating to the
> corresponding vif. This is because we cannot inject our own environment vars
> into the tap hotplug events. However this means that if the tap is initially
> named with a user specified name (which will not match the expected scheme) we
> fail to do anything useful with the device. So now we create the initial tap
> device with the standard "vifX.Y-emu" name and the hotplug script will handle
> the rename to the desired name. This is also how PV vif devices work -- they
> are always created by netback with the name vifX.Y and renamed in the script.
>
> Lastly also move libxl__device_* to a better place in the header, otherwise the
> comment about evgen stuff isn't next to the associated functions (noticed jsut
> because I was going to add nic_devname near to the setdefault functions)
>
> Signed-off-by: Ian Campbell<ian.campbell@citrix.com>
>
> diff -r a14b1dd0feee -r 44fc76485f79 docs/misc/xl-network-configuration.markdown
> --- a/docs/misc/xl-network-configuration.markdown	Wed Apr 25 10:53:52 2012 +0100
> +++ b/docs/misc/xl-network-configuration.markdown	Wed Apr 25 12:55:57 2012 +0100
> @@ -93,11 +93,14 @@ are:
>
>   ### vifname
>
> -This keyword is valid for HVM guest devices with `type=ioemu` only.
> +Specifies the backend device name for the virtual device.
>
> -Specifies the backend device name for an emulated device. The default
> -is `tapDOMID.DEVID` where `DOMID` is the guest domain ID and `DEVID`
> -is the device number.
> +If the domain is an HVM domain then the associated emulated (tap)
> +device will have a "-emu" suffice added.
> +
> +The default name for the virtual device is `vifDOMID.DEVID` where
> +`DOMID` is the guest domain ID and `DEVID` is the device
> +number. Likewise the default tap name is `vifDOMID.DEVID-emu`.
>
>   ### script
>
> diff -r a14b1dd0feee -r 44fc76485f79 tools/hotplug/Linux/vif-common.sh
> --- a/tools/hotplug/Linux/vif-common.sh	Wed Apr 25 10:53:52 2012 +0100
> +++ b/tools/hotplug/Linux/vif-common.sh	Wed Apr 25 12:55:57 2012 +0100
> @@ -85,12 +85,23 @@ elif [ "$type_if" = tap ]; then
>       : ${INTERFACE:?}
>
>       # Get xenbus_path from device name.
> -    # The name is built like that: "tap${domid}.${devid}".
> -    dev_=${dev#tap}
> +    # The name is built like that: "vif${domid}.${devid}-emu".
> +    dev_=${dev#vif}
> +    dev_=${dev_%-emu}
>       domid=${dev_%.*}
>       devid=${dev_#*.}
>
>       XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid"
> +    vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
> +    if [ "$vifname" ]
> +    then
> +        vifname="${vifname}-emu"
> +        if [ "$command" == "add" ]&&  ! ip link show "$vifname">&/dev/null
> +        then
> +            do_or_die ip link set "$dev" name "$vifname"
> +        fi
> +        dev="$vifname"
> +    fi
>   fi
>
>   ip=${ip:-}
> diff -r a14b1dd0feee -r 44fc76485f79 tools/hotplug/Linux/xen-backend.rules
> --- a/tools/hotplug/Linux/xen-backend.rules	Wed Apr 25 10:53:52 2012 +0100
> +++ b/tools/hotplug/Linux/xen-backend.rules	Wed Apr 25 12:55:57 2012 +0100
> @@ -13,4 +13,4 @@ KERNEL=="blktap-control", NAME="xen/blkt
>   KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
>   KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
>   KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
> -SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
> +SUBSYSTEM=="net", KERNEL=="vif*-emu", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
> diff -r a14b1dd0feee -r 44fc76485f79 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Wed Apr 25 10:53:52 2012 +0100
> +++ b/tools/libxl/libxl.c	Wed Apr 25 12:55:57 2012 +0100
> @@ -2095,6 +2095,21 @@ int libxl_device_nic_getinfo(libxl_ctx *
>       return 0;
>   }
>
> +const char *libxl__device_nic_devname(libxl__gc *gc,
> +                                      uint32_t domid,
> +                                      uint32_t devid,
> +                                      libxl_nic_type type)
> +{
> +    switch (type) {
> +    case LIBXL_NIC_TYPE_VIF:
> +        return GCSPRINTF("vif%u.%d", domid, devid);
> +    case LIBXL_NIC_TYPE_IOEMU:
> +        return GCSPRINTF("vif%u.%d" TAP_DEVICE_SUFFIX, domid, devid);
> +    default:
> +        abort();
> +    }
> +}
> +
>   /******************************************************************************/
>   int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
>                                 libxl__device_console *console,
> diff -r a14b1dd0feee -r 44fc76485f79 tools/libxl/libxl_dm.c
> --- a/tools/libxl/libxl_dm.c	Wed Apr 25 10:53:52 2012 +0100
> +++ b/tools/libxl/libxl_dm.c	Wed Apr 25 12:55:57 2012 +0100
> @@ -209,12 +209,9 @@ static char ** libxl__build_device_model
>               if (vifs[i].nictype == LIBXL_NIC_TYPE_IOEMU) {
>                   char *smac = libxl__sprintf(gc,
>                                      LIBXL_MAC_FMT, LIBXL_MAC_BYTES(vifs[i].mac));
> -                char *ifname;
> -                if (!vifs[i].ifname)
> -                    ifname = libxl__sprintf(gc,
> -                                            "tap%d.%d", domid, vifs[i].devid);
> -                else
> -                    ifname = vifs[i].ifname;
> +                const char *ifname = libxl__device_nic_devname(gc,
> +                                                domid, vifs[i].devid,
> +                                                LIBXL_NIC_TYPE_IOEMU);
>                   flexarray_vappend(dm_args,
>                                   "-net", libxl__sprintf(gc, "nic,vlan=%d,macaddr=%s,model=%s",
>                                                          vifs[i].devid, smac, vifs[i].model),
> @@ -449,13 +446,9 @@ static char ** libxl__build_device_model
>               if (vifs[i].nictype == LIBXL_NIC_TYPE_IOEMU) {
>                   char *smac = libxl__sprintf(gc,
>                                   LIBXL_MAC_FMT, LIBXL_MAC_BYTES(vifs[i].mac));
> -                char *ifname;
> -                if (!vifs[i].ifname) {
> -                    ifname = libxl__sprintf(gc, "tap%d.%d",
> -                                            guest_domid, vifs[i].devid);
> -                } else {
> -                    ifname = vifs[i].ifname;
> -                }
> +                const char *ifname = libxl__device_nic_devname(gc,
> +                                                guest_domid, vifs[i].devid,
> +                                                LIBXL_NIC_TYPE_IOEMU);
>                   flexarray_append(dm_args, "-device");
>                   flexarray_append(dm_args,
>                      libxl__sprintf(gc, "%s,id=nic%d,netdev=net%d,mac=%s",
> diff -r a14b1dd0feee -r 44fc76485f79 tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h	Wed Apr 25 10:53:52 2012 +0100
> +++ b/tools/libxl/libxl_internal.h	Wed Apr 25 12:55:57 2012 +0100
> @@ -83,6 +83,7 @@
>   #define STUBDOM_CONSOLE_RESTORE 2
>   #define STUBDOM_CONSOLE_SERIAL 3
>   #define STUBDOM_SPECIAL_CONSOLES 3
> +#define TAP_DEVICE_SUFFIX "-emu"
>
>   #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
>
> @@ -196,17 +197,6 @@ _hidden libxl__ev_xswatch *libxl__watch_
>    * version of the _evdisable_FOO function; the internal one is
>    * used during cleanup.
>    */
> -_hidden int libxl__domain_create_info_setdefault(libxl__gc *gc,
> -                                        libxl_domain_create_info *c_info);
> -_hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
> -                                        libxl_domain_build_info *b_info);
> -_hidden int libxl__device_disk_setdefault(libxl__gc *gc,
> -                                          libxl_device_disk *disk);
> -_hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic);
> -_hidden int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb);
> -_hidden int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb);
> -_hidden int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci);
> -
>   struct libxl__evgen_domain_death {
>       uint32_t domid;
>       unsigned shutdown_reported:1, death_reported:1;
> @@ -704,6 +694,21 @@ _hidden int libxl__wait_for_backend(libx
>    *     All libxl API functions are expected to have arranged for this
>    *     to be called before using any values within these structures.
>    */
> +_hidden int libxl__domain_create_info_setdefault(libxl__gc *gc,
> +                                        libxl_domain_create_info *c_info);
> +_hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
> +                                        libxl_domain_build_info *b_info);
> +_hidden int libxl__device_disk_setdefault(libxl__gc *gc,
> +                                          libxl_device_disk *disk);
> +_hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic);
> +_hidden int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb);
> +_hidden int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb);
> +_hidden int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci);
> +
> +_hidden const char *libxl__device_nic_devname(libxl__gc *gc,
> +                                              uint32_t domid,
> +                                              uint32_t devid,
> +                                              libxl_nic_type type);
>
>   /* Arranges that dev will be removed from its guest.  When
>    * this is done, the ao will be completed.  An error
> diff -r a14b1dd0feee -r 44fc76485f79 tools/python/xen/xend/image.py
> --- a/tools/python/xen/xend/image.py	Wed Apr 25 10:53:52 2012 +0100
> +++ b/tools/python/xen/xend/image.py	Wed Apr 25 12:55:57 2012 +0100
> @@ -917,11 +917,7 @@ class HVMImageHandler(ImageHandler):
>               ret.append("-net")
>               ret.append("nic,vlan=%d,macaddr=%s,model=%s" %
>                          (nics, mac, model))
> -            vifname = devinfo.get('vifname')
> -            if vifname:
> -                vifname = "tap-" + vifname
> -            else:
> -                vifname = "tap%d.%d" % (self.vm.getDomid(), nics-1)
> +            vifname = "vif%d.%d-emu" % (self.vm.getDomid(), nics-1)
>               ret.append("-net")
>               ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
>                          (nics, vifname, bridge))
>
>


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

Re: [patch] xen udev rule interfering with openvpn

[Ian Campbell]

On Wed, 2012-04-25 at 14:16 +0100, Roger Pau Monne wrote:
> Ian Campbell escribió:
> > On Wed, 2012-04-25 at 11:14 +0100, Ian Campbell wrote:
> >> On Wed, 2012-04-25 at 11:11 +0100, Ian Jackson wrote:
> >>> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
> >>>> So for vifname="foo" it is a no-brainer to call the vif "foo" and the
> >>>> emulated tap device "foo-emu".
> >>>>
> >>>> But what about the case where no vifname is given, in that case vif is
> >>>> named "vif<DOM>.<DEV>". But what to call the tap? Previously I was
> >>>> changing the name from tap<DOM>.<DEV>  to xentap<DOM>.<DEV>  but perhaps
> >>>> now "vif<DOM>.<DEV>-emu" makes more sense/is more consistent?
> >>> I think either is fine.  While we're changing it it probably makes
> >>> sense to use "vif..." as indeed it is more consistent.
> >> OK, vifX.Y and vifX.Y-emu it is...
> >
> > This turned out to be a bit more complex than I was expecting, mostly
> > because the use of "vifname" with tap devices was already broken. I
> > think this does the right things with each type of device.
> >
> > Roger, not sure what knock on effect this has on your series. The main
> > thing is likely to be that you needn't find the vifname since you
> > actually want the standard name not the user supplied once on most
> > occasions.
> 
>  From what I see, I have to pass the name returned by
> libxl__device_nic_devname to the hotplug scripts, is that right?

AFAICT pretty much, yes.

Ian.



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

Re: [patch] xen udev rule interfering with openvpn

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
> libxl/xend: name tap devices vifX.Y-emu

Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>

Re: [patch] xen udev rule interfering with openvpn

[Teck Choon Giam]

[-- Attachment #1: Type: text/plain, Size: 6800 bytes --]

On Fri, May 11, 2012 at 10:53 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
>> libxl/xend: name tap devices vifX.Y-emu
>
> Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>

This is my backport version which excludes the following:

Lastly also move libxl__device_* to a better place in the header, otherwise the
comment about evgen stuff isn't next to the associated functions (noticed jsut
because I was going to add nic_devname near to the setdefault functions)

I have tested with xm and xl with and without vifname set in domU
config for hvmdomain and pvdomain.

For your review and comments please.

Thanks.

Kindest regards,
Giam Teck Choon




libxl/xend: name tap devices vifX.Y-emu

This prevents the udev scripts from operating on other tap devices (e.g.
openvpn etc)

Correct the documentation for the "vifname" option which suggested it applied
to HVM tap devices only, which is not the case.

Reported by Michael Young.

Also fix the use of vifname with emulated devices. This is slightly complex.
The current hotplug scripts rely on being able to parse the "tapX.Y" (now
"vifX.Y-emu") name in order to locate the xenstore backend dir relating to the
corresponding vif. This is because we cannot inject our own environment vars
into the tap hotplug events. However this means that if the tap is initially
named with a user specified name (which will not match the expected scheme) we
fail to do anything useful with the device. So now we create the initial tap
device with the standard "vifX.Y-emu" name and the hotplug script will handle
the rename to the desired name. This is also how PV vif devices work -- they
are always created by netback with the name vifX.Y and renamed in the script.

xen-unstable changeset: 25290:7a6dcecb1781
Signed-off-by: Giam Teck Choon <giamteckchoon@gmail.com>
---
 tools/hotplug/Linux/vif-common.sh     |   15 +++++++++++++--
 tools/hotplug/Linux/xen-backend.rules |    2 +-
 tools/libxl/libxl_dm.c                |   17 ++++++-----------
 tools/python/xen/xend/image.py        |    6 +-----
 4 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/tools/hotplug/Linux/vif-common.sh
b/tools/hotplug/Linux/vif-common.sh
index c9c5d41..fff22bb 100644
--- a/tools/hotplug/Linux/vif-common.sh
+++ b/tools/hotplug/Linux/vif-common.sh
@@ -85,12 +85,23 @@ elif [ "$type_if" = tap ]; then
     : ${INTERFACE:?}

     # Get xenbus_path from device name.
-    # The name is built like that: "tap${domid}.${devid}".
-    dev_=${dev#tap}
+    # The name is built like that: "vif${domid}.${devid}-emu".
+    dev_=${dev#vif}
+    dev_=${dev_%-emu}
     domid=${dev_%.*}
     devid=${dev_#*.}

     XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid"
+    vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
+    if [ "$vifname" ]
+    then
+        vifname="${vifname}-emu"
+        if [ "$command" == "add" ] && ! ip link show "$vifname" >&/dev/null
+        then
+            do_or_die ip link set "$dev" name "$vifname"
+        fi
+        dev="$vifname"
+    fi
 fi

 ip=${ip:-}
diff --git a/tools/hotplug/Linux/xen-backend.rules
b/tools/hotplug/Linux/xen-backend.rules
index 40f2658..405387f 100644
--- a/tools/hotplug/Linux/xen-backend.rules
+++ b/tools/hotplug/Linux/xen-backend.rules
@@ -13,4 +13,4 @@ KERNEL=="blktap-control",
NAME="xen/blktap-2/control", MODE="0600"
 KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
 KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
 KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
-SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add",
RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
+SUBSYSTEM=="net", KERNEL=="vif*-emu", ACTION=="add",
RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 1ffcc90..2c030ca 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -134,11 +134,9 @@ static char **
libxl_build_device_model_args_old(libxl__gc *gc,
                 char *smac = libxl__sprintf(gc,
"%02x:%02x:%02x:%02x:%02x:%02x",
                                            vifs[i].mac[0],
vifs[i].mac[1], vifs[i].mac[2],
                                            vifs[i].mac[3],
vifs[i].mac[4], vifs[i].mac[5]);
-                char *ifname;
-                if (!vifs[i].ifname)
-                    ifname = libxl__sprintf(gc, "tap%d.%d",
info->domid, vifs[i].devid);
-                else
-                    ifname = vifs[i].ifname;
+                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
+                                                    info->domid,
+                                                    vifs[i].devid);
                 flexarray_vappend(dm_args,
                                 "-net", libxl__sprintf(gc,
"nic,vlan=%d,macaddr=%s,model=%s",
                                                        vifs[i].devid,
smac, vifs[i].model),
@@ -271,12 +269,9 @@ static char **
libxl_build_device_model_args_new(libxl__gc *gc,
                 char *smac = libxl__sprintf(gc,
"%02x:%02x:%02x:%02x:%02x:%02x",
                                            vifs[i].mac[0],
vifs[i].mac[1], vifs[i].mac[2],
                                            vifs[i].mac[3],
vifs[i].mac[4], vifs[i].mac[5]);
-                char *ifname;
-                if (!vifs[i].ifname) {
-                    ifname = libxl__sprintf(gc, "tap%d.%d",
info->domid, vifs[i].devid);
-                } else {
-                    ifname = vifs[i].ifname;
-                }
+                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
+                                                    info->domid,
+                                                    vifs[i].devid);
                 flexarray_append(dm_args, "-net");
                 flexarray_append(dm_args, libxl__sprintf(gc,
"nic,vlan=%d,macaddr=%s,model=%s",
                             vifs[i].devid, smac, vifs[i].model));
diff --git a/tools/python/xen/xend/image.py b/tools/python/xen/xend/image.py
index f1464ac..3c8d80c 100644
--- a/tools/python/xen/xend/image.py
+++ b/tools/python/xen/xend/image.py
@@ -917,11 +917,7 @@ class HVMImageHandler(ImageHandler):
             ret.append("-net")
             ret.append("nic,vlan=%d,macaddr=%s,model=%s" %
                        (nics, mac, model))
-            vifname = devinfo.get('vifname')
-            if vifname:
-                vifname = "tap-" + vifname
-            else:
-                vifname = "tap%d.%d" % (self.vm.getDomid(), nics-1)
+            vifname = "vif%d.%d-emu" % (self.vm.getDomid(), nics-1)
             ret.append("-net")
             ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
                        (nics, vifname, bridge))

[-- Attachment #2: backport-unstable-25290-to-xen-4.1-testing.patch --]
[-- Type: application/octet-stream, Size: 6052 bytes --]

libxl/xend: name tap devices vifX.Y-emu

This prevents the udev scripts from operating on other tap devices (e.g.
openvpn etc)

Correct the documentation for the "vifname" option which suggested it applied
to HVM tap devices only, which is not the case.

Reported by Michael Young.

Also fix the use of vifname with emulated devices. This is slightly complex.
The current hotplug scripts rely on being able to parse the "tapX.Y" (now
"vifX.Y-emu") name in order to locate the xenstore backend dir relating to the
corresponding vif. This is because we cannot inject our own environment vars
into the tap hotplug events. However this means that if the tap is initially
named with a user specified name (which will not match the expected scheme) we
fail to do anything useful with the device. So now we create the initial tap
device with the standard "vifX.Y-emu" name and the hotplug script will handle
the rename to the desired name. This is also how PV vif devices work -- they
are always created by netback with the name vifX.Y and renamed in the script.

xen-unstable changeset: 25290:7a6dcecb1781
Signed-off-by: Giam Teck Choon <giamteckchoon@gmail.com>
---
 tools/hotplug/Linux/vif-common.sh     |   15 +++++++++++++--
 tools/hotplug/Linux/xen-backend.rules |    2 +-
 tools/libxl/libxl_dm.c                |   17 ++++++-----------
 tools/python/xen/xend/image.py        |    6 +-----
 4 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/tools/hotplug/Linux/vif-common.sh b/tools/hotplug/Linux/vif-common.sh
index c9c5d41..fff22bb 100644
--- a/tools/hotplug/Linux/vif-common.sh
+++ b/tools/hotplug/Linux/vif-common.sh
@@ -85,12 +85,23 @@ elif [ "$type_if" = tap ]; then
     : ${INTERFACE:?}
 
     # Get xenbus_path from device name.
-    # The name is built like that: "tap${domid}.${devid}".
-    dev_=${dev#tap}
+    # The name is built like that: "vif${domid}.${devid}-emu".
+    dev_=${dev#vif}
+    dev_=${dev_%-emu}
     domid=${dev_%.*}
     devid=${dev_#*.}
 
     XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid"
+    vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
+    if [ "$vifname" ]
+    then
+        vifname="${vifname}-emu"
+        if [ "$command" == "add" ] && ! ip link show "$vifname" >&/dev/null
+        then
+            do_or_die ip link set "$dev" name "$vifname"
+        fi
+        dev="$vifname"
+    fi
 fi
 
 ip=${ip:-}
diff --git a/tools/hotplug/Linux/xen-backend.rules b/tools/hotplug/Linux/xen-backend.rules
index 40f2658..405387f 100644
--- a/tools/hotplug/Linux/xen-backend.rules
+++ b/tools/hotplug/Linux/xen-backend.rules
@@ -13,4 +13,4 @@ KERNEL=="blktap-control", NAME="xen/blktap-2/control", MODE="0600"
 KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
 KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
 KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
-SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
+SUBSYSTEM=="net", KERNEL=="vif*-emu", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 1ffcc90..2c030ca 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -134,11 +134,9 @@ static char ** libxl_build_device_model_args_old(libxl__gc *gc,
                 char *smac = libxl__sprintf(gc, "%02x:%02x:%02x:%02x:%02x:%02x",
                                            vifs[i].mac[0], vifs[i].mac[1], vifs[i].mac[2],
                                            vifs[i].mac[3], vifs[i].mac[4], vifs[i].mac[5]);
-                char *ifname;
-                if (!vifs[i].ifname)
-                    ifname = libxl__sprintf(gc, "tap%d.%d", info->domid, vifs[i].devid);
-                else
-                    ifname = vifs[i].ifname;
+                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
+                                                    info->domid,
+                                                    vifs[i].devid);
                 flexarray_vappend(dm_args,
                                 "-net", libxl__sprintf(gc, "nic,vlan=%d,macaddr=%s,model=%s",
                                                        vifs[i].devid, smac, vifs[i].model),
@@ -271,12 +269,9 @@ static char ** libxl_build_device_model_args_new(libxl__gc *gc,
                 char *smac = libxl__sprintf(gc, "%02x:%02x:%02x:%02x:%02x:%02x",
                                            vifs[i].mac[0], vifs[i].mac[1], vifs[i].mac[2],
                                            vifs[i].mac[3], vifs[i].mac[4], vifs[i].mac[5]);
-                char *ifname;
-                if (!vifs[i].ifname) {
-                    ifname = libxl__sprintf(gc, "tap%d.%d", info->domid, vifs[i].devid);
-                } else {
-                    ifname = vifs[i].ifname;
-                }
+                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
+                                                    info->domid,
+                                                    vifs[i].devid);
                 flexarray_append(dm_args, "-net");
                 flexarray_append(dm_args, libxl__sprintf(gc, "nic,vlan=%d,macaddr=%s,model=%s",
                             vifs[i].devid, smac, vifs[i].model));
diff --git a/tools/python/xen/xend/image.py b/tools/python/xen/xend/image.py
index f1464ac..3c8d80c 100644
--- a/tools/python/xen/xend/image.py
+++ b/tools/python/xen/xend/image.py
@@ -917,11 +917,7 @@ class HVMImageHandler(ImageHandler):
             ret.append("-net")
             ret.append("nic,vlan=%d,macaddr=%s,model=%s" %
                        (nics, mac, model))
-            vifname = devinfo.get('vifname')
-            if vifname:
-                vifname = "tap-" + vifname
-            else:
-                vifname = "tap%d.%d" % (self.vm.getDomid(), nics-1)
+            vifname = "vif%d.%d-emu" % (self.vm.getDomid(), nics-1)
             ret.append("-net")
             ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
                        (nics, vifname, bridge))

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: [patch] xen udev rule interfering with openvpn

[Teck Choon Giam]

On Sat, May 12, 2012 at 7:53 AM, Teck Choon Giam
<giamteckchoon@gmail.com> wrote:
> On Fri, May 11, 2012 at 10:53 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
>> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
>>> libxl/xend: name tap devices vifX.Y-emu
>>
>> Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
>
> This is my backport version which excludes the following:
>
> Lastly also move libxl__device_* to a better place in the header, otherwise the
> comment about evgen stuff isn't next to the associated functions (noticed jsut
> because I was going to add nic_devname near to the setdefault functions)
>
> I have tested with xm and xl with and without vifname set in domU
> config for hvmdomain and pvdomain.

Sorry, when re-test one of the test case failed... xm create hvmdomain
with vifname set.  I must have missed certain test case :(

Kindest regards,
Giam Teck Choon



>
> For your review and comments please.
>
> Thanks.
>
> Kindest regards,
> Giam Teck Choon
>
>
>
>
> libxl/xend: name tap devices vifX.Y-emu
>
> This prevents the udev scripts from operating on other tap devices (e.g.
> openvpn etc)
>
> Correct the documentation for the "vifname" option which suggested it applied
> to HVM tap devices only, which is not the case.
>
> Reported by Michael Young.
>
> Also fix the use of vifname with emulated devices. This is slightly complex.
> The current hotplug scripts rely on being able to parse the "tapX.Y" (now
> "vifX.Y-emu") name in order to locate the xenstore backend dir relating to the
> corresponding vif. This is because we cannot inject our own environment vars
> into the tap hotplug events. However this means that if the tap is initially
> named with a user specified name (which will not match the expected scheme) we
> fail to do anything useful with the device. So now we create the initial tap
> device with the standard "vifX.Y-emu" name and the hotplug script will handle
> the rename to the desired name. This is also how PV vif devices work -- they
> are always created by netback with the name vifX.Y and renamed in the script.
>
> xen-unstable changeset: 25290:7a6dcecb1781
> Signed-off-by: Giam Teck Choon <giamteckchoon@gmail.com>
> ---
>  tools/hotplug/Linux/vif-common.sh     |   15 +++++++++++++--
>  tools/hotplug/Linux/xen-backend.rules |    2 +-
>  tools/libxl/libxl_dm.c                |   17 ++++++-----------
>  tools/python/xen/xend/image.py        |    6 +-----
>  4 files changed, 21 insertions(+), 19 deletions(-)
>
> diff --git a/tools/hotplug/Linux/vif-common.sh
> b/tools/hotplug/Linux/vif-common.sh
> index c9c5d41..fff22bb 100644
> --- a/tools/hotplug/Linux/vif-common.sh
> +++ b/tools/hotplug/Linux/vif-common.sh
> @@ -85,12 +85,23 @@ elif [ "$type_if" = tap ]; then
>     : ${INTERFACE:?}
>
>     # Get xenbus_path from device name.
> -    # The name is built like that: "tap${domid}.${devid}".
> -    dev_=${dev#tap}
> +    # The name is built like that: "vif${domid}.${devid}-emu".
> +    dev_=${dev#vif}
> +    dev_=${dev_%-emu}
>     domid=${dev_%.*}
>     devid=${dev_#*.}
>
>     XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid"
> +    vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
> +    if [ "$vifname" ]
> +    then
> +        vifname="${vifname}-emu"
> +        if [ "$command" == "add" ] && ! ip link show "$vifname" >&/dev/null
> +        then
> +            do_or_die ip link set "$dev" name "$vifname"
> +        fi
> +        dev="$vifname"
> +    fi
>  fi
>
>  ip=${ip:-}
> diff --git a/tools/hotplug/Linux/xen-backend.rules
> b/tools/hotplug/Linux/xen-backend.rules
> index 40f2658..405387f 100644
> --- a/tools/hotplug/Linux/xen-backend.rules
> +++ b/tools/hotplug/Linux/xen-backend.rules
> @@ -13,4 +13,4 @@ KERNEL=="blktap-control",
> NAME="xen/blktap-2/control", MODE="0600"
>  KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
>  KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
>  KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
> -SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add",
> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
> +SUBSYSTEM=="net", KERNEL=="vif*-emu", ACTION=="add",
> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 1ffcc90..2c030ca 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -134,11 +134,9 @@ static char **
> libxl_build_device_model_args_old(libxl__gc *gc,
>                 char *smac = libxl__sprintf(gc,
> "%02x:%02x:%02x:%02x:%02x:%02x",
>                                            vifs[i].mac[0],
> vifs[i].mac[1], vifs[i].mac[2],
>                                            vifs[i].mac[3],
> vifs[i].mac[4], vifs[i].mac[5]);
> -                char *ifname;
> -                if (!vifs[i].ifname)
> -                    ifname = libxl__sprintf(gc, "tap%d.%d",
> info->domid, vifs[i].devid);
> -                else
> -                    ifname = vifs[i].ifname;
> +                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
> +                                                    info->domid,
> +                                                    vifs[i].devid);
>                 flexarray_vappend(dm_args,
>                                 "-net", libxl__sprintf(gc,
> "nic,vlan=%d,macaddr=%s,model=%s",
>                                                        vifs[i].devid,
> smac, vifs[i].model),
> @@ -271,12 +269,9 @@ static char **
> libxl_build_device_model_args_new(libxl__gc *gc,
>                 char *smac = libxl__sprintf(gc,
> "%02x:%02x:%02x:%02x:%02x:%02x",
>                                            vifs[i].mac[0],
> vifs[i].mac[1], vifs[i].mac[2],
>                                            vifs[i].mac[3],
> vifs[i].mac[4], vifs[i].mac[5]);
> -                char *ifname;
> -                if (!vifs[i].ifname) {
> -                    ifname = libxl__sprintf(gc, "tap%d.%d",
> info->domid, vifs[i].devid);
> -                } else {
> -                    ifname = vifs[i].ifname;
> -                }
> +                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
> +                                                    info->domid,
> +                                                    vifs[i].devid);
>                 flexarray_append(dm_args, "-net");
>                 flexarray_append(dm_args, libxl__sprintf(gc,
> "nic,vlan=%d,macaddr=%s,model=%s",
>                             vifs[i].devid, smac, vifs[i].model));
> diff --git a/tools/python/xen/xend/image.py b/tools/python/xen/xend/image.py
> index f1464ac..3c8d80c 100644
> --- a/tools/python/xen/xend/image.py
> +++ b/tools/python/xen/xend/image.py
> @@ -917,11 +917,7 @@ class HVMImageHandler(ImageHandler):
>             ret.append("-net")
>             ret.append("nic,vlan=%d,macaddr=%s,model=%s" %
>                        (nics, mac, model))
> -            vifname = devinfo.get('vifname')
> -            if vifname:
> -                vifname = "tap-" + vifname
> -            else:
> -                vifname = "tap%d.%d" % (self.vm.getDomid(), nics-1)
> +            vifname = "vif%d.%d-emu" % (self.vm.getDomid(), nics-1)
>             ret.append("-net")
>             ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
>                        (nics, vifname, bridge))

Re: [patch] xen udev rule interfering with openvpn

[Teck Choon Giam]

On Sat, May 12, 2012 at 3:29 PM, Teck Choon Giam
<giamteckchoon@gmail.com> wrote:
> On Sat, May 12, 2012 at 7:53 AM, Teck Choon Giam
> <giamteckchoon@gmail.com> wrote:
>> On Fri, May 11, 2012 at 10:53 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
>>> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
>>>> libxl/xend: name tap devices vifX.Y-emu
>>>
>>> Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
>>
>> This is my backport version which excludes the following:
>>
>> Lastly also move libxl__device_* to a better place in the header, otherwise the
>> comment about evgen stuff isn't next to the associated functions (noticed jsut
>> because I was going to add nic_devname near to the setdefault functions)
>>
>> I have tested with xm and xl with and without vifname set in domU
>> config for hvmdomain and pvdomain.
>
> Sorry, when re-test one of the test case failed... xm create hvmdomain
> with vifname set.  I must have missed certain test case :(

The same test case failed for xen unstable latest changeset 25326:cd4dd23a831d.

My tests as below:

1. xm create pvdomain without vifname set
2. xl create pvdomain without vifname set
3. xm create hvmdomain without vifname set
4. xl create hvmdomain without vifname set
5. xm create pvdomain with vifname set
6. xl create pvdomain with vifname set
7. xm create hvmdomain with vifname set
8. xl create hvmdomain with vifname set

Thanks.

Kindest regards,


>
> Kindest regards,
> Giam Teck Choon
>
>
>
>>
>> For your review and comments please.
>>
>> Thanks.
>>
>> Kindest regards,
>> Giam Teck Choon
>>
>>
>>
>>
>> libxl/xend: name tap devices vifX.Y-emu
>>
>> This prevents the udev scripts from operating on other tap devices (e.g.
>> openvpn etc)
>>
>> Correct the documentation for the "vifname" option which suggested it applied
>> to HVM tap devices only, which is not the case.
>>
>> Reported by Michael Young.
>>
>> Also fix the use of vifname with emulated devices. This is slightly complex.
>> The current hotplug scripts rely on being able to parse the "tapX.Y" (now
>> "vifX.Y-emu") name in order to locate the xenstore backend dir relating to the
>> corresponding vif. This is because we cannot inject our own environment vars
>> into the tap hotplug events. However this means that if the tap is initially
>> named with a user specified name (which will not match the expected scheme) we
>> fail to do anything useful with the device. So now we create the initial tap
>> device with the standard "vifX.Y-emu" name and the hotplug script will handle
>> the rename to the desired name. This is also how PV vif devices work -- they
>> are always created by netback with the name vifX.Y and renamed in the script.
>>
>> xen-unstable changeset: 25290:7a6dcecb1781
>> Signed-off-by: Giam Teck Choon <giamteckchoon@gmail.com>
>> ---
>>  tools/hotplug/Linux/vif-common.sh     |   15 +++++++++++++--
>>  tools/hotplug/Linux/xen-backend.rules |    2 +-
>>  tools/libxl/libxl_dm.c                |   17 ++++++-----------
>>  tools/python/xen/xend/image.py        |    6 +-----
>>  4 files changed, 21 insertions(+), 19 deletions(-)
>>
>> diff --git a/tools/hotplug/Linux/vif-common.sh
>> b/tools/hotplug/Linux/vif-common.sh
>> index c9c5d41..fff22bb 100644
>> --- a/tools/hotplug/Linux/vif-common.sh
>> +++ b/tools/hotplug/Linux/vif-common.sh
>> @@ -85,12 +85,23 @@ elif [ "$type_if" = tap ]; then
>>     : ${INTERFACE:?}
>>
>>     # Get xenbus_path from device name.
>> -    # The name is built like that: "tap${domid}.${devid}".
>> -    dev_=${dev#tap}
>> +    # The name is built like that: "vif${domid}.${devid}-emu".
>> +    dev_=${dev#vif}
>> +    dev_=${dev_%-emu}
>>     domid=${dev_%.*}
>>     devid=${dev_#*.}
>>
>>     XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid"
>> +    vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
>> +    if [ "$vifname" ]
>> +    then
>> +        vifname="${vifname}-emu"
>> +        if [ "$command" == "add" ] && ! ip link show "$vifname" >&/dev/null
>> +        then
>> +            do_or_die ip link set "$dev" name "$vifname"
>> +        fi
>> +        dev="$vifname"
>> +    fi
>>  fi
>>
>>  ip=${ip:-}
>> diff --git a/tools/hotplug/Linux/xen-backend.rules
>> b/tools/hotplug/Linux/xen-backend.rules
>> index 40f2658..405387f 100644
>> --- a/tools/hotplug/Linux/xen-backend.rules
>> +++ b/tools/hotplug/Linux/xen-backend.rules
>> @@ -13,4 +13,4 @@ KERNEL=="blktap-control",
>> NAME="xen/blktap-2/control", MODE="0600"
>>  KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
>>  KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
>>  KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
>> -SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add",
>> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
>> +SUBSYSTEM=="net", KERNEL=="vif*-emu", ACTION=="add",
>> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>> index 1ffcc90..2c030ca 100644
>> --- a/tools/libxl/libxl_dm.c
>> +++ b/tools/libxl/libxl_dm.c
>> @@ -134,11 +134,9 @@ static char **
>> libxl_build_device_model_args_old(libxl__gc *gc,
>>                 char *smac = libxl__sprintf(gc,
>> "%02x:%02x:%02x:%02x:%02x:%02x",
>>                                            vifs[i].mac[0],
>> vifs[i].mac[1], vifs[i].mac[2],
>>                                            vifs[i].mac[3],
>> vifs[i].mac[4], vifs[i].mac[5]);
>> -                char *ifname;
>> -                if (!vifs[i].ifname)
>> -                    ifname = libxl__sprintf(gc, "tap%d.%d",
>> info->domid, vifs[i].devid);
>> -                else
>> -                    ifname = vifs[i].ifname;
>> +                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
>> +                                                    info->domid,
>> +                                                    vifs[i].devid);
>>                 flexarray_vappend(dm_args,
>>                                 "-net", libxl__sprintf(gc,
>> "nic,vlan=%d,macaddr=%s,model=%s",
>>                                                        vifs[i].devid,
>> smac, vifs[i].model),
>> @@ -271,12 +269,9 @@ static char **
>> libxl_build_device_model_args_new(libxl__gc *gc,
>>                 char *smac = libxl__sprintf(gc,
>> "%02x:%02x:%02x:%02x:%02x:%02x",
>>                                            vifs[i].mac[0],
>> vifs[i].mac[1], vifs[i].mac[2],
>>                                            vifs[i].mac[3],
>> vifs[i].mac[4], vifs[i].mac[5]);
>> -                char *ifname;
>> -                if (!vifs[i].ifname) {
>> -                    ifname = libxl__sprintf(gc, "tap%d.%d",
>> info->domid, vifs[i].devid);
>> -                } else {
>> -                    ifname = vifs[i].ifname;
>> -                }
>> +                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
>> +                                                    info->domid,
>> +                                                    vifs[i].devid);
>>                 flexarray_append(dm_args, "-net");
>>                 flexarray_append(dm_args, libxl__sprintf(gc,
>> "nic,vlan=%d,macaddr=%s,model=%s",
>>                             vifs[i].devid, smac, vifs[i].model));
>> diff --git a/tools/python/xen/xend/image.py b/tools/python/xen/xend/image.py
>> index f1464ac..3c8d80c 100644
>> --- a/tools/python/xen/xend/image.py
>> +++ b/tools/python/xen/xend/image.py
>> @@ -917,11 +917,7 @@ class HVMImageHandler(ImageHandler):
>>             ret.append("-net")
>>             ret.append("nic,vlan=%d,macaddr=%s,model=%s" %
>>                        (nics, mac, model))
>> -            vifname = devinfo.get('vifname')
>> -            if vifname:
>> -                vifname = "tap-" + vifname
>> -            else:
>> -                vifname = "tap%d.%d" % (self.vm.getDomid(), nics-1)
>> +            vifname = "vif%d.%d-emu" % (self.vm.getDomid(), nics-1)
>>             ret.append("-net")
>>             ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
>>                        (nics, vifname, bridge))

Re: [patch] xen udev rule interfering with openvpn

[Ian Campbell]

On Sat, 2012-05-12 at 23:00 +0100, Teck Choon Giam wrote:
> On Sat, May 12, 2012 at 3:29 PM, Teck Choon Giam
> <giamteckchoon@gmail.com> wrote:
> > On Sat, May 12, 2012 at 7:53 AM, Teck Choon Giam
> > <giamteckchoon@gmail.com> wrote:
> >> On Fri, May 11, 2012 at 10:53 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
> >>> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
> >>>> libxl/xend: name tap devices vifX.Y-emu
> >>>
> >>> Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
> >>
> >> This is my backport version which excludes the following:
> >>
> >> Lastly also move libxl__device_* to a better place in the header, otherwise the
> >> comment about evgen stuff isn't next to the associated functions (noticed jsut
> >> because I was going to add nic_devname near to the setdefault functions)
> >>
> >> I have tested with xm and xl with and without vifname set in domU
> >> config for hvmdomain and pvdomain.
> >
> > Sorry, when re-test one of the test case failed... xm create hvmdomain
> > with vifname set.  I must have missed certain test case :(
> 
> The same test case failed for xen unstable latest changeset 25326:cd4dd23a831d.

Oh dear.

> My tests as below:

Which ones fail?

> 1. xm create pvdomain without vifname set
> 2. xl create pvdomain without vifname set
> 3. xm create hvmdomain without vifname set
> 4. xl create hvmdomain without vifname set
> 5. xm create pvdomain with vifname set
> 6. xl create pvdomain with vifname set
> 7. xm create hvmdomain with vifname set
> 8. xl create hvmdomain with vifname set
> 
> Thanks.
> 
> Kindest regards,
> 
> 
> >
> > Kindest regards,
> > Giam Teck Choon
> >
> >
> >
> >>
> >> For your review and comments please.
> >>
> >> Thanks.
> >>
> >> Kindest regards,
> >> Giam Teck Choon
> >>
> >>
> >>
> >>
> >> libxl/xend: name tap devices vifX.Y-emu
> >>
> >> This prevents the udev scripts from operating on other tap devices (e.g.
> >> openvpn etc)
> >>
> >> Correct the documentation for the "vifname" option which suggested it applied
> >> to HVM tap devices only, which is not the case.
> >>
> >> Reported by Michael Young.
> >>
> >> Also fix the use of vifname with emulated devices. This is slightly complex.
> >> The current hotplug scripts rely on being able to parse the "tapX.Y" (now
> >> "vifX.Y-emu") name in order to locate the xenstore backend dir relating to the
> >> corresponding vif. This is because we cannot inject our own environment vars
> >> into the tap hotplug events. However this means that if the tap is initially
> >> named with a user specified name (which will not match the expected scheme) we
> >> fail to do anything useful with the device. So now we create the initial tap
> >> device with the standard "vifX.Y-emu" name and the hotplug script will handle
> >> the rename to the desired name. This is also how PV vif devices work -- they
> >> are always created by netback with the name vifX.Y and renamed in the script.
> >>
> >> xen-unstable changeset: 25290:7a6dcecb1781
> >> Signed-off-by: Giam Teck Choon <giamteckchoon@gmail.com>
> >> ---
> >>  tools/hotplug/Linux/vif-common.sh     |   15 +++++++++++++--
> >>  tools/hotplug/Linux/xen-backend.rules |    2 +-
> >>  tools/libxl/libxl_dm.c                |   17 ++++++-----------
> >>  tools/python/xen/xend/image.py        |    6 +-----
> >>  4 files changed, 21 insertions(+), 19 deletions(-)
> >>
> >> diff --git a/tools/hotplug/Linux/vif-common.sh
> >> b/tools/hotplug/Linux/vif-common.sh
> >> index c9c5d41..fff22bb 100644
> >> --- a/tools/hotplug/Linux/vif-common.sh
> >> +++ b/tools/hotplug/Linux/vif-common.sh
> >> @@ -85,12 +85,23 @@ elif [ "$type_if" = tap ]; then
> >>     : ${INTERFACE:?}
> >>
> >>     # Get xenbus_path from device name.
> >> -    # The name is built like that: "tap${domid}.${devid}".
> >> -    dev_=${dev#tap}
> >> +    # The name is built like that: "vif${domid}.${devid}-emu".
> >> +    dev_=${dev#vif}
> >> +    dev_=${dev_%-emu}
> >>     domid=${dev_%.*}
> >>     devid=${dev_#*.}
> >>
> >>     XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid"
> >> +    vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
> >> +    if [ "$vifname" ]
> >> +    then
> >> +        vifname="${vifname}-emu"
> >> +        if [ "$command" == "add" ] && ! ip link show "$vifname" >&/dev/null
> >> +        then
> >> +            do_or_die ip link set "$dev" name "$vifname"
> >> +        fi
> >> +        dev="$vifname"
> >> +    fi
> >>  fi
> >>
> >>  ip=${ip:-}
> >> diff --git a/tools/hotplug/Linux/xen-backend.rules
> >> b/tools/hotplug/Linux/xen-backend.rules
> >> index 40f2658..405387f 100644
> >> --- a/tools/hotplug/Linux/xen-backend.rules
> >> +++ b/tools/hotplug/Linux/xen-backend.rules
> >> @@ -13,4 +13,4 @@ KERNEL=="blktap-control",
> >> NAME="xen/blktap-2/control", MODE="0600"
> >>  KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
> >>  KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
> >>  KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
> >> -SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add",
> >> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
> >> +SUBSYSTEM=="net", KERNEL=="vif*-emu", ACTION=="add",
> >> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
> >> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> >> index 1ffcc90..2c030ca 100644
> >> --- a/tools/libxl/libxl_dm.c
> >> +++ b/tools/libxl/libxl_dm.c
> >> @@ -134,11 +134,9 @@ static char **
> >> libxl_build_device_model_args_old(libxl__gc *gc,
> >>                 char *smac = libxl__sprintf(gc,
> >> "%02x:%02x:%02x:%02x:%02x:%02x",
> >>                                            vifs[i].mac[0],
> >> vifs[i].mac[1], vifs[i].mac[2],
> >>                                            vifs[i].mac[3],
> >> vifs[i].mac[4], vifs[i].mac[5]);
> >> -                char *ifname;
> >> -                if (!vifs[i].ifname)
> >> -                    ifname = libxl__sprintf(gc, "tap%d.%d",
> >> info->domid, vifs[i].devid);
> >> -                else
> >> -                    ifname = vifs[i].ifname;
> >> +                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
> >> +                                                    info->domid,
> >> +                                                    vifs[i].devid);
> >>                 flexarray_vappend(dm_args,
> >>                                 "-net", libxl__sprintf(gc,
> >> "nic,vlan=%d,macaddr=%s,model=%s",
> >>                                                        vifs[i].devid,
> >> smac, vifs[i].model),
> >> @@ -271,12 +269,9 @@ static char **
> >> libxl_build_device_model_args_new(libxl__gc *gc,
> >>                 char *smac = libxl__sprintf(gc,
> >> "%02x:%02x:%02x:%02x:%02x:%02x",
> >>                                            vifs[i].mac[0],
> >> vifs[i].mac[1], vifs[i].mac[2],
> >>                                            vifs[i].mac[3],
> >> vifs[i].mac[4], vifs[i].mac[5]);
> >> -                char *ifname;
> >> -                if (!vifs[i].ifname) {
> >> -                    ifname = libxl__sprintf(gc, "tap%d.%d",
> >> info->domid, vifs[i].devid);
> >> -                } else {
> >> -                    ifname = vifs[i].ifname;
> >> -                }
> >> +                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
> >> +                                                    info->domid,
> >> +                                                    vifs[i].devid);
> >>                 flexarray_append(dm_args, "-net");
> >>                 flexarray_append(dm_args, libxl__sprintf(gc,
> >> "nic,vlan=%d,macaddr=%s,model=%s",
> >>                             vifs[i].devid, smac, vifs[i].model));
> >> diff --git a/tools/python/xen/xend/image.py b/tools/python/xen/xend/image.py
> >> index f1464ac..3c8d80c 100644
> >> --- a/tools/python/xen/xend/image.py
> >> +++ b/tools/python/xen/xend/image.py
> >> @@ -917,11 +917,7 @@ class HVMImageHandler(ImageHandler):
> >>             ret.append("-net")
> >>             ret.append("nic,vlan=%d,macaddr=%s,model=%s" %
> >>                        (nics, mac, model))
> >> -            vifname = devinfo.get('vifname')
> >> -            if vifname:
> >> -                vifname = "tap-" + vifname
> >> -            else:
> >> -                vifname = "tap%d.%d" % (self.vm.getDomid(), nics-1)
> >> +            vifname = "vif%d.%d-emu" % (self.vm.getDomid(), nics-1)
> >>             ret.append("-net")
> >>             ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
> >>                        (nics, vifname, bridge))

Re: [patch] xen udev rule interfering with openvpn

[Teck Choon Giam]

On Sun, May 13, 2012 at 6:30 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Sat, 2012-05-12 at 23:00 +0100, Teck Choon Giam wrote:
>> On Sat, May 12, 2012 at 3:29 PM, Teck Choon Giam
>> <giamteckchoon@gmail.com> wrote:
>> > On Sat, May 12, 2012 at 7:53 AM, Teck Choon Giam
>> > <giamteckchoon@gmail.com> wrote:
>> >> On Fri, May 11, 2012 at 10:53 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
>> >>> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
>> >>>> libxl/xend: name tap devices vifX.Y-emu
>> >>>
>> >>> Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
>> >>
>> >> This is my backport version which excludes the following:
>> >>
>> >> Lastly also move libxl__device_* to a better place in the header, otherwise the
>> >> comment about evgen stuff isn't next to the associated functions (noticed jsut
>> >> because I was going to add nic_devname near to the setdefault functions)
>> >>
>> >> I have tested with xm and xl with and without vifname set in domU
>> >> config for hvmdomain and pvdomain.
>> >
>> > Sorry, when re-test one of the test case failed... xm create hvmdomain
>> > with vifname set.  I must have missed certain test case :(
>>
>> The same test case failed for xen unstable latest changeset 25326:cd4dd23a831d.
>
> Oh dear.
>
>> My tests as below:
>
> Which ones fail?
>
>> 1. xm create pvdomain without vifname set
>> 2. xl create pvdomain without vifname set
>> 3. xm create hvmdomain without vifname set
>> 4. xl create hvmdomain without vifname set
>> 5. xm create pvdomain with vifname set
>> 6. xl create pvdomain with vifname set
>> 7. xm create hvmdomain with vifname set

xm create hvmdomain with vifname set


I track down the problem already.  It happens that xm and xl behave
differently when creating $dev device?

In short, just set $dev down before:
do_or_die ip link set "$dev" name "$vifname"
Then set $vifname up after the above fix my problem.

Is the above suitable in upstream/unstable?  If yes, can you fix that
in xen-unstable or you need me to submit a patch for review for
xen-unstable?

With the below partial of my latest backport patch fix the problem but
I have to re-run all tests to double confirm all are fix and good to
go :p

diff --git a/tools/hotplug/Linux/vif-common.sh
b/tools/hotplug/Linux/vif-common.sh
index c9c5d41..86c0aaa 100644
--- a/tools/hotplug/Linux/vif-common.sh
+++ b/tools/hotplug/Linux/vif-common.sh
@@ -85,12 +85,25 @@ elif [ "$type_if" = tap ]; then
     : ${INTERFACE:?}

     # Get xenbus_path from device name.
-    # The name is built like that: "tap${domid}.${devid}".
-    dev_=${dev#tap}
+    # The name is built like that: "vif${domid}.${devid}-emu".
+    dev_=${dev#vif}
+    dev_=${dev_%-emu}
     domid=${dev_%.*}
     devid=${dev_#*.}

     XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid"
+    vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
+    if [ "$vifname" ]
+    then
+        vifname="${vifname}-emu"
+        if [ "$command" == "add" ] && ! ip link show "$vifname" >&/dev/null
+        then
+            ip link set "$dev" down
+            do_or_die ip link set "$dev" name "$vifname"
+            ip link set "$vifname" up
+        fi
+        dev="$vifname"
+    fi
 fi


Thanks.

Kindest regards,
Giam Teck Choon



>> 8. xl create hvmdomain with vifname set
>>
>> Thanks.
>>
>> Kindest regards,
>>
>>
>> >
>> > Kindest regards,
>> > Giam Teck Choon
>> >
>> >
>> >
>> >>
>> >> For your review and comments please.
>> >>
>> >> Thanks.
>> >>
>> >> Kindest regards,
>> >> Giam Teck Choon
>> >>
>> >>
>> >>
>> >>
>> >> libxl/xend: name tap devices vifX.Y-emu
>> >>
>> >> This prevents the udev scripts from operating on other tap devices (e.g.
>> >> openvpn etc)
>> >>
>> >> Correct the documentation for the "vifname" option which suggested it applied
>> >> to HVM tap devices only, which is not the case.
>> >>
>> >> Reported by Michael Young.
>> >>
>> >> Also fix the use of vifname with emulated devices. This is slightly complex.
>> >> The current hotplug scripts rely on being able to parse the "tapX.Y" (now
>> >> "vifX.Y-emu") name in order to locate the xenstore backend dir relating to the
>> >> corresponding vif. This is because we cannot inject our own environment vars
>> >> into the tap hotplug events. However this means that if the tap is initially
>> >> named with a user specified name (which will not match the expected scheme) we
>> >> fail to do anything useful with the device. So now we create the initial tap
>> >> device with the standard "vifX.Y-emu" name and the hotplug script will handle
>> >> the rename to the desired name. This is also how PV vif devices work -- they
>> >> are always created by netback with the name vifX.Y and renamed in the script.
>> >>
>> >> xen-unstable changeset: 25290:7a6dcecb1781
>> >> Signed-off-by: Giam Teck Choon <giamteckchoon@gmail.com>
>> >> ---
>> >>  tools/hotplug/Linux/vif-common.sh     |   15 +++++++++++++--
>> >>  tools/hotplug/Linux/xen-backend.rules |    2 +-
>> >>  tools/libxl/libxl_dm.c                |   17 ++++++-----------
>> >>  tools/python/xen/xend/image.py        |    6 +-----
>> >>  4 files changed, 21 insertions(+), 19 deletions(-)
>> >>
>> >> diff --git a/tools/hotplug/Linux/vif-common.sh
>> >> b/tools/hotplug/Linux/vif-common.sh
>> >> index c9c5d41..fff22bb 100644
>> >> --- a/tools/hotplug/Linux/vif-common.sh
>> >> +++ b/tools/hotplug/Linux/vif-common.sh
>> >> @@ -85,12 +85,23 @@ elif [ "$type_if" = tap ]; then
>> >>     : ${INTERFACE:?}
>> >>
>> >>     # Get xenbus_path from device name.
>> >> -    # The name is built like that: "tap${domid}.${devid}".
>> >> -    dev_=${dev#tap}
>> >> +    # The name is built like that: "vif${domid}.${devid}-emu".
>> >> +    dev_=${dev#vif}
>> >> +    dev_=${dev_%-emu}
>> >>     domid=${dev_%.*}
>> >>     devid=${dev_#*.}
>> >>
>> >>     XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid"
>> >> +    vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
>> >> +    if [ "$vifname" ]
>> >> +    then
>> >> +        vifname="${vifname}-emu"
>> >> +        if [ "$command" == "add" ] && ! ip link show "$vifname" >&/dev/null
>> >> +        then
>> >> +            do_or_die ip link set "$dev" name "$vifname"
>> >> +        fi
>> >> +        dev="$vifname"
>> >> +    fi
>> >>  fi
>> >>
>> >>  ip=${ip:-}
>> >> diff --git a/tools/hotplug/Linux/xen-backend.rules
>> >> b/tools/hotplug/Linux/xen-backend.rules
>> >> index 40f2658..405387f 100644
>> >> --- a/tools/hotplug/Linux/xen-backend.rules
>> >> +++ b/tools/hotplug/Linux/xen-backend.rules
>> >> @@ -13,4 +13,4 @@ KERNEL=="blktap-control",
>> >> NAME="xen/blktap-2/control", MODE="0600"
>> >>  KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
>> >>  KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
>> >>  KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
>> >> -SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add",
>> >> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
>> >> +SUBSYSTEM=="net", KERNEL=="vif*-emu", ACTION=="add",
>> >> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
>> >> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>> >> index 1ffcc90..2c030ca 100644
>> >> --- a/tools/libxl/libxl_dm.c
>> >> +++ b/tools/libxl/libxl_dm.c
>> >> @@ -134,11 +134,9 @@ static char **
>> >> libxl_build_device_model_args_old(libxl__gc *gc,
>> >>                 char *smac = libxl__sprintf(gc,
>> >> "%02x:%02x:%02x:%02x:%02x:%02x",
>> >>                                            vifs[i].mac[0],
>> >> vifs[i].mac[1], vifs[i].mac[2],
>> >>                                            vifs[i].mac[3],
>> >> vifs[i].mac[4], vifs[i].mac[5]);
>> >> -                char *ifname;
>> >> -                if (!vifs[i].ifname)
>> >> -                    ifname = libxl__sprintf(gc, "tap%d.%d",
>> >> info->domid, vifs[i].devid);
>> >> -                else
>> >> -                    ifname = vifs[i].ifname;
>> >> +                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
>> >> +                                                    info->domid,
>> >> +                                                    vifs[i].devid);
>> >>                 flexarray_vappend(dm_args,
>> >>                                 "-net", libxl__sprintf(gc,
>> >> "nic,vlan=%d,macaddr=%s,model=%s",
>> >>                                                        vifs[i].devid,
>> >> smac, vifs[i].model),
>> >> @@ -271,12 +269,9 @@ static char **
>> >> libxl_build_device_model_args_new(libxl__gc *gc,
>> >>                 char *smac = libxl__sprintf(gc,
>> >> "%02x:%02x:%02x:%02x:%02x:%02x",
>> >>                                            vifs[i].mac[0],
>> >> vifs[i].mac[1], vifs[i].mac[2],
>> >>                                            vifs[i].mac[3],
>> >> vifs[i].mac[4], vifs[i].mac[5]);
>> >> -                char *ifname;
>> >> -                if (!vifs[i].ifname) {
>> >> -                    ifname = libxl__sprintf(gc, "tap%d.%d",
>> >> info->domid, vifs[i].devid);
>> >> -                } else {
>> >> -                    ifname = vifs[i].ifname;
>> >> -                }
>> >> +                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
>> >> +                                                    info->domid,
>> >> +                                                    vifs[i].devid);
>> >>                 flexarray_append(dm_args, "-net");
>> >>                 flexarray_append(dm_args, libxl__sprintf(gc,
>> >> "nic,vlan=%d,macaddr=%s,model=%s",
>> >>                             vifs[i].devid, smac, vifs[i].model));
>> >> diff --git a/tools/python/xen/xend/image.py b/tools/python/xen/xend/image.py
>> >> index f1464ac..3c8d80c 100644
>> >> --- a/tools/python/xen/xend/image.py
>> >> +++ b/tools/python/xen/xend/image.py
>> >> @@ -917,11 +917,7 @@ class HVMImageHandler(ImageHandler):
>> >>             ret.append("-net")
>> >>             ret.append("nic,vlan=%d,macaddr=%s,model=%s" %
>> >>                        (nics, mac, model))
>> >> -            vifname = devinfo.get('vifname')
>> >> -            if vifname:
>> >> -                vifname = "tap-" + vifname
>> >> -            else:
>> >> -                vifname = "tap%d.%d" % (self.vm.getDomid(), nics-1)
>> >> +            vifname = "vif%d.%d-emu" % (self.vm.getDomid(), nics-1)
>> >>             ret.append("-net")
>> >>             ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
>> >>                        (nics, vifname, bridge))
>
>

Re: [patch] xen udev rule interfering with openvpn

[Teck Choon Giam]

[-- Attachment #1: Type: text/plain, Size: 11776 bytes --]

On Sun, May 13, 2012 at 7:37 AM, Teck Choon Giam
<giamteckchoon@gmail.com> wrote:
> On Sun, May 13, 2012 at 6:30 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> On Sat, 2012-05-12 at 23:00 +0100, Teck Choon Giam wrote:
>>> On Sat, May 12, 2012 at 3:29 PM, Teck Choon Giam
>>> <giamteckchoon@gmail.com> wrote:
>>> > On Sat, May 12, 2012 at 7:53 AM, Teck Choon Giam
>>> > <giamteckchoon@gmail.com> wrote:
>>> >> On Fri, May 11, 2012 at 10:53 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
>>> >>> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
>>> >>>> libxl/xend: name tap devices vifX.Y-emu
>>> >>>
>>> >>> Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
>>> >>
>>> >> This is my backport version which excludes the following:
>>> >>
>>> >> Lastly also move libxl__device_* to a better place in the header, otherwise the
>>> >> comment about evgen stuff isn't next to the associated functions (noticed jsut
>>> >> because I was going to add nic_devname near to the setdefault functions)
>>> >>
>>> >> I have tested with xm and xl with and without vifname set in domU
>>> >> config for hvmdomain and pvdomain.
>>> >
>>> > Sorry, when re-test one of the test case failed... xm create hvmdomain
>>> > with vifname set.  I must have missed certain test case :(
>>>
>>> The same test case failed for xen unstable latest changeset 25326:cd4dd23a831d.
>>
>> Oh dear.
>>
>>> My tests as below:
>>
>> Which ones fail?
>>
>>> 1. xm create pvdomain without vifname set
>>> 2. xl create pvdomain without vifname set
>>> 3. xm create hvmdomain without vifname set
>>> 4. xl create hvmdomain without vifname set
>>> 5. xm create pvdomain with vifname set
>>> 6. xl create pvdomain with vifname set
>>> 7. xm create hvmdomain with vifname set
>
> xm create hvmdomain with vifname set
>
>
> I track down the problem already.  It happens that xm and xl behave
> differently when creating $dev device?
>
> In short, just set $dev down before:
> do_or_die ip link set "$dev" name "$vifname"
> Then set $vifname up after the above fix my problem.
>
> Is the above suitable in upstream/unstable?  If yes, can you fix that
> in xen-unstable or you need me to submit a patch for review for
> xen-unstable?
>
> With the below partial of my latest backport patch fix the problem but
> I have to re-run all tests to double confirm all are fix and good to
> go :p

Attached is my final backport for xen-4.1-testing which passed all my
tests as stated below:

1. xm create pvdomain without vifname set
2. xl create pvdomain without vifname set
3. xm create hvmdomain without vifname set
4. xl create hvmdomain without vifname set
5. xm create pvdomain with vifname set
6. xl create pvdomain with vifname set
7. xm create hvmdomain with vifname set
8. xl create hvmdomain with vifname set

My initial backport patch failed for test no. 7 and when I perform
test for all the above in latest xen-unstable it failed in test no. 7
as well.

For review and comments please.

Thanks.

Kindest regards,
Giam Teck Choon


>
> diff --git a/tools/hotplug/Linux/vif-common.sh
> b/tools/hotplug/Linux/vif-common.sh
> index c9c5d41..86c0aaa 100644
> --- a/tools/hotplug/Linux/vif-common.sh
> +++ b/tools/hotplug/Linux/vif-common.sh
> @@ -85,12 +85,25 @@ elif [ "$type_if" = tap ]; then
>     : ${INTERFACE:?}
>
>     # Get xenbus_path from device name.
> -    # The name is built like that: "tap${domid}.${devid}".
> -    dev_=${dev#tap}
> +    # The name is built like that: "vif${domid}.${devid}-emu".
> +    dev_=${dev#vif}
> +    dev_=${dev_%-emu}
>     domid=${dev_%.*}
>     devid=${dev_#*.}
>
>     XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid"
> +    vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
> +    if [ "$vifname" ]
> +    then
> +        vifname="${vifname}-emu"
> +        if [ "$command" == "add" ] && ! ip link show "$vifname" >&/dev/null
> +        then
> +            ip link set "$dev" down
> +            do_or_die ip link set "$dev" name "$vifname"
> +            ip link set "$vifname" up
> +        fi
> +        dev="$vifname"
> +    fi
>  fi
>
>
> Thanks.
>
> Kindest regards,
> Giam Teck Choon
>
>
>
>>> 8. xl create hvmdomain with vifname set
>>>
>>> Thanks.
>>>
>>> Kindest regards,
>>>
>>>
>>> >
>>> > Kindest regards,
>>> > Giam Teck Choon
>>> >
>>> >
>>> >
>>> >>
>>> >> For your review and comments please.
>>> >>
>>> >> Thanks.
>>> >>
>>> >> Kindest regards,
>>> >> Giam Teck Choon
>>> >>
>>> >>
>>> >>
>>> >>
>>> >> libxl/xend: name tap devices vifX.Y-emu
>>> >>
>>> >> This prevents the udev scripts from operating on other tap devices (e.g.
>>> >> openvpn etc)
>>> >>
>>> >> Correct the documentation for the "vifname" option which suggested it applied
>>> >> to HVM tap devices only, which is not the case.
>>> >>
>>> >> Reported by Michael Young.
>>> >>
>>> >> Also fix the use of vifname with emulated devices. This is slightly complex.
>>> >> The current hotplug scripts rely on being able to parse the "tapX.Y" (now
>>> >> "vifX.Y-emu") name in order to locate the xenstore backend dir relating to the
>>> >> corresponding vif. This is because we cannot inject our own environment vars
>>> >> into the tap hotplug events. However this means that if the tap is initially
>>> >> named with a user specified name (which will not match the expected scheme) we
>>> >> fail to do anything useful with the device. So now we create the initial tap
>>> >> device with the standard "vifX.Y-emu" name and the hotplug script will handle
>>> >> the rename to the desired name. This is also how PV vif devices work -- they
>>> >> are always created by netback with the name vifX.Y and renamed in the script.
>>> >>
>>> >> xen-unstable changeset: 25290:7a6dcecb1781
>>> >> Signed-off-by: Giam Teck Choon <giamteckchoon@gmail.com>
>>> >> ---
>>> >>  tools/hotplug/Linux/vif-common.sh     |   15 +++++++++++++--
>>> >>  tools/hotplug/Linux/xen-backend.rules |    2 +-
>>> >>  tools/libxl/libxl_dm.c                |   17 ++++++-----------
>>> >>  tools/python/xen/xend/image.py        |    6 +-----
>>> >>  4 files changed, 21 insertions(+), 19 deletions(-)
>>> >>
>>> >> diff --git a/tools/hotplug/Linux/vif-common.sh
>>> >> b/tools/hotplug/Linux/vif-common.sh
>>> >> index c9c5d41..fff22bb 100644
>>> >> --- a/tools/hotplug/Linux/vif-common.sh
>>> >> +++ b/tools/hotplug/Linux/vif-common.sh
>>> >> @@ -85,12 +85,23 @@ elif [ "$type_if" = tap ]; then
>>> >>     : ${INTERFACE:?}
>>> >>
>>> >>     # Get xenbus_path from device name.
>>> >> -    # The name is built like that: "tap${domid}.${devid}".
>>> >> -    dev_=${dev#tap}
>>> >> +    # The name is built like that: "vif${domid}.${devid}-emu".
>>> >> +    dev_=${dev#vif}
>>> >> +    dev_=${dev_%-emu}
>>> >>     domid=${dev_%.*}
>>> >>     devid=${dev_#*.}
>>> >>
>>> >>     XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid"
>>> >> +    vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
>>> >> +    if [ "$vifname" ]
>>> >> +    then
>>> >> +        vifname="${vifname}-emu"
>>> >> +        if [ "$command" == "add" ] && ! ip link show "$vifname" >&/dev/null
>>> >> +        then
>>> >> +            do_or_die ip link set "$dev" name "$vifname"
>>> >> +        fi
>>> >> +        dev="$vifname"
>>> >> +    fi
>>> >>  fi
>>> >>
>>> >>  ip=${ip:-}
>>> >> diff --git a/tools/hotplug/Linux/xen-backend.rules
>>> >> b/tools/hotplug/Linux/xen-backend.rules
>>> >> index 40f2658..405387f 100644
>>> >> --- a/tools/hotplug/Linux/xen-backend.rules
>>> >> +++ b/tools/hotplug/Linux/xen-backend.rules
>>> >> @@ -13,4 +13,4 @@ KERNEL=="blktap-control",
>>> >> NAME="xen/blktap-2/control", MODE="0600"
>>> >>  KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
>>> >>  KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
>>> >>  KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
>>> >> -SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add",
>>> >> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
>>> >> +SUBSYSTEM=="net", KERNEL=="vif*-emu", ACTION=="add",
>>> >> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
>>> >> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>> >> index 1ffcc90..2c030ca 100644
>>> >> --- a/tools/libxl/libxl_dm.c
>>> >> +++ b/tools/libxl/libxl_dm.c
>>> >> @@ -134,11 +134,9 @@ static char **
>>> >> libxl_build_device_model_args_old(libxl__gc *gc,
>>> >>                 char *smac = libxl__sprintf(gc,
>>> >> "%02x:%02x:%02x:%02x:%02x:%02x",
>>> >>                                            vifs[i].mac[0],
>>> >> vifs[i].mac[1], vifs[i].mac[2],
>>> >>                                            vifs[i].mac[3],
>>> >> vifs[i].mac[4], vifs[i].mac[5]);
>>> >> -                char *ifname;
>>> >> -                if (!vifs[i].ifname)
>>> >> -                    ifname = libxl__sprintf(gc, "tap%d.%d",
>>> >> info->domid, vifs[i].devid);
>>> >> -                else
>>> >> -                    ifname = vifs[i].ifname;
>>> >> +                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
>>> >> +                                                    info->domid,
>>> >> +                                                    vifs[i].devid);
>>> >>                 flexarray_vappend(dm_args,
>>> >>                                 "-net", libxl__sprintf(gc,
>>> >> "nic,vlan=%d,macaddr=%s,model=%s",
>>> >>                                                        vifs[i].devid,
>>> >> smac, vifs[i].model),
>>> >> @@ -271,12 +269,9 @@ static char **
>>> >> libxl_build_device_model_args_new(libxl__gc *gc,
>>> >>                 char *smac = libxl__sprintf(gc,
>>> >> "%02x:%02x:%02x:%02x:%02x:%02x",
>>> >>                                            vifs[i].mac[0],
>>> >> vifs[i].mac[1], vifs[i].mac[2],
>>> >>                                            vifs[i].mac[3],
>>> >> vifs[i].mac[4], vifs[i].mac[5]);
>>> >> -                char *ifname;
>>> >> -                if (!vifs[i].ifname) {
>>> >> -                    ifname = libxl__sprintf(gc, "tap%d.%d",
>>> >> info->domid, vifs[i].devid);
>>> >> -                } else {
>>> >> -                    ifname = vifs[i].ifname;
>>> >> -                }
>>> >> +                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
>>> >> +                                                    info->domid,
>>> >> +                                                    vifs[i].devid);
>>> >>                 flexarray_append(dm_args, "-net");
>>> >>                 flexarray_append(dm_args, libxl__sprintf(gc,
>>> >> "nic,vlan=%d,macaddr=%s,model=%s",
>>> >>                             vifs[i].devid, smac, vifs[i].model));
>>> >> diff --git a/tools/python/xen/xend/image.py b/tools/python/xen/xend/image.py
>>> >> index f1464ac..3c8d80c 100644
>>> >> --- a/tools/python/xen/xend/image.py
>>> >> +++ b/tools/python/xen/xend/image.py
>>> >> @@ -917,11 +917,7 @@ class HVMImageHandler(ImageHandler):
>>> >>             ret.append("-net")
>>> >>             ret.append("nic,vlan=%d,macaddr=%s,model=%s" %
>>> >>                        (nics, mac, model))
>>> >> -            vifname = devinfo.get('vifname')
>>> >> -            if vifname:
>>> >> -                vifname = "tap-" + vifname
>>> >> -            else:
>>> >> -                vifname = "tap%d.%d" % (self.vm.getDomid(), nics-1)
>>> >> +            vifname = "vif%d.%d-emu" % (self.vm.getDomid(), nics-1)
>>> >>             ret.append("-net")
>>> >>             ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
>>> >>                        (nics, vifname, bridge))
>>
>>

[-- Attachment #2: backport-unstable-25290-to-xen-4.1-testing.patch --]
[-- Type: application/octet-stream, Size: 6130 bytes --]

libxl/xend: name tap devices vifX.Y-emu

This prevents the udev scripts from operating on other tap devices (e.g.
openvpn etc)

Correct the documentation for the "vifname" option which suggested it applied
to HVM tap devices only, which is not the case.

Reported by Michael Young.

Also fix the use of vifname with emulated devices. This is slightly complex.
The current hotplug scripts rely on being able to parse the "tapX.Y" (now
"vifX.Y-emu") name in order to locate the xenstore backend dir relating to the
corresponding vif. This is because we cannot inject our own environment vars
into the tap hotplug events. However this means that if the tap is initially
named with a user specified name (which will not match the expected scheme) we
fail to do anything useful with the device. So now we create the initial tap
device with the standard "vifX.Y-emu" name and the hotplug script will handle
the rename to the desired name. This is also how PV vif devices work -- they
are always created by netback with the name vifX.Y and renamed in the script.

xen-unstable changeset: 25290:7a6dcecb1781
Signed-off-by: Giam Teck Choon <giamteckchoon@gmail.com>
---
 tools/hotplug/Linux/vif-common.sh     |   17 +++++++++++++++--
 tools/hotplug/Linux/xen-backend.rules |    2 +-
 tools/libxl/libxl_dm.c                |   17 ++++++-----------
 tools/python/xen/xend/image.py        |    6 +-----
 4 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/tools/hotplug/Linux/vif-common.sh b/tools/hotplug/Linux/vif-common.sh
index c9c5d41..86c0aaa 100644
--- a/tools/hotplug/Linux/vif-common.sh
+++ b/tools/hotplug/Linux/vif-common.sh
@@ -85,12 +85,25 @@ elif [ "$type_if" = tap ]; then
     : ${INTERFACE:?}
 
     # Get xenbus_path from device name.
-    # The name is built like that: "tap${domid}.${devid}".
-    dev_=${dev#tap}
+    # The name is built like that: "vif${domid}.${devid}-emu".
+    dev_=${dev#vif}
+    dev_=${dev_%-emu}
     domid=${dev_%.*}
     devid=${dev_#*.}
 
     XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid"
+    vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
+    if [ "$vifname" ]
+    then
+        vifname="${vifname}-emu"
+        if [ "$command" == "add" ] && ! ip link show "$vifname" >&/dev/null
+        then
+            ip link set "$dev" down
+            do_or_die ip link set "$dev" name "$vifname"
+            ip link set "$vifname" up
+        fi
+        dev="$vifname"
+    fi
 fi
 
 ip=${ip:-}
diff --git a/tools/hotplug/Linux/xen-backend.rules b/tools/hotplug/Linux/xen-backend.rules
index 40f2658..405387f 100644
--- a/tools/hotplug/Linux/xen-backend.rules
+++ b/tools/hotplug/Linux/xen-backend.rules
@@ -13,4 +13,4 @@ KERNEL=="blktap-control", NAME="xen/blktap-2/control", MODE="0600"
 KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
 KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
 KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
-SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
+SUBSYSTEM=="net", KERNEL=="vif*-emu", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 1ffcc90..2c030ca 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -134,11 +134,9 @@ static char ** libxl_build_device_model_args_old(libxl__gc *gc,
                 char *smac = libxl__sprintf(gc, "%02x:%02x:%02x:%02x:%02x:%02x",
                                            vifs[i].mac[0], vifs[i].mac[1], vifs[i].mac[2],
                                            vifs[i].mac[3], vifs[i].mac[4], vifs[i].mac[5]);
-                char *ifname;
-                if (!vifs[i].ifname)
-                    ifname = libxl__sprintf(gc, "tap%d.%d", info->domid, vifs[i].devid);
-                else
-                    ifname = vifs[i].ifname;
+                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
+                                                    info->domid,
+                                                    vifs[i].devid);
                 flexarray_vappend(dm_args,
                                 "-net", libxl__sprintf(gc, "nic,vlan=%d,macaddr=%s,model=%s",
                                                        vifs[i].devid, smac, vifs[i].model),
@@ -271,12 +269,9 @@ static char ** libxl_build_device_model_args_new(libxl__gc *gc,
                 char *smac = libxl__sprintf(gc, "%02x:%02x:%02x:%02x:%02x:%02x",
                                            vifs[i].mac[0], vifs[i].mac[1], vifs[i].mac[2],
                                            vifs[i].mac[3], vifs[i].mac[4], vifs[i].mac[5]);
-                char *ifname;
-                if (!vifs[i].ifname) {
-                    ifname = libxl__sprintf(gc, "tap%d.%d", info->domid, vifs[i].devid);
-                } else {
-                    ifname = vifs[i].ifname;
-                }
+                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
+                                                    info->domid,
+                                                    vifs[i].devid);
                 flexarray_append(dm_args, "-net");
                 flexarray_append(dm_args, libxl__sprintf(gc, "nic,vlan=%d,macaddr=%s,model=%s",
                             vifs[i].devid, smac, vifs[i].model));
diff --git a/tools/python/xen/xend/image.py b/tools/python/xen/xend/image.py
index f1464ac..3c8d80c 100644
--- a/tools/python/xen/xend/image.py
+++ b/tools/python/xen/xend/image.py
@@ -917,11 +917,7 @@ class HVMImageHandler(ImageHandler):
             ret.append("-net")
             ret.append("nic,vlan=%d,macaddr=%s,model=%s" %
                        (nics, mac, model))
-            vifname = devinfo.get('vifname')
-            if vifname:
-                vifname = "tap-" + vifname
-            else:
-                vifname = "tap%d.%d" % (self.vm.getDomid(), nics-1)
+            vifname = "vif%d.%d-emu" % (self.vm.getDomid(), nics-1)
             ret.append("-net")
             ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
                        (nics, vifname, bridge))

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: [patch] xen udev rule interfering with openvpn

[Teck Choon Giam]

On Sun, May 13, 2012 at 7:37 AM, Teck Choon Giam
<giamteckchoon@gmail.com> wrote:
> On Sun, May 13, 2012 at 6:30 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> On Sat, 2012-05-12 at 23:00 +0100, Teck Choon Giam wrote:
>>> On Sat, May 12, 2012 at 3:29 PM, Teck Choon Giam
>>> <giamteckchoon@gmail.com> wrote:
>>> > On Sat, May 12, 2012 at 7:53 AM, Teck Choon Giam
>>> > <giamteckchoon@gmail.com> wrote:
>>> >> On Fri, May 11, 2012 at 10:53 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
>>> >>> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
>>> >>>> libxl/xend: name tap devices vifX.Y-emu
>>> >>>
>>> >>> Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
>>> >>
>>> >> This is my backport version which excludes the following:
>>> >>
>>> >> Lastly also move libxl__device_* to a better place in the header, otherwise the
>>> >> comment about evgen stuff isn't next to the associated functions (noticed jsut
>>> >> because I was going to add nic_devname near to the setdefault functions)
>>> >>
>>> >> I have tested with xm and xl with and without vifname set in domU
>>> >> config for hvmdomain and pvdomain.
>>> >
>>> > Sorry, when re-test one of the test case failed... xm create hvmdomain
>>> > with vifname set.  I must have missed certain test case :(
>>>
>>> The same test case failed for xen unstable latest changeset 25326:cd4dd23a831d.
>>
>> Oh dear.
>>
>>> My tests as below:
>>
>> Which ones fail?
>>
>>> 1. xm create pvdomain without vifname set
>>> 2. xl create pvdomain without vifname set
>>> 3. xm create hvmdomain without vifname set
>>> 4. xl create hvmdomain without vifname set
>>> 5. xm create pvdomain with vifname set
>>> 6. xl create pvdomain with vifname set
>>> 7. xm create hvmdomain with vifname set
>
> xm create hvmdomain with vifname set
>
>
> I track down the problem already.  It happens that xm and xl behave
> differently when creating $dev device?
>
> In short, just set $dev down before:
> do_or_die ip link set "$dev" name "$vifname"
> Then set $vifname up after the above fix my problem.
>
> Is the above suitable in upstream/unstable?  If yes, can you fix that
> in xen-unstable or you need me to submit a patch for review for
> xen-unstable?

Sorry, I know developers are busy and don't mean to be demanding.
This is just a note in case you have overlook this as I am waiting for
your valuable input.

Thanks.

Kindest regards,
Giam Teck Choon


>
> With the below partial of my latest backport patch fix the problem but
> I have to re-run all tests to double confirm all are fix and good to
> go :p
>
> diff --git a/tools/hotplug/Linux/vif-common.sh
> b/tools/hotplug/Linux/vif-common.sh
> index c9c5d41..86c0aaa 100644
> --- a/tools/hotplug/Linux/vif-common.sh
> +++ b/tools/hotplug/Linux/vif-common.sh
> @@ -85,12 +85,25 @@ elif [ "$type_if" = tap ]; then
>     : ${INTERFACE:?}
>
>     # Get xenbus_path from device name.
> -    # The name is built like that: "tap${domid}.${devid}".
> -    dev_=${dev#tap}
> +    # The name is built like that: "vif${domid}.${devid}-emu".
> +    dev_=${dev#vif}
> +    dev_=${dev_%-emu}
>     domid=${dev_%.*}
>     devid=${dev_#*.}
>
>     XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid"
> +    vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
> +    if [ "$vifname" ]
> +    then
> +        vifname="${vifname}-emu"
> +        if [ "$command" == "add" ] && ! ip link show "$vifname" >&/dev/null
> +        then
> +            ip link set "$dev" down
> +            do_or_die ip link set "$dev" name "$vifname"
> +            ip link set "$vifname" up
> +        fi
> +        dev="$vifname"
> +    fi
>  fi
>
>
> Thanks.
>
> Kindest regards,
> Giam Teck Choon
>
>
>
>>> 8. xl create hvmdomain with vifname set
>>>
>>> Thanks.
>>>
>>> Kindest regards,
>>>
>>>
>>> >
>>> > Kindest regards,
>>> > Giam Teck Choon
>>> >
>>> >
>>> >
>>> >>
>>> >> For your review and comments please.
>>> >>
>>> >> Thanks.
>>> >>
>>> >> Kindest regards,
>>> >> Giam Teck Choon
>>> >>
>>> >>
>>> >>
>>> >>
>>> >> libxl/xend: name tap devices vifX.Y-emu
>>> >>
>>> >> This prevents the udev scripts from operating on other tap devices (e.g.
>>> >> openvpn etc)
>>> >>
>>> >> Correct the documentation for the "vifname" option which suggested it applied
>>> >> to HVM tap devices only, which is not the case.
>>> >>
>>> >> Reported by Michael Young.
>>> >>
>>> >> Also fix the use of vifname with emulated devices. This is slightly complex.
>>> >> The current hotplug scripts rely on being able to parse the "tapX.Y" (now
>>> >> "vifX.Y-emu") name in order to locate the xenstore backend dir relating to the
>>> >> corresponding vif. This is because we cannot inject our own environment vars
>>> >> into the tap hotplug events. However this means that if the tap is initially
>>> >> named with a user specified name (which will not match the expected scheme) we
>>> >> fail to do anything useful with the device. So now we create the initial tap
>>> >> device with the standard "vifX.Y-emu" name and the hotplug script will handle
>>> >> the rename to the desired name. This is also how PV vif devices work -- they
>>> >> are always created by netback with the name vifX.Y and renamed in the script.
>>> >>
>>> >> xen-unstable changeset: 25290:7a6dcecb1781
>>> >> Signed-off-by: Giam Teck Choon <giamteckchoon@gmail.com>
>>> >> ---
>>> >>  tools/hotplug/Linux/vif-common.sh     |   15 +++++++++++++--
>>> >>  tools/hotplug/Linux/xen-backend.rules |    2 +-
>>> >>  tools/libxl/libxl_dm.c                |   17 ++++++-----------
>>> >>  tools/python/xen/xend/image.py        |    6 +-----
>>> >>  4 files changed, 21 insertions(+), 19 deletions(-)
>>> >>
>>> >> diff --git a/tools/hotplug/Linux/vif-common.sh
>>> >> b/tools/hotplug/Linux/vif-common.sh
>>> >> index c9c5d41..fff22bb 100644
>>> >> --- a/tools/hotplug/Linux/vif-common.sh
>>> >> +++ b/tools/hotplug/Linux/vif-common.sh
>>> >> @@ -85,12 +85,23 @@ elif [ "$type_if" = tap ]; then
>>> >>     : ${INTERFACE:?}
>>> >>
>>> >>     # Get xenbus_path from device name.
>>> >> -    # The name is built like that: "tap${domid}.${devid}".
>>> >> -    dev_=${dev#tap}
>>> >> +    # The name is built like that: "vif${domid}.${devid}-emu".
>>> >> +    dev_=${dev#vif}
>>> >> +    dev_=${dev_%-emu}
>>> >>     domid=${dev_%.*}
>>> >>     devid=${dev_#*.}
>>> >>
>>> >>     XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid"
>>> >> +    vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
>>> >> +    if [ "$vifname" ]
>>> >> +    then
>>> >> +        vifname="${vifname}-emu"
>>> >> +        if [ "$command" == "add" ] && ! ip link show "$vifname" >&/dev/null
>>> >> +        then
>>> >> +            do_or_die ip link set "$dev" name "$vifname"
>>> >> +        fi
>>> >> +        dev="$vifname"
>>> >> +    fi
>>> >>  fi
>>> >>
>>> >>  ip=${ip:-}
>>> >> diff --git a/tools/hotplug/Linux/xen-backend.rules
>>> >> b/tools/hotplug/Linux/xen-backend.rules
>>> >> index 40f2658..405387f 100644
>>> >> --- a/tools/hotplug/Linux/xen-backend.rules
>>> >> +++ b/tools/hotplug/Linux/xen-backend.rules
>>> >> @@ -13,4 +13,4 @@ KERNEL=="blktap-control",
>>> >> NAME="xen/blktap-2/control", MODE="0600"
>>> >>  KERNEL=="gntdev", NAME="xen/%k", MODE="0600"
>>> >>  KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600"
>>> >>  KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600"
>>> >> -SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add",
>>> >> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
>>> >> +SUBSYSTEM=="net", KERNEL=="vif*-emu", ACTION=="add",
>>> >> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
>>> >> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>> >> index 1ffcc90..2c030ca 100644
>>> >> --- a/tools/libxl/libxl_dm.c
>>> >> +++ b/tools/libxl/libxl_dm.c
>>> >> @@ -134,11 +134,9 @@ static char **
>>> >> libxl_build_device_model_args_old(libxl__gc *gc,
>>> >>                 char *smac = libxl__sprintf(gc,
>>> >> "%02x:%02x:%02x:%02x:%02x:%02x",
>>> >>                                            vifs[i].mac[0],
>>> >> vifs[i].mac[1], vifs[i].mac[2],
>>> >>                                            vifs[i].mac[3],
>>> >> vifs[i].mac[4], vifs[i].mac[5]);
>>> >> -                char *ifname;
>>> >> -                if (!vifs[i].ifname)
>>> >> -                    ifname = libxl__sprintf(gc, "tap%d.%d",
>>> >> info->domid, vifs[i].devid);
>>> >> -                else
>>> >> -                    ifname = vifs[i].ifname;
>>> >> +                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
>>> >> +                                                    info->domid,
>>> >> +                                                    vifs[i].devid);
>>> >>                 flexarray_vappend(dm_args,
>>> >>                                 "-net", libxl__sprintf(gc,
>>> >> "nic,vlan=%d,macaddr=%s,model=%s",
>>> >>                                                        vifs[i].devid,
>>> >> smac, vifs[i].model),
>>> >> @@ -271,12 +269,9 @@ static char **
>>> >> libxl_build_device_model_args_new(libxl__gc *gc,
>>> >>                 char *smac = libxl__sprintf(gc,
>>> >> "%02x:%02x:%02x:%02x:%02x:%02x",
>>> >>                                            vifs[i].mac[0],
>>> >> vifs[i].mac[1], vifs[i].mac[2],
>>> >>                                            vifs[i].mac[3],
>>> >> vifs[i].mac[4], vifs[i].mac[5]);
>>> >> -                char *ifname;
>>> >> -                if (!vifs[i].ifname) {
>>> >> -                    ifname = libxl__sprintf(gc, "tap%d.%d",
>>> >> info->domid, vifs[i].devid);
>>> >> -                } else {
>>> >> -                    ifname = vifs[i].ifname;
>>> >> -                }
>>> >> +                const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu",
>>> >> +                                                    info->domid,
>>> >> +                                                    vifs[i].devid);
>>> >>                 flexarray_append(dm_args, "-net");
>>> >>                 flexarray_append(dm_args, libxl__sprintf(gc,
>>> >> "nic,vlan=%d,macaddr=%s,model=%s",
>>> >>                             vifs[i].devid, smac, vifs[i].model));
>>> >> diff --git a/tools/python/xen/xend/image.py b/tools/python/xen/xend/image.py
>>> >> index f1464ac..3c8d80c 100644
>>> >> --- a/tools/python/xen/xend/image.py
>>> >> +++ b/tools/python/xen/xend/image.py
>>> >> @@ -917,11 +917,7 @@ class HVMImageHandler(ImageHandler):
>>> >>             ret.append("-net")
>>> >>             ret.append("nic,vlan=%d,macaddr=%s,model=%s" %
>>> >>                        (nics, mac, model))
>>> >> -            vifname = devinfo.get('vifname')
>>> >> -            if vifname:
>>> >> -                vifname = "tap-" + vifname
>>> >> -            else:
>>> >> -                vifname = "tap%d.%d" % (self.vm.getDomid(), nics-1)
>>> >> +            vifname = "vif%d.%d-emu" % (self.vm.getDomid(), nics-1)
>>> >>             ret.append("-net")
>>> >>             ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
>>> >>                        (nics, vifname, bridge))
>>
>>

Re: [patch] xen udev rule interfering with openvpn

[Ian Campbell]

On Sun, 2012-05-13 at 01:39 +0100, Teck Choon Giam wrote:
> On Sun, May 13, 2012 at 7:37 AM, Teck Choon Giam
> <giamteckchoon@gmail.com> wrote:
> > On Sun, May 13, 2012 at 6:30 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> >> On Sat, 2012-05-12 at 23:00 +0100, Teck Choon Giam wrote:
> >>> On Sat, May 12, 2012 at 3:29 PM, Teck Choon Giam
> >>> <giamteckchoon@gmail.com> wrote:
> >>> > On Sat, May 12, 2012 at 7:53 AM, Teck Choon Giam
> >>> > <giamteckchoon@gmail.com> wrote:
> >>> >> On Fri, May 11, 2012 at 10:53 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
> >>> >>> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
> >>> >>>> libxl/xend: name tap devices vifX.Y-emu
> >>> >>>
> >>> >>> Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
> >>> >>
> >>> >> This is my backport version which excludes the following:
> >>> >>
> >>> >> Lastly also move libxl__device_* to a better place in the header, otherwise the
> >>> >> comment about evgen stuff isn't next to the associated functions (noticed jsut
> >>> >> because I was going to add nic_devname near to the setdefault functions)
> >>> >>
> >>> >> I have tested with xm and xl with and without vifname set in domU
> >>> >> config for hvmdomain and pvdomain.
> >>> >
> >>> > Sorry, when re-test one of the test case failed... xm create hvmdomain
> >>> > with vifname set.  I must have missed certain test case :(
> >>>
> >>> The same test case failed for xen unstable latest changeset 25326:cd4dd23a831d.
> >>
> >> Oh dear.
> >>
> >>> My tests as below:
> >>
> >> Which ones fail?
> >>
> >>> 1. xm create pvdomain without vifname set
> >>> 2. xl create pvdomain without vifname set
> >>> 3. xm create hvmdomain without vifname set
> >>> 4. xl create hvmdomain without vifname set
> >>> 5. xm create pvdomain with vifname set
> >>> 6. xl create pvdomain with vifname set
> >>> 7. xm create hvmdomain with vifname set
> >
> > xm create hvmdomain with vifname set
> >
> >
> > I track down the problem already.  It happens that xm and xl behave
> > differently when creating $dev device?
> >
> > In short, just set $dev down before:
> > do_or_die ip link set "$dev" name "$vifname"
> > Then set $vifname up after the above fix my problem.
> >
> > Is the above suitable in upstream/unstable?  If yes, can you fix that
> > in xen-unstable or you need me to submit a patch for review for
> > xen-unstable?
> >
> > With the below partial of my latest backport patch fix the problem but
> > I have to re-run all tests to double confirm all are fix and good to
> > go :p
> 
> Attached is my final backport for xen-4.1-testing which passed all my
> tests as stated below:
> 
> 1. xm create pvdomain without vifname set
> 2. xl create pvdomain without vifname set
> 3. xm create hvmdomain without vifname set
> 4. xl create hvmdomain without vifname set
> 5. xm create pvdomain with vifname set
> 6. xl create pvdomain with vifname set
> 7. xm create hvmdomain with vifname set
> 8. xl create hvmdomain with vifname set
> 
> My initial backport patch failed for test no. 7 and when I perform
> test for all the above in latest xen-unstable it failed in test no. 7
> as well.

OK, can we concentrate on the xen-unstable failure first, hopefully
addressing that will naturally fix 4.1 too but I don't wan't to get
confused between the two.

How does case #7 fail? Do you get both devices created but not placed on
the bridge or something else?

What names do the devices end up with? ("ifconfig -a", while guest is
running, "brctl show" also useful)

What is the qemu command line? (from ps, while guest is running)

What is the name in xenstore? ("xenstore-ls -fp", again while guest is
running)

Thanks, sorry for the delay in responding to this one.

Ian.

Re: [patch] xen udev rule interfering with openvpn

[Ian Campbell]

On Mon, 2012-05-21 at 13:24 +0100, Teck Choon Giam wrote:
> Sorry, I know developers are busy and don't mean to be demanding.
> This is just a note in case you have overlook this as I am waiting for
> your valuable input.

You've done the right thing by reminding us, thanks!

Ian

Re: [patch] xen udev rule interfering with openvpn

[Teck Choon Giam]

On Mon, May 21, 2012 at 8:31 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Sun, 2012-05-13 at 01:39 +0100, Teck Choon Giam wrote:
>> On Sun, May 13, 2012 at 7:37 AM, Teck Choon Giam
>> <giamteckchoon@gmail.com> wrote:
>> > On Sun, May 13, 2012 at 6:30 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> >> On Sat, 2012-05-12 at 23:00 +0100, Teck Choon Giam wrote:
>> >>> On Sat, May 12, 2012 at 3:29 PM, Teck Choon Giam
>> >>> <giamteckchoon@gmail.com> wrote:
>> >>> > On Sat, May 12, 2012 at 7:53 AM, Teck Choon Giam
>> >>> > <giamteckchoon@gmail.com> wrote:
>> >>> >> On Fri, May 11, 2012 at 10:53 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
>> >>> >>> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"):
>> >>> >>>> libxl/xend: name tap devices vifX.Y-emu
>> >>> >>>
>> >>> >>> Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
>> >>> >>
>> >>> >> This is my backport version which excludes the following:
>> >>> >>
>> >>> >> Lastly also move libxl__device_* to a better place in the header, otherwise the
>> >>> >> comment about evgen stuff isn't next to the associated functions (noticed jsut
>> >>> >> because I was going to add nic_devname near to the setdefault functions)
>> >>> >>
>> >>> >> I have tested with xm and xl with and without vifname set in domU
>> >>> >> config for hvmdomain and pvdomain.
>> >>> >
>> >>> > Sorry, when re-test one of the test case failed... xm create hvmdomain
>> >>> > with vifname set.  I must have missed certain test case :(
>> >>>
>> >>> The same test case failed for xen unstable latest changeset 25326:cd4dd23a831d.
>> >>
>> >> Oh dear.
>> >>
>> >>> My tests as below:
>> >>
>> >> Which ones fail?
>> >>
>> >>> 1. xm create pvdomain without vifname set
>> >>> 2. xl create pvdomain without vifname set
>> >>> 3. xm create hvmdomain without vifname set
>> >>> 4. xl create hvmdomain without vifname set
>> >>> 5. xm create pvdomain with vifname set
>> >>> 6. xl create pvdomain with vifname set
>> >>> 7. xm create hvmdomain with vifname set
>> >
>> > xm create hvmdomain with vifname set
>> >
>> >
>> > I track down the problem already.  It happens that xm and xl behave
>> > differently when creating $dev device?
>> >
>> > In short, just set $dev down before:
>> > do_or_die ip link set "$dev" name "$vifname"
>> > Then set $vifname up after the above fix my problem.
>> >
>> > Is the above suitable in upstream/unstable?  If yes, can you fix that
>> > in xen-unstable or you need me to submit a patch for review for
>> > xen-unstable?
>> >
>> > With the below partial of my latest backport patch fix the problem but
>> > I have to re-run all tests to double confirm all are fix and good to
>> > go :p
>>
>> Attached is my final backport for xen-4.1-testing which passed all my
>> tests as stated below:
>>
>> 1. xm create pvdomain without vifname set
>> 2. xl create pvdomain without vifname set
>> 3. xm create hvmdomain without vifname set
>> 4. xl create hvmdomain without vifname set
>> 5. xm create pvdomain with vifname set
>> 6. xl create pvdomain with vifname set
>> 7. xm create hvmdomain with vifname set
>> 8. xl create hvmdomain with vifname set
>>
>> My initial backport patch failed for test no. 7 and when I perform
>> test for all the above in latest xen-unstable it failed in test no. 7
>> as well.
>
> OK, can we concentrate on the xen-unstable failure first, hopefully
> addressing that will naturally fix 4.1 too but I don't wan't to get
> confused between the two.

Sure.  My previous mail is actually asking what to do with #7 in
xen-unstable not in xen-4.1-testing.

>
> How does case #7 fail? Do you get both devices created but not placed on
> the bridge or something else?

I am not sure just in #7 fail to create.  The error as below:

# xm create hvmdomaintest-vifname.cfg
Using config file "./hvmdomaintest-vifname.cfg".
Error: Device 0 (vif) could not be connected. ip link set vif1.0-emu
name b1-emu failed


>
> What names do the devices end up with? ("ifconfig -a", while guest is
> running, "brctl show" also useful)

Can't even create the hvmdomain with vifname set.  If you mean trying
to capture the ifconfig -a output while xm create hvmdomain with
vifname set... the interval for xm create hvmdomain with vifname set
is too short for me to issue ifconfig -a and brctl show output in
another terminal :(

> What is the qemu command line? (from ps, while guest is running)

Not relevant since can't create the hvmdomain with vifname set.

>
> What is the name in xenstore? ("xenstore-ls -fp", again while guest is
> running)

Not relevant since can't create the hvmdomain with vifname set.

>
> Thanks, sorry for the delay in responding to this one.

It is fine and I totally understand developers are busy and mostly
miss certain posts or threads or emails.

Just to confirm... are we going to "throw away" xm in 4.2 and use only xl?

Thanks.

Kindest regards,
Giam Teck Choon


>
> Ian.
>

Re: [patch] xen udev rule interfering with openvpn

[Ian Campbell]

On Mon, 2012-05-21 at 13:51 +0100, Teck Choon Giam wrote:

> > How does case #7 fail? Do you get both devices created but not placed on
> > the bridge or something else?
> 
> I am not sure just in #7 fail to create.  The error as below:
> 
> # xm create hvmdomaintest-vifname.cfg
> Using config file "./hvmdomaintest-vifname.cfg".
> Error: Device 0 (vif) could not be connected. ip link set vif1.0-emu
> name b1-emu failed

So am I correct that you use vifname="b1"? I wonder if there was any
output from this command. Did anything appear in
var/log/xen/xen-hotplug.log ?

> >
> > What names do the devices end up with? ("ifconfig -a", while guest is
> > running, "brctl show" also useful)
> 
> Can't even create the hvmdomain with vifname set.
>   If you mean trying
> to capture the ifconfig -a output while xm create hvmdomain with
> vifname set... the interval for xm create hvmdomain with vifname set
> is too short for me to issue ifconfig -a and brctl show output in
> another terminal :(

Yes, that's something of a problem.

One approach you could try is to add the commands to the vif-bridge
script and re-direct to a file. One useful place to do that might be in
vif-common.sh just before the
	do_or_die ip link set "$dev" name "$vifname"
calls. e.g. add
	( ifconfig -a ; brctl show ) >> /tmp/hotplug.dbg.log
or something. You might also want to add ">> /tmp/hotplug.dbg.log" to
the door_die so that it's output can also be logged.

> Just to confirm... are we going to "throw away" xm in 4.2 and use only xl?

Not yet, they will both existing in 4.2 but xl will now be considered
the default. We hope to be able to get rid of xm in the 4.3 time frame,
but that depends on a variety of factors.

> 
> Thanks.
> 
> Kindest regards,
> Giam Teck Choon
> 
> 
> >
> > Ian.
> >

Re: [patch] xen udev rule interfering with openvpn

[Teck Choon Giam]

On Mon, May 21, 2012 at 9:04 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Mon, 2012-05-21 at 13:51 +0100, Teck Choon Giam wrote:
>
>> > How does case #7 fail? Do you get both devices created but not placed on
>> > the bridge or something else?
>>
>> I am not sure just in #7 fail to create.  The error as below:
>>
>> # xm create hvmdomaintest-vifname.cfg
>> Using config file "./hvmdomaintest-vifname.cfg".
>> Error: Device 0 (vif) could not be connected. ip link set vif1.0-emu
>> name b1-emu failed
>
> So am I correct that you use vifname="b1"? I wonder if there was any
> output from this command. Did anything appear in
> var/log/xen/xen-hotplug.log ?

Related hvmdomain-vifname.cfg as below:

# cat hvmdomaintest-vifname.cfg
#kernel = "/usr/lib/xen/boot/hvmloader"
builder = 'hvm'
# Shadow pagetable memory for the domain, in MB.
# If not explicictly set, xend will pick an appropriate value.
# Should be at least 2KB per MB of domain memory, plus a few MB per vcpu.
shadow_memory = 32
memory = 3072
maxmem = 3072
name = "hvmdomaintest"
vif = [ 'vifname=b1,type=ioemu,mac=EDITOUT,bridge=xenbr0,ip=EDITOUT',
'vifname=b2,type=ioemu,mac=EDITOUT,bridge=xenbr1,ip=EDITOUT' ]
disk = [ 'phy:/dev/XenGroup/hvmdomaintest,ioemu:hda,w',
'file:/home/xen/images/X17-59463_Windows_7_Ultimate_Service_Pack_1_x86_ISO_32-bit.iso,hdc:cdrom,r'
]
vmid=1
vcpus=4
ne2000=0
boot='cd'
sdl=0
vncviewer=0
vncpasswd='EDITOUT'
vnclisten="XXX.XXX.XXX.XXX"
vnc=1
vncdisplay=1
vncunused=0
usbdevice='tablet'
#acpi=0
viridian=1
#-----------------------------------------------------------------------------
#   enable sound card support, [sb16|es1370|all|..,..], default none
#soundhw='sb16'
#-----------------------------------------------------------------------------
#    set the real time clock to local time [default=0 i.e. set to utc]
#localtime=0
localtime=1
#-----------------------------------------------------------------------------
#    set the real time clock offset in seconds [default=0 i.e. same as dom0]
#rtc_timeoffset=28800
#rtc_timeoffset=0
#on_poweroff = 'destroy'
#on_reboot   = 'restart'
#on_crash    = 'restart'


Related xen-hotplug.log as below:

RTNETLINK answers: Operation not supported
RTNETLINK answers: Device or resource busy
RTNETLINK answers: Operation not supported
RTNETLINK answers: Device or resource busy


>
>> >
>> > What names do the devices end up with? ("ifconfig -a", while guest is
>> > running, "brctl show" also useful)
>>
>> Can't even create the hvmdomain with vifname set.
>>   If you mean trying
>> to capture the ifconfig -a output while xm create hvmdomain with
>> vifname set... the interval for xm create hvmdomain with vifname set
>> is too short for me to issue ifconfig -a and brctl show output in
>> another terminal :(
>
> Yes, that's something of a problem.
>
> One approach you could try is to add the commands to the vif-bridge
> script and re-direct to a file. One useful place to do that might be in
> vif-common.sh just before the
>        do_or_die ip link set "$dev" name "$vifname"
> calls. e.g. add
>        ( ifconfig -a ; brctl show ) >> /tmp/hotplug.dbg.log
> or something. You might also want to add ">> /tmp/hotplug.dbg.log" to
> the door_die so that it's output can also be logged.

I actually done that as below:

(ifconfig -a && brctl show) >> /root/test-output.log
printenv >> /root/test-output.log
above the line: do_or_die ip link set "$dev" name "$vifname"

And the appending output as below about /root/test-output.log:

# cat ../test-output.log
b1        Link encap:Ethernet  HWaddr FE:FF:FF:FF:FF:FF
          UP BROADCAST RUNNING PROMISC MULTICAST  MTU:1500  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:32
          RX bytes:0 (0.0 b)  TX bytes:0 (0.0 b)

b2        Link encap:Ethernet  HWaddr FE:FF:FF:FF:FF:FF
          BROADCAST MULTICAST  MTU:1500  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:32
          RX bytes:0 (0.0 b)  TX bytes:0 (0.0 b)

eth0      Link encap:Ethernet  HWaddr 00:25:90:3D:0D:12
          inet6 addr: fe80::225:90ff:fe3d:d12/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:14589 errors:0 dropped:61 overruns:0 frame:0
          TX packets:1167 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000
          RX bytes:957339 (934.9 KiB)  TX bytes:228032 (222.6 KiB)
          Memory:fba80000-fbb00000

eth1      Link encap:Ethernet  HWaddr 00:25:90:3D:0D:13
          inet6 addr: fe80::225:90ff:fe3d:d13/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:3454 errors:0 dropped:60 overruns:0 frame:0
          TX packets:47 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000
          RX bytes:298974 (291.9 KiB)  TX bytes:3130 (3.0 KiB)
          Memory:fb980000-fba00000

lo        Link encap:Local Loopback
          inet addr:127.0.0.1  Mask:255.0.0.0
          inet6 addr: ::1/128 Scope:Host
          UP LOOPBACK RUNNING  MTU:16436  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0
          RX bytes:0 (0.0 b)  TX bytes:0 (0.0 b)

vif5.0-emu Link encap:Ethernet  HWaddr 1A:58:5C:16:5C:02
          inet6 addr: fe80::1858:5cff:fe16:5c02/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:4 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:500
          RX bytes:0 (0.0 b)  TX bytes:280 (280.0 b)

vif5.1-emu Link encap:Ethernet  HWaddr 6A:A7:BB:1E:A6:95
          inet6 addr: fe80::68a7:bbff:fe1e:a695/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:3 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:500
          RX bytes:0 (0.0 b)  TX bytes:222 (222.0 b)

xenbr0    Link encap:Ethernet  HWaddr 00:25:90:3D:0D:12
          inet addr:203.175.161.8  Bcast:203.175.161.255  Mask:255.255.255.0
          inet6 addr: fe80::225:90ff:fe3d:d12/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:13978 errors:0 dropped:0 overruns:0 frame:0
          TX packets:1145 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0
          RX bytes:702446 (685.9 KiB)  TX bytes:226828 (221.5 KiB)

xenbr1    Link encap:Ethernet  HWaddr 00:25:90:3D:0D:13
          inet addr:192.168.100.18  Bcast:192.168.100.255  Mask:255.255.255.0
          inet6 addr: fe80::225:90ff:fe3d:d13/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:2506 errors:0 dropped:0 overruns:0 frame:0
          TX packets:25 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0
          RX bytes:121776 (118.9 KiB)  TX bytes:1926 (1.8 KiB)

bridge name	bridge id		STP enabled	interfaces
xenbr0		8000.0025903d0d12	no		b1
							eth0
							vif5.0-emu
xenbr1		8000.0025903d0d13	no		b2
							eth1
							vif5.1-emu
b1        Link encap:Ethernet  HWaddr FE:FF:FF:FF:FF:FF
          UP BROADCAST RUNNING PROMISC MULTICAST  MTU:1500  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:32
          RX bytes:0 (0.0 b)  TX bytes:0 (0.0 b)

b2        Link encap:Ethernet  HWaddr FE:FF:FF:FF:FF:FF
          BROADCAST MULTICAST  MTU:1500  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:32
          RX bytes:0 (0.0 b)  TX bytes:0 (0.0 b)

eth0      Link encap:Ethernet  HWaddr 00:25:90:3D:0D:12
          inet6 addr: fe80::225:90ff:fe3d:d12/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:14589 errors:0 dropped:61 overruns:0 frame:0
          TX packets:1167 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000
          RX bytes:957339 (934.9 KiB)  TX bytes:228032 (222.6 KiB)
          Memory:fba80000-fbb00000

eth1      Link encap:Ethernet  HWaddr 00:25:90:3D:0D:13
          inet6 addr: fe80::225:90ff:fe3d:d13/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:3454 errors:0 dropped:60 overruns:0 frame:0
          TX packets:47 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000
          RX bytes:298974 (291.9 KiB)  TX bytes:3130 (3.0 KiB)
          Memory:fb980000-fba00000

lo        Link encap:Local Loopback
          inet addr:127.0.0.1  Mask:255.0.0.0
          inet6 addr: ::1/128 Scope:Host
          UP LOOPBACK RUNNING  MTU:16436  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0
          RX bytes:0 (0.0 b)  TX bytes:0 (0.0 b)

vif5.0-emu Link encap:Ethernet  HWaddr 1A:58:5C:16:5C:02
          inet6 addr: fe80::1858:5cff:fe16:5c02/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:4 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:500
          RX bytes:0 (0.0 b)  TX bytes:280 (280.0 b)

vif5.1-emu Link encap:Ethernet  HWaddr 6A:A7:BB:1E:A6:95
          inet6 addr: fe80::68a7:bbff:fe1e:a695/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:3 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:500
          RX bytes:0 (0.0 b)  TX bytes:222 (222.0 b)

xenbr0    Link encap:Ethernet  HWaddr 00:25:90:3D:0D:12
          inet addr:203.175.161.8  Bcast:203.175.161.255  Mask:255.255.255.0
          inet6 addr: fe80::225:90ff:fe3d:d12/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:13978 errors:0 dropped:0 overruns:0 frame:0
          TX packets:1145 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0
          RX bytes:702446 (685.9 KiB)  TX bytes:226828 (221.5 KiB)

xenbr1    Link encap:Ethernet  HWaddr 00:25:90:3D:0D:13
          inet addr:192.168.100.18  Bcast:192.168.100.255  Mask:255.255.255.0
          inet6 addr: fe80::225:90ff:fe3d:d13/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:2506 errors:0 dropped:0 overruns:0 frame:0
          TX packets:25 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0
          RX bytes:121776 (118.9 KiB)  TX bytes:1926 (1.8 KiB)

bridge name	bridge id		STP enabled	interfaces
xenbr0		8000.0025903d0d12	no		b1
							eth0
							vif5.0-emu
xenbr1		8000.0025903d0d13	no		b2
							eth1
							vif5.1-emu
NET_MATCHID=
SUBSYSTEM=net
DEVPATH=/devices/virtual/net/vif5.0-emu
PATH=/usr/bin:/usr/sbin:/usr/lib/xen/bin:/usr/lib64/xen/bin:/sbin:/bin:/usr/bin:/usr/sbin:/usr/local/bin:/bin:/usr/bin
ACTION=add
UDEV_LOG=3
PWD=/
LANG=POSIX
SHLVL=1
IFINDEX=24
INTERFACE=vif5.0-emu
SEQNUM=2521
_=/usr/bin/printenv
NET_MATCHID=
SUBSYSTEM=net
DEVPATH=/devices/virtual/net/vif5.1-emu
PATH=/usr/bin:/usr/sbin:/usr/lib/xen/bin:/usr/lib64/xen/bin:/sbin:/bin:/usr/bin:/usr/sbin:/usr/local/bin:/bin:/usr/bin
ACTION=add
UDEV_LOG=3
PWD=/
LANG=POSIX
SHLVL=1
IFINDEX=25
INTERFACE=vif5.1-emu
SEQNUM=2524
_=/usr/bin/printenv


>
>> Just to confirm... are we going to "throw away" xm in 4.2 and use only xl?
>
> Not yet, they will both existing in 4.2 but xl will now be considered
> the default. We hope to be able to get rid of xm in the 4.3 time frame,
> but that depends on a variety of factors.

Ok.  Thanks for letting me know ;)

Thanks.

Kindest regards,
Giam Teck Choon


>
>>
>> Thanks.
>>
>> Kindest regards,
>> Giam Teck Choon
>>
>>
>> >
>> > Ian.
>> >
>
>

Re: [patch] xen udev rule interfering with openvpn

[Ian Campbell]

On Mon, 2012-05-21 at 14:16 +0100, Teck Choon Giam wrote:

> vif5.0-emu Link encap:Ethernet  HWaddr 1A:58:5C:16:5C:02
>           inet6 addr: fe80::1858:5cff:fe16:5c02/64 Scope:Link
>           UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
>           RX packets:0 errors:0 dropped:0 overruns:0 frame:0
>           TX packets:4 errors:0 dropped:0 overruns:0 carrier:0
>           collisions:0 txqueuelen:500
>           RX bytes:0 (0.0 b)  TX bytes:280 (280.0 b)

The fact that this interface is up at this point is interesting, didn't
you mention something about this at another time?

In the tap dev case I made the rename into:
            do_or_die ifconfig "$dev" down
            do_or_die ip link set "$dev" name "$vifname"
        
which seemed to workaround the issue for me.

I think the reason this effects xm and not xl is that libxl uses
script=none to disable qemu-ifup while xend does not and instead ends up
using qemu-ifup which does some fiddling with the device too, including
bringing it up.

The proper fix is probably to change xend, I'm a bit wary of this,
especially for a 4.1 backport, but the following looks right and works
for me. It's a bit more complex since in libxl we seem to only do this
for Linux (i.e. not NetBSD) and I guess we should do the same in xend
too.

Ian

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1337692747 -3600
# Node ID 426bbf58cea4559464b6e5d3ff0f65324a5f5926
# Parent  72ca5bc4eb6b91fa8dff51d439bd05f5586179df
xend: do not run a hotplug script from qemu on Linux

The current vif-hotplug-common.sh for renaming the tap device is failing
because it is racing with this script and therefore the device is unexpectedly
up when we come to rename it.

Fix this in the same way as libxl does, by disabling the script (only on
Linux).

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 72ca5bc4eb6b -r 426bbf58cea4 tools/python/xen/xend/image.py
--- a/tools/python/xen/xend/image.py	Tue May 22 11:29:50 2012 +0100
+++ b/tools/python/xen/xend/image.py	Tue May 22 14:19:07 2012 +0100
@@ -919,8 +919,13 @@ class HVMImageHandler(ImageHandler):
                        (nics, mac, model))
             vifname = "vif%d.%d-emu" % (self.vm.getDomid(), nics-1)
             ret.append("-net")
-            ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
-                       (nics, vifname, bridge))
+            if osdep.tapif_script is not None:
+                script=",script=%s,downscript=%s" % \
+                        (osdep.tapif_script, osdep.tapif_script)
+            else:
+                script=""
+            ret.append("tap,vlan=%d,ifname=%s,bridge=%s%s" %
+                       (nics, vifname, bridge, script))
 
         if nics == 0:
             ret.append("-net")
diff -r 72ca5bc4eb6b -r 426bbf58cea4 tools/python/xen/xend/osdep.py
--- a/tools/python/xen/xend/osdep.py	Tue May 22 11:29:50 2012 +0100
+++ b/tools/python/xen/xend/osdep.py	Tue May 22 14:19:07 2012 +0100
@@ -30,6 +30,10 @@ _vif_script = {
     "SunOS": "vif-vnic"
 }
 
+_tapif_script = {
+    "Linux": "no",
+}
+
 PROC_XEN_BALLOON = '/proc/xen/balloon'
 SYSFS_XEN_MEMORY = '/sys/devices/system/xen_memory/xen_memory0'
 
@@ -257,6 +261,7 @@ def _get(var, default=None):
 
 xend_autorestart = _get(_xend_autorestart)
 vif_script = _get(_vif_script, "vif-bridge")
+tapif_script = _get(_tapif_script)
 lookup_balloon_stat = _get(_balloon_stat, _linux_balloon_stat)
 get_cpuinfo = _get(_get_cpuinfo, _linux_get_cpuinfo)
 prefork = _get(_get_prefork, _default_prefork)

Re: [patch] xen udev rule interfering with openvpn

[Teck Choon Giam]

On Tue, May 22, 2012 at 9:19 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Mon, 2012-05-21 at 14:16 +0100, Teck Choon Giam wrote:
>
>> vif5.0-emu Link encap:Ethernet  HWaddr 1A:58:5C:16:5C:02
>>           inet6 addr: fe80::1858:5cff:fe16:5c02/64 Scope:Link
>>           UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
>>           RX packets:0 errors:0 dropped:0 overruns:0 frame:0
>>           TX packets:4 errors:0 dropped:0 overruns:0 carrier:0
>>           collisions:0 txqueuelen:500
>>           RX bytes:0 (0.0 b)  TX bytes:280 (280.0 b)
>
> The fact that this interface is up at this point is interesting, didn't
> you mention something about this at another time?

Yes, I did mentioned that $dev is up when test with xm create
hvmdomain with vifname set which cause test #7 to fail.

>
> In the tap dev case I made the rename into:
>            do_or_die ifconfig "$dev" down
>            do_or_die ip link set "$dev" name "$vifname"
>
> which seemed to workaround the issue for me.

The workaround is identical for mine by bringing the $dev down.

>
> I think the reason this effects xm and not xl is that libxl uses
> script=none to disable qemu-ifup while xend does not and instead ends up
> using qemu-ifup which does some fiddling with the device too, including
> bringing it up.

Ok, so default for xend is using script=qemu-ifup if script is not
set?  Am I right about this?

>
> The proper fix is probably to change xend, I'm a bit wary of this,
> especially for a 4.1 backport, but the following looks right and works
> for me. It's a bit more complex since in libxl we seem to only do this
> for Linux (i.e. not NetBSD) and I guess we should do the same in xend
> too.

Err... if we are going to change default behaviour will we be
affecting those users who is upgrading from xen-4.1 to xen-4.2?

If your fix patch is going into xen-unstable for sure, I will re-run
my tests by then.  I hope it doesn't affect current domUs
configuration (I mean we shouldn't need to change domU configuration)
especially when users prefer to use xm then xl in xen-4.2.

Thanks.

Kindest regards,
Giam Teck Choon


>
> Ian
>
> # HG changeset patch
> # User Ian Campbell <ian.campbell@citrix.com>
> # Date 1337692747 -3600
> # Node ID 426bbf58cea4559464b6e5d3ff0f65324a5f5926
> # Parent  72ca5bc4eb6b91fa8dff51d439bd05f5586179df
> xend: do not run a hotplug script from qemu on Linux
>
> The current vif-hotplug-common.sh for renaming the tap device is failing
> because it is racing with this script and therefore the device is unexpectedly
> up when we come to rename it.
>
> Fix this in the same way as libxl does, by disabling the script (only on
> Linux).
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>
> diff -r 72ca5bc4eb6b -r 426bbf58cea4 tools/python/xen/xend/image.py
> --- a/tools/python/xen/xend/image.py    Tue May 22 11:29:50 2012 +0100
> +++ b/tools/python/xen/xend/image.py    Tue May 22 14:19:07 2012 +0100
> @@ -919,8 +919,13 @@ class HVMImageHandler(ImageHandler):
>                        (nics, mac, model))
>             vifname = "vif%d.%d-emu" % (self.vm.getDomid(), nics-1)
>             ret.append("-net")
> -            ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
> -                       (nics, vifname, bridge))
> +            if osdep.tapif_script is not None:
> +                script=",script=%s,downscript=%s" % \
> +                        (osdep.tapif_script, osdep.tapif_script)
> +            else:
> +                script=""
> +            ret.append("tap,vlan=%d,ifname=%s,bridge=%s%s" %
> +                       (nics, vifname, bridge, script))
>
>         if nics == 0:
>             ret.append("-net")
> diff -r 72ca5bc4eb6b -r 426bbf58cea4 tools/python/xen/xend/osdep.py
> --- a/tools/python/xen/xend/osdep.py    Tue May 22 11:29:50 2012 +0100
> +++ b/tools/python/xen/xend/osdep.py    Tue May 22 14:19:07 2012 +0100
> @@ -30,6 +30,10 @@ _vif_script = {
>     "SunOS": "vif-vnic"
>  }
>
> +_tapif_script = {
> +    "Linux": "no",
> +}
> +
>  PROC_XEN_BALLOON = '/proc/xen/balloon'
>  SYSFS_XEN_MEMORY = '/sys/devices/system/xen_memory/xen_memory0'
>
> @@ -257,6 +261,7 @@ def _get(var, default=None):
>
>  xend_autorestart = _get(_xend_autorestart)
>  vif_script = _get(_vif_script, "vif-bridge")
> +tapif_script = _get(_tapif_script)
>  lookup_balloon_stat = _get(_balloon_stat, _linux_balloon_stat)
>  get_cpuinfo = _get(_get_cpuinfo, _linux_get_cpuinfo)
>  prefork = _get(_get_prefork, _default_prefork)
>
>

Re: [patch] xen udev rule interfering with openvpn

[Ian Campbell]

On Wed, 2012-05-23 at 03:22 +0100, Teck Choon Giam wrote:

> >
> > I think the reason this effects xm and not xl is that libxl uses
> > script=none to disable qemu-ifup while xend does not and instead ends up
> > using qemu-ifup which does some fiddling with the device too, including
> > bringing it up.
> 
> Ok, so default for xend is using script=qemu-ifup if script is not
> set?  Am I right about this?

Yes.

> > The proper fix is probably to change xend, I'm a bit wary of this,
> > especially for a 4.1 backport, but the following looks right and works
> > for me. It's a bit more complex since in libxl we seem to only do this
> > for Linux (i.e. not NetBSD) and I guess we should do the same in xend
> > too.
> 
> Err... if we are going to change default behaviour will we be
> affecting those users who is upgrading from xen-4.1 to xen-4.2?

This was already a deliberate change made in xl, it does not effect the
guest config, only the mechanisms by which that configuration is
achieved. I think extending this to xend in order not to break xend in
4.2 is worthwhile.

I don't think we should be backporting any of this to 4.1 though.

> If your fix patch is going into xen-unstable for sure, I will re-run
> my tests by then.  I hope it doesn't affect current domUs
> configuration (I mean we shouldn't need to change domU configuration)
> especially when users prefer to use xm then xl in xen-4.2.

There should be no guest config change necessary.

Ian.

> 
> Thanks.
> 
> Kindest regards,
> Giam Teck Choon
> 
> 
> >
> > Ian
> >
> > # HG changeset patch
> > # User Ian Campbell <ian.campbell@citrix.com>
> > # Date 1337692747 -3600
> > # Node ID 426bbf58cea4559464b6e5d3ff0f65324a5f5926
> > # Parent  72ca5bc4eb6b91fa8dff51d439bd05f5586179df
> > xend: do not run a hotplug script from qemu on Linux
> >
> > The current vif-hotplug-common.sh for renaming the tap device is failing
> > because it is racing with this script and therefore the device is unexpectedly
> > up when we come to rename it.
> >
> > Fix this in the same way as libxl does, by disabling the script (only on
> > Linux).
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> >
> > diff -r 72ca5bc4eb6b -r 426bbf58cea4 tools/python/xen/xend/image.py
> > --- a/tools/python/xen/xend/image.py    Tue May 22 11:29:50 2012 +0100
> > +++ b/tools/python/xen/xend/image.py    Tue May 22 14:19:07 2012 +0100
> > @@ -919,8 +919,13 @@ class HVMImageHandler(ImageHandler):
> >                        (nics, mac, model))
> >             vifname = "vif%d.%d-emu" % (self.vm.getDomid(), nics-1)
> >             ret.append("-net")
> > -            ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
> > -                       (nics, vifname, bridge))
> > +            if osdep.tapif_script is not None:
> > +                script=",script=%s,downscript=%s" % \
> > +                        (osdep.tapif_script, osdep.tapif_script)
> > +            else:
> > +                script=""
> > +            ret.append("tap,vlan=%d,ifname=%s,bridge=%s%s" %
> > +                       (nics, vifname, bridge, script))
> >
> >         if nics == 0:
> >             ret.append("-net")
> > diff -r 72ca5bc4eb6b -r 426bbf58cea4 tools/python/xen/xend/osdep.py
> > --- a/tools/python/xen/xend/osdep.py    Tue May 22 11:29:50 2012 +0100
> > +++ b/tools/python/xen/xend/osdep.py    Tue May 22 14:19:07 2012 +0100
> > @@ -30,6 +30,10 @@ _vif_script = {
> >     "SunOS": "vif-vnic"
> >  }
> >
> > +_tapif_script = {
> > +    "Linux": "no",
> > +}
> > +
> >  PROC_XEN_BALLOON = '/proc/xen/balloon'
> >  SYSFS_XEN_MEMORY = '/sys/devices/system/xen_memory/xen_memory0'
> >
> > @@ -257,6 +261,7 @@ def _get(var, default=None):
> >
> >  xend_autorestart = _get(_xend_autorestart)
> >  vif_script = _get(_vif_script, "vif-bridge")
> > +tapif_script = _get(_tapif_script)
> >  lookup_balloon_stat = _get(_balloon_stat, _linux_balloon_stat)
> >  get_cpuinfo = _get(_get_cpuinfo, _linux_get_cpuinfo)
> >  prefork = _get(_get_prefork, _default_prefork)
> >
> >

Re: [patch] xen udev rule interfering with openvpn

[Teck Choon Giam]

On Wed, May 23, 2012 at 5:37 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Wed, 2012-05-23 at 03:22 +0100, Teck Choon Giam wrote:
>
>> >
>> > I think the reason this effects xm and not xl is that libxl uses
>> > script=none to disable qemu-ifup while xend does not and instead ends up
>> > using qemu-ifup which does some fiddling with the device too, including
>> > bringing it up.
>>
>> Ok, so default for xend is using script=qemu-ifup if script is not
>> set?  Am I right about this?
>
> Yes.

Thanks for clarifying.

>
>> > The proper fix is probably to change xend, I'm a bit wary of this,
>> > especially for a 4.1 backport, but the following looks right and works
>> > for me. It's a bit more complex since in libxl we seem to only do this
>> > for Linux (i.e. not NetBSD) and I guess we should do the same in xend
>> > too.
>>
>> Err... if we are going to change default behaviour will we be
>> affecting those users who is upgrading from xen-4.1 to xen-4.2?
>
> This was already a deliberate change made in xl, it does not effect the
> guest config, only the mechanisms by which that configuration is
> achieved. I think extending this to xend in order not to break xend in
> 4.2 is worthwhile.

Noted.

>
> I don't think we should be backporting any of this to 4.1 though.

You mean your tap to -emu patch series including the latest fix patch
you posted shouldn't be backporting to 4.1?  If this is so, I am fine
since there isn't much difference for me as personally I kept few
custom patches in own xen packages.  Of course whatever get into
upstream is better though :p

>
>> If your fix patch is going into xen-unstable for sure, I will re-run
>> my tests by then.  I hope it doesn't affect current domUs
>> configuration (I mean we shouldn't need to change domU configuration)
>> especially when users prefer to use xm then xl in xen-4.2.
>
> There should be no guest config change necessary.

Noted.

>
> Ian.

Thanks for taking time to provide fix and responses.

Kindest regards,
Giam Teck Choon



>
>>
>> Thanks.
>>
>> Kindest regards,
>> Giam Teck Choon
>>
>>
>> >
>> > Ian
>> >
>> > # HG changeset patch
>> > # User Ian Campbell <ian.campbell@citrix.com>
>> > # Date 1337692747 -3600
>> > # Node ID 426bbf58cea4559464b6e5d3ff0f65324a5f5926
>> > # Parent  72ca5bc4eb6b91fa8dff51d439bd05f5586179df
>> > xend: do not run a hotplug script from qemu on Linux
>> >
>> > The current vif-hotplug-common.sh for renaming the tap device is failing
>> > because it is racing with this script and therefore the device is unexpectedly
>> > up when we come to rename it.
>> >
>> > Fix this in the same way as libxl does, by disabling the script (only on
>> > Linux).
>> >
>> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>> >
>> > diff -r 72ca5bc4eb6b -r 426bbf58cea4 tools/python/xen/xend/image.py
>> > --- a/tools/python/xen/xend/image.py    Tue May 22 11:29:50 2012 +0100
>> > +++ b/tools/python/xen/xend/image.py    Tue May 22 14:19:07 2012 +0100
>> > @@ -919,8 +919,13 @@ class HVMImageHandler(ImageHandler):
>> >                        (nics, mac, model))
>> >             vifname = "vif%d.%d-emu" % (self.vm.getDomid(), nics-1)
>> >             ret.append("-net")
>> > -            ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
>> > -                       (nics, vifname, bridge))
>> > +            if osdep.tapif_script is not None:
>> > +                script=",script=%s,downscript=%s" % \
>> > +                        (osdep.tapif_script, osdep.tapif_script)
>> > +            else:
>> > +                script=""
>> > +            ret.append("tap,vlan=%d,ifname=%s,bridge=%s%s" %
>> > +                       (nics, vifname, bridge, script))
>> >
>> >         if nics == 0:
>> >             ret.append("-net")
>> > diff -r 72ca5bc4eb6b -r 426bbf58cea4 tools/python/xen/xend/osdep.py
>> > --- a/tools/python/xen/xend/osdep.py    Tue May 22 11:29:50 2012 +0100
>> > +++ b/tools/python/xen/xend/osdep.py    Tue May 22 14:19:07 2012 +0100
>> > @@ -30,6 +30,10 @@ _vif_script = {
>> >     "SunOS": "vif-vnic"
>> >  }
>> >
>> > +_tapif_script = {
>> > +    "Linux": "no",
>> > +}
>> > +
>> >  PROC_XEN_BALLOON = '/proc/xen/balloon'
>> >  SYSFS_XEN_MEMORY = '/sys/devices/system/xen_memory/xen_memory0'
>> >
>> > @@ -257,6 +261,7 @@ def _get(var, default=None):
>> >
>> >  xend_autorestart = _get(_xend_autorestart)
>> >  vif_script = _get(_vif_script, "vif-bridge")
>> > +tapif_script = _get(_tapif_script)
>> >  lookup_balloon_stat = _get(_balloon_stat, _linux_balloon_stat)
>> >  get_cpuinfo = _get(_get_cpuinfo, _linux_get_cpuinfo)
>> >  prefork = _get(_get_prefork, _default_prefork)
>> >
>> >
>
>

Re: [patch] xen udev rule interfering with openvpn

[Teck Choon Giam]

On Wed, May 23, 2012 at 9:04 PM, Teck Choon Giam
<giamteckchoon@gmail.com> wrote:
> On Wed, May 23, 2012 at 5:37 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> On Wed, 2012-05-23 at 03:22 +0100, Teck Choon Giam wrote:
>>
>>> >
>>> > I think the reason this effects xm and not xl is that libxl uses
>>> > script=none to disable qemu-ifup while xend does not and instead ends up
>>> > using qemu-ifup which does some fiddling with the device too, including
>>> > bringing it up.
>>>
>>> Ok, so default for xend is using script=qemu-ifup if script is not
>>> set?  Am I right about this?
>>
>> Yes.
>
> Thanks for clarifying.
>
>>
>>> > The proper fix is probably to change xend, I'm a bit wary of this,
>>> > especially for a 4.1 backport, but the following looks right and works
>>> > for me. It's a bit more complex since in libxl we seem to only do this
>>> > for Linux (i.e. not NetBSD) and I guess we should do the same in xend
>>> > too.
>>>
>>> Err... if we are going to change default behaviour will we be
>>> affecting those users who is upgrading from xen-4.1 to xen-4.2?
>>
>> This was already a deliberate change made in xl, it does not effect the
>> guest config, only the mechanisms by which that configuration is
>> achieved. I think extending this to xend in order not to break xend in
>> 4.2 is worthwhile.
>
> Noted.
>
>>
>> I don't think we should be backporting any of this to 4.1 though.
>
> You mean your tap to -emu patch series including the latest fix patch
> you posted shouldn't be backporting to 4.1?  If this is so, I am fine
> since there isn't much difference for me as personally I kept few
> custom patches in own xen packages.  Of course whatever get into
> upstream is better though :p

Just an update.  I have run my tests with your fix patch in
xen-unstable changeset 25386:340062faf298 and all are working fine.

Thanks again.

Kindest regards,
Giam Teck Choon


>
>>
>>> If your fix patch is going into xen-unstable for sure, I will re-run
>>> my tests by then.  I hope it doesn't affect current domUs
>>> configuration (I mean we shouldn't need to change domU configuration)
>>> especially when users prefer to use xm then xl in xen-4.2.
>>
>> There should be no guest config change necessary.
>
> Noted.
>
>>
>> Ian.
>
> Thanks for taking time to provide fix and responses.
>
> Kindest regards,
> Giam Teck Choon
>
>
>
>>
>>>
>>> Thanks.
>>>
>>> Kindest regards,
>>> Giam Teck Choon
>>>
>>>
>>> >
>>> > Ian
>>> >
>>> > # HG changeset patch
>>> > # User Ian Campbell <ian.campbell@citrix.com>
>>> > # Date 1337692747 -3600
>>> > # Node ID 426bbf58cea4559464b6e5d3ff0f65324a5f5926
>>> > # Parent  72ca5bc4eb6b91fa8dff51d439bd05f5586179df
>>> > xend: do not run a hotplug script from qemu on Linux
>>> >
>>> > The current vif-hotplug-common.sh for renaming the tap device is failing
>>> > because it is racing with this script and therefore the device is unexpectedly
>>> > up when we come to rename it.
>>> >
>>> > Fix this in the same way as libxl does, by disabling the script (only on
>>> > Linux).
>>> >
>>> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>>> >
>>> > diff -r 72ca5bc4eb6b -r 426bbf58cea4 tools/python/xen/xend/image.py
>>> > --- a/tools/python/xen/xend/image.py    Tue May 22 11:29:50 2012 +0100
>>> > +++ b/tools/python/xen/xend/image.py    Tue May 22 14:19:07 2012 +0100
>>> > @@ -919,8 +919,13 @@ class HVMImageHandler(ImageHandler):
>>> >                        (nics, mac, model))
>>> >             vifname = "vif%d.%d-emu" % (self.vm.getDomid(), nics-1)
>>> >             ret.append("-net")
>>> > -            ret.append("tap,vlan=%d,ifname=%s,bridge=%s" %
>>> > -                       (nics, vifname, bridge))
>>> > +            if osdep.tapif_script is not None:
>>> > +                script=",script=%s,downscript=%s" % \
>>> > +                        (osdep.tapif_script, osdep.tapif_script)
>>> > +            else:
>>> > +                script=""
>>> > +            ret.append("tap,vlan=%d,ifname=%s,bridge=%s%s" %
>>> > +                       (nics, vifname, bridge, script))
>>> >
>>> >         if nics == 0:
>>> >             ret.append("-net")
>>> > diff -r 72ca5bc4eb6b -r 426bbf58cea4 tools/python/xen/xend/osdep.py
>>> > --- a/tools/python/xen/xend/osdep.py    Tue May 22 11:29:50 2012 +0100
>>> > +++ b/tools/python/xen/xend/osdep.py    Tue May 22 14:19:07 2012 +0100
>>> > @@ -30,6 +30,10 @@ _vif_script = {
>>> >     "SunOS": "vif-vnic"
>>> >  }
>>> >
>>> > +_tapif_script = {
>>> > +    "Linux": "no",
>>> > +}
>>> > +
>>> >  PROC_XEN_BALLOON = '/proc/xen/balloon'
>>> >  SYSFS_XEN_MEMORY = '/sys/devices/system/xen_memory/xen_memory0'
>>> >
>>> > @@ -257,6 +261,7 @@ def _get(var, default=None):
>>> >
>>> >  xend_autorestart = _get(_xend_autorestart)
>>> >  vif_script = _get(_vif_script, "vif-bridge")
>>> > +tapif_script = _get(_tapif_script)
>>> >  lookup_balloon_stat = _get(_balloon_stat, _linux_balloon_stat)
>>> >  get_cpuinfo = _get(_get_cpuinfo, _linux_get_cpuinfo)
>>> >  prefork = _get(_get_prefork, _default_prefork)
>>> >
>>> >
>>
>>