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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
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))
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
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-" + vifnameThe 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
> > 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-" + vifnameYou are absolutely right, not sure how I missed that! I''m out-of-office today but I''ll resend later. Ian.
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))
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.
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.
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.
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.
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.
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.
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
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 :pI 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
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 > >
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.
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.
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.
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))
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
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
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>
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)) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
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))
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))
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))
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 setxm 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)) > >
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 :pAttached 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)) >> >>_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
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)) >> >>
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.
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
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. >
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 failedSo 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. > >
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_MATCHIDSUBSYSTEM=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_MATCHIDSUBSYSTEM=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. >> > > >
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)
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) > >
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) > > > >
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) >> > >> > > >
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 :pJust 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) >>> > >>> > >> >>