WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] xc: deal with xen/evtchn and xen/gntdev device n

To: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xc: deal with xen/evtchn and xen/gntdev device names
From: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Date: Wed, 02 Jun 2010 09:09:14 -0700
Cc: Bastian Blank <waldi@xxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Wed, 02 Jun 2010 09:12:29 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1275470944.24218.23601.camel@xxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <4C00690B.2020303@xxxxxxxx> <1275379012.24218.20337.camel@xxxxxxxxxxxxxxxxxxxxxx> <4C0536BF.7070605@xxxxxxxx> <1275470944.24218.23601.camel@xxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100430 Fedora/3.0.4-2.fc12 Lightning/1.0b2pre Thunderbird/3.0.4
On 06/02/2010 02:29 AM, Ian Campbell wrote:
> On Tue, 2010-06-01 at 17:35 +0100, Jeremy Fitzhardinge wrote:
>   
>> On 06/01/2010 12:56 AM, Ian Campbell wrote:
>>     
>>> Does any of this logic really belong in libxc in the first place? 
>>>   
>>>       
>> No.  It's a relic of a simpler time.
>>
>>     
>>> Can we not just rip it all out and make it the distro/platform's
>>> responsibility to ensure these devices exist and are correct? Perhaps
>>> that might involve shipping some default/example udev rules instead.
>>>   
>>>       
>> I only kept it because I didn't want to break any existing installs, but
>> updating the kernel's name is going to do that anyway.  Unfortunately
>> the current libxc code is broken in the worst possible way - it will
>> unlink an existing correct device and then fail to replace it - so even
>> if the system has set it up correctly it will still screw things up.
>>
>> I think we're just going to have to have a flag day and fix kernel,
>> libxc and udev (assuming it needs it) all at once...
>>     
> I don't think we need a flag day. It seems like we already ship a udev
> rule (in tools/hotplug/Linux/xen-backend.rules) which correctly
> created /dev/xen/evtchn with the current kernel and which is apparently
> unnecessary (but harmless) with the proposed kernel change.
>   

My main concern is that an old libxc will screw anyone with new kernel
and udev.

> I removed all the mknod stuff from my tools and things kept working as
> expected.
>
> So lets try the following. I think once it has settled down in unstable
> we could consider it for inclusion in the stable branch.
>
> --- 
> tools: assume that special Xen devices have been created by the platform
>
> Remove all the magic surrounding the special Xen devices in Linux
> specific code whereby we attempt to figure out what the correct
> major:minor number is and check the the existing device has these
> numbers etc. In 2010 we really should be able to trust that the platform
> has created the devices correctly or provide correct configuration
> settings such that they are without resorting to tearing down the
> platform configured state and rebuilding it.
>
> tools/hotplug/Linux/xen-backend.rules already contains the necessary
> udev rules to create /dev/xen/evtchn and friends in the correct place.
>
>   

Looks fine to me.

    J

> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
>
> diff -r 2f765c9825b2 -r 2962c9c4d26f tools/blktap/drivers/blktapctrl_linux.c
> --- a/tools/blktap/drivers/blktapctrl_linux.c Tue Jun 01 10:40:06 2010 +0100
> +++ b/tools/blktap/drivers/blktapctrl_linux.c Wed Jun 02 10:01:09 2010 +0100
> @@ -79,31 +79,11 @@
>  
>  int blktap_interface_open(void)
>  {
> -     char *devname;
> -     int ret;
>       int ctlfd;
>  
> -     /* Attach to blktap0 */
> -     if (asprintf(&devname,"%s/%s0", BLKTAP_DEV_DIR, BLKTAP_DEV_NAME) == -1)
> -             goto open_failed;
> -
> -     ret = xc_find_device_number("blktap0");
> -     if (ret < 0) {
> -             DPRINTF("couldn't find device number for 'blktap0'\n");
> -             goto open_failed;
> -     }
> -
> -     blktap_major = major(ret);
> -     make_blktap_dev(devname,blktap_major, 0);
> -
> -     ctlfd = open(devname, O_RDWR);
> -     if (ctlfd == -1) {
> +     ctlfd = open(BLKTAP_DEV_DIR "/" BLKTAP_DEV_NAME "0", O_RDWR);
> +     if (ctlfd == -1)
>               DPRINTF("blktap0 open failed\n");
> -             goto open_failed;
> -     }
>  
>       return ctlfd;
> -
> -open_failed:
> -     return -1;
>  }
> diff -r 2f765c9825b2 -r 2962c9c4d26f tools/libxc/xc_linux.c
> --- a/tools/libxc/xc_linux.c  Tue Jun 01 10:40:06 2010 +0100
> +++ b/tools/libxc/xc_linux.c  Wed Jun 02 10:01:09 2010 +0100
> @@ -316,126 +316,11 @@
>                        (unsigned long)hypercall);
>  }
>  
> -#define MTAB "/proc/mounts"
> -#define MAX_PATH 255
> -#define _STR(x) #x
> -#define STR(x) _STR(x)
> -
> -static int find_sysfsdir(char *sysfsdir)
> -{
> -    FILE *fp;
> -    char type[MAX_PATH + 1];
> -
> -    if ( (fp = fopen(MTAB, "r")) == NULL )
> -        return -1;
> -
> -    while ( fscanf(fp, "%*s %"STR(MAX_PATH)"s %"STR(MAX_PATH)"s %*s %*d 
> %*d\n",
> -                   sysfsdir, type) == 2 )
> -        if ( strncmp(type, "sysfs", 5) == 0 )
> -            break;
> -
> -    fclose(fp);
> -
> -    return ((strncmp(type, "sysfs", 5) == 0) ? 0 : -1);
> -}
> -
> -int xc_find_device_number(const char *name)
> -{
> -    FILE *fp;
> -    int i, major, minor;
> -    char sysfsdir[MAX_PATH + 1];
> -    static char *classlist[] = { "xen", "misc" };
> -
> -    for ( i = 0; i < (sizeof(classlist) / sizeof(classlist[0])); i++ )
> -    {
> -        if ( find_sysfsdir(sysfsdir) < 0 )
> -            goto not_found;
> -
> -        /* <base>/class/<classname>/<devname>/dev */
> -        strncat(sysfsdir, "/class/", MAX_PATH);
> -        strncat(sysfsdir, classlist[i], MAX_PATH);
> -        strncat(sysfsdir, "/", MAX_PATH);
> -        strncat(sysfsdir, name, MAX_PATH);
> -        strncat(sysfsdir, "/dev", MAX_PATH);
> -
> -        if ( (fp = fopen(sysfsdir, "r")) != NULL )
> -            goto found;
> -    }
> -
> - not_found:
> -    errno = -ENOENT;
> -    return -1;
> -
> - found:
> -    if ( fscanf(fp, "%d:%d", &major, &minor) != 2 )
> -    {
> -        fclose(fp);
> -        goto not_found;
> -    }
> -
> -    fclose(fp);
> -
> -    return makedev(major, minor);
> -}
> -
> -#define DEVXEN "/dev/xen"
> -
> -static int make_dev_xen(void)
> -{
> -    if ( mkdir(DEVXEN, 0755) != 0 )
> -    {
> -        struct stat st;
> -        if ( (stat(DEVXEN, &st) != 0) || !S_ISDIR(st.st_mode) )
> -            return -1;
> -    }
> -
> -    return 0;
> -}
> -
> -static int xendev_open(const char *dev)
> -{
> -    int fd, devnum;
> -    struct stat st;
> -    char *devname, *devpath;
> -
> -    devname = devpath = NULL;
> -    fd = -1;
> -
> -    if ( asprintf(&devname, "xen!%s", dev) == 0 )
> -        goto fail;
> -
> -    if ( asprintf(&devpath, "%s/%s", DEVXEN, dev) == 0 )
> -        goto fail;
> -
> -    devnum = xc_find_device_number(dev);
> -    if ( devnum == -1 )
> -        devnum = xc_find_device_number(devname);
> -
> -    /*
> -     * If we know what the correct device is and the path doesn't exist or 
> -     * isn't a device, then remove it so we can create the device.
> -     */
> -    if ( (devnum != -1) &&
> -         ((stat(devpath, &st) != 0) || !S_ISCHR(st.st_mode)) )
> -    {
> -        unlink(devpath);
> -        if ( (make_dev_xen() == -1) ||
> -             (mknod(devpath, S_IFCHR|0600, devnum) != 0) )
> -            goto fail;
> -    }
> -
> -    fd = open(devpath, O_RDWR);
> -
> - fail:
> -    free(devname);
> -    free(devpath);
> -
> -    return fd;
> -}
> +#define DEVXEN "/dev/xen/"
>  
>  int xc_evtchn_open(void)
>  {
> -    return xendev_open("evtchn");
> +    return open(DEVXEN "evtchn", O_RDWR);
>  }
>  
>  int xc_evtchn_close(int xce_handle)
> @@ -551,7 +436,7 @@
>  
>  int xc_gnttab_open(xc_interface *xch)
>  {
> -    return xendev_open("gntdev");
> +    return open(DEVXEN "gntdev", O_RDWR);
>  }
>  
>  int xc_gnttab_close(xc_interface *xch, int xcg_handle)
>
>
>   


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel