Jeremy Fitzhardinge
2010-May-29 01:08 UTC
[Xen-devel] [PATCH] xc: deal with xen/evtchn and xen/gntdev device names
This patch makes xc_linux properly deal with: 1. discovering and creating device nodes if necessary 2. the new form of xen/<dev> device names soon to be used by the kernel This changes the logic slightly: - If a device node already exists with the proper name, then it uses it as-is, assuming it has already been correctly created. - If the path doesn''t exist, or it exists but isn''t a device node, and it has successfully found the major/minor for the device, then (re)create the device node. Since this logic is identical for gntdev and evtchn, make a common function to handle both. Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com> diff -r f146aa75120c tools/libxc/xc_linux.c --- a/tools/libxc/xc_linux.c Thu May 27 12:14:32 2010 -0700 +++ b/tools/libxc/xc_linux.c Fri May 28 16:33:11 2010 -0700 @@ -378,34 +378,73 @@ return makedev(major, minor); } -#define EVTCHN_DEV_NAME "/dev/xen/evtchn" +#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) + goto fail; + + if (mknod(devpath, S_IFCHR|0600, devnum) != 0) { + PERROR("Couldn''t make %s", devpath); + goto fail; + } + } + + fd = open(devpath, O_RDWR); + + if (fd == -1) + PERROR("Could not open %s", devpath); + +fail: + free(devname); + free(devpath); + + return fd; +} + int xc_evtchn_open(void) { - struct stat st; - int fd; - int devnum; - - devnum = xc_find_device_number("evtchn"); - - /* Make sure any existing device file links to correct device. */ - if ( (lstat(EVTCHN_DEV_NAME, &st) != 0) || !S_ISCHR(st.st_mode) || - (st.st_rdev != devnum) ) - (void)unlink(EVTCHN_DEV_NAME); - - reopen: - if ( (fd = open(EVTCHN_DEV_NAME, O_RDWR)) == -1 ) - { - if ( (errno == ENOENT) && - ((mkdir("/dev/xen", 0755) == 0) || (errno == EEXIST)) && - (mknod(EVTCHN_DEV_NAME, S_IFCHR|0600, devnum) == 0) ) - goto reopen; - - PERROR("Could not open event channel interface"); - return -1; - } - - return fd; + return xendev_open("evtchn"); } int xc_evtchn_close(int xce_handle) @@ -519,34 +558,9 @@ errno = saved_errno; } -#define GNTTAB_DEV_NAME "/dev/xen/gntdev" - int xc_gnttab_open(void) { - struct stat st; - int fd; - int devnum; - - devnum = xc_find_device_number("gntdev"); - - /* Make sure any existing device file links to correct device. */ - if ( (lstat(GNTTAB_DEV_NAME, &st) != 0) || !S_ISCHR(st.st_mode) || - (st.st_rdev != devnum) ) - (void)unlink(GNTTAB_DEV_NAME); - -reopen: - if ( (fd = open(GNTTAB_DEV_NAME, O_RDWR)) == -1 ) - { - if ( (errno == ENOENT) && - ((mkdir("/dev/xen", 0755) == 0) || (errno == EEXIST)) && - (mknod(GNTTAB_DEV_NAME, S_IFCHR|0600, devnum) == 0) ) - goto reopen; - - PERROR("Could not open grant table interface"); - return -1; - } - - return fd; + return xendev_open("gntdev"); } int xc_gnttab_close(int xcg_handle) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Bastian Blank
2010-May-29 06:53 UTC
Re: [Xen-devel] [PATCH] xc: deal with xen/evtchn and xen/gntdev device names
On Fri, May 28, 2010 at 06:08:27PM -0700, Jeremy Fitzhardinge wrote:> + if (asprintf(&devname, "xen!%s", dev) == 0) > + if (asprintf(&devpath, "%s/%s", DEVXEN, dev) == 0)asprintf is a glibc extension. Is it really necessary to support long device names for fun or would some 64 byte on the stack be enough?> + devnum = xc_find_device_number(dev); > + if (devnum == -1) > + devnum = xc_find_device_number(devname);Checks should always go from the special ("xen!...") to the gerneral. Also the device is dev_t, not integer.> + /* > + * 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))) {Why should it exist and be something else then a character device? The admin is free to break the system as she likes.> + fd = open(devpath, O_RDWR);Whitespace. Bastian -- ... bacteriological warfare ... hard to believe we were once foolish enough to play around with that. -- McCoy, "The Omega Glory", stardate unknown _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Jun-01 07:56 UTC
Re: [Xen-devel] [PATCH] xc: deal with xen/evtchn and xen/gntdev device names
On Sat, 2010-05-29 at 02:08 +0100, Jeremy Fitzhardinge wrote:> This patch makes xc_linux properly deal with: > 1. discovering and creating device nodes if necessary > 2. the new form of xen/<dev> device names soon to be used by the kernel > > This changes the logic slightly: > - If a device node already exists with the proper name, then it uses it > as-is, assuming it has already been correctly created. > - If the path doesn''t exist, or it exists but isn''t a device node, and > it has successfully found the major/minor for the device, then > (re)create the device node. > > Since this logic is identical for gntdev and evtchn, make a common function > to handle both.Does any of this logic really belong in libxc in the first place? 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. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Bastian Blank
2010-Jun-01 08:17 UTC
Re: [Xen-devel] [PATCH] xc: deal with xen/evtchn and xen/gntdev device names
On Tue, Jun 01, 2010 at 08:56:52AM +0100, Ian Campbell wrote:> Does any of this logic really belong in libxc in the first place?No.> Can we not just rip it all out and make it the distro/platform''s > responsibility to ensure these devices exist and are correct?Thats what I forced in Debian right now.> Perhaps > that might involve shipping some default/example udev rules instead.With the fixed kernel this needs no rules. Even with devtmpfs (some people call it devfs version 2) and no udev it will work then. Bastian -- Vulcans believe peace should not depend on force. -- Amanda, "Journey to Babel", stardate 3842.3 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2010-Jun-01 16:35 UTC
Re: [Xen-devel] [PATCH] xc: deal with xen/evtchn and xen/gntdev device names
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... J _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Jun-02 09:29 UTC
Re: [Xen-devel] [PATCH] xc: deal with xen/evtchn and xen/gntdev device names
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. 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. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> 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@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2010-Jun-02 16:09 UTC
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@citrix.com> > > 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@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Jun-03 08:31 UTC
Re: [Xen-devel] [PATCH] xc: deal with xen/evtchn and xen/gntdev device names
On Wed, 2010-06-02 at 17:09 +0100, Jeremy Fitzhardinge wrote:> On 06/02/2010 02:29 AM, Ian Campbell wrote: > > On Tue, 2010-06-01 at 17:35 +0100, Jeremy Fitzhardinge wrote: > > 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.Is it any more likely to screw them with a new kernel than with an old one? If so I think that''s an argument for propagating the removal of this functionality into stable trees sooner rather than later rather than papering over the craziness for even longer. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2010-Jun-03 19:32 UTC
Re: [Xen-devel] [PATCH] xc: deal with xen/evtchn and xen/gntdev device names
On 06/03/2010 01:31 AM, Ian Campbell wrote:> On Wed, 2010-06-02 at 17:09 +0100, Jeremy Fitzhardinge wrote: > >> On 06/02/2010 02:29 AM, Ian Campbell wrote: >> >>> On Tue, 2010-06-01 at 17:35 +0100, Jeremy Fitzhardinge wrote: >>> 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. >> > Is it any more likely to screw them with a new kernel than with an old > one? >Yeah, because libxc''s rummage around in sysfs will actually work. If we rename the devices to be correct, it won''t find them and it just ends up deleting the old device and either failing to create a new one, or creating it with a bogus major/minor.> If so I think that''s an argument for propagating the removal of this > functionality into stable trees sooner rather than later rather than > papering over the craziness for even longer. >Yep. J _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Jun-04 09:55 UTC
Re: [Xen-devel] [PATCH] xc: deal with xen/evtchn and xen/gntdev device names
On Thu, 2010-06-03 at 20:32 +0100, Jeremy Fitzhardinge wrote:> On 06/03/2010 01:31 AM, Ian Campbell wrote: > > On Wed, 2010-06-02 at 17:09 +0100, Jeremy Fitzhardinge wrote: > > > >> On 06/02/2010 02:29 AM, Ian Campbell wrote: > >> > >>> On Tue, 2010-06-01 at 17:35 +0100, Jeremy Fitzhardinge wrote: > >>> 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. > >> > > Is it any more likely to screw them with a new kernel than with an old > > one? > > > > Yeah, because libxc''s rummage around in sysfs will actually work. If we > rename the devices to be correct, it won''t find them and it just ends up > deleting the old device and either failing to create a new one, or > creating it with a bogus major/minor.That sucks ;-)> > If so I think that''s an argument for propagating the removal of this > > functionality into stable trees sooner rather than later rather than > > papering over the craziness for even longer. > > > > Yep.Keir has applied my patch so I''ll keep an eye out for fallout and suggest it for backporting to the stable branch when it looks like we''ve gotten away with it. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Jun-04 10:01 UTC
Re: [Xen-devel] [PATCH] xc: deal with xen/evtchn and xen/gntdev device names
On 04/06/2010 10:55, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:>>> If so I think that''s an argument for propagating the removal of this >>> functionality into stable trees sooner rather than later rather than >>> papering over the craziness for even longer. > > Keir has applied my patch so I''ll keep an eye out for fallout and > suggest it for backporting to the stable branch when it looks like we''ve > gotten away with it.Any fallout should be dealt with by getting users to set up proper udev rules, I''m sure. I backported to 4.0 for 4.0.1, so I have confidence. ;-) -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Dan Magenheimer
2010-Jun-04 15:27 UTC
RE: [Xen-devel] [PATCH] xc: deal with xen/evtchn and xen/gntdev device names
Since udev magic is something that few developers should need to deal with, would it be too much to ask for a good wiki page to list likely failure conditions and how to solve them BEFORE clueless developers (including me) run into them?> -----Original Message----- > From: Ian Campbell [mailto:Ian.Campbell@citrix.com] > Sent: Friday, June 04, 2010 3:55 AM > To: Jeremy Fitzhardinge > Cc: Bastian Blank; Xen-devel > Subject: Re: [Xen-devel] [PATCH] xc: deal with xen/evtchn and > xen/gntdev device names > > On Thu, 2010-06-03 at 20:32 +0100, Jeremy Fitzhardinge wrote: > > On 06/03/2010 01:31 AM, Ian Campbell wrote: > > > On Wed, 2010-06-02 at 17:09 +0100, Jeremy Fitzhardinge wrote: > > > > > >> On 06/02/2010 02:29 AM, Ian Campbell wrote: > > >> > > >>> On Tue, 2010-06-01 at 17:35 +0100, Jeremy Fitzhardinge wrote: > > >>> 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. > > >> > > > Is it any more likely to screw them with a new kernel than with an > old > > > one? > > > > > > > Yeah, because libxc''s rummage around in sysfs will actually work. If > we > > rename the devices to be correct, it won''t find them and it just ends > up > > deleting the old device and either failing to create a new one, or > > creating it with a bogus major/minor. > > That sucks ;-) > > > > If so I think that''s an argument for propagating the removal of > this > > > functionality into stable trees sooner rather than later rather > than > > > papering over the craziness for even longer. > > > > > > > Yep. > > Keir has applied my patch so I''ll keep an eye out for fallout and > suggest it for backporting to the stable branch when it looks like > we''ve > gotten away with it. > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Jun-04 15:31 UTC
RE: [Xen-devel] [PATCH] xc: deal with xen/evtchn and xen/gntdev device names
On Fri, 2010-06-04 at 16:27 +0100, Dan Magenheimer wrote:> Since udev magic is something that few developers should > need to deal with, would it be too much to ask for a good > wiki page to list likely failure conditions and how to > solve them BEFORE clueless developers (including me) run > into them?The expectation is that things will mostly Just Work. If this turns out not to be the case then the wiki would be an appropriate place to document the issues and solutions as we determine what they are. Ian.> > > -----Original Message----- > > From: Ian Campbell [mailto:Ian.Campbell@citrix.com] > > Sent: Friday, June 04, 2010 3:55 AM > > To: Jeremy Fitzhardinge > > Cc: Bastian Blank; Xen-devel > > Subject: Re: [Xen-devel] [PATCH] xc: deal with xen/evtchn and > > xen/gntdev device names > > > > On Thu, 2010-06-03 at 20:32 +0100, Jeremy Fitzhardinge wrote: > > > On 06/03/2010 01:31 AM, Ian Campbell wrote: > > > > On Wed, 2010-06-02 at 17:09 +0100, Jeremy Fitzhardinge wrote: > > > > > > > >> On 06/02/2010 02:29 AM, Ian Campbell wrote: > > > >> > > > >>> On Tue, 2010-06-01 at 17:35 +0100, Jeremy Fitzhardinge wrote: > > > >>> 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. > > > >> > > > > Is it any more likely to screw them with a new kernel than with an > > old > > > > one? > > > > > > > > > > Yeah, because libxc''s rummage around in sysfs will actually work. If > > we > > > rename the devices to be correct, it won''t find them and it just ends > > up > > > deleting the old device and either failing to create a new one, or > > > creating it with a bogus major/minor. > > > > That sucks ;-) > > > > > > If so I think that''s an argument for propagating the removal of > > this > > > > functionality into stable trees sooner rather than later rather > > than > > > > papering over the craziness for even longer. > > > > > > > > > > Yep. > > > > Keir has applied my patch so I''ll keep an eye out for fallout and > > suggest it for backporting to the stable branch when it looks like > > we''ve > > gotten away with it. > > > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xensource.com > > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel