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
|
Home