From mboxrd@z Thu Jan 1 00:00:00 1970 From: Roy Marples Date: Fri, 04 Aug 2006 23:32:40 +0000 Subject: Re: [PATCH] Only lookup uid/gid when applying rules Message-Id: <200608050032.41425.uberlord@gentoo.org> MIME-Version: 1 Content-Type: multipart/mixed; boundary="Boundary-00=_Zk90ECYvtXuen3u" List-Id: References: <200608011135.25769.uberlord@gentoo.org> In-Reply-To: <200608011135.25769.uberlord@gentoo.org> To: linux-hotplug@vger.kernel.org --Boundary-00=_Zk90ECYvtXuen3u Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline 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 Gentoo/Linux Developer (baselayout, networking) --Boundary-00=_Zk90ECYvtXuen3u Content-Type: text/x-diff; charset="iso-8859-15"; name="udev-lookup.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="udev-lookup.patch" 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) { --Boundary-00=_Zk90ECYvtXuen3u Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline ------------------------------------------------------------------------- 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 --Boundary-00=_Zk90ECYvtXuen3u Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ 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 --Boundary-00=_Zk90ECYvtXuen3u--