Re: [patch] xen udev rule interfering with openvpn

[Roger Pau Monne <roger.pau@citrix.com>]

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