r20442: Slight rewrite of the change notify infrastructure. This now survives the

first of the raw-notify subtests, the one-level test_notify_dir without any
flags around yet.

The tricky part was getting the data structures right, I hope the next tests
don't let that fall over.

fsp->notify is now by default NULL, meaning that nobody has issued a
changenotify call. This means nobody is interested in changes for this
directory.

If that has happened, notify_change_buf collects the changes if no current
request is outstanding, and it collects the requests if no change has happened
since the last request.

Happy New Year, somewhere on this planet it's already 2007 :-)

Volker

P.S: Jeremy, there's a question for you in smbd/files.c line 367.
(This used to be commit ce0ad24988075465addcac0b9afc872e909135af)

diff --git a/source3/include/smb.h b/source3/include/smb.h
index c4d46600288c020a4d55e4b0568b13d09951d3b7..5f69ac23fa050cea758679f675e7797db39ff6b8 100644 (file)
--- a/source3/include/smb.h
+++ b/source3/include/smb.h
@@ -428,14 +428,46 @@ struct vfs_fsp_data {
      */
 };
 
+/* the basic packet size, assuming no words or bytes */
+#define smb_size 39
+
 struct notify_change {
        uint32_t action;
        char *name;
 };
 
-struct notify_changes {
-       uint32_t num_changes;
+struct notify_change_request {
+       struct notify_change_request *prev, *next;
+       struct files_struct *fsp;       /* backpointer for cancel by mid */
+       char request_buf[smb_size];
+       uint32 max_param_count;
+       struct notify_mid_map *mid_map;
+};
+
+/*
+ * For NTCancel, we need to find the notify_change_request indexed by
+ * mid. Separate list here.
+ */
+
+struct notify_mid_map {
+       struct notify_mid_map *prev, *next;
+       struct notify_change_request *req;
+       uint16 mid;
+};
+
+struct notify_change_buf {
+       /*
+        * If no requests are pending, changes are queued here. Simple array,
+        * we only append.
+        */
+       unsigned num_changes;
        struct notify_change *changes;
+
+       /*
+        * If no changes are around requests are queued here. Using a linked
+        * list, because we have to append at the end and delete from the top.
+        */
+       struct notify_change_request *requests;
 };
 
 typedef struct files_struct {
@@ -481,7 +513,7 @@ typedef struct files_struct {
        struct vfs_fsp_data *vfs_extension;
        FAKE_FILE_HANDLE *fake_file_handle;
 
-       struct notify_changes *notify;
+       struct notify_change_buf *notify;
 } files_struct;
 
 #include "ntquotas.h"
@@ -891,9 +923,6 @@ struct bitmap {
        unsigned int n;
 };
 
-/* the basic packet size, assuming no words or bytes */
-#define smb_size 39
-
 /* offsets into message for common items */
 #define smb_com 8
 #define smb_rcls 9

diff --git a/source3/smbd/close.c b/source3/smbd/close.c
index a3ddcae11d004645c351efc05ba46136a359af45..282c4acb244db03272a7ef3e0c7c22b73a68b1ce 100644 (file)
--- a/source3/smbd/close.c
+++ b/source3/smbd/close.c
@@ -416,7 +416,8 @@ static int close_directory(files_struct *fsp, enum file_close_type close_type)
                process_pending_change_notify_queue((time_t)0);
        } else {
                TALLOC_FREE(lck);
-               remove_pending_change_notify_requests_by_fid(fsp, NT_STATUS_CANCELLED);
+               remove_pending_change_notify_requests_by_fid(
+                       fsp, NT_STATUS_OK);
        }
 
        /*

diff --git a/source3/smbd/files.c b/source3/smbd/files.c
index 8df7a29a65d70a75e9d82d7d601c2ff2e1066eb1..982de4c55fd0658298c24ddfb1ac6f600de60134 100644 (file)
--- a/source3/smbd/files.c
+++ b/source3/smbd/files.c
@@ -100,12 +100,6 @@ NTSTATUS file_new(connection_struct *conn, files_struct **result)
 
        ZERO_STRUCTP(fsp->fh);
 
-       if (!(fsp->notify = TALLOC_ZERO_P(NULL, struct notify_changes))) {
-               SAFE_FREE(fsp->fh);
-               SAFE_FREE(fsp);
-               return NT_STATUS_NO_MEMORY;
-       }
-
        fsp->fh->ref_count = 1;
        fsp->fh->fd = -1;
 
@@ -367,33 +361,48 @@ files_struct *file_find_di_next(files_struct *start_fsp)
        return NULL;
 }
 
-/****************************************************************************
- Find the directory fsp given a device and inode with the lowest
- file_id. First use is for notify actions.
-****************************************************************************/
+/*
+ * Same as file_find_di_first/next, but also finds non-fd opens.
+ *
+ * Jeremy, do we really need the fsp->fh->fd != -1 ??
+ */
 
-files_struct *file_find_dir_lowest_id(SMB_DEV_T dev, SMB_INO_T inode)
+struct files_struct *fsp_find_di_first(SMB_DEV_T dev, SMB_INO_T inode)
 {
        files_struct *fsp;
-       files_struct *min_fsp = NULL;
 
-       for (fsp = Files; fsp; fsp = fsp->next) {
-               if (!fsp->is_directory
-                   || fsp->dev != dev || fsp->inode != inode) {
-                       continue;
-               }
+       if (fsp_fi_cache.dev == dev && fsp_fi_cache.inode == inode) {
+               /* Positive or negative cache hit. */
+               return fsp_fi_cache.fsp;
+       }
 
-               if (min_fsp == NULL) {
-                       min_fsp = fsp;
-                       continue;
-               }
+       fsp_fi_cache.dev = dev;
+       fsp_fi_cache.inode = inode;
 
-               if (fsp->fh->file_id < min_fsp->fh->file_id) {
-                       min_fsp = fsp;
+       for (fsp=Files;fsp;fsp=fsp->next) {
+               if ((fsp->dev == dev) && (fsp->inode == inode)) {
+                       /* Setup positive cache. */
+                       fsp_fi_cache.fsp = fsp;
+                       return fsp;
                }
        }
 
-       return min_fsp;
+       /* Setup negative cache. */
+       fsp_fi_cache.fsp = NULL;
+       return NULL;
+}
+
+struct files_struct *fsp_find_di_next(files_struct *start_fsp)
+{
+       files_struct *fsp;
+
+       for (fsp = start_fsp->next;fsp;fsp=fsp->next) {
+               if ( (fsp->dev == start_fsp->dev)
+                    && (fsp->inode == start_fsp->inode) )
+                       return fsp;
+       }
+
+       return NULL;
 }
 
 /****************************************************************************

diff --git a/source3/smbd/notify.c b/source3/smbd/notify.c
index 674505ac5bc72869f3de35a87d26867ef20819ae..9957c1fc5d4c104261c4313bb023f662eb38078f 100644 (file)
--- a/source3/smbd/notify.c
+++ b/source3/smbd/notify.c
@@ -22,6 +22,7 @@
 #include "includes.h"
 
 static struct cnotify_fns *cnotify;
+static struct notify_mid_map *notify_changes_by_mid;
 
 /****************************************************************************
  This is the structure to queue to implement NT change
@@ -41,14 +42,15 @@ struct change_notify {
 
 static struct change_notify *change_notify_list;
 
-static BOOL notify_marshall_changes(struct notify_changes *changes,
+static BOOL notify_marshall_changes(unsigned num_changes,
+                                   struct notify_change *changes,
                                    prs_struct *ps)
 {
        int i;
        UNISTR uni_name;
 
-       for (i=0; i<changes->num_changes; i++) {
-               struct notify_change *c = &changes->changes[i];
+       for (i=0; i<num_changes; i++) {
+               struct notify_change *c = &changes[i];
                size_t namelen;
                uint32 u32_tmp; /* Temp arg to prs_uint32 to avoid
                                 * signed/unsigned issues */
@@ -66,7 +68,7 @@ static BOOL notify_marshall_changes(struct notify_changes *changes,
                 * Offset to next entry, only if there is one
                 */
 
-               u32_tmp = (i == changes->num_changes-1) ? 0 : namelen + 12;
+               u32_tmp = (i == num_changes-1) ? 0 : namelen + 12;
                if (!prs_uint32("offset", ps, 1, &u32_tmp)) goto fail;
 
                u32_tmp = c->action;
@@ -96,8 +98,7 @@ static BOOL notify_marshall_changes(struct notify_changes *changes,
  Setup the common parts of the return packet and send it.
 *****************************************************************************/
 
-static void change_notify_reply_packet(const char *request_buf,
-                                      NTSTATUS error_code)
+void change_notify_reply_packet(const char *request_buf, NTSTATUS error_code)
 {
        char outbuf[smb_size+38];
 
@@ -118,14 +119,14 @@ static void change_notify_reply_packet(const char *request_buf,
 }
 
 void change_notify_reply(const char *request_buf, uint32 max_param_count,
-                        files_struct *fsp)
+                        unsigned num_changes, struct notify_change *changes)
 {
        char *outbuf = NULL;
        prs_struct ps;
        size_t buflen = smb_size+38+max_param_count;
 
        if (!prs_init(&ps, 0, NULL, False)
-           || !notify_marshall_changes(fsp->notify, &ps)) {
+           || !notify_marshall_changes(num_changes, changes, &ps)) {
                change_notify_reply_packet(request_buf, NT_STATUS_NO_MEMORY);
                goto done;
        }
@@ -154,8 +155,6 @@ void change_notify_reply(const char *request_buf, uint32 max_param_count,
  done:
        SAFE_FREE(outbuf);
        prs_mem_free(&ps);
-       fsp->notify->num_changes = 0;
-       TALLOC_FREE(fsp->notify->changes);
 }
 
 /****************************************************************************
@@ -171,39 +170,100 @@ static void change_notify_remove(struct change_notify *cnbp)
        SAFE_FREE(cnbp);
 }
 
-/****************************************************************************
- Delete entries by fnum from the change notify pending queue.
-*****************************************************************************/
+NTSTATUS change_notify_add_request(const char *inbuf, uint32 max_param_count,
+                                  struct files_struct *fsp)
+{
+       struct notify_change_request *request = NULL;
+       struct notify_mid_map *map = NULL;
+
+       if (!(request = SMB_MALLOC_P(struct notify_change_request))
+           || !(map = SMB_MALLOC_P(struct notify_mid_map))) {
+               SAFE_FREE(request);
+               return NT_STATUS_NO_MEMORY;
+       }
 
-void remove_pending_change_notify_requests_by_fid(files_struct *fsp, NTSTATUS status)
+       request->mid_map = map;
+       map->req = request;
+
+       memcpy(request->request_buf, inbuf, sizeof(request->request_buf));
+       request->max_param_count = max_param_count;
+       request->fsp = fsp;
+       DLIST_ADD_END(fsp->notify->requests, request,
+                     struct notify_change_request *);
+
+       map->mid = SVAL(inbuf, smb_mid);
+       DLIST_ADD(notify_changes_by_mid, map);
+
+       /* Push the MID of this packet on the signing queue. */
+       srv_defer_sign_response(SVAL(inbuf,smb_mid));
+
+       return NT_STATUS_OK;
+}
+
+static void change_notify_remove_request(struct notify_change_request *remove_req)
 {
-       struct change_notify *cnbp, *next;
+       files_struct *fsp;
+       struct notify_change_request *req;
 
-       for (cnbp=change_notify_list; cnbp; cnbp=next) {
-               next=cnbp->next;
-               if (cnbp->fsp->fnum == fsp->fnum) {
-                       change_notify_reply_packet(cnbp->request_buf, status);
-                       change_notify_remove(cnbp);
+       /*
+        * Paranoia checks, the fsp referenced must must have the request in
+        * its list of pending requests
+        */
+
+       fsp = remove_req->fsp;
+       SMB_ASSERT(fsp->notify != NULL);
+
+       for (req = fsp->notify->requests; req; req = req->next) {
+               if (req == remove_req) {
+                       break;
                }
        }
+       SMB_ASSERT(req != NULL);
+
+       DLIST_REMOVE(fsp->notify->requests, req);
+       DLIST_REMOVE(notify_changes_by_mid, req->mid_map);
+       SAFE_FREE(req->mid_map);
+       SAFE_FREE(req);
 }
 
 /****************************************************************************
  Delete entries by mid from the change notify pending queue. Always send reply.
 *****************************************************************************/
 
-void remove_pending_change_notify_requests_by_mid(int mid)
+void remove_pending_change_notify_requests_by_mid(uint16 mid)
 {
-       struct change_notify *cnbp, *next;
+       struct notify_mid_map *map;
 
-       for (cnbp=change_notify_list; cnbp; cnbp=next) {
-               next=cnbp->next;
-               if(SVAL(cnbp->request_buf,smb_mid) == mid) {
-                       change_notify_reply_packet(cnbp->request_buf,
-                                                  NT_STATUS_CANCELLED);
-                       change_notify_remove(cnbp);
+       for (map = notify_changes_by_mid; map; map = map->next) {
+               if (map->mid == mid) {
+                       break;
                }
        }
+
+       if (map == NULL) {
+               return;
+       }
+
+       change_notify_reply_packet(map->req->request_buf, NT_STATUS_CANCELLED);
+       change_notify_remove_request(map->req);
+}
+
+/****************************************************************************
+ Delete entries by fnum from the change notify pending queue.
+*****************************************************************************/
+
+void remove_pending_change_notify_requests_by_fid(files_struct *fsp,
+                                                 NTSTATUS status)
+{
+       if (fsp->notify == NULL) {
+               return;
+       }
+
+       while (fsp->notify->requests != NULL) {
+               change_notify_reply_packet(
+                       fsp->notify->requests->request_buf, status);
+               change_notify_remove_request(fsp->notify->requests);
+       }
 }
 
 /****************************************************************************
@@ -271,7 +331,8 @@ BOOL process_pending_change_notify_queue(time_t t)
                                  cnbp->fsp->notify->num_changes));
                        change_notify_reply(cnbp->request_buf,
                                            cnbp->max_param_count,
-                                           cnbp->fsp);
+                                           cnbp->fsp->notify->num_changes,
+                                           cnbp->fsp->notify->changes);
                        change_notify_remove(cnbp);
                        continue;
                }
@@ -283,7 +344,8 @@ BOOL process_pending_change_notify_queue(time_t t)
                                  "%s changed !\n", cnbp->fsp->fsp_name ));
                        change_notify_reply(cnbp->request_buf,
                                            cnbp->max_param_count,
-                                           cnbp->fsp);
+                                           cnbp->fsp->notify->num_changes,
+                                           cnbp->fsp->notify->changes);
                        change_notify_remove(cnbp);
                }
        }
@@ -472,43 +534,42 @@ void notify_action(connection_struct *conn, const char *parent,
        TALLOC_FREE(lck);
 }
 
-static void notify_message_callback(int msgtype, struct process_id pid,
-                                   void *buf, size_t len)
+static void notify_fsp(files_struct *fsp, struct notify_message *msg)
 {
-       struct notify_message msg;
-       files_struct *fsp;
-       struct notify_change *changes, *change;
-       struct change_notify *cnbp;
+       struct notify_change *change, *changes;
 
-       if (!buf_to_notify_message(buf, len, &msg)) {
+       if (fsp->notify == NULL) {
+               /*
+                * Nobody is waiting, don't queue
+                */
                return;
        }
 
-       DEBUG(10, ("Received notify_message for 0x%x/%.0f: %d\n",
-                  (unsigned)msg.dev, (double)msg.inode, msg.action));
+       if (fsp->notify->requests != NULL) {
+               /*
+                * Someone is waiting for the change, trigger the reply
+                * immediately
+                */
 
-       fsp = NULL;
+               struct notify_change_request *req = fsp->notify->requests;
+               struct notify_change onechange;
 
-       for (cnbp = change_notify_list; cnbp != NULL; cnbp = cnbp->next) {
-               if ((cnbp->fsp->dev == msg.dev)
-                   && (cnbp->fsp->inode == msg.inode)) {
-                       break;
-               }
-       }
+               onechange.action = msg->action;
+               onechange.name = msg->name;
 
-       if (cnbp != NULL) {
-               DEBUG(10, ("Found pending change notify for %s\n",
-                          cnbp->fsp->fsp_name));
-               fsp = cnbp->fsp;
-               SMB_ASSERT(fsp->notify->num_changes == 0);
-       }
+               change_notify_reply(req->request_buf, req->max_param_count,
+                                   1, &onechange);
 
-       if ((fsp == NULL)
-           && !(fsp = file_find_dir_lowest_id(msg.dev, msg.inode))) {
-               DEBUG(10, ("notify_message: did not find fsp\n"));
+               DLIST_REMOVE(fsp->notify->requests, req);
+               SAFE_FREE(req);
                return;
        }
 
+       /*
+        * Someone has triggered a notify previously, queue the change for
+        * later. TODO: Limit the number of changes queued.
+        */
+
        if (!(changes = TALLOC_REALLOC_ARRAY(
                      fsp->notify, fsp->notify->changes,
                      struct notify_change, fsp->notify->num_changes+1))) {
@@ -520,24 +581,33 @@ static void notify_message_callback(int msgtype, struct process_id pid,
 
        change = &(fsp->notify->changes[fsp->notify->num_changes]);
 
-       if (!(change->name = talloc_strdup(changes, msg.name))) {
+       if (!(change->name = talloc_strdup(changes, msg->name))) {
                DEBUG(0, ("talloc_strdup failed\n"));
                return;
        }
-       change->action = msg.action;
+       change->action = msg->action;
        fsp->notify->num_changes += 1;
 
-       if (cnbp != NULL) {
-               /*
-                * Respond directly, we have a someone waiting for this change
-                */
-               DEBUG(10, ("Found pending cn for %s, responding directly\n",
-                          cnbp->fsp->fsp_name));
-               change_notify_reply(cnbp->request_buf, cnbp->max_param_count,
-                                   cnbp->fsp);
-               change_notify_remove(cnbp);
+       return;
+}
+
+static void notify_message_callback(int msgtype, struct process_id pid,
+                                   void *buf, size_t len)
+{
+       struct notify_message msg;
+       files_struct *fsp;
+
+       if (!buf_to_notify_message(buf, len, &msg)) {
                return;
        }
+
+       DEBUG(10, ("Received notify_message for 0x%x/%.0f: %d\n",
+                  (unsigned)msg.dev, (double)msg.inode, msg.action));
+
+       for(fsp = fsp_find_di_first(msg.dev, msg.inode); fsp;
+           fsp = fsp_find_di_next(fsp)) {
+               notify_fsp(fsp, &msg);
+       }
 }
 
 /****************************************************************************

diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
index f51d01fb9c8e40abb7c7dea7ae893f85f98a54ff..0ef962ca0f6db34c8e5a063848921d40c1a43bdc 100644 (file)
--- a/source3/smbd/nttrans.c
+++ b/source3/smbd/nttrans.c
@@ -1826,6 +1826,7 @@ static int call_nt_transact_notify_change(connection_struct *conn, char *inbuf,
        uint16 *setup = *ppsetup;
        files_struct *fsp;
        uint32 flags;
+       NTSTATUS status;
 
        if(setup_count < 6) {
                return ERROR_DOS(ERRDOS,ERRbadfunc);
@@ -1847,9 +1848,27 @@ static int call_nt_transact_notify_change(connection_struct *conn, char *inbuf,
                return ERROR_DOS(ERRDOS,ERRbadfid);
        }
 
+       if (fsp->notify == NULL) {
+               if (!(fsp->notify = TALLOC_ZERO_P(
+                             NULL, struct notify_change_buf))) {
+                       return ERROR_NT(NT_STATUS_NO_MEMORY);
+               }
+       }
+
        if (fsp->notify->num_changes > 0) {
 
-               change_notify_reply(inbuf, max_param_count, fsp);
+               /*
+                * We've got changes pending, respond immediately
+                */
+
+               SMB_ASSERT(fsp->notify->requests == NULL);
+
+               change_notify_reply(inbuf, max_param_count,
+                                   fsp->notify->num_changes,
+                                   fsp->notify->changes);
+
+               TALLOC_FREE(fsp->notify->changes);
+               fsp->notify->num_changes = 0;
 
                /*
                 * change_notify_reply() above has independently sent its
@@ -1858,8 +1877,13 @@ static int call_nt_transact_notify_change(connection_struct *conn, char *inbuf,
                return -1;
        }
 
-       if (!change_notify_set(inbuf, fsp, conn, flags, max_param_count)) {
-               return(UNIXERROR(ERRDOS,ERRbadfid));
+       /*
+        * No changes pending, queue the request
+        */
+
+       status = change_notify_add_request(inbuf, max_param_count, fsp);
+       if (!NT_STATUS_IS_OK(status)) {
+               return ERROR_NT(status);
        }
 
        return -1;

diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 935e8033a57c79b5b14f290f9299eec6d02a7b58..c6b3c17c016ae3fb2abd10cb2e766731343c784b 100644 (file)
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -2020,6 +2020,9 @@ NTSTATUS unlink_internals(connection_struct *conn, uint32 dirtype,
                mangle_check_cache( mask, sizeof(pstring)-1, conn->params );
        
        if (!has_wild) {
+               char *dir;
+               const char *fname;
+
                pstrcat(directory,"/");
                pstrcat(directory,mask);
                error = can_delete(conn,directory,dirtype,bad_path,False);
@@ -2029,6 +2032,14 @@ NTSTATUS unlink_internals(connection_struct *conn, uint32 dirtype,
                if (SMB_VFS_UNLINK(conn,directory) == 0) {
                        count++;
                }
+
+               if (parent_dirname_talloc(tmp_talloc_ctx(), orig_name,
+                                         &dir, &fname)) {
+                       notify_action(conn, dir, fname,
+                                     NOTIFY_ACTION_REMOVED);
+                       TALLOC_FREE(dir); /* not strictly necessary */
+               }
+
        } else {
                struct smb_Dir *dir_hnd = NULL;
                const char *dname;
@@ -2086,6 +2097,7 @@ NTSTATUS unlink_internals(connection_struct *conn, uint32 dirtype,
                                }
                                if (SMB_VFS_UNLINK(conn,fname) == 0)
                                        count++;
+                               notify_action(conn, directory, dname, NOTIFY_ACTION_REMOVED);
                                DEBUG(3,("unlink_internals: succesful unlink [%s]\n",fname));
                        }
                        CloseDir(dir_hnd);
@@ -2096,18 +2108,6 @@ NTSTATUS unlink_internals(connection_struct *conn, uint32 dirtype,
                error = map_nt_error_from_unix(errno);
        }
 
-       {
-               char *dir;
-               const char *fname;
-
-               if (parent_dirname_talloc(tmp_talloc_ctx(), orig_name,
-                                         &dir, &fname)) {
-                       notify_action(conn, dir, fname,
-                                     NOTIFY_ACTION_REMOVED);
-                       TALLOC_FREE(dir); /* not strictly necessary */
-               }
-       }
-
        return error;
 }