account: Watch parent directory instead of files directly

This commit changes the way the inotify watching is done. Instead of
watching files directly, we watch the directory they reside in and just
filter out changes in files we don't want to watch. This brings a benefit
of reliable notification through file deletions and other inode number
changing changes. It's also less racy as we had to recreate watches on
IN_IGNORED events with possibility of missing some events. File deletions
or atomic replaces were always problematic.

There are some lingering issues however. The most severe is a problem of
shadow files and libuser calls. At the time the event is processed further
in the gather() method, shadow files may not been updated yet. This causes
libuser to miss some information required for proper CIM object instance
construction, potentially propagating half-baked data to the user. This
could be solved by implementing a settle timeout, possibly compressing
more fast-coming events into one. For the moment a simple artificial
sleep has been put before returning from the watcher in good hope shadow
files are updated properly.

Then there's a current problem of throwing out the rest of the read buffer,
losing notifications of other files if watched. This is by current design
of the watcher callback and could be fixed by implementing proper event
queue.

2 files changed, 34 insertions, 63 deletions

diff --git a/src/account/indication_common.c b/src/account/indication_common.c
index 88d16a0..17d570f 100644
--- a/src/account/indication_common.c
+++ b/src/account/indication_common.c
@@ -45,8 +45,11 @@ static const char* allowed_classes[] = {
 
 #define EVENT_SIZE (sizeof(struct inotify_event))
 #define BUF_LEN (10 * EVENT_SIZE + NAME_MAX + 1)
-#define PASSWD_FILE "/etc/passwd"
-#define GROUP_FILE "/etc/group"
+#define WATCH_PATH "/etc/"
+#define PASSWD_FILE_NAME "passwd"
+#define GROUP_FILE_NAME "group"
+
+#define SETTLE_DELAY 1000 * 250  /* usec */
 
 bool filter_checker(const CMPISelectExp *filter)
 {
@@ -107,44 +110,34 @@ static int timecmp(struct timespec a, struct timespec b)
     }
 }
 
-#define ADD_WATCH(fd, wd, file)\
-    (wd) = inotify_add_watch((fd), (file), IN_CLOSE_WRITE | IN_MODIFY |\
-                                           IN_DELETE | IN_DELETE_SELF);\
-    if ((wd) < 0) {\
-        goto bail;\
-    }\
-
 bool watcher_init(AccountIndication *ind)
 {
-    ind->wd_pwd = 0;
-    ind->wd_grp = 0;
+    ind->wd = -1;
     ind->inotify_fd = inotify_init();
 
     if (ind->inotify_fd < 0)
         return false;
 
     /* Get initial timestamps, at the beginning of watching. */
-    ind->last_pwd = get_last_mod(PASSWD_FILE);
-    ind->last_grp = get_last_mod(GROUP_FILE);
-
-    ADD_WATCH(ind->inotify_fd, ind->wd_pwd, PASSWD_FILE);
-    ADD_WATCH(ind->inotify_fd, ind->wd_grp, GROUP_FILE);
+    ind->last_pwd = get_last_mod(WATCH_PATH PASSWD_FILE_NAME);
+    ind->last_grp = get_last_mod(WATCH_PATH GROUP_FILE_NAME);
 
+    ind->wd = inotify_add_watch(ind->inotify_fd, WATCH_PATH,
+                                IN_CLOSE_WRITE | IN_CREATE | IN_MODIFY | IN_MOVED_TO);
+    if (ind->wd < 0) {
+        watcher_destroy(ind);
+        return false;
+    }
     return true;
-
-bail:
-    watcher_destroy(ind);
-    return false;
 }
 
 void watcher_destroy(AccountIndication *ind)
 {
     if (ind->inotify_fd >= 0) {
-        if (ind->wd_pwd > 0)
-            inotify_rm_watch(ind->inotify_fd, ind->wd_pwd);
-        if (ind->wd_grp > 0)
-            inotify_rm_watch(ind->inotify_fd, ind->wd_grp);
+        if (ind->wd >= 0)
+            inotify_rm_watch(ind->inotify_fd, ind->wd);
         close(ind->inotify_fd);
+        ind->wd = -1;
         ind->inotify_fd = -1;
     }
 }
@@ -167,53 +160,31 @@ bool watcher(AccountIndication *ind, void **data)
         }
         while (i + (ssize_t) EVENT_SIZE < len) {
             struct inotify_event *event = (struct inotify_event *) &buffer[i];
-            if (i + (ssize_t) EVENT_SIZE + event->len >= len) {
+            if (i + (ssize_t) EVENT_SIZE + event->len > len) {
                 error("Unable to create watcher, inotify initialization failed");
                 watcher_destroy(ind);
                 watcher_init(ind);
                 return false;
             }
-            switch (event->mask) {
-                case IN_MODIFY:
-                case IN_CLOSE_WRITE:
-                case IN_DELETE:
-                case IN_DELETE_SELF:
-                    if (event->wd == ind->wd_grp) {
-                        curr_grp = get_last_mod(GROUP_FILE);
-                        if (timecmp(ind->last_grp, curr_grp) == -1) {
-                            ind->last_grp = curr_grp;
-                            return true;
-                        }
-                    } else {
-                        curr_pwd = get_last_mod(GROUP_FILE);
-                        if (timecmp(ind->last_pwd, curr_pwd) == -1) {
-                            ind->last_pwd = curr_pwd;
-                            return true;
-                        }
+            if (event->len > 1) {
+                if (strcmp(&event->name[0], PASSWD_FILE_NAME) == 0) {
+                    curr_pwd = get_last_mod(WATCH_PATH PASSWD_FILE_NAME);
+                    if (timecmp(ind->last_pwd, curr_pwd) == -1) {
+                        ind->last_pwd = curr_pwd;
+                        usleep(SETTLE_DELAY);
+                        return true;
                     }
-                    break;
-                case IN_IGNORED:
-                    if (event->wd == ind->wd_grp) {
-                        ADD_WATCH(ind->inotify_fd, ind->wd_grp, GROUP_FILE);
-                        curr_grp = get_last_mod(GROUP_FILE);
-                        if (timecmp(ind->last_grp, curr_grp) == -1) {
-                            ind->last_grp = curr_grp;
-                            return true;
-                        }
-                    } else {
-                        ADD_WATCH(ind->inotify_fd, ind->wd_pwd, PASSWD_FILE);
-                        curr_pwd = get_last_mod(GROUP_FILE);
-                        if (timecmp(ind->last_pwd, curr_pwd) == -1) {
-                            ind->last_pwd = curr_pwd;
-                            return true;
-                        }
+                } else
+                if (strcmp(&event->name[0], GROUP_FILE_NAME) == 0) {
+                    curr_grp = get_last_mod(WATCH_PATH GROUP_FILE_NAME);
+                    if (timecmp(ind->last_grp, curr_grp) == -1) {
+                        ind->last_grp = curr_grp;
+                        usleep(SETTLE_DELAY);
+                        return true;
                     }
-                    break;
+                }
             }
             i += EVENT_SIZE + event->len;
         }
     } while (1);
-
-bail:
-    return false;
 }
diff --git a/src/account/indication_common.h b/src/account/indication_common.h
index 6b7bc6d..06eaa7b 100644
--- a/src/account/indication_common.h
+++ b/src/account/indication_common.h
@@ -19,7 +19,7 @@
  */
 
 typedef struct {
-    int wd_pwd, wd_grp;
+    int wd;
     int inotify_fd;
     struct timespec last_pwd, last_grp;
 } AccountIndication;