libmount: make public top-level monitor FD only

We need full control on changes evaluation, so it's better to
hide all in our private epoll. This change also significantly
simplify the API.

 mn = mnt_new_monitor();
 mnt_monitor_enable_userapce(mn, TRUE, NULL);
 mnt_monitor_enable_kenrel(mn, TRUE);

 fd = mnt_monitor_get_fd(mn);
 ...
   <use 'fd' in epoll controlled by your application>
 ...
 while (mnt_monitor_next_changed(mn, &filename, NULL) == 0)
 	printf("%s: change detected\n", filename);

Signed-off-by: Karel Zak <kzak@redhat.com>

1 files changed, 221 insertions, 162 deletions

diff --git a/libmount/src/monitor.c b/libmount/src/monitor.c
index 6d6b2056a..9e997fee6 100644
--- a/libmount/src/monitor.c
+++ b/libmount/src/monitor.c
@@ -24,11 +24,15 @@ enum {
 	MNT_MONITOR_TYPE_USERSPACE
 };
 
+struct monitor_opers;
+
 struct monitor_entry {
-	int			fd;		/* public file descriptor */
+	int			fd;		/* private entry file descriptor */
 	char			*path;		/* path to the monitored file */
 	int			type;		/* MNT_MONITOR_TYPE_* */
 
+	const struct monitor_opers *opers;
+
 	unsigned int		enable : 1;
 
 	struct list_head	ents;
@@ -36,10 +40,16 @@ struct monitor_entry {
 
 struct libmnt_monitor {
 	int			refcount;
+	int			fd;		/* public monitor file descriptor */
 
 	struct list_head	ents;
 };
 
+struct monitor_opers {
+	int (*op_get_fd)(struct libmnt_monitor *, struct monitor_entry *);
+	int (*op_verify_change)(struct libmnt_monitor *, struct monitor_entry *);
+};
+
 static int monitor_enable_entry(struct libmnt_monitor *mn,
 				struct monitor_entry *me, int enable);
 
@@ -58,6 +68,7 @@ struct libmnt_monitor *mnt_new_monitor(void)
 		return NULL;
 
 	mn->refcount = 1;
+	mn->fd = -1;
 	INIT_LIST_HEAD(&mn->ents);
 
 	DBG(MONITOR, ul_debugobj(mn, "alloc"));
@@ -101,11 +112,16 @@ void mnt_unref_monitor(struct libmnt_monitor *mn)
 
 	mn->refcount--;
 	if (mn->refcount <= 0) {
+		if (mn->fd >= 0)
+			close(mn->fd);
+
 		while (!list_empty(&mn->ents)) {
 			struct monitor_entry *me = list_entry(mn->ents.next,
 						  struct monitor_entry, ents);
 			free_monitor_entry(me);
 		}
+
+		free(mn);
 	}
 }
 
@@ -126,134 +142,65 @@ static struct monitor_entry *monitor_new_entry(struct libmnt_monitor *mn)
 	return me;
 }
 
-static struct monitor_entry *monitor_get_entry(struct libmnt_monitor *mn, int type)
+static int monitor_next_entry(struct libmnt_monitor *mn,
+			      struct libmnt_iter *itr,
+			      struct monitor_entry **me)
 {
-	struct list_head *p;
+	int rc = 1;
 
 	assert(mn);
-	assert(type);
+	assert(itr);
+	assert(me);
 
-	list_for_each(p, &mn->ents) {
-		struct monitor_entry *me;
+	if (!mn || !itr || !me)
+		return -EINVAL;
 
-		me = list_entry(p, struct monitor_entry, ents);
-		if (me->type == type)
-			return me;
+	*me = NULL;
+
+	if (!itr->head)
+		MNT_ITER_INIT(itr, &mn->ents);
+	if (itr->p != itr->head) {
+		MNT_ITER_ITERATE(itr, *me, struct monitor_entry, ents);
+		rc = 0;
 	}
 
-	return NULL;
+	return rc;
 }
 
-static struct monitor_entry *monitor_get_entry_by_fd(struct libmnt_monitor *mn, int fd)
+static struct monitor_entry *monitor_get_entry(struct libmnt_monitor *mn, int type)
 {
-	struct list_head *p;
-
-	assert(mn);
-
-	if (fd < 0)
-		return NULL;
-
-	list_for_each(p, &mn->ents) {
-		struct monitor_entry *me;
+	struct libmnt_iter itr;
+	struct monitor_entry *me;
 
-		me = list_entry(p, struct monitor_entry, ents);
-		if (me->fd == fd)
+	mnt_reset_iter(&itr, MNT_ITER_FORWARD);
+	while (monitor_next_entry(mn, &itr, &me) == 0) {
+		if (me->type == type)
 			return me;
 	}
-
-	DBG(MONITOR, ul_debugobj(mn, "failed to get entry for fd=%d", fd));
 	return NULL;
 }
 
 
-
-/**
- * mnt_monitor_enable_userspace:
- * @mn: monitor
- * @enable: 0 or 1
- * @filename: overwrites default
- *
- * Enables or disables userspace monitor. If the monitor does not exist and
- * enable=1 then allocates new resources necessary for the monitor.
- *
- * If high-level monitor has been already initialized (by mnt_monitor_get_fd()
- * or mnt_wait_monitor()) then it's updated according to @enable.
- *
- * The @filename is used only first time when you enable the monitor. It's
- * impossible to have more than one userspace monitor.
- *
- * Note that the current implementation of the userspace monitor is based on
- * inotify. On systems (libc) without inotify_init1() the function return
- * -ENOSYS. The dependence on inotify is implemenation specific and may be
- * changed later.
- *
- * Return: 0 on success and <0 on error
+/*
+ * Userspace monitor
  */
-int mnt_monitor_enable_userspace(struct libmnt_monitor *mn, int enable, const char *filename)
-{
-	struct monitor_entry *me;
-	int rc = 0;
-
-	if (!mn)
-		return -EINVAL;
-
-	me = monitor_get_entry(mn, MNT_MONITOR_TYPE_USERSPACE);
-	if (me)
-		return monitor_enable_entry(mn, me, enable);
-	if (!enable)
-		return 0;
-
-	DBG(MONITOR, ul_debugobj(mn, "allocate new userspace monitor"));
-
-	/* create a new entry */
-	if (!mnt_has_regular_mtab(&filename, NULL))	/* /etc/mtab */
-		filename = mnt_get_utab_path();		/* /run/mount/utab */
-	if (!filename) {
-		DBG(MONITOR, ul_debugobj(mn, "failed to get userspace mount table path"));
-		return -EINVAL;
-	}
-
-	me = monitor_new_entry(mn);
-	if (!me)
-		goto err;
-
-	me->type = MNT_MONITOR_TYPE_USERSPACE;
-	me->path = strdup(filename);
-	if (!me->path)
-		goto err;
 
-	DBG(MONITOR, ul_debugobj(mn, "allocate new userspace monitor: OK"));
-	return monitor_enable_entry(mn, me, 1);
-err:
-	rc = -errno;
-	free_monitor_entry(me);
-	return rc;
-}
-
-/**
- * mnt_monitor_userspace_get_fd:
- * @mn: monitor pointer
- *
- * Returns: file descriptor to previously enabled userspace monitor or <0 on error.
- */
-#ifdef HAVE_INOTIFY_INIT1
-int mnt_monitor_userspace_get_fd(struct libmnt_monitor *mn)
+static int userspace_monitor_get_fd(struct libmnt_monitor *mn,
+				    struct monitor_entry *me)
 {
-	struct monitor_entry *me;
 	int wd, rc;
 	char *dirname, *sep;
 
 	assert(mn);
+	assert(me);
 
-	me = monitor_get_entry(mn, MNT_MONITOR_TYPE_USERSPACE);
 	if (!me || me->enable == 0)	/* not-initialized or disabled */
 		return -EINVAL;
-
 	if (me->fd >= 0)
 		return me->fd;		/* already initialized */
 
 	assert(me->path);
-	DBG(MONITOR, ul_debugobj(mn, "open userspace monitor for %s", me->path));
+	DBG(MONITOR, ul_debugobj(mn, " open userspace monitor for %s", me->path));
 
 	dirname = me->path;
 	sep = stripoff_last_component(dirname);	/* add \0 between dir/filename */
@@ -281,14 +228,14 @@ int mnt_monitor_userspace_get_fd(struct libmnt_monitor *mn)
 	if (sep && sep > dirname)
 		*(sep - 1) = '/';		/* set '/' back to the path */
 
-	DBG(MONITOR, ul_debugobj(mn, "new fd=%d", me->fd));
 	return me->fd;
 err:
+	DBG(MONITOR, ul_debugobj(mn, "failed to create userspace monitor [rc=%d]", rc));
 	return -errno;
 }
 
-static int monitor_userspace_is_changed(struct libmnt_monitor *mn,
-					struct monitor_entry *me)
+static int userspace_monitor_verify_change(struct libmnt_monitor *mn,
+					   struct monitor_entry *me)
 {
 	char wanted[NAME_MAX + 1];
 	char buf[sizeof(struct inotify_event) + NAME_MAX + 1];
@@ -308,8 +255,6 @@ static int monitor_userspace_is_changed(struct libmnt_monitor *mn,
 	wanted[sizeof(wanted) - 1] = '\0';
 	rc = 0;
 
-	DBG(MONITOR, ul_debugobj(mn, "wanted file: '%s'", wanted));
-
 	while ((r = read(me->fd, buf, sizeof(buf))) > 0) {
 		for (p = buf; p < buf + r; ) {
 			event = (struct inotify_event *) p;
@@ -325,20 +270,77 @@ static int monitor_userspace_is_changed(struct libmnt_monitor *mn,
 	return rc;
 }
 
-#else /* HAVE_INOTIFY_INIT1 */
-int mnt_monitor_enable_userspace(
-		struct libmnt_monitor *mn  __attribute__((unused)),
-		int enable  __attribute__((unused)),
-		const char *filename  __attribute__((unused)))
-{
-	return -ENOSYS;
-}
-int mnt_monitor_userspace_get_fd(
-		struct libmnt_monitor *mn __attribute__((unused)))
+static const struct monitor_opers userspace_opers = {
+	.op_get_fd		= userspace_monitor_get_fd,
+	.op_verify_change	= userspace_monitor_verify_change
+};
+
+
+/**
+ * mnt_monitor_enable_userspace:
+ * @mn: monitor
+ * @enable: 0 or 1
+ * @filename: overwrites default
+ *
+ * Enables or disables userspace monitor. If the monitor does not exist and
+ * enable=1 then allocates new resources necessary for the monitor.
+ *
+ * If high-level monitor has been already initialized (by mnt_monitor_get_fd()
+ * or mnt_wait_monitor()) then it's updated according to @enable.
+ *
+ * The @filename is used only first time when you enable the monitor. It's
+ * impossible to have more than one userspace monitor.
+ *
+ * Return: 0 on success and <0 on error
+ */
+int mnt_monitor_enable_userspace(struct libmnt_monitor *mn, int enable, const char *filename)
 {
-	return -ENOSYS;
+	struct monitor_entry *me;
+	int rc = 0;
+
+	if (!mn)
+		return -EINVAL;
+
+	me = monitor_get_entry(mn, MNT_MONITOR_TYPE_USERSPACE);
+	if (me) {
+		rc = monitor_enable_entry(mn, me, enable);
+		if (!enable && me->fd) {
+			close(me->fd);		/* disable inotify notification */
+			me->fd = -1;
+		}
+		return rc;
+	}
+	if (!enable)
+		return 0;
+
+	DBG(MONITOR, ul_debugobj(mn, "allocate new userspace monitor"));
+
+	/* create a new entry */
+	if (!mnt_has_regular_mtab(&filename, NULL))	/* /etc/mtab */
+		filename = mnt_get_utab_path();		/* /run/mount/utab */
+	if (!filename) {
+		DBG(MONITOR, ul_debugobj(mn, "failed to get userspace mount table path"));
+		return -EINVAL;
+	}
+
+	me = monitor_new_entry(mn);
+	if (!me)
+		goto err;
+
+	me->type = MNT_MONITOR_TYPE_USERSPACE;
+	me->opers = &userspace_opers;
+	me->path = strdup(filename);
+	if (!me->path)
+		goto err;
+
+	return monitor_enable_entry(mn, me, 1);
+err:
+	rc = -errno;
+	free_monitor_entry(me);
+	DBG(MONITOR, ul_debugobj(mn, "failed to allocate userspace monitor [rc=%d]", rc));
+	return rc;
 }
-#endif
+
 
 static int monitor_enable_entry(struct libmnt_monitor *mn,
 				struct monitor_entry *me, int enable)
@@ -352,56 +354,107 @@ static int monitor_enable_entry(struct libmnt_monitor *mn,
 	return 0;
 }
 
-/**
- * mnt_monitor_get_filename:
- * @mn: monitor
- * @fd: event file descriptor
- *
- * Returns: filename monitored by @fd or NULL on error.
- */
-const char *mnt_monitor_get_filename(struct libmnt_monitor *mn, int fd)
+int mnt_monitor_close_fd(struct libmnt_monitor *mn)
 {
-	struct monitor_entry *me = monitor_get_entry_by_fd(mn, fd);
+	if (mn && mn->fd >= 0) {
+		close(mn->fd);
+		mn->fd = -1;
+	}
 
-	if (!me)
-		return NULL;
-	return me->path;
+	return 0;
 }
 
-/**
- * mnt_monitor_is_changed:
- * @mn: monitor
- * @fd: event file descriptor
- *
- * Returns: 1 of the file monitored by @fd has been changed
- */
-int mnt_monitor_is_changed(struct libmnt_monitor *mn, int fd)
+int mnt_monitor_get_fd(struct libmnt_monitor *mn)
 {
-	struct monitor_entry *me = monitor_get_entry_by_fd(mn, fd);
+	struct libmnt_iter itr;
+	struct monitor_entry *me;
 	int rc = 0;
 
-	if (!me)
-		return 0;
+	if (!mn)
+		return -EINVAL;
+	if (mn->fd >= 0)
+		return mn->fd;
 
-	switch (me->type) {
-	case MNT_MONITOR_TYPE_USERSPACE:
-		rc = monitor_userspace_is_changed(mn, me);
-		break;
-	default:
-		return 0;
+	DBG(MONITOR, ul_debugobj(mn, "create top-level monitor fd"));
+	mn->fd = epoll_create1(EPOLL_CLOEXEC);
+	if (mn->fd < 0)
+		return -errno;
+
+	mnt_reset_iter(&itr, MNT_ITER_FORWARD);
+
+	DBG(MONITOR, ul_debugobj(mn, "adding monitor entries to epoll (fd=%d)", mn->fd));
+	while (monitor_next_entry(mn, &itr, &me) == 0) {
+		int fd;
+		struct epoll_event ev = { .events = EPOLLPRI | EPOLLIN };
+
+		if (!me->enable)
+			continue;
+
+		fd = me->opers->op_get_fd(mn, me);
+		if (fd < 0)
+			goto err;
+
+		DBG(MONITOR, ul_debugobj(mn, " add fd=%d (for %s)", fd, me->path));
+
+		ev.data.ptr = (void *) me;
+		if (epoll_ctl(mn->fd, EPOLL_CTL_ADD, fd, &ev) < 0)
+			goto err;
 	}
 
-	DBG(MONITOR, ul_debugobj(mn, "fd=%d %s", me->fd, rc ? "changed" : "unchanged"));
+	DBG(MONITOR, ul_debugobj(mn, "successfully created monitor"));
+	return mn->fd;
+err:
+	rc = errno ? -errno : -EINVAL;
+	close(mn->fd);
+	mn->fd = -1;
+	DBG(MONITOR, ul_debugobj(mn, "failed to create monitor [rc=%d]", rc));
 	return rc;
 }
 
+int mnt_monitor_next_changed(struct libmnt_monitor *mn,
+			     const char **filename,
+			     int *type)
+{
+	int rc;
+
+	if (!mn || mn->fd < 0)
+		return -EINVAL;
+
+	do {
+		struct monitor_entry *me;
+		struct epoll_event events[1];
+
+		rc = epoll_wait(mn->fd, events, 1, 0);
+		if (rc < 0)
+			return -errno;		/* error */
+		if (rc == 0)
+			return 1;		/* nothing */
+
+		me = (struct monitor_entry *) events[0].data.ptr;
+		if (!me)
+			continue;
+
+		if (me->opers->op_verify_change != NULL &&
+		    me->opers->op_verify_change(mn, me) != 1)
+			continue;		/* false positive */
+
+		if (filename)
+			*filename = me->path;
+		if (type)
+			*type = me->type;
+		return 0;
+	} while (1);
+
+	return 0;
+}
 
 #ifdef TEST_PROGRAM
 
+/* monitor @fd by epoll */
 static int my_epoll(struct libmnt_monitor *mn, int fd)
 {
 	int efd = -1, rc = -1;
-	struct epoll_event ev = { .events = 0 };
+	struct epoll_event ev;
 
 	assert(mn);
 	assert(fd >= 0);
@@ -423,21 +476,20 @@ static int my_epoll(struct libmnt_monitor *mn, int fd)
 
 	printf("waiting for changes...\n");
 	do {
+		const char *filename = NULL;
 		struct epoll_event events[1];
-		int n, nfds = epoll_wait(efd, events, 1, -1);
+		int n = epoll_wait(efd, events, 1, -1);
 
-		if (nfds < 0) {
+		if (n < 0) {
 			rc = -errno;
 			warn("polling error");
 			goto done;
 		}
+		if (n == 0 || events[0].data.fd != fd)
+			continue;
 
-		for (n = 0; n < nfds; n++) {
-			if (events[n].data.fd == fd &&
-			    mnt_monitor_is_changed(mn, fd) == 1)
-				printf("%s: change detected\n",
-						mnt_monitor_get_filename(mn, fd));
-		}
+		while (mnt_monitor_next_changed(mn, &filename, NULL) == 0)
+			printf("%s: change detected\n", filename);
 	} while (1);
 
 	rc = 0;
@@ -447,10 +499,13 @@ done:
 	return rc;
 }
 
-int test_low_user(struct libmnt_test *ts, int argc, char *argv[])
+/*
+ * create a monitor and add the monitor fd to epoll
+ */
+int test_epoll(struct libmnt_test *ts, int argc, char *argv[])
 {
 	struct libmnt_monitor *mn;
-	int fd, rc = -1;
+	int i, fd, rc = -1;
 
 	mn = mnt_new_monitor();
 	if (!mn) {
@@ -458,14 +513,18 @@ int test_low_user(struct libmnt_test *ts, int argc, char *argv[])
 		goto done;
 	}
 
-	if (mnt_monitor_enable_userspace(mn, TRUE, NULL)) {
-		warn("failed to initialize userspace monitor");
-		goto done;
+	for (i = 1; i < argc; i++) {
+		if (strcmp(argv[i], "userspace") == 0) {
+			if (mnt_monitor_enable_userspace(mn, TRUE, NULL)) {
+				warn("failed to initialize userspace monitor");
+				goto done;
+			}
+		}
 	}
 
-	fd = mnt_monitor_userspace_get_fd(mn);
+	fd = mnt_monitor_get_fd(mn);
 	if (fd < 0) {
-		warn("failed to initialize userspace monitor fd");
+		warn("failed to initialize monitor fd");
 		goto done;
 	}
 
@@ -479,7 +538,7 @@ done:
 int main(int argc, char *argv[])
 {
 	struct libmnt_test tss[] = {
-		{ "--low-userspace", test_low_user, "tests low-level userspace monitor" },
+		{ "--epoll", test_epoll, "<userspace kernel ...>  test monitor in epoll" },
 		{ NULL }
 	};