udev patch

udev patch

[Stefanos Harhalakis]

[-- Attachment #1.1.1: Type: text/plain, Size: 1107 bytes --]

Hi there,

  Currently udev does not reread changed rule files when using symlinks. 
Debian uses the symlink approach resulting to something like this:

 020_permissions.rules -> ../permissions.rules
 025_libgphoto2.rules -> ../libgphoto2.rules

  Because udev uses inotify on the rules directory it cannot detect changes to 
the destination file of a link. The attached patch solves this problem by 
using inotify on the directory where each symlink destination resides.

  I've tried using inotify on the destination filenames but this doesn't work 
as expected. For example vi will unlink and recreate the files. This means 
that the symlink will not point anywere for some time which is enough for 
udev to loose the destination. After that the link needs to be rechecked 
which is not possible (and efficient).

  Please consider the attached patch since it removes the need  
of 'udevcontrol reload_rules' when working with symlinks. 

  If I have to set a license for the patch the it is GPLv2.

Thanks in advance
Harhalakis Stefanos

p.s. CC replies
p.s.2. Be gentle :-)

[-- Attachment #1.1.2: patch-inotify_links.diff --]
[-- Type: text/x-diff, Size: 3852 bytes --]

diff -ru udev-103.orig/udevd.c udev-103/udevd.c
--- udev-103.orig/udevd.c	2006-10-20 15:43:35.000000000 +0300
+++ udev-103/udevd.c	2006-12-09 19:42:09.432522000 +0200
@@ -929,6 +929,100 @@
 	}
 }
 
+static void check_link(const char *filename)
+{
+	struct stat	stats;
+	struct stat	lstats;
+	char		fn[PATH_SIZE+1];
+	char		fn2[PATH_SIZE*2+3];
+	char		*p;
+	ssize_t		sz;
+
+	if (lstat(filename, &lstats) != 0)
+	{
+		err("could not read '%s': %s", filename, strerror(errno));
+		return;
+	}
+
+	/* Check if it is a link */
+	if (!S_ISLNK(lstats.st_mode))
+		return;
+
+	/* Get the destination */
+	dbg("found link: %s", filename);
+	sz=readlink(filename, fn, PATH_SIZE);
+	if (sz>=PATH_SIZE)
+	{
+		err("oversized link destination of '%s'", filename);
+		return;
+	}
+
+	fn[sz]=0;
+
+	/* Form the full destination path */
+	if (fn[0]=='/')
+	{
+		strcpy(fn2, fn);
+	} else
+	{
+		snprintf(fn2, PATH_SIZE*2+2, "%s/%s", udev_rules_filename, fn);
+		fn2[PATH_SIZE*2+2]=0;
+	}
+
+	/* Check if destination if a file */
+	stat(fn2, &stats);
+	if (!S_ISREG(stats.st_mode))
+		return;
+
+	/* Find the parent directory */
+	p=fn2+strlen(fn2);
+
+	while (p>=fn2)
+	{
+		if (*p=='/')
+		{
+			*p=0;
+			break;
+		}
+		p--;
+	}
+
+	if (p==fn2)
+		err("failed to get parrent directory of '%s'", fn2);
+
+	dbg("adding notify for:'%s'", fn2);
+	inotify_add_watch(inotify_fd, fn2, IN_CREATE | IN_DELETE | IN_MOVE | IN_CLOSE_WRITE);
+}
+
+static void start_inotify(void)
+{
+	struct name_entry	*name_loop, *name_tmp;
+
+	/* watch rules directory */
+	inotify_fd = inotify_init();
+	if (inotify_fd >= 0)
+	{
+		inotify_add_watch(inotify_fd, udev_rules_filename, IN_CREATE | IN_DELETE | IN_MOVE | IN_CLOSE_WRITE);
+	} else if (errno == ENOSYS)
+	{
+		err("the kernel does not support inotify, udevd can't monitor configuration file changes");
+		return;
+	} else
+	{
+		err("inotify_init failed: %s", strerror(errno));
+		return;
+	}
+
+	LIST_HEAD(name_list);
+
+	add_matching_files(&name_list, udev_rules_filename, RULEFILE_SUFFIX);
+	list_for_each_entry_safe(name_loop, name_tmp, &name_list, node) {
+		check_link(name_loop->name);
+		list_del(&name_loop->node);
+		free(name_loop);
+	}
+}
+
 int main(int argc, char *argv[], char *envp[])
 {
 	int retval;
@@ -1101,14 +1195,7 @@
 	sigaction(SIGCHLD, &act, NULL);
 	sigaction(SIGHUP, &act, NULL);
 
-	/* watch rules directory */
-	inotify_fd = inotify_init();
-	if (inotify_fd >= 0)
-		inotify_add_watch(inotify_fd, udev_rules_filename, IN_CREATE | IN_DELETE | IN_MOVE | IN_CLOSE_WRITE);
-	else if (errno == ENOSYS)
-		err("the kernel does not support inotify, udevd can't monitor configuration file changes");
-	else
-		err("inotify_init failed: %s", strerror(errno));
+	start_inotify();
 
 	/* maximum limit of forked childs */
 	value = getenv("UDEVD_MAX_CHILDS");
@@ -1145,15 +1232,15 @@
 	if (debug_trace)
 		putenv("DEBUG=1");
 
-	maxfd = udevd_sock;
-	maxfd = UDEV_MAX(maxfd, uevent_netlink_sock);
-	maxfd = UDEV_MAX(maxfd, signal_pipe[READ_END]);
-	maxfd = UDEV_MAX(maxfd, inotify_fd);
-
 	while (!udev_exit) {
 		struct udevd_uevent_msg *msg;
 		int fdcount;
 
+		maxfd = udevd_sock;
+		maxfd = UDEV_MAX(maxfd, uevent_netlink_sock);
+		maxfd = UDEV_MAX(maxfd, signal_pipe[READ_END]);
+		maxfd = UDEV_MAX(maxfd, inotify_fd);
+
 		FD_ZERO(&readfds);
 		FD_SET(signal_pipe[READ_END], &readfds);
 		FD_SET(udevd_sock, &readfds);
@@ -1209,6 +1296,12 @@
 		/* rules changed, set by inotify or a HUP signal */
 		if (reload_config) {
 			reload_config = 0;
+			/* Restart inotify to handle possible file changes */
+			if (inotify_fd>0)
+			{
+				close(inotify_fd);
+				start_inotify();
+			}
 			udev_rules_cleanup(&rules);
 			udev_rules_init(&rules, 1);
 		}

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 347 bytes --]

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[-- Attachment #3: Type: text/plain, Size: 226 bytes --]

_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: udev patch

[Bryan Kadzban]

[-- Attachment #1.1: Type: text/plain, Size: 666 bytes --]

Stefanos Harhalakis wrote:
> Currently udev does not reread changed rule files when using
> symlinks. Debian uses the symlink approach

I have often wondered why Debian uses symlinks for the rules files.  Is
it just for compatibility with an ancient version of udev, that put the
rules somewhere else?  Or is it something with package upgrades, to
handle it better when users decide to remove rules files?  (If that's
the case, then could the normal dpkg config-file handling -- or anything
that wouldn't require the files to be kept in two places -- work?)  Or
is it something else?

Or has this already been discussed to death on Debian lists?  ;-)


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

[-- Attachment #2: Type: text/plain, Size: 347 bytes --]

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[-- Attachment #3: Type: text/plain, Size: 226 bytes --]

_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: udev patch

[Stefanos Harhalakis]

[-- Attachment #1.1: Type: text/plain, Size: 2401 bytes --]

On Saturday 09 December 2006 21:42, Bryan Kadzban wrote:
> Stefanos Harhalakis wrote:
> > Currently udev does not reread changed rule files when using
> > symlinks. Debian uses the symlink approach
>
> I have often wondered why Debian uses symlinks for the rules files.  Is
> it just for compatibility with an ancient version of udev, that put the
> rules somewhere else?  Or is it something with package upgrades, to
> handle it better when users decide to remove rules files?  (If that's
> the case, then could the normal dpkg config-file handling -- or anything
> that wouldn't require the files to be kept in two places -- work?)  Or
> is it something else?
>
> Or has this already been discussed to death on Debian lists?  ;-)

  I'm not a debian developer, maintainer or anything more than a debian user, 
so I'll just copy & paste a part from the README.Debian of the udev package:

-----
MAINTAINERS BEWARE: the use of /etc/udev/rules.d/ by other packages is
discouraged, except when only RUN rules are added.
If you think your package needs to create a file there, then please
contact the udev package maintainer and explain your needs.
Packages should NEVER create files in /etc/udev/rules.d/, but create a
symlink the first time the package is installed (and never try again, to
allow the local system administrator to remove it).
-----

  A mail that illustrates this can be found at:
http://lists.alioth.debian.org/pipermail/pkg-alsa-devel/2006-March/003158.html

  There is a debhelper for udev rules called 'dh_installudev' which says:

-----
DESCRIPTION
       dh_installudev is a debhelper program that is responsible for
       installing udev rules files.

       Files named debian/package.udev will be installed as etc/udev/pack‐
       age.rules in the package build directory.

       Then postinst and postrm commands are automatically generated to enable
       the rules file when the package is first installed, by creating a sym‐
       link to it in the /etc/udev/rules.d/ directory. See dh_installdeb(1)
       for an explanation of how this works.
-----

  So I believe that this matches both of your answers, about better 
removal/addition and about the long discussion :-)

  I've CC'd this to Marco d'Itri <md@linux.it>, which is the debian udev 
package maintainer and may answer your questions in more depth.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 347 bytes --]

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[-- Attachment #3: Type: text/plain, Size: 226 bytes --]

_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: udev patch

[Bryan Kadzban]

[-- Attachment #1.1: Type: text/plain, Size: 1245 bytes --]

Stefanos Harhalakis wrote:
> Packages should NEVER create files in /etc/udev/rules.d/, but create
> a symlink the first time the package is installed (and never try
> again, to allow the local system administrator to remove it).

Shouldn't this apply to any config directory for any package, e.g.
Debian's Apache package's /etc/apache/{mods,sites}-enabled?  Yet
at least one time that I upgraded Apache, both of those directories were
reset, which broke the web server setup that I had.  (I believe this was
the upgrade from Apache 2.0 to 2.2, but I'm not sure on that.)

But regardless of that, here's another solution that might allow both:
Maybe you could have the sysadmin rename the files they want to disable
(to e.g. package.rules.off) so that udev ignores them, and then have the
dh_installudev script treat these files like removed symlinks.  Then at
least inotify would work, without having to lstat the files and figure
out which directory their target is in.  (Especially when the target is
itself a symlink, etc., etc...)  Maybe that's not possible, though.

> So I believe that this matches both of your answers, about better 
> removal/addition and about the long discussion :-)

It does, thank you.  :-)


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

[-- Attachment #2: Type: text/plain, Size: 347 bytes --]

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[-- Attachment #3: Type: text/plain, Size: 226 bytes --]

_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: udev patch

[Marco d'Itri]

[-- Attachment #1.1: Type: text/plain, Size: 934 bytes --]

On Dec 09, Bryan Kadzban <bryan@kadzban.is-a-geek.net> wrote:

> I have often wondered why Debian uses symlinks for the rules files.  Is
> it just for compatibility with an ancient version of udev, that put the
> rules somewhere else?  Or is it something with package upgrades, to
> handle it better when users decide to remove rules files?  (If that's
> the case, then could the normal dpkg config-file handling -- or anything
> that wouldn't require the files to be kept in two places -- work?)  Or
> is it something else?
Conffiles handling is suboptimal when a file is deleted.
*Some* symlinks are needed, to choose the /dev layout, while other
nowadays could probably become simple files.
In the beginning I tried to have something as flexible as possible to
account for future changes, after the release I will try to remove some
complexity.

Anyway, I would like to see this patch merged.

-- 
ciao,
Marco

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 347 bytes --]

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[-- Attachment #3: Type: text/plain, Size: 226 bytes --]

_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel