|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [patch] xen udev rule interfering with openvpn
On Sun, May 13, 2012 at 6:30 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> 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@xxxxxxxxx> wrote:
>> > On Sat, May 12, 2012 at 7:53 AM, Teck Choon Giam
>> > <giamteckchoon@xxxxxxxxx> wrote:
>> >> On Fri, May 11, 2012 at 10:53 PM, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
>> >> 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@xxxxxxxxxxxxx>
>> >>
>> >> 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@xxxxxxxxx>
>> >> ---
>> >> 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))
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |