SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #34698 from yuwata/udev-node-lock-file

Browse Source 
udev-node: remove stack directories and their lock files by workers, rather than manager
This commit is contained in:
Yu Watanabe
2024-10-12 08:12:10 +09:00
committed by GitHub
parent 8ffc83d3d4 5a22b27b5c
commit d106887bb5
5 changed files with 65 additions and 165 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/udev/udev-manager.c
-8
View File
@@ -583,9 +583,6 @@ static int event_queue_start(Manager *manager) {
if (!manager->events || manager->exit || manager->stop_exec_queue)
return 0;
/* To make the stack directory /run/udev/links cleaned up later. */
manager->udev_node_needs_cleanup = true;
r = event_source_disable(manager->kill_workers_event);
if (r < 0)
log_warning_errno(r, "Failed to disable event source for cleaning up idle workers, ignoring: %m");
@@ -1181,11 +1178,6 @@ static int on_post(sd_event_source *s, void *userdata) {
/* There are no idle workers. */
if (manager->udev_node_needs_cleanup) {
(void) udev_node_cleanup();
manager->udev_node_needs_cleanup = false;
}
if (manager->exit)
return sd_event_exit(manager->event, 0);
src/udev/udev-manager.h
-1
View File
@@ -44,7 +44,6 @@ typedef struct Manager {
int timeout_signal;
bool blockdev_read_only;
bool udev_node_needs_cleanup;
bool stop_exec_queue;
bool exit;
} Manager;
src/udev/udev-node.c
+47 -151
View File
@@ -28,48 +28,6 @@
#define UDEV_NODE_HASH_KEY SD_ID128_MAKE(b9,6a,f1,ce,40,31,44,1a,9e,19,ec,8b,ae,f3,e3,2f)
int udev_node_cleanup(void) {
_cleanup_closedir_ DIR *dir = NULL;
/* This must not be called when any workers exist. It would cause a race between mkdir() called
* by stack_directory_lock() and unlinkat() called by this. */
dir = opendir("/run/udev/links");
if (!dir) {
if (errno == ENOENT)
return 0;
return log_debug_errno(errno, "Failed to open directory '/run/udev/links', ignoring: %m");
}
FOREACH_DIRENT_ALL(de, dir, break) {
_cleanup_free_ char *lockfile = NULL;
if (de->d_name[0] == '.')
continue;
if (de->d_type != DT_DIR)
continue;
/* As commented in the above, this is called when no worker exists, hence the file is not
* locked. On a later uevent, the lock file will be created if necessary. So, we can safely
* remove the file now. */
lockfile = path_join(de->d_name, ".lock");
if (!lockfile)
return log_oom_debug();
if (unlinkat(dirfd(dir), lockfile, 0) < 0 && errno != ENOENT) {
log_debug_errno(errno, "Failed to remove '/run/udev/links/%s', ignoring: %m", lockfile);
continue;
}
if (unlinkat(dirfd(dir), de->d_name, AT_REMOVEDIR) < 0 && errno != ENOTEMPTY)
log_debug_errno(errno, "Failed to remove '/run/udev/links/%s', ignoring: %m", de->d_name);
}
return 0;
}
static int node_remove_symlink(sd_device *dev, const char *slink) {
assert(dev);
assert(slink);
@@ -339,7 +297,7 @@ toolong:
}
static int stack_directory_get_name(const char *slink, char **ret) {
_cleanup_free_ char *s = NULL, *dirname = NULL;
_cleanup_free_ char *s = NULL;
char name_enc[NAME_MAX+1];
const char *name;
int r;
@@ -360,45 +318,59 @@ static int stack_directory_get_name(const char *slink, char **ret) {
udev_node_escape_path(name, name_enc, sizeof(name_enc));
dirname = path_join("/run/udev/links", name_enc);
if (!dirname)
return -ENOMEM;
*ret = TAKE_PTR(dirname);
return 0;
return strdup_to(ret, name_enc);
}
static int stack_directory_open(sd_device *dev, const char *slink, int *ret_dirfd, int *ret_lockfd) {
_cleanup_close_ int dirfd = -EBADF, lockfd = -EBADF;
_cleanup_free_ char *dirname = NULL;
static int stack_directory_open_and_lock(
sd_device *dev,
const char *slink,
char **ret_dirpath,
int *ret_dirfd,
LockFile *ret_lockfile) {
_cleanup_(release_lock_file) LockFile lockfile = LOCK_FILE_INIT;
_cleanup_close_ int dirfd = -EBADF;
_cleanup_free_ char *name = NULL, *dirpath = NULL, *lockname = NULL;
int r;
assert(dev);
assert(slink);
assert(ret_dirpath);
assert(ret_dirfd);
assert(ret_lockfd);
assert(ret_lockfile);
r = stack_directory_get_name(slink, &dirname);
r = stack_directory_get_name(slink, &name);
if (r < 0)
return log_device_debug_errno(dev, r, "Failed to build stack directory name for '%s': %m", slink);
r = mkdir_parents(dirname, 0755);
FOREACH_STRING(s, "/run/udev/links/", "/run/udev/links.lock/") {
r = mkdir_p(s, 0755);
if (r < 0)
return log_device_debug_errno(dev, r, "Failed to create '%s': %m", s);
}
/* 1. Take a lock for the stack directory. */
lockname = path_join("/run/udev/links.lock/", name);
if (!lockname)
return -ENOMEM;
r = make_lock_file(lockname, LOCK_EX, &lockfile);
if (r < 0)
return log_device_debug_errno(dev, r, "Failed to create stack directory '%s': %m", dirname);
return log_device_debug_errno(dev, r, "Failed to create and lock '%s': %m", lockname);
dirfd = open_mkdir(dirname, O_CLOEXEC | O_DIRECTORY | O_NOFOLLOW | O_RDONLY, 0755);
/* 2. Create and open the stack directory. Do not create the stack directory before taking a lock,
* otherwise the directory may be removed by another worker. */
dirpath = path_join("/run/udev/links/", name);
if (!dirpath)
return -ENOMEM;
dirfd = open_mkdir(dirpath, O_CLOEXEC | O_DIRECTORY | O_NOFOLLOW | O_RDONLY, 0755);
if (dirfd < 0)
return log_device_debug_errno(dev, dirfd, "Failed to open stack directory '%s': %m", dirname);
lockfd = openat(dirfd, ".lock", O_CLOEXEC | O_NOFOLLOW | O_RDONLY | O_CREAT, 0600);
if (lockfd < 0)
return log_device_debug_errno(dev, errno, "Failed to create lock file for stack directory '%s': %m", dirname);
if (flock(lockfd, LOCK_EX) < 0)
return log_device_debug_errno(dev, errno, "Failed to place a lock on lock file for %s: %m", dirname);
return log_device_debug_errno(dev, dirfd, "Failed to open stack directory '%s': %m", dirpath);
*ret_dirpath = TAKE_PTR(dirpath);
*ret_dirfd = TAKE_FD(dirfd);
*ret_lockfd = TAKE_FD(lockfd);
*ret_lockfile = TAKE_GENERIC(lockfile, LockFile, LOCK_FILE_INIT);
return 0;
}
@@ -434,98 +406,22 @@ static int node_get_current(const char *slink, int dirfd, char **ret_id, int *re
return 0;
}
static int link_update_diskseq(sd_device *dev, const char *slink, bool add) {
_cleanup_free_ char *buf = NULL;
const char *fname, *diskseq;
int r;
assert(dev);
assert(slink);
if (!device_in_subsystem(dev, "block"))
return 0;
fname = path_startswith(slink, "/dev/disk/by-diskseq");
if (isempty(fname))
return 0;
if (device_is_devtype(dev, "partition")) {
_cleanup_free_ char *suffix = NULL;
const char *partn, *p;
/* Check if the symlink has an expected suffix "-part%n". See 60-persistent-storage.rules. */
r = sd_device_get_sysnum(dev, &partn);
if (r < 0) {
/* Cannot verify the symlink is owned by this device. Let's create the stack directory for the symlink. */
log_device_debug_errno(dev, r, "Failed to get sysnum, but symlink '%s' is requested, ignoring: %m", slink);
return 0;
}
suffix = strjoin("-part", partn);
if (!suffix)
return -ENOMEM;
p = endswith(fname, suffix);
if (!p) {
log_device_debug(dev, "Unexpected by-diskseq symlink '%s' is requested, proceeding anyway.", slink);
return 0;
}
buf = strndup(fname, p - fname);
if (!buf)
return -ENOMEM;
fname = buf;
}
/* Check if the diskseq part of the symlink is in digits. */
if (!in_charset(fname, DIGITS)) {
log_device_debug(dev, "Unexpected by-diskseq symlink '%s' is requested, proceeding anyway.", slink);
return 0; /* unexpected by-diskseq symlink */
}
/* On removal, we cannot verify the diskseq. Skipping further check below. */
if (!add) {
r = node_remove_symlink(dev, slink);
if (r < 0)
return r;
return 1; /* done */
}
/* Check if the diskseq matches with the DISKSEQ property. */
r = sd_device_get_property_value(dev, "DISKSEQ", &diskseq);
if (r < 0) {
log_device_debug_errno(dev, r, "Failed to get DISKSEQ property, but symlink '%s' is requested, ignoring: %m", slink);
return 0;
}
if (!streq(fname, diskseq)) {
log_device_debug(dev, "Unexpected by-diskseq symlink '%s' is requested (DISKSEQ=%s), proceeding anyway.", slink, diskseq);
return 0;
}
r = node_create_symlink(dev, /* devnode = */ NULL, slink);
if (r < 0)
return r;
return 1; /* done */
}
static int link_update(sd_device *dev, const char *slink, bool add) {
/* On cleaning up,
* 1. close the stack directory,
* 2. remove the stack directory if it is empty,
* 3. then finally release the lock.
* Hence, the variables must be declared in the reverse order. */
_cleanup_(release_lock_file) LockFile lockfile = LOCK_FILE_INIT; /* #3 */
_cleanup_(rmdir_and_freep) char *dirpath = NULL; /* #2 */
_cleanup_close_ int dirfd = -EBADF; /* #1 */
_cleanup_free_ char *current_id = NULL, *devnode = NULL;
_cleanup_close_ int dirfd = -EBADF, lockfd = -EBADF;
int r, current_prio;
assert(dev);
assert(slink);
r = link_update_diskseq(dev, slink, add);
if (r != 0)
return r;
r = stack_directory_open(dev, slink, &dirfd, &lockfd);
r = stack_directory_open_and_lock(dev, slink, &dirpath, &dirfd, &lockfile);
if (r < 0)
return r;
src/udev/udev-node.h
-1
View File
@@ -24,6 +24,5 @@ int static_node_apply_permissions(
int udev_node_remove(sd_device *dev);
int udev_node_update(sd_device *dev, sd_device *dev_old);
int udev_node_cleanup(void);
size_t udev_node_escape_path(const char *src, char *dest, size_t size);
test/units/TEST-17-UDEV.diskseq.sh
+18 -4
View File
@@ -1,6 +1,7 @@
#!/usr/bin/env bash
# SPDX-License-Identifier: LGPL-2.1-or-later
# shellcheck disable=SC2010
# shellcheck disable=SC2012
# shellcheck disable=SC2317
set -ex
set -o pipefail
@@ -43,15 +44,28 @@ systemctl daemon-reload
udevadm settle
# If an initrd from the host is used, stack directories for by-diskseq symlinks
# may already exist. Save the number of the directories here.
NUM_DISKSEQ_EXPECTED=$(ls /run/udev/links | grep -c by-diskseq || :)
# Check if no lock file exists, if the lock directory exists.
if [[ -d /run/udev/links.lock/ ]]; then
[[ "$(ls /run/udev/links.lock/ | wc -l)" == 0 ]]
fi
# Save the current number of the directories.
NUM_DISKSEQ=$(ls /run/udev/links/ | grep -c by-diskseq || :)
systemctl start --no-block test-diskseq.service
for _ in {0..100}; do
sleep .1
assert_eq "$(ls /run/udev/links | grep -c by-diskseq || :)" "$NUM_DISKSEQ_EXPECTED"
n=$(ls /run/udev/links/ | grep -c by-diskseq || :)
(( n <= NUM_DISKSEQ + 1 ))
done
systemctl stop test-diskseq.service || :
udevadm settle
# Check if the lock directory exists, but no lock file exists in it.
[[ -d /run/udev/links.lock/ ]]
[[ "$(ls /run/udev/links.lock/ | wc -l)" == 0 ]]
exit 0