From mboxrd@z Thu Jan 1 00:00:00 1970 From: Teck Choon Giam Subject: Re: [patch] xen udev rule interfering with openvpn Date: Sun, 13 May 2012 06:00:33 +0800 Message-ID: References: <1334658395.23948.6.camel@zakaz.uk.xensource.com> <1334817587.11493.44.camel@dagon.hellion.org.uk> <1334912603.28331.2.camel@zakaz.uk.xensource.com> <20369.15528.270106.567037@mariner.uk.xensource.com> <1334918900.28331.47.camel@zakaz.uk.xensource.com> <20369.16555.46229.798603@mariner.uk.xensource.com> <1334919613.28331.53.camel@zakaz.uk.xensource.com> <20369.17085.330843.561841@mariner.uk.xensource.com> <1335347949.28015.19.camel@zakaz.uk.xensource.com> <20375.52677.287182.934829@mariner.uk.xensource.com> <1335348880.28015.24.camel@zakaz.uk.xensource.com> <1335358736.28015.41.camel@zakaz.uk.xensource.com> <20397.10224.96552.711065@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Jackson Cc: Roger Pau Monne , "xen-devel@lists.xen.org" , Ian Campbell , M A Young List-Id: xen-devel@lists.xenproject.org On Sat, May 12, 2012 at 3:29 PM, Teck Choon Giam wrote: > On Sat, May 12, 2012 at 7:53 AM, Teck Choon Giam > wrote: >> On Fri, May 11, 2012 at 10:53 PM, Ian Jackson 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 >> >> This is my backport version which excludes the following: >> >> Lastly also move libxl__device_* to a better place in the header, otherw= ise the >> comment about evgen stuff isn't next to the associated functions (notice= d 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. =A0I must have missed certain test case :( The same test case failed for xen unstable latest changeset 25326:cd4dd23a8= 31d. 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 ap= plied >> 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 comp= lex. >> 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 initi= ally >> named with a user specified name (which will not match the expected sche= me) 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 h= andle >> 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 sc= ript. >> >> xen-unstable changeset: 25290:7a6dcecb1781 >> Signed-off-by: Giam Teck Choon >> --- >> =A0tools/hotplug/Linux/vif-common.sh =A0 =A0 | =A0 15 +++++++++++++-- >> =A0tools/hotplug/Linux/xen-backend.rules | =A0 =A02 +- >> =A0tools/libxl/libxl_dm.c =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0| =A0 17 ++++++= ----------- >> =A0tools/python/xen/xend/image.py =A0 =A0 =A0 =A0| =A0 =A06 +----- >> =A04 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" =3D tap ]; then >> =A0 =A0 : ${INTERFACE:?} >> >> =A0 =A0 # Get xenbus_path from device name. >> - =A0 =A0# The name is built like that: "tap${domid}.${devid}". >> - =A0 =A0dev_=3D${dev#tap} >> + =A0 =A0# The name is built like that: "vif${domid}.${devid}-emu". >> + =A0 =A0dev_=3D${dev#vif} >> + =A0 =A0dev_=3D${dev_%-emu} >> =A0 =A0 domid=3D${dev_%.*} >> =A0 =A0 devid=3D${dev_#*.} >> >> =A0 =A0 XENBUS_PATH=3D"/local/domain/0/backend/vif/$domid/$devid" >> + =A0 =A0vifname=3D$(xenstore_read_default "$XENBUS_PATH/vifname" "") >> + =A0 =A0if [ "$vifname" ] >> + =A0 =A0then >> + =A0 =A0 =A0 =A0vifname=3D"${vifname}-emu" >> + =A0 =A0 =A0 =A0if [ "$command" =3D=3D "add" ] && ! ip link show "$vifn= ame" >&/dev/null >> + =A0 =A0 =A0 =A0then >> + =A0 =A0 =A0 =A0 =A0 =A0do_or_die ip link set "$dev" name "$vifname" >> + =A0 =A0 =A0 =A0fi >> + =A0 =A0 =A0 =A0dev=3D"$vifname" >> + =A0 =A0fi >> =A0fi >> >> =A0ip=3D${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=3D=3D"blktap-control", >> NAME=3D"xen/blktap-2/control", MODE=3D"0600" >> =A0KERNEL=3D=3D"gntdev", NAME=3D"xen/%k", MODE=3D"0600" >> =A0KERNEL=3D=3D"pci_iomul", NAME=3D"xen/%k", MODE=3D"0600" >> =A0KERNEL=3D=3D"tapdev[a-z]*", NAME=3D"xen/blktap-2/tapdev%m", MODE=3D"0= 600" >> -SUBSYSTEM=3D=3D"net", KERNEL=3D=3D"tap*", ACTION=3D=3D"add", >> RUN+=3D"/etc/xen/scripts/vif-setup $env{ACTION} type_if=3Dtap" >> +SUBSYSTEM=3D=3D"net", KERNEL=3D=3D"vif*-emu", ACTION=3D=3D"add", >> RUN+=3D"/etc/xen/scripts/vif-setup $env{ACTION} type_if=3Dtap" >> 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, >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 char *smac =3D libxl__sprintf(gc, >> "%02x:%02x:%02x:%02x:%02x:%02x", >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0vifs[i].mac[0], >> vifs[i].mac[1], vifs[i].mac[2], >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0vifs[i].mac[3], >> vifs[i].mac[4], vifs[i].mac[5]); >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0char *ifname; >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (!vifs[i].ifname) >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ifname =3D libxl__sprintf(gc, "= tap%d.%d", >> info->domid, vifs[i].devid); >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0else >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ifname =3D vifs[i].ifname; >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0const char *ifname =3D libxl__sprintf(g= c, "vif%d.%d-emu", >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0info->domid, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0vifs[i].devid); >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 flexarray_vappend(dm_args, >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "-net", = libxl__sprintf(gc, >> "nic,vlan=3D%d,macaddr=3D%s,model=3D%s", >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0vifs[i].devid, >> smac, vifs[i].model), >> @@ -271,12 +269,9 @@ static char ** >> libxl_build_device_model_args_new(libxl__gc *gc, >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 char *smac =3D libxl__sprintf(gc, >> "%02x:%02x:%02x:%02x:%02x:%02x", >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0vifs[i].mac[0], >> vifs[i].mac[1], vifs[i].mac[2], >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0vifs[i].mac[3], >> vifs[i].mac[4], vifs[i].mac[5]); >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0char *ifname; >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (!vifs[i].ifname) { >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ifname =3D libxl__sprintf(gc, "= tap%d.%d", >> info->domid, vifs[i].devid); >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} else { >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ifname =3D vifs[i].ifname; >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0const char *ifname =3D libxl__sprintf(g= c, "vif%d.%d-emu", >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0info->domid, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0vifs[i].devid); >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 flexarray_append(dm_args, "-net"); >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 flexarray_append(dm_args, libxl__sprintf= (gc, >> "nic,vlan=3D%d,macaddr=3D%s,model=3D%s", >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 vifs[i].devid, s= mac, vifs[i].model)); >> diff --git a/tools/python/xen/xend/image.py b/tools/python/xen/xend/imag= e.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): >> =A0 =A0 =A0 =A0 =A0 =A0 ret.append("-net") >> =A0 =A0 =A0 =A0 =A0 =A0 ret.append("nic,vlan=3D%d,macaddr=3D%s,model=3D%= s" % >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(nics, mac, model)) >> - =A0 =A0 =A0 =A0 =A0 =A0vifname =3D devinfo.get('vifname') >> - =A0 =A0 =A0 =A0 =A0 =A0if vifname: >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0vifname =3D "tap-" + vifname >> - =A0 =A0 =A0 =A0 =A0 =A0else: >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0vifname =3D "tap%d.%d" % (self.vm.getDo= mid(), nics-1) >> + =A0 =A0 =A0 =A0 =A0 =A0vifname =3D "vif%d.%d-emu" % (self.vm.getDomid(= ), nics-1) >> =A0 =A0 =A0 =A0 =A0 =A0 ret.append("-net") >> =A0 =A0 =A0 =A0 =A0 =A0 ret.append("tap,vlan=3D%d,ifname=3D%s,bridge=3D%= s" % >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(nics, vifname, bridge))