[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] xc: deal with xen/evtchn and xen/gntdev device names
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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |