[PATCHES] Improve reporting of inotify notifications

Christof Schmitt cs at samba.org
Fri Jul 15 22:52:54 UTC 2016


These patches fix a case seen in the smb2.notify.tree test running
against a server with notify:inotify enabled. Without these patches, the
notifiy requests that ask only for FILE_NOTIFY_CHANGE_FILE_NAME (i=17
and i=18 in the array) also receive the changes for directories. This is
fixed by querying the inotify event mask and only report the events that
map to that mask.

Even with these patches, the smb2.notify.tree receives more events than
expected, but that is expected due to the double notification from
inotify and smbd internal events.

Christof
-------------- next part --------------
From 79f7531158bc4a689b31f7492e62030a9c2c7181 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 14 Jul 2016 13:35:15 -0700
Subject: [PATCH 1/4] smbtorture: Correctly initialize notify request in
 smb2.notify.tree

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 source4/torture/smb2/notify.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/source4/torture/smb2/notify.c b/source4/torture/smb2/notify.c
index e045f25..de4ff58 100644
--- a/source4/torture/smb2/notify.c
+++ b/source4/torture/smb2/notify.c
@@ -1883,7 +1883,9 @@ static bool torture_smb2_notify_tree(struct torture_context *torture,
 	do {
 		/* count events that have happened in each dir */
 		for (i=0;i<ARRAY_SIZE(dirs);i++) {
+			notify.smb2.in.completion_filter = dirs[i].filter;
 			notify.smb2.in.file.handle = dirs[i].h1;
+			notify.smb2.in.recursive = dirs[i].recursive;
 			req = smb2_notify_send(tree, &(notify.smb2));
 			smb2_cancel(req);
 			notify.smb2.out.num_changes = 0;
-- 
1.8.3.1


From a15892bedfb8b1ad2e54b19ef73530ec6bc33b0f Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 14 Jul 2016 15:44:46 -0700
Subject: [PATCH 2/4] smbd: Allow passing notify filter from inotify and fam

This only adds a parameter to the callback without any functional
change.

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 source3/smbd/notify_fam.c      |  8 +++++---
 source3/smbd/notify_inotify.c  | 11 +++++++----
 source3/smbd/notifyd/notifyd.c | 11 +++++++----
 source3/smbd/notifyd/notifyd.h |  3 ++-
 source3/smbd/proto.h           |  6 ++++--
 5 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/source3/smbd/notify_fam.c b/source3/smbd/notify_fam.c
index d8d6946..2cf1e35 100644
--- a/source3/smbd/notify_fam.c
+++ b/source3/smbd/notify_fam.c
@@ -52,7 +52,8 @@ struct fam_watch_context {
 	struct sys_notify_context *sys_ctx;
 	void (*callback)(struct sys_notify_context *ctx,
 			 void *private_data,
-			 struct notify_event *ev);
+			 struct notify_event *ev,
+			 uint32_t filter);
 	void *private_data;
 	uint32_t mask; /* the inotify mask */
 	uint32_t filter; /* the windows completion filter */
@@ -205,7 +206,7 @@ static void fam_handler(struct tevent_context *event_ctx,
 	}
 	ne.dir = ctx->path;
 
-	ctx->callback(ctx->sys_ctx, ctx->private_data, &ne);
+	ctx->callback(ctx->sys_ctx, ctx->private_data, &ne, UINT32_MAX);
 }
 
 static int fam_watch_context_destructor(struct fam_watch_context *ctx)
@@ -228,7 +229,8 @@ int fam_watch(TALLOC_CTX *mem_ctx,
 	      uint32_t *subdir_filter,
 	      void (*callback)(struct sys_notify_context *ctx,
 			       void *private_data,
-			       struct notify_event *ev),
+			       struct notify_event *ev,
+			       uint32_t filter),
 	      void *private_data,
 	      void *handle_p)
 {
diff --git a/source3/smbd/notify_inotify.c b/source3/smbd/notify_inotify.c
index 78fb654..88a54ec 100644
--- a/source3/smbd/notify_inotify.c
+++ b/source3/smbd/notify_inotify.c
@@ -48,7 +48,8 @@ struct inotify_watch_context {
 	int wd;
 	void (*callback)(struct sys_notify_context *ctx, 
 			 void *private_data,
-			 struct notify_event *ev);
+			 struct notify_event *ev,
+			 uint32_t filter);
 	void *private_data;
 	uint32_t mask; /* the inotify mask */
 	uint32_t filter; /* the windows completion filter */
@@ -164,7 +165,7 @@ static void inotify_dispatch(struct inotify_private *in,
 		next = w->next;
 		if (w->wd == e->wd && filter_match(w, e)) {
 			ne.dir = w->path;
-			w->callback(in->ctx, w->private_data, &ne);
+			w->callback(in->ctx, w->private_data, &ne, UINT32_MAX);
 		}
 	}
 
@@ -185,7 +186,8 @@ static void inotify_dispatch(struct inotify_private *in,
 			if (w->wd == e->wd && filter_match(w, e) &&
 			    !(w->filter & FILE_NOTIFY_CHANGE_CREATION)) {
 				ne.dir = w->path;
-				w->callback(in->ctx, w->private_data, &ne);
+				w->callback(in->ctx, w->private_data, &ne,
+					    UINT32_MAX);
 			}
 		}
 	}
@@ -355,7 +357,8 @@ int inotify_watch(TALLOC_CTX *mem_ctx,
 		  uint32_t *subdir_filter,
 		  void (*callback)(struct sys_notify_context *ctx,
 				   void *private_data,
-				   struct notify_event *ev),
+				   struct notify_event *ev,
+				   uint32_t filter),
 		  void *private_data,
 		  void *handle_p)
 {
diff --git a/source3/smbd/notifyd/notifyd.c b/source3/smbd/notifyd/notifyd.c
index 45b029b..519109f 100644
--- a/source3/smbd/notifyd/notifyd.c
+++ b/source3/smbd/notifyd/notifyd.c
@@ -140,7 +140,8 @@ static void notifyd_broadcast_reclog(struct ctdbd_connection *ctdbd_conn,
 				     struct messaging_reclog *log);
 #endif
 static void notifyd_sys_callback(struct sys_notify_context *ctx,
-				 void *private_data, struct notify_event *ev);
+				 void *private_data, struct notify_event *ev,
+				 uint32_t filter);
 
 #ifdef CLUSTER_SUPPORT
 static struct tevent_req *notifyd_broadcast_reclog_send(
@@ -163,7 +164,8 @@ static int sys_notify_watch_dummy(
 	uint32_t *subdir_filter,
 	void (*callback)(struct sys_notify_context *ctx,
 			 void *private_data,
-			 struct notify_event *ev),
+			 struct notify_event *ev,
+			 uint32_t filter),
 	void *private_data,
 	void *handle_p)
 {
@@ -513,7 +515,8 @@ fail:
 }
 
 static void notifyd_sys_callback(struct sys_notify_context *ctx,
-				 void *private_data, struct notify_event *ev)
+				 void *private_data, struct notify_event *ev,
+				 uint32_t filter)
 {
 	struct messaging_context *msg_ctx = talloc_get_type_abort(
 		private_data, struct messaging_context);
@@ -524,7 +527,7 @@ static void notifyd_sys_callback(struct sys_notify_context *ctx,
 	msg = (struct notify_trigger_msg) {
 		.when = timespec_current(),
 		.action = ev->action,
-		.filter = UINT32_MAX
+		.filter = filter,
 	};
 
 	iov[0].iov_base = &msg;
diff --git a/source3/smbd/notifyd/notifyd.h b/source3/smbd/notifyd/notifyd.h
index 672ffba..82fdd6e 100644
--- a/source3/smbd/notifyd/notifyd.h
+++ b/source3/smbd/notifyd/notifyd.h
@@ -129,7 +129,8 @@ typedef int (*sys_notify_watch_fn)(TALLOC_CTX *mem_ctx,
 				   uint32_t *subdir_filter,
 				   void (*callback)(struct sys_notify_context *ctx,
 						    void *private_data,
-						    struct notify_event *ev),
+						    struct notify_event *ev,
+						    uint32_t filter),
 				   void *private_data,
 				   void *handle_p);
 
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 81bdc87..f330b4c 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -560,7 +560,8 @@ int inotify_watch(TALLOC_CTX *mem_ctx,
 		  uint32_t *subdir_filter,
 		  void (*callback)(struct sys_notify_context *ctx,
 				   void *private_data,
-				   struct notify_event *ev),
+				   struct notify_event *ev,
+				   uint32_t filter),
 		  void *private_data,
 		  void *handle_p);
 
@@ -571,7 +572,8 @@ int fam_watch(TALLOC_CTX *mem_ctx,
 	      uint32_t *subdir_filter,
 	      void (*callback)(struct sys_notify_context *ctx,
 			       void *private_data,
-			       struct notify_event *ev),
+			       struct notify_event *ev,
+			       uint32_t filter),
 	      void *private_data,
 	      void *handle_p);
 
-- 
1.8.3.1


From 995911521b4056ba7e457b200c509d54a0a8974b Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Fri, 15 Jul 2016 12:15:15 -0700
Subject: [PATCH 3/4] notify_inotify: Move mapping table to top of file

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 source3/smbd/notify_inotify.c | 61 +++++++++++++++++++++----------------------
 1 file changed, 30 insertions(+), 31 deletions(-)

diff --git a/source3/smbd/notify_inotify.c b/source3/smbd/notify_inotify.c
index 88a54ec..f7034f1 100644
--- a/source3/smbd/notify_inotify.c
+++ b/source3/smbd/notify_inotify.c
@@ -58,6 +58,36 @@ struct inotify_watch_context {
 
 
 /*
+  map from a change notify mask to a inotify mask. Remove any bits
+  which we can handle
+*/
+static const struct {
+	uint32_t notify_mask;
+	uint32_t inotify_mask;
+} inotify_mapping[] = {
+	{FILE_NOTIFY_CHANGE_FILE_NAME,   IN_CREATE|IN_DELETE|IN_MOVED_FROM|IN_MOVED_TO},
+	{FILE_NOTIFY_CHANGE_DIR_NAME,    IN_CREATE|IN_DELETE|IN_MOVED_FROM|IN_MOVED_TO},
+	{FILE_NOTIFY_CHANGE_ATTRIBUTES,  IN_ATTRIB|IN_MOVED_TO|IN_MOVED_FROM|IN_MODIFY},
+	{FILE_NOTIFY_CHANGE_LAST_WRITE,  IN_ATTRIB},
+	{FILE_NOTIFY_CHANGE_LAST_ACCESS, IN_ATTRIB},
+	{FILE_NOTIFY_CHANGE_EA,          IN_ATTRIB},
+	{FILE_NOTIFY_CHANGE_SECURITY,    IN_ATTRIB}
+};
+
+static uint32_t inotify_map(uint32_t *filter)
+{
+	int i;
+	uint32_t out=0;
+	for (i=0;i<ARRAY_SIZE(inotify_mapping);i++) {
+		if (inotify_mapping[i].notify_mask & *filter) {
+			out |= inotify_mapping[i].inotify_mask;
+			*filter &= ~inotify_mapping[i].notify_mask;
+		}
+	}
+	return out;
+}
+
+/*
   destroy the inotify private context
 */
 static int inotify_destructor(struct inotify_private *in)
@@ -288,37 +318,6 @@ static int inotify_setup(struct sys_notify_context *ctx)
 	return 0;
 }
 
-
-/*
-  map from a change notify mask to a inotify mask. Remove any bits
-  which we can handle
-*/
-static const struct {
-	uint32_t notify_mask;
-	uint32_t inotify_mask;
-} inotify_mapping[] = {
-	{FILE_NOTIFY_CHANGE_FILE_NAME,   IN_CREATE|IN_DELETE|IN_MOVED_FROM|IN_MOVED_TO},
-	{FILE_NOTIFY_CHANGE_DIR_NAME,    IN_CREATE|IN_DELETE|IN_MOVED_FROM|IN_MOVED_TO},
-	{FILE_NOTIFY_CHANGE_ATTRIBUTES,  IN_ATTRIB|IN_MOVED_TO|IN_MOVED_FROM|IN_MODIFY},
-	{FILE_NOTIFY_CHANGE_LAST_WRITE,  IN_ATTRIB},
-	{FILE_NOTIFY_CHANGE_LAST_ACCESS, IN_ATTRIB},
-	{FILE_NOTIFY_CHANGE_EA,          IN_ATTRIB},
-	{FILE_NOTIFY_CHANGE_SECURITY,    IN_ATTRIB}
-};
-
-static uint32_t inotify_map(uint32_t *filter)
-{
-	int i;
-	uint32_t out=0;
-	for (i=0;i<ARRAY_SIZE(inotify_mapping);i++) {
-		if (inotify_mapping[i].notify_mask & *filter) {
-			out |= inotify_mapping[i].inotify_mask;
-			*filter &= ~inotify_mapping[i].notify_mask;
-		}
-	}
-	return out;
-}
-
 /*
   destroy a watch
 */
-- 
1.8.3.1


From a0808c75b3747d37609df9d3e35f6fee5e445dbe Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Fri, 15 Jul 2016 12:16:18 -0700
Subject: [PATCH 4/4] notify_inotify: Map inotify mask back to filter

Instead of reporting that an inotify event triggered all possible filter
masks, map the inotify event back to the filter mask. This is slightly
more accurate, although there can still be mismatches due to the
mapping.

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 source3/smbd/notify_inotify.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/source3/smbd/notify_inotify.c b/source3/smbd/notify_inotify.c
index f7034f1..3848dd6 100644
--- a/source3/smbd/notify_inotify.c
+++ b/source3/smbd/notify_inotify.c
@@ -88,6 +88,30 @@ static uint32_t inotify_map(uint32_t *filter)
 }
 
 /*
+ * Map inotify mask back to filter. This returns all filters that
+ * could have created the inotify watch.
+ */
+static uint32_t inotify_map_mask_to_filter(uint32_t mask)
+{
+	int i;
+	uint32_t filter = 0;
+
+	for (i = 0; i < ARRAY_SIZE(inotify_mapping); i++) {
+		if (inotify_mapping[0].inotify_mask & mask) {
+			filter |= inotify_mapping[i].notify_mask;
+		}
+	}
+
+	if (mask & IN_ISDIR) {
+		filter &= ~FILE_NOTIFY_CHANGE_FILE_NAME;
+	} else {
+		filter &= ~FILE_NOTIFY_CHANGE_DIR_NAME;
+	}
+
+	return filter;
+}
+
+/*
   destroy the inotify private context
 */
 static int inotify_destructor(struct inotify_private *in)
@@ -153,6 +177,7 @@ static void inotify_dispatch(struct inotify_private *in,
 {
 	struct inotify_watch_context *w, *next;
 	struct notify_event ne;
+	uint32_t filter;
 
 	DEBUG(10, ("inotify_dispatch called with mask=%x, name=[%s]\n",
 		   e->mask, e->len ? e->name : ""));
@@ -187,15 +212,17 @@ static void inotify_dispatch(struct inotify_private *in,
 	}
 	ne.path = e->name;
 
-	DEBUG(10, ("inotify_dispatch: ne.action = %d, ne.path = %s\n",
-		   ne.action, ne.path));
+	filter = inotify_map_mask_to_filter(e->mask);
+
+	DBG_DEBUG("ne.action = %d, ne.path = %s, filter = %d\n",
+		  ne.action, ne.path, filter);
 
 	/* find any watches that have this watch descriptor */
 	for (w=in->watches;w;w=next) {
 		next = w->next;
 		if (w->wd == e->wd && filter_match(w, e)) {
 			ne.dir = w->path;
-			w->callback(in->ctx, w->private_data, &ne, UINT32_MAX);
+			w->callback(in->ctx, w->private_data, &ne, filter);
 		}
 	}
 
@@ -217,7 +244,7 @@ static void inotify_dispatch(struct inotify_private *in,
 			    !(w->filter & FILE_NOTIFY_CHANGE_CREATION)) {
 				ne.dir = w->path;
 				w->callback(in->ctx, w->private_data, &ne,
-					    UINT32_MAX);
+					    filter);
 			}
 		}
 	}
-- 
1.8.3.1



More information about the samba-technical mailing list