The current implementation directly monitor /proc/self/mountinfo and /run/mount/utab files. It's really not optimal because utab file is private libmount stuff without any official guaranteed semantic.
The libmount since v2.26 provides API to monitor mount kernel & userspace changes. This patch replaces the current implementation with libmount based solution. Now the manager.h includes <libmount.h>, so $MOUNT_CFLAGS has been necessary to add to many tests CFLAGS. Note that mnt_monitor_event_cleanup() in v2.26 is broken, so the patch uses mnt_monitor_next_change(). It's exactly the same solution which uses the current libmount HEAD (mnt_monitor_event_cleanup() is API shorcut only). --- V2: - update README - add missing (void) Makefile.am | 33 ++++++++++++------ README | 2 +- configure.ac | 2 +- src/core/manager.c | 2 +- src/core/manager.h | 5 ++- src/core/mount.c | 100 ++++++++++++----------------------------------------- 6 files changed, 50 insertions(+), 94 deletions(-) diff --git a/Makefile.am b/Makefile.am index 64038a5..c1a97de 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1329,7 +1329,8 @@ systemd_SOURCES = \ systemd_CFLAGS = \ $(AM_CFLAGS) \ - $(SECCOMP_CFLAGS) + $(SECCOMP_CFLAGS) \ + $(MOUNT_CFLAGS) systemd_LDADD = \ libsystemd-core.la \ @@ -1532,7 +1533,8 @@ test_engine_SOURCES = \ test_engine_CFLAGS = \ $(AM_CFLAGS) \ - $(SECCOMP_CFLAGS) + $(SECCOMP_CFLAGS) \ + $(MOUNT_CFLAGS) test_engine_LDADD = \ libsystemd-core.la \ @@ -1543,7 +1545,8 @@ test_job_type_SOURCES = \ test_job_type_CFLAGS = \ $(AM_CFLAGS) \ - $(SECCOMP_CFLAGS) + $(SECCOMP_CFLAGS) \ + $(MOUNT_CFLAGS) test_job_type_LDADD = \ libsystemd-core.la \ @@ -1587,7 +1590,8 @@ test_unit_name_SOURCES = \ test_unit_name_CFLAGS = \ $(AM_CFLAGS) \ - $(SECCOMP_CFLAGS) + $(SECCOMP_CFLAGS) \ + $(MOUNT_CFLAGS) test_unit_name_LDADD = \ libsystemd-core.la \ @@ -1598,7 +1602,8 @@ test_unit_file_SOURCES = \ test_unit_file_CFLAGS = \ $(AM_CFLAGS) \ - $(SECCOMP_CFLAGS) + $(SECCOMP_CFLAGS) \ + $(MOUNT_CFLAGS) test_unit_file_LDADD = \ libsystemd-core.la \ @@ -1811,7 +1816,8 @@ test_tables_CPPFLAGS = \ test_tables_CFLAGS = \ $(AM_CFLAGS) \ - $(SECCOMP_CFLAGS) + $(SECCOMP_CFLAGS) \ + $(MOUNT_CFLAGS) test_tables_LDADD = \ libsystemd-logs.la \ @@ -1944,7 +1950,8 @@ test_cgroup_mask_SOURCES = \ src/test/test-cgroup-mask.c test_cgroup_mask_CPPFLAGS = \ - $(AM_CPPFLAGS) + $(AM_CPPFLAGS) \ + $(MOUNT_CFLAGS) test_cgroup_mask_CFLAGS = \ $(AM_CFLAGS) \ @@ -1990,7 +1997,8 @@ test_path_SOURCES = \ src/test/test-path.c test_path_CFLAGS = \ - $(AM_CFLAGS) + $(AM_CFLAGS) \ + $(MOUNT_CFLAGS) test_path_LDADD = \ libsystemd-core.la @@ -1999,7 +2007,8 @@ test_execute_SOURCES = \ src/test/test-execute.c test_execute_CFLAGS = \ - $(AM_CFLAGS) + $(AM_CFLAGS) \ + $(MOUNT_CFLAGS) test_execute_LDADD = \ libsystemd-core.la @@ -2027,7 +2036,8 @@ test_sched_prio_SOURCES = \ src/test/test-sched-prio.c test_sched_prio_CPPFLAGS = \ - $(AM_CPPFLAGS) + $(AM_CPPFLAGS) \ + $(MOUNT_CFLAGS) test_sched_prio_CFLAGS = \ $(AM_CFLAGS) \ @@ -2104,7 +2114,8 @@ systemd_analyze_SOURCES = \ systemd_analyze_CFLAGS = \ $(AM_CFLAGS) \ - $(SECCOMP_CFLAGS) + $(SECCOMP_CFLAGS) \ + $(MOUNT_CFLAGS) systemd_analyze_LDADD = \ libsystemd-core.la \ diff --git a/README b/README index b810044..19abeb2 100644 --- a/README +++ b/README @@ -114,7 +114,7 @@ REQUIREMENTS: glibc >= 2.16 libcap - libmount >= 2.20 (from util-linux) + libmount >= 2.26 (from util-linux) libseccomp >= 1.0.0 (optional) libblkid >= 2.24 (from util-linux) (optional) libkmod >= 15 (optional) diff --git a/configure.ac b/configure.ac index 0532c54..61f9a0f 100644 --- a/configure.ac +++ b/configure.ac @@ -438,7 +438,7 @@ AM_CONDITIONAL(HAVE_BLKID, [test "$have_blkid" = "yes"]) # ------------------------------------------------------------------------------ have_libmount=no -PKG_CHECK_MODULES(MOUNT, [ mount >= 2.20 ], +PKG_CHECK_MODULES(MOUNT, [ mount >= 2.26 ], [AC_DEFINE(HAVE_LIBMOUNT, 1, [Define if libmount is available]) have_libmount=yes], have_libmount=no) if test "x$have_libmount" = xno; then AC_MSG_ERROR([*** libmount support required but libraries not found]) diff --git a/src/core/manager.c b/src/core/manager.c index ae473d0..10ab83a 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -568,7 +568,7 @@ int manager_new(ManagerRunningAs running_as, bool test_run, Manager **_m) { m->idle_pipe[0] = m->idle_pipe[1] = m->idle_pipe[2] = m->idle_pipe[3] = -1; - m->pin_cgroupfs_fd = m->notify_fd = m->signal_fd = m->time_change_fd = m->dev_autofs_fd = m->private_listen_fd = m->kdbus_fd = m->utab_inotify_fd = -1; + m->pin_cgroupfs_fd = m->notify_fd = m->signal_fd = m->time_change_fd = m->dev_autofs_fd = m->private_listen_fd = m->kdbus_fd = -1; m->current_job_id = 1; /* start as id #1, so that we can leave #0 around as "null-like" value */ m->ask_password_inotify_fd = -1; diff --git a/src/core/manager.h b/src/core/manager.h index 4ef869d..5cd8884 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -23,6 +23,7 @@ #include <stdbool.h> #include <stdio.h> +#include <libmount.h> #include "sd-bus.h" #include "sd-event.h" @@ -176,10 +177,8 @@ struct Manager { Hashmap *devices_by_sysfs; /* Data specific to the mount subsystem */ - FILE *proc_self_mountinfo; + struct libmnt_monitor *mount_monitor; sd_event_source *mount_event_source; - int utab_inotify_fd; - sd_event_source *mount_utab_event_source; /* Data specific to the swap filesystem */ FILE *proc_swaps; diff --git a/src/core/mount.c b/src/core/mount.c index ba1dcf1..970ffdb 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -23,8 +23,6 @@ #include <stdio.h> #include <sys/epoll.h> #include <signal.h> -#include <libmount.h> -#include <sys/inotify.h> #include "manager.h" #include "unit.h" @@ -1539,16 +1537,13 @@ static int mount_load_proc_self_mountinfo(Manager *m, bool set_flags) { } static void mount_shutdown(Manager *m) { + assert(m); m->mount_event_source = sd_event_source_unref(m->mount_event_source); - m->mount_utab_event_source = sd_event_source_unref(m->mount_utab_event_source); - if (m->proc_self_mountinfo) { - fclose(m->proc_self_mountinfo); - m->proc_self_mountinfo = NULL; - } - m->utab_inotify_fd = safe_close(m->utab_inotify_fd); + mnt_unref_monitor(m->mount_monitor); + m->mount_monitor = NULL; } static int mount_get_timeout(Unit *u, uint64_t *timeout) { @@ -1567,53 +1562,41 @@ static int mount_get_timeout(Unit *u, uint64_t *timeout) { static int mount_enumerate(Manager *m) { int r; + assert(m); mnt_init_debug(0); - if (!m->proc_self_mountinfo) { - m->proc_self_mountinfo = fopen("/proc/self/mountinfo", "re"); - if (!m->proc_self_mountinfo) - return -errno; + if (!m->mount_monitor) { + int fd; - r = sd_event_add_io(m->event, &m->mount_event_source, fileno(m->proc_self_mountinfo), EPOLLPRI, mount_dispatch_io, m); - if (r < 0) + m->mount_monitor = mnt_new_monitor(); + if (!m->mount_monitor) { + r = -ENOMEM; goto fail; + } - /* Dispatch this before we dispatch SIGCHLD, so that - * we always get the events from /proc/self/mountinfo - * before the SIGCHLD of /usr/bin/mount. */ - r = sd_event_source_set_priority(m->mount_event_source, -10); - if (r < 0) + r = mnt_monitor_enable_kernel(m->mount_monitor, 1); + if (r) goto fail; - - (void) sd_event_source_set_description(m->mount_event_source, "mount-mountinfo-dispatch"); - } - - if (m->utab_inotify_fd < 0) { - m->utab_inotify_fd = inotify_init1(IN_NONBLOCK|IN_CLOEXEC); - if (m->utab_inotify_fd < 0) { - r = -errno; + r = mnt_monitor_enable_userspace(m->mount_monitor, 1, NULL); + if (r) goto fail; - } - - (void) mkdir_p_label("/run/mount", 0755); - r = inotify_add_watch(m->utab_inotify_fd, "/run/mount", IN_MOVED_TO); - if (r < 0) { - r = -errno; + /* mnt_unref_monitor() will close the fd */ + fd = r = mnt_monitor_get_fd(m->mount_monitor); + if (r < 0) goto fail; - } - r = sd_event_add_io(m->event, &m->mount_utab_event_source, m->utab_inotify_fd, EPOLLIN, mount_dispatch_io, m); + r = sd_event_add_io(m->event, &m->mount_event_source, fd, EPOLLIN | EPOLLPRI, mount_dispatch_io, m); if (r < 0) goto fail; - r = sd_event_source_set_priority(m->mount_utab_event_source, -10); + r = sd_event_source_set_priority(m->mount_event_source, -10); if (r < 0) goto fail; - (void) sd_event_source_set_description(m->mount_utab_event_source, "mount-utab-dispatch"); + (void) sd_event_source_set_description(m->mount_event_source, "mount-monitor-dispatch"); } r = mount_load_proc_self_mountinfo(m, false); @@ -1638,46 +1621,9 @@ static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents, assert(m); assert(revents & (EPOLLPRI | EPOLLIN)); - /* The manager calls this for every fd event happening on the - * /proc/self/mountinfo file, which informs us about mounting - * table changes, and for /run/mount events which we watch - * for mount options. */ - - if (fd == m->utab_inotify_fd) { - bool rescan = false; - - /* FIXME: We *really* need to replace this with - * libmount's own API for this, we should not hardcode - * internal behaviour of libmount here. */ - - for (;;) { - union inotify_event_buffer buffer; - struct inotify_event *e; - ssize_t l; - - l = read(fd, &buffer, sizeof(buffer)); - if (l < 0) { - if (errno == EAGAIN || errno == EINTR) - break; - - log_error_errno(errno, "Failed to read utab inotify: %m"); - break; - } - - FOREACH_INOTIFY_EVENT(e, buffer, l) { - /* Only care about changes to utab, - * but we have to monitor the - * directory to reliably get - * notifications about when utab is - * replaced using rename(2) */ - if ((e->mask & IN_Q_OVERFLOW) || streq(e->name, "utab")) - rescan = true; - } - } - - if (!rescan) - return 0; - } + if (fd == mnt_monitor_get_fd(m->mount_monitor)) + /* Drain all changes from the monitor */ + while (mnt_monitor_next_change(m->mount_monitor, NULL, NULL) == 0); r = mount_load_proc_self_mountinfo(m, true); if (r < 0) { -- 2.4.1 _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel