From: Kay Sievers Date: Wed, 9 Sep 2009 16:18:17 +0000 (+0200) Subject: simplify "symlink name stack" X-Git-Tag: 174~777 X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=6c29f2b942358d4dd9d3e7c65c13c3612dded3cc simplify "symlink name stack" With well defined and kernel-supplied node names, we no longer need to support a possible stack of conflicting symlinks and node names. Only symlinks with identical names can be claimed by multiple devices. This shrinks the former /dev/.udev/names/ significantly. Also the /dev/{block,char}/MAJ:MIN" links are excluded from the name stack - they are unique and can not conflict. --- diff --git a/NEWS b/NEWS index dcbf3a66b..ff6d93205 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,10 @@ Please do not port anything to the new format again, everything in /dev/.udev is and always was private to udev, and may and will change any time without prior notice. +Multiple devices claiming the same names in /dev are limited to symlinks +only now. Mixing identical symlink names and node names is not supported. +This reduces the amount of data in the database significantly. + NAME="%k" causes a warning now. It's is and always was completely superfluous. It will break kernel supplied DEVNAMEs and therefore it needs to be removed from all rules. diff --git a/TODO b/TODO index 05b763efa..e1c06d5c6 100644 --- a/TODO +++ b/TODO @@ -1,11 +1,4 @@ - o drop support for node names in name stack, support only symlinks - With well defined and kernel-supplied node names, we no longer need - to support a possible stack of conflicting symlinks and node names. - From there on, only symlinks with identical names can be claimed - by multiple devices. It will simplify the logic a lot and shrink - /dev/.udev/names/ significantly. Also exclude "*/MAJ:MIN" link names - from the name stack, they can not conflict. o remove most NAME= rules (they are provided by the 2.6.31 kernel) o convert firmware.sh to C o "udevadm control" commands will only accept the -- syntax diff --git a/libudev/libudev-device.c b/libudev/libudev-device.c index 22d834927..5d1ad9f22 100644 --- a/libudev/libudev-device.c +++ b/libudev/libudev-device.c @@ -123,7 +123,7 @@ int udev_device_read_db(struct udev_device *udev_device) next = &next[1]; } util_strscpyl(devlink, sizeof(devlink), udev_get_dev_path(udev_device->udev), "/", lnk, NULL); - udev_device_add_devlink(udev_device, devlink); + udev_device_add_devlink(udev_device, devlink, 0); } info(udev_device->udev, "device %p filled with db symlink data '%s'\n", udev_device, udev_device->devnode); return 0; @@ -150,7 +150,7 @@ int udev_device_read_db(struct udev_device *udev_device) break; case 'S': util_strscpyl(filename, sizeof(filename), udev_get_dev_path(udev_device->udev), "/", val, NULL); - udev_device_add_devlink(udev_device, filename); + udev_device_add_devlink(udev_device, filename, 0); break; case 'L': udev_device_set_devlink_priority(udev_device, atoi(val)); @@ -1118,11 +1118,16 @@ int udev_device_set_devnode(struct udev_device *udev_device, const char *devnode return 0; } -int udev_device_add_devlink(struct udev_device *udev_device, const char *devlink) +int udev_device_add_devlink(struct udev_device *udev_device, const char *devlink, int unique) { + struct udev_list_entry *list_entry; + udev_device->devlinks_uptodate = 0; - if (udev_list_entry_add(udev_device->udev, &udev_device->devlinks_list, devlink, NULL, 1, 0) == NULL) + list_entry = udev_list_entry_add(udev_device->udev, &udev_device->devlinks_list, devlink, NULL, 1, 0); + if (list_entry == NULL) return -ENOMEM; + if (unique) + udev_list_entry_set_flag(list_entry, 1); return 0; } diff --git a/libudev/libudev-monitor.c b/libudev/libudev-monitor.c index 657f23d72..ee855afac 100644 --- a/libudev/libudev-monitor.c +++ b/libudev/libudev-monitor.c @@ -628,12 +628,12 @@ retry: next = strchr(slink, ' '); while (next != NULL) { next[0] = '\0'; - udev_device_add_devlink(udev_device, slink); + udev_device_add_devlink(udev_device, slink, 0); slink = &next[1]; next = strchr(slink, ' '); } if (slink[0] != '\0') - udev_device_add_devlink(udev_device, slink); + udev_device_add_devlink(udev_device, slink, 0); } else if (strncmp(key, "DRIVER=", 7) == 0) { udev_device_set_driver(udev_device, &key[7]); } else if (strncmp(key, "ACTION=", 7) == 0) { diff --git a/libudev/libudev-private.h b/libudev/libudev-private.h index e90c79cb3..285b9d48c 100644 --- a/libudev/libudev-private.h +++ b/libudev/libudev-private.h @@ -69,7 +69,7 @@ int udev_device_set_syspath(struct udev_device *udev_device, const char *syspath int udev_device_set_subsystem(struct udev_device *udev_device, const char *subsystem); int udev_device_set_devtype(struct udev_device *udev_device, const char *devtype); int udev_device_set_devnode(struct udev_device *udev_device, const char *devnode); -int udev_device_add_devlink(struct udev_device *udev_device, const char *devlink); +int udev_device_add_devlink(struct udev_device *udev_device, const char *devlink, int unique); void udev_device_cleanup_devlinks_list(struct udev_device *udev_device); struct udev_list_entry *udev_device_add_property(struct udev_device *udev_device, const char *key, const char *value); struct udev_list_entry *udev_device_add_property_from_string(struct udev_device *udev_device, const char *property); diff --git a/rules/rules.d/50-udev-default.rules b/rules/rules.d/50-udev-default.rules index 4e9a73751..200e2b4e5 100644 --- a/rules/rules.d/50-udev-default.rules +++ b/rules/rules.d/50-udev-default.rules @@ -1,7 +1,7 @@ # do not edit this file, it will be overwritten on update -SUBSYSTEM=="block", SYMLINK+="block/%M:%m" -SUBSYSTEM!="block", SYMLINK+="char/%M:%m" +SUBSYSTEM=="block", SYMLINK{unique}+="block/%M:%m" +SUBSYSTEM!="block", SYMLINK{unique}+="char/%M:%m" KERNEL=="pty[pqrstuvwxyzabcdef][0123456789abcdef]", GROUP="tty", MODE="0660" KERNEL=="tty[pqrstuvwxyzabcdef][0123456789abcdef]", GROUP="tty", MODE="0660" diff --git a/udev/udev-event.c b/udev/udev-event.c index 7b4e4ac5c..701c06182 100644 --- a/udev/udev-event.c +++ b/udev/udev-event.c @@ -688,9 +688,9 @@ exit_add: char devnode[UTIL_PATH_SIZE]; info(event->udev, "'%s' not found in database, using kernel name '%s'\n", - udev_device_get_syspath(dev), udev_device_get_sysname(dev)); + udev_device_get_syspath(dev), udev_device_get_knodename(dev)); util_strscpyl(devnode, sizeof(devnode), - udev_get_dev_path(event->udev), "/", udev_device_get_sysname(dev), NULL); + udev_get_dev_path(event->udev), "/", udev_device_get_knodename(dev), NULL); udev_device_set_devnode(dev, devnode); } diff --git a/udev/udev-node.c b/udev/udev-node.c index 36f6f6d99..f17c48a7b 100644 --- a/udev/udev-node.c +++ b/udev/udev-node.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2003-2008 Kay Sievers + * Copyright (C) 2003-2009 Kay Sievers * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -31,29 +32,6 @@ #define TMP_FILE_EXT ".udev-tmp" -/* reverse mapping from the device file name to the devpath */ -static int name_index(struct udev_device *dev, const char *name, int add) -{ - struct udev *udev = udev_device_get_udev(dev); - char name_enc[UTIL_PATH_SIZE]; - char filename[UTIL_PATH_SIZE * 2]; - - util_path_encode(&name[strlen(udev_get_dev_path(udev))+1], name_enc, sizeof(name_enc)); - snprintf(filename, sizeof(filename), "%s/.udev/names/%s/%u:%u", udev_get_dev_path(udev), name_enc, - major(udev_device_get_devnum(dev)), minor(udev_device_get_devnum(dev))); - - if (add) { - dbg(udev, "creating index: '%s'\n", filename); - util_create_path(udev, filename); - symlink(udev_device_get_devpath(dev), filename); - } else { - dbg(udev, "removing index: '%s'\n", filename); - unlink(filename); - util_delete_path(udev, filename); - } - return 0; -} - int udev_node_mknod(struct udev_device *dev, const char *file, dev_t devnum, mode_t mode, uid_t uid, gid_t gid) { struct udev *udev = udev_device_get_udev(dev); @@ -168,7 +146,7 @@ static int node_symlink(struct udev *udev, const char *node, const char *slink) info(udev, "found existing node instead of symlink '%s'\n", slink); if (lstat(node, &stats2) == 0) { if ((stats.st_mode & S_IFMT) == (stats2.st_mode & S_IFMT) && - stats.st_rdev == stats2.st_rdev) { + stats.st_rdev == stats2.st_rdev && stats.st_ino != stats2.st_ino) { info(udev, "replace device node '%s' with symlink to our node '%s'\n", slink, node); } else { @@ -222,143 +200,115 @@ exit: return err; } -static int name_index_get_devices(struct udev *udev, const char *name, struct udev_list_node *dev_list) +/* find device node of device with highest priority */ +static const char *link_find_prioritized(struct udev_device *dev, bool add, const char *stackdir, char *buf, size_t bufsize) { - char dirname[UTIL_PATH_SIZE]; - char *s; - size_t l; + struct udev *udev = udev_device_get_udev(dev); DIR *dir; - int count = 0; - - s = dirname; - l = util_strpcpyl(&s, sizeof(dirname), udev_get_dev_path(udev), - "/.udev/names/", NULL); - util_path_encode(&name[strlen(udev_get_dev_path(udev))+1], s, l); - dir = opendir(dirname); - if (dir == NULL) { - dbg(udev, "no index directory '%s': %m\n", dirname); - count = -1; - goto out; + int priority = 0; + const char *target = NULL; + + if (add) { + priority = udev_device_get_devlink_priority(dev); + util_strscpy(buf, bufsize, udev_device_get_devnode(dev)); + target = buf; } - dbg(udev, "found index directory '%s'\n", dirname); - while (1) { + dir = opendir(stackdir); + if (dir == NULL) + return target; + for (;;) { + struct udev_device *dev_db; struct dirent *dent; char devpath[UTIL_PATH_SIZE]; char syspath[UTIL_PATH_SIZE]; - int len; + ssize_t len; dent = readdir(dir); if (dent == NULL || dent->d_name[0] == '\0') break; if (dent->d_name[0] == '.') continue; - + dbg(udev, "found '%s/%s'\n", stackdir, dent->d_name); len = readlinkat(dirfd(dir), dent->d_name, devpath, sizeof(devpath)); - if (len < 0 || (size_t)len >= sizeof(devpath)) + if (len <= 0 || len == (ssize_t)sizeof(devpath)) continue; devpath[len] = '\0'; util_strscpyl(syspath, sizeof(syspath), udev_get_sys_path(udev), devpath, NULL); - udev_list_entry_add(udev, dev_list, syspath, NULL, 1, 0); - count++; + info(udev, "found '%s' claiming '%s'\n", syspath, stackdir); + + /* did we find ourself? */ + if (strcmp(udev_device_get_syspath(dev), syspath) == 0) + continue; + + dev_db = udev_device_new_from_syspath(udev, syspath); + if (dev_db != NULL) { + const char *devnode; + + devnode = udev_device_get_devnode(dev_db); + if (devnode != NULL) { + dbg(udev, "compare priority of '%s'(%i) > '%s'(%i)\n", target, priority, + udev_device_get_devnode(dev_db), udev_device_get_devlink_priority(dev_db)); + if (target == NULL || udev_device_get_devlink_priority(dev_db) > priority) { + info(udev, "'%s' claims priority %i for '%s'\n", + syspath, udev_device_get_devlink_priority(dev_db), stackdir); + priority = udev_device_get_devlink_priority(dev_db); + util_strscpy(buf, bufsize, devnode); + target = buf; + } + } + udev_device_unref(dev_db); + } } - closedir(dir); -out: - return count; + return target; } -static int update_link(struct udev_device *dev, const char *slink) +/* manage "stack of names" with possibly specified device priorities */ +static void link_update(struct udev_device *dev, const char *slink, bool add) { struct udev *udev = udev_device_get_udev(dev); - struct udev_list_node dev_list; - struct udev_list_entry *dev_entry; - char target[UTIL_PATH_SIZE]; - int count; - int priority = 0; - int rc = 0; + char name_enc[UTIL_PATH_SIZE]; + char filename[UTIL_PATH_SIZE * 2]; + char dirname[UTIL_PATH_SIZE]; + const char *target; + char buf[UTIL_PATH_SIZE]; dbg(udev, "update symlink '%s' of '%s'\n", slink, udev_device_get_syspath(dev)); - udev_list_init(&dev_list); - count = name_index_get_devices(udev, slink, &dev_list); - if (count > 1) - info(udev, "found %i devices with name '%s'\n", count, slink); + util_path_encode(&slink[strlen(udev_get_dev_path(udev))+1], name_enc, sizeof(name_enc)); + snprintf(dirname, sizeof(dirname), "%s/.udev/links/%s", udev_get_dev_path(udev), name_enc); + snprintf(filename, sizeof(filename), "%s/%c%u:%u", dirname, + strcmp(udev_device_get_subsystem(dev), "block") == 0 ? 'b' : 'c', + major(udev_device_get_devnum(dev)), minor(udev_device_get_devnum(dev))); + + if (!add) { + dbg(udev, "removing index: '%s'\n", filename); + unlink(filename); + util_delete_path(udev, filename); + } - /* if we don't have a reference, delete it */ - if (count <= 0) { + target = link_find_prioritized(dev, add, dirname, buf, sizeof(buf)); + if (target == NULL) { info(udev, "no reference left, remove '%s'\n", slink); unlink(slink); util_delete_path(udev, slink); - goto out; - } - - /* find the device with the highest priority */ - target[0] = '\0'; - udev_list_entry_foreach(dev_entry, udev_list_get_entry(&dev_list)) { - const char *syspath; - struct udev_device *dev_db; - const char *devnode; - - syspath = udev_list_entry_get_name(dev_entry); - dbg(udev, "found '%s' for '%s'\n", syspath, slink); - - /* did we find ourself? we win, if we have the same priority */ - if (strcmp(udev_device_get_syspath(dev), syspath) == 0) { - dbg(udev, "compare (our own) priority of '%s' %i >= %i\n", - udev_device_get_devpath(dev), udev_device_get_devlink_priority(dev), priority); - if (strcmp(udev_device_get_devnode(dev), slink) == 0) { - info(udev, "'%s' is our device node, database inconsistent, skip link update\n", - udev_device_get_devnode(dev)); - } else if (target[0] == '\0' || udev_device_get_devlink_priority(dev) >= priority) { - priority = udev_device_get_devlink_priority(dev); - util_strscpy(target, sizeof(target), udev_device_get_devnode(dev)); - } - continue; - } - - /* another device, read priority from database */ - dev_db = udev_device_new_from_syspath(udev, syspath); - if (dev_db == NULL) - continue; - devnode = udev_device_get_devnode(dev_db); - if (devnode != NULL) { - if (strcmp(devnode, slink) == 0) { - info(udev, "'%s' is a device node of '%s', skip link update\n", - devnode, syspath); - } else { - dbg(udev, "compare priority of '%s' %i > %i\n", - udev_device_get_devpath(dev_db), - udev_device_get_devlink_priority(dev_db), - priority); - if (target[0] == '\0' || udev_device_get_devlink_priority(dev_db) > priority) { - priority = udev_device_get_devlink_priority(dev_db); - util_strscpy(target, sizeof(target), devnode); - } - } - } - udev_device_unref(dev_db); + } else { + util_create_path(udev, slink); + info(udev, "creating link '%s' to '%s'\n", slink, target); + node_symlink(udev, target, slink); } - udev_list_cleanup_entries(udev, &dev_list); - if (target[0] == '\0') { - info(udev, "no current target for '%s' found\n", slink); - rc = 1; - goto out; + if (add) { + dbg(udev, "creating index: '%s'\n", filename); + util_create_path(udev, filename); + symlink(udev_device_get_devpath(dev), filename); } - - /* create symlink to the target with the highest priority */ - info(udev, "'%s' with target '%s' has the highest priority %i, create it\n", slink, target, priority); - util_create_path(udev, slink); - node_symlink(udev, target, slink); -out: - return rc; } void udev_node_update_old_links(struct udev_device *dev, struct udev_device *dev_old) { struct udev *udev = udev_device_get_udev(dev); struct udev_list_entry *list_entry; - const char *devnode_old; /* update possible left-over symlinks */ udev_list_entry_foreach(list_entry, udev_device_get_devlinks_list_entry(dev_old)) { @@ -366,10 +316,6 @@ void udev_node_update_old_links(struct udev_device *dev, struct udev_device *dev struct udev_list_entry *list_entry_current; int found; - /* check if old link name is now our node name */ - if (strcmp(name, udev_device_get_devnode(dev)) == 0) - continue; - /* check if old link name still belongs to this device */ found = 0; udev_list_entry_foreach(list_entry_current, udev_device_get_devlinks_list_entry(dev)) { @@ -385,23 +331,7 @@ void udev_node_update_old_links(struct udev_device *dev, struct udev_device *dev info(udev, "update old name, '%s' no longer belonging to '%s'\n", name, udev_device_get_devpath(dev)); - name_index(dev, name, 0); - update_link(dev, name); - } - - /* - * if the node name has changed, delete the node, - * and possibly restore a symlink of a different device - */ - devnode_old = udev_device_get_devnode(dev_old); - if (devnode_old != NULL) { - const char *devnode = udev_device_get_devnode(dev); - - if (devnode != NULL && strcmp(devnode_old, devnode) != 0) { - info(udev, "node has changed from '%s' to '%s'\n", devnode_old, devnode); - name_index(dev, devnode_old, 0); - update_link(dev, devnode_old); - } + link_update(dev, name, 0); } } @@ -441,13 +371,15 @@ int udev_node_add(struct udev_device *dev, mode_t mode, uid_t uid, gid_t gid) } } - /* add node to name index */ - name_index(dev, udev_device_get_devnode(dev), 1); - /* create/update symlinks, add symlinks to name index */ udev_list_entry_foreach(list_entry, udev_device_get_devlinks_list_entry(dev)) { - name_index(dev, udev_list_entry_get_name(list_entry), 1); - update_link(dev, udev_list_entry_get_name(list_entry)); + if (udev_list_entry_get_flag(list_entry)) { + /* simple unmanaged link name */ + util_create_path(udev, udev_list_entry_get_name(list_entry)); + node_symlink(udev, udev_device_get_devnode(dev), udev_list_entry_get_name(list_entry)); + } else { + link_update(dev, udev_list_entry_get_name(list_entry), 1); + } } exit: return err; @@ -463,14 +395,9 @@ int udev_node_remove(struct udev_device *dev) int err = 0; int num; - /* remove node from name index */ - name_index(dev, udev_device_get_devnode(dev), 0); - /* remove,update symlinks, remove symlinks from name index */ - udev_list_entry_foreach(list_entry, udev_device_get_devlinks_list_entry(dev)) { - name_index(dev, udev_list_entry_get_name(list_entry), 0); - update_link(dev, udev_list_entry_get_name(list_entry)); - } + udev_list_entry_foreach(list_entry, udev_device_get_devlinks_list_entry(dev)) + link_update(dev, udev_list_entry_get_name(list_entry), 0); devnode = udev_device_get_devnode(dev); if (devnode == NULL) diff --git a/udev/udev-rules.c b/udev/udev-rules.c index a85800adf..904ddac72 100644 --- a/udev/udev-rules.c +++ b/udev/udev-rules.c @@ -190,7 +190,8 @@ struct token { unsigned int value_off; union { unsigned int attr_off; - int ignore_error; + int devlink_unique; + int fail_on_error; unsigned int rule_goto; mode_t mode; uid_t uid; @@ -1011,7 +1012,6 @@ static int rule_add_key(struct rule_tmp *rule_tmp, enum token_type type, case TK_A_GROUP: case TK_A_MODE: case TK_A_NAME: - case TK_A_DEVLINK: case TK_A_GOTO: token->key.value_off = add_string(rule_tmp->rules, value); break; @@ -1024,6 +1024,10 @@ static int rule_add_key(struct rule_tmp *rule_tmp, enum token_type type, token->key.value_off = add_string(rule_tmp->rules, value); token->key.attr_off = add_string(rule_tmp->rules, attr); break; + case TK_A_DEVLINK: + token->key.value_off = add_string(rule_tmp->rules, value); + token->key.devlink_unique = *(int *)data; + break; case TK_M_TEST: token->key.value_off = add_string(rule_tmp->rules, value); if (data != NULL) @@ -1037,7 +1041,7 @@ static int rule_add_key(struct rule_tmp *rule_tmp, enum token_type type, break; case TK_A_RUN: token->key.value_off = add_string(rule_tmp->rules, value); - token->key.ignore_error = *(int *)data; + token->key.fail_on_error = *(int *)data; break; case TK_A_INOTIFY_WATCH: case TK_A_NUM_FAKE_PART: @@ -1432,10 +1436,16 @@ static int add_rule(struct udev_rules *rules, char *line, } if (strcmp(key, "SYMLINK") == 0) { - if (op < OP_MATCH_MAX) + if (op < OP_MATCH_MAX) { rule_add_key(&rule_tmp, TK_M_DEVLINK, op, value, NULL); - else - rule_add_key(&rule_tmp, TK_A_DEVLINK, op, value, NULL); + } else { + int flag = 0; + + attr = get_key_attribute(rules->udev, key + sizeof("SYMLINK")-1); + if (attr != NULL && strstr(attr, "unique") != NULL) + flag = 1; + rule_add_key(&rule_tmp, TK_A_DEVLINK, op, value, &flag); + } rule_tmp.rule.rule.flags = 1; continue; } @@ -2445,26 +2455,22 @@ int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event while (isspace(pos[0])) pos++; next = strchr(pos, ' '); - while (next) { + while (next != NULL) { next[0] = '\0'; - info(event->udev, "LINK '%s' %s:%u\n", - pos, - &rules->buf[rule->rule.filename_off], - rule->rule.filename_line); + info(event->udev, "LINK '%s' %s:%u\n", pos, + &rules->buf[rule->rule.filename_off], rule->rule.filename_line); util_strscpyl(filename, sizeof(filename), udev_get_dev_path(event->udev), "/", pos, NULL); - udev_device_add_devlink(event->dev, filename); + udev_device_add_devlink(event->dev, filename, cur->key.devlink_unique); while (isspace(next[1])) next++; pos = &next[1]; next = strchr(pos, ' '); } if (pos[0] != '\0') { - info(event->udev, "LINK '%s' %s:%u\n", - pos, - &rules->buf[rule->rule.filename_off], - rule->rule.filename_line); + info(event->udev, "LINK '%s' %s:%u\n", pos, + &rules->buf[rule->rule.filename_off], rule->rule.filename_line); util_strscpyl(filename, sizeof(filename), udev_get_dev_path(event->udev), "/", pos, NULL); - udev_device_add_devlink(event->dev, filename); + udev_device_add_devlink(event->dev, filename, cur->key.devlink_unique); } } break; @@ -2511,7 +2517,7 @@ int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event rule->rule.filename_line); list_entry = udev_list_entry_add(event->udev, &event->run_list, &rules->buf[cur->key.value_off], NULL, 1, 0); - if (cur->key.ignore_error) + if (cur->key.fail_on_error) udev_list_entry_set_flag(list_entry, 1); break; }