[systemd-devel] [PATCH] mount: use libmount to monitor mountinfo & utab
Karel Zak
kzak at redhat.com
Mon Jun 1 05:07:57 PDT 2015
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).
---
Makefile.am | 33 ++++++++++++------
configure.ac | 2 +-
src/core/manager.c | 2 +-
src/core/manager.h | 5 ++-
src/core/mount.c | 100 ++++++++++++-----------------------------------------
5 files changed, 49 insertions(+), 93 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index ed5135d..3815e72 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1352,7 +1352,8 @@ systemd_SOURCES = \
systemd_CFLAGS = \
$(AM_CFLAGS) \
- $(SECCOMP_CFLAGS)
+ $(SECCOMP_CFLAGS) \
+ $(MOUNT_CFLAGS)
systemd_LDADD = \
libsystemd-core.la \
@@ -1554,7 +1555,8 @@ test_engine_SOURCES = \
test_engine_CFLAGS = \
$(AM_CFLAGS) \
- $(SECCOMP_CFLAGS)
+ $(SECCOMP_CFLAGS) \
+ $(MOUNT_CFLAGS)
test_engine_LDADD = \
libsystemd-core.la \
@@ -1565,7 +1567,8 @@ test_job_type_SOURCES = \
test_job_type_CFLAGS = \
$(AM_CFLAGS) \
- $(SECCOMP_CFLAGS)
+ $(SECCOMP_CFLAGS) \
+ $(MOUNT_CFLAGS)
test_job_type_LDADD = \
libsystemd-core.la \
@@ -1609,7 +1612,8 @@ test_unit_name_SOURCES = \
test_unit_name_CFLAGS = \
$(AM_CFLAGS) \
- $(SECCOMP_CFLAGS)
+ $(SECCOMP_CFLAGS) \
+ $(MOUNT_CFLAGS)
test_unit_name_LDADD = \
libsystemd-core.la \
@@ -1620,7 +1624,8 @@ test_unit_file_SOURCES = \
test_unit_file_CFLAGS = \
$(AM_CFLAGS) \
- $(SECCOMP_CFLAGS)
+ $(SECCOMP_CFLAGS) \
+ $(MOUNT_CFLAGS)
test_unit_file_LDADD = \
libsystemd-core.la \
@@ -1838,7 +1843,8 @@ test_tables_CPPFLAGS = \
test_tables_CFLAGS = \
$(AM_CFLAGS) \
- $(SECCOMP_CFLAGS)
+ $(SECCOMP_CFLAGS) \
+ $(MOUNT_CFLAGS)
test_tables_LDADD = \
libsystemd-logs.la \
@@ -1973,7 +1979,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) \
@@ -2022,7 +2029,8 @@ test_path_SOURCES = \
src/test/test-path.c
test_path_CFLAGS = \
- $(AM_CFLAGS)
+ $(AM_CFLAGS) \
+ $(MOUNT_CFLAGS)
test_path_LDADD = \
libsystemd-core.la
@@ -2031,7 +2039,8 @@ test_execute_SOURCES = \
src/test/test-execute.c
test_execute_CFLAGS = \
- $(AM_CFLAGS)
+ $(AM_CFLAGS) \
+ $(MOUNT_CFLAGS)
test_execute_LDADD = \
libsystemd-core.la
@@ -2061,7 +2070,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) \
@@ -2133,7 +2143,8 @@ systemd_analyze_SOURCES = \
systemd_analyze_CFLAGS = \
$(AM_CFLAGS) \
- $(SECCOMP_CFLAGS)
+ $(SECCOMP_CFLAGS) \
+ $(MOUNT_CFLAGS)
systemd_analyze_LDADD = \
libsystemd-core.la \
diff --git a/configure.ac b/configure.ac
index 48cedb5..74ec386 100644
--- a/configure.ac
+++ b/configure.ac
@@ -454,7 +454,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 b931b0d..6881bb2 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -567,7 +567,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..3b6a519 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");
+ 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
More information about the systemd-devel
mailing list