Re: [PATCH] Only lookup uid/gid when applying rules

[Roy Marples <uberlord@gentoo.org>]

[-- Attachment #1: Type: text/plain, Size: 1084 bytes --]

On Tuesday 01 August 2006 13:35, you wrote:
> On Tue, 2006-08-01 at 11:35 +0100, Roy Marples wrote:
> > Attached is a patch that stops udev from doing a uid/gid lookup unless it
> > is actually going to use the rule.
>
> Nope, that doesn't really help, we want to to resolve the names once on
> udevd startup and not from every event process again and again.

OK, this patch again removes the uid/gid lookup when loading rules as it's the 
root cause of the problem. I also don't see a flag being of any benefit.

However, this patch stores the looked up uid/gid back in the rule for future 
event processes on the same rule. So, this should also make us more efficient 
by only looking up the uid/gid for each rule when it's need, but only once 
per rule.

Hopefully this satisfies your criteria :)

NOTE: I will be unable to test this against an LDAP server with the actual 
issue in question until Monday/tuesday as it's a work server, but I thought 
I'd post it here still for review.

Thanks

-- 
Roy Marples <uberlord@gentoo.org>
Gentoo/Linux Developer (baselayout, networking)

[-- Attachment #2: udev-lookup.patch --]
[-- Type: text/x-diff, Size: 4831 bytes --]

diff -u udev-096.orig/udev.c udev-096/udev.c
--- udev-096.orig/udev.c	2006-08-05 00:16:20.000000000 +0100
+++ udev-096/udev.c	2006-08-05 00:17:11.000000000 +0100
@@ -128,7 +128,7 @@
 	}
 
 	sysfs_init();
-	udev_rules_init(&rules, 0);
+	udev_rules_init(&rules);
 
 	dev = sysfs_device_get(devpath);
 	if (dev == NULL) {
diff -u udev-096.orig/udevd.c udev-096/udevd.c
--- udev-096.orig/udevd.c	2006-08-05 00:16:20.000000000 +0100
+++ udev-096/udevd.c	2006-08-05 00:17:11.000000000 +0100
@@ -908,7 +908,7 @@
 
 	/* parse the rules and keep it in memory */
 	sysfs_init();
-	udev_rules_init(&rules, 1);
+	udev_rules_init(&rules);
 
 	export_initial_seqnum();
 
@@ -1088,7 +1088,7 @@
 		if (reload_config) {
 			reload_config = 0;
 			udev_rules_cleanup(&rules);
-			udev_rules_init(&rules, 1);
+			udev_rules_init(&rules);
 		}
 
 		/* forked child has returned */
diff -u udev-096.orig/udev_device.c udev-096/udev_device.c
--- udev-096.orig/udev_device.c	2006-08-05 00:16:20.000000000 +0100
+++ udev-096/udev_device.c	2006-08-05 00:17:11.000000000 +0100
@@ -162,6 +162,27 @@
 			goto exit;
 		}
 
+		/* If we don't have owner and group id's, look them up now and
+		   store them back in the rule for other devices. */
+		unsigned long id;
+		char *endptr;
+		uid_t uid = 0;
+		gid_t gid = 0;
+
+		id = strtoul(udev->owner, &endptr, 10);
+		if (endptr[0] != '\0') {
+			if (strcmp(udev->owner, "root") != 0)
+				uid = lookup_user(udev->owner);
+			sprintf(udev->owner, "%u", (unsigned int) uid);
+		}
+
+		id = strtoul(udev->group, &endptr, 10);
+		if (endptr[0] != '\0') {
+			if (strcmp(udev->group, "root") != 0)
+				gid = lookup_group(udev->group);
+			sprintf(udev->group, "%u", (unsigned int) gid);
+		}
+
 		/* read current database entry, we may want to cleanup symlinks */
 		udev_old = udev_device_init();
 		if (udev_old != NULL) {
diff -u udev-096.orig/udev_rules.h udev-096/udev_rules.h
--- udev-096.orig/udev_rules.h	2006-08-05 00:16:20.000000000 +0100
+++ udev-096/udev_rules.h	2006-08-05 00:17:11.000000000 +0100
@@ -98,10 +98,9 @@
 	char *buf;
 	size_t bufsize;
 	size_t current;
-	int resolve_names;
 };
 
-extern int udev_rules_init(struct udev_rules *rules, int resolve_names);
+extern int udev_rules_init(struct udev_rules *rules);
 extern void udev_rules_cleanup(struct udev_rules *rules);
 
 extern void udev_rules_iter_init(struct udev_rules *rules);
diff -u udev-096.orig/udev_rules_parse.c udev-096/udev_rules_parse.c
--- udev-096.orig/udev_rules_parse.c	2006-08-05 00:16:20.000000000 +0100
+++ udev-096/udev_rules_parse.c	2006-08-05 00:17:11.000000000 +0100
@@ -473,38 +473,12 @@
 
 		if (strcasecmp(key, "OWNER") == 0) {
 			valid = 1;
-			if (rules->resolve_names && (!strchr(value, '$') && !strchr(value, '%'))) {
-				char *endptr;
-				strtoul(value, &endptr, 10);
-				if (endptr[0] != '\0') {
-					char owner[32];
-					uid_t uid = lookup_user(value);
-					dbg("replacing username='%s' by id=%i", value, uid);
-					sprintf(owner, "%u", (unsigned int) uid);
-					add_rule_key(rule, &rule->owner, operation, owner);
-					continue;
-				}
-			}
-
 			add_rule_key(rule, &rule->owner, operation, value);
 			continue;
 		}
 
 		if (strcasecmp(key, "GROUP") == 0) {
 			valid = 1;
-			if (rules->resolve_names && (!strchr(value, '$') && !strchr(value, '%'))) {
-				char *endptr;
-				strtoul(value, &endptr, 10);
-				if (endptr[0] != '\0') {
-					char group[32];
-					gid_t gid = lookup_group(value);
-					dbg("replacing groupname='%s' by id=%i", value, gid);
-					sprintf(group, "%u", (unsigned int) gid);
-					add_rule_key(rule, &rule->group, operation, group);
-					continue;
-				}
-			}
-
 			add_rule_key(rule, &rule->group, operation, value);
 			continue;
 		}
@@ -637,13 +611,12 @@
 	return retval;
 }
 
-int udev_rules_init(struct udev_rules *rules, int resolve_names)
+int udev_rules_init(struct udev_rules *rules)
 {
 	struct stat stats;
 	int retval;
 
 	memset(rules, 0x00, sizeof(struct udev_rules));
-	rules->resolve_names = resolve_names;
 
 	/* parse rules file or all matching files in directory */
 	if (stat(udev_rules_filename, &stats) != 0)
diff -u udev-096.orig/udevstart.c udev-096/udevstart.c
--- udev-096.orig/udevstart.c	2006-08-05 00:16:20.000000000 +0100
+++ udev-096/udevstart.c	2006-08-05 00:17:11.000000000 +0100
@@ -361,7 +361,7 @@
 	alarm(UDEV_ALARM_TIMEOUT);
 
 	sysfs_init();
-	udev_rules_init(&rules, 1);
+	udev_rules_init(&rules);
 
 	udev_scan_class(&device_list);
 	udev_scan_block(&device_list);
diff -u udev-096.orig/udevtest.c udev-096/udevtest.c
--- udev-096.orig/udevtest.c	2006-08-05 00:16:20.000000000 +0100
+++ udev-096/udevtest.c	2006-08-05 00:17:11.000000000 +0100
@@ -83,7 +83,7 @@
 			devpath = argv[1];
 
 	sysfs_init();
-	udev_rules_init(&rules, 0);
+	udev_rules_init(&rules);
 
 	dev = sysfs_device_get(devpath);
 	if (dev == NULL) {

[-- Attachment #3: Type: text/plain, Size: 348 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 #4: 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