The costs are 1) the memory cost of storing the tdb info in memory and 2) the performance cost of having to check each file to figure out which one changed everytime there is a file notification event. The memory use could be reduced by storing the table on disk instead of in memory, but that would incur a cost in performance and could result in a lot of *tdb files sitting around (one per directory watched). The only way to reduce the performance cost is to make the kernel stuff return a file name with its notification. This change wouldn't be too hard to add to the kernel, but I'd rather not require a hacked kernel to make samba work as it should.
The only thing that doesn't work quite as it should is that the code does not report a file name change event as such, but instead just treats a name change as a file remove and then a file add. Supporting this would make the code a lot more complex (I'd have to keep a separate, inode indexed table of file stats in addition to the file name indexed one), and I can't imagine when a remove/add instead of a move would break a client, though it might cause performance issues.
Also, though this is a separate issue, the file change stuff still doesn't support deep notifications as it should as far as I can tell (which is to say not at all).
-hal
diff -u -r samba-2.2.7a.dist/source/include/smb.h samba-2.2.7a/source/include/smb.h
--- samba-2.2.7a.dist/source/include/smb.h Wed Dec 4 12:16:36 2002
+++ samba-2.2.7a/source/include/smb.h Wed Jan 22 16:26:56 2003
@@ -1202,6 +1202,16 @@
#define FILE_NOTIFY_CHANGE_SECURITY 0x100
#define FILE_NOTIFY_CHANGE_FILE_NAME 0x200
+/* file notification actions */
+#define FILE_ACTION_ADDED 0x00000001
+#define FILE_ACTION_REMOVED 0x00000002
+#define FILE_ACTION_MODIFIED 0x00000003
+#define FILE_ACTION_RENAMED_OLD_NAME 0x00000004
+#define FILE_ACTION_RENAMED_NEW_NAME 0x00000005
+#define FILE_ACTION_ADDED_STREAM 0x00000006
+#define FILE_ACTION_REMOVED_STREAM 0x00000007
+#define FILE_ACTION_MODIFIED_STREAM 0x00000008
+
/* where to find the base of the SMB packet proper */
#define smb_base(buf) (((char *)(buf))+4)
diff -u -r samba-2.2.7a.dist/source/smbd/notify.c samba-2.2.7a/source/smbd/notify.c
--- samba-2.2.7a.dist/source/smbd/notify.c Fri Feb 1 17:14:47 2002
+++ samba-2.2.7a/source/smbd/notify.c Thu Feb 6 16:08:07 2003
@@ -25,6 +25,16 @@
static struct cnotify_fns *cnotify;
/****************************************************************************
+This structure holds a list of files and associated notification actions.
+*****************************************************************************/
+struct file_action {
+ struct file_action *next, *prev;
+ int action;
+ char *filename;
+ int filename_length;
+};
+
+/****************************************************************************
This is the structure to queue to implement NT change
notify. It consists of smb_size bytes stored from the
transact command (to keep the mid, tid etc around).
@@ -38,14 +48,250 @@
uint32 flags;
char request_buf[smb_size];
void *change_data;
+ TDB_CONTEXT *file_data;
+ struct file_action *file_actions;
};
static struct change_notify *change_notify_list;
/****************************************************************************
+ Return a file action struct with the given filename and fileaction
+*****************************************************************************/
+struct file_action *change_notify_get_file_action(char *filename,
+ int fileaction) {
+
+ struct file_action *fa;
+
+ if (!(fa = (struct file_action *)malloc(sizeof(struct file_action)))) {
+ DEBUG(0, ("malloc failed!"));
+ }
+
+ fa->action = fileaction;
+ fa->filename = strdup(filename);
+ fa->filename_length = strlen(filename);
+
+ return fa;
+}
+
+/****************************************************************************
+ Check to make sure that the file in the given cnbp.file_data record
+ still exists.
+*****************************************************************************/
+int change_notify_file_data_exists(TDB_CONTEXT *tdb, TDB_DATA key,
+ TDB_DATA data, void *status) {
+
+ struct file_action *fa;
+ struct change_notify *cnbp = (struct change_notify *)status;
+ char *filename;
+ char *path = ((struct change_notify *)cnbp)->fsp->fsp_name;
+ int filename_len = strlen((char *)key.dptr) + strlen((char *)path) + 2;
+ int fd;
+
+ if (!(filename = (char *)malloc(filename_len))) {
+ DEBUG(0, ("malloc failed"));
+ }
+
+ filename[0] = '\0';
+ safe_strcat(filename, (char *)path, filename_len);
+ safe_strcat(filename, "/", filename_len);
+ safe_strcat(filename, (char *)key.dptr, filename_len);
+
+ if ((fd = open(filename, O_RDONLY)) == -1) {
+ fa = change_notify_get_file_action
+ ((char *)key.dptr, FILE_ACTION_REMOVED);
+ DLIST_ADD(cnbp->file_actions, fa);
+ } else {
+ close(fd);
+ }
+
+ return 0;
+}
+
+/****************************************************************************
+ Get a list of all the file notification actions
+*****************************************************************************/
+void change_notify_find_file_actions(struct change_notify *cnbp) {
+
+ void *dp;
+ char *fname;
+ TDB_DATA tdb_key, tdb_data;
+ SMB_STRUCT_STAT old_st, new_st;
+ pstring full_name;
+ size_t fullname_len, remaining_len;
+ char *p;
+ struct file_action *fa;
+
+ cnbp->file_actions = NULL;
+
+ DEBUG(10, ("find file actions\n"));
+
+ if (!(dp = OpenDir(cnbp->conn, cnbp->fsp->fsp_name, True))) {
+ DEBUG(0, ("Failed to open directory '%s'",
+ cnbp->fsp->fsp_name));
+ }
+
+ pstrcpy(full_name, cnbp->fsp->fsp_name);
+ pstrcat(full_name, "/");
+
+ fullname_len = strlen(full_name);
+ remaining_len = sizeof(full_name) - fullname_len - 1;
+ p = &full_name[fullname_len];
+
+ while (fname = ReadDirName(dp)) {
+
+ if (strequal(fname, ".") || strequal(fname, "..")) {
+ continue;
+ }
+
+ DEBUG(10, ("check file: %s\n", fname));
+
+ safe_strcpy(p, fname, remaining_len);
+
+ vfs_stat(cnbp->conn, full_name, &old_st);
+
+ tdb_key.dptr = (void *)fname;
+ tdb_key.dsize = strlen(fname) + 1;
+ tdb_data = tdb_fetch(cnbp->file_data, tdb_key);
+ if (!tdb_data.dptr) {
+ fa = change_notify_get_file_action
+ (fname, FILE_ACTION_ADDED);
+ DLIST_ADD(cnbp->file_actions, fa);
+ continue;
+ }
+
+ new_st = *((SMB_STRUCT_STAT *)tdb_data.dptr);
+ free(tdb_data.dptr);
+
+ if ((old_st.st_atime != new_st.st_atime) ||
+ (old_st.st_mtime != new_st.st_mtime) ||
+ (old_st.st_ctime != new_st.st_ctime)) {
+ fa = change_notify_get_file_action
+ (fname, FILE_ACTION_MODIFIED);
+ DLIST_ADD(cnbp->file_actions, fa);
+
+ }
+ }
+
+ /* check for deleted files */
+ if (tdb_traverse(cnbp->file_data, &change_notify_file_data_exists, cnbp) ==
+ -1) {
+ DEBUG(0, ("traverse failed: %s", tdb_error(cnbp->file_data)));
+ }
+}
+
+
+/****************************************************************************
+ Get the total length in bytes of the file notify information list that
+ will be sent with the reply
+*****************************************************************************/
+int change_notify_get_file_notify_length(struct change_notify *cnbp) {
+
+ int file_notify_length = 0;
+ int filename_length;
+ struct file_action *p = cnbp->file_actions;
+
+ while (p) {
+ filename_length = (p->filename_length + 1) * 2;
+ filename_length += filename_length % 4; /* word aligned */
+ file_notify_length += filename_length + 12;
+ p = p->next;
+ }
+
+ DEBUG(10, ("file notify length: %d\n", file_notify_length));
+
+ return file_notify_length;
+}
+
+/****************************************************************************
+ Append the file notify information onto the end of the smb reply packet.
+ Assumes that the reply packet is the correct length
+ (use change_notify_get_file_notify_length above).
+*****************************************************************************/
+void change_notify_append_file_notify_information(char *reply,
+ struct change_notify *cnbp) {
+
+ struct file_action *fa = cnbp->file_actions;
+ char *p;
+ uint16 *w_filename;
+ int filename_len, w_filename_len;
+ int i, j;
+
+ /* go to location in the reply where the file notify list should be */
+ p = reply + smb_vwv;
+
+ while (fa) {
+ filename_len = strlen(fa->filename) + 1;
+ w_filename_len = filename_len * 2;
+
+ /* length of the file notify information struct must be
+ a multiple of 4 */
+ w_filename_len += (w_filename_len % 4);
+
+ /* byte offset to next record, 0 if this is the last record */
+ if (fa->next) {
+ SIVAL(p, 0, w_filename_len + 12); p += 4;
+ } else {
+ SIVAL(p, 0, 0); p += 4;
+ }
+
+ /* notify action */
+ SIVAL(p, 0, fa->action); p += 4;
+
+ /* length of filename, using WCHAR instead of char */
+ SIVAL(p, 0, w_filename_len); p += 4;
+
+ /* filename */
+ w_filename = (uint16 *)p;
+ for (i = 0; i < filename_len; i++) {
+ w_filename[i] = (uint16)(fa->filename[i]);
+ }
+
+ p += w_filename_len;
+
+ fa = fa->next;
+ }
+}
+
+
+/****************************************************************************
+ Setup a reply packet with the names of individual files changed and send it
+*****************************************************************************/
+static void change_notify_reply_notify(struct change_notify *cnbp) {
+
+ void *file_actions;
+ int file_actions_len;
+
+ char *outbuf;
+ int outbuf_len;
+ int file_notify_len;
+ uint16 *w_filename;
+ int w_filename_len;
+ int i;
+
+ char *inbuf = cnbp->request_buf;
+
+ change_notify_find_file_actions(cnbp);
+
+ file_notify_len = change_notify_get_file_notify_length(cnbp);
+
+ outbuf_len = smb_size + file_notify_len;
+ outbuf = malloc(outbuf_len);
+ memset(outbuf, '\0', outbuf_len);
+ construct_reply_common(inbuf, outbuf);
+
+ /* number of bytes in file notify info list */
+ SCVAL(outbuf, smb_wct, file_notify_len);
+
+ change_notify_append_file_notify_information(outbuf, cnbp);
+
+ if (!send_smb(smbd_server_fd(),outbuf))
+ exit_server("change_notify_reply_notify: send_smb failed.");
+}
+
+/****************************************************************************
Setup the common parts of the return packet and send it.
*****************************************************************************/
-static void change_notify_reply_packet(char *inbuf, NTSTATUS error_code)
+static void change_notify_reply_error(char *inbuf, NTSTATUS error_code)
{
char outbuf[smb_size+38];
@@ -59,9 +305,10 @@
* in it. This is a longer packet than a simple error.
*/
set_message(outbuf,18,0,False);
-
+
if (!send_smb(smbd_server_fd(),outbuf))
- exit_server("change_notify_reply_packet: send_smb failed.");
+ exit_server("change_notify_reply_error: send_smb failed.");
+
}
/****************************************************************************
@@ -71,6 +318,16 @@
static void change_notify_remove(struct change_notify *cnbp)
{
+ struct file_action *fa;
+
+ while (fa = cnbp->file_actions) {
+ DLIST_REMOVE(cnbp->file_actions, fa);
+ safe_free(fa->filename);
+ safe_free(fa);
+ }
+
+ tdb_close(cnbp->file_data);
+
cnotify->remove_notify(cnbp->change_data);
DLIST_REMOVE(change_notify_list, cnbp);
ZERO_STRUCTP(cnbp);
@@ -104,7 +361,7 @@
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_reply_error(cnbp->request_buf,NT_STATUS_CANCELLED);
change_notify_remove(cnbp);
}
}
@@ -126,7 +383,7 @@
* the filename are identical.
*/
if((cnbp->fsp->conn == fsp->conn) &&
strequal(cnbp->fsp->fsp_name,fsp->fsp_name)) {
-
change_notify_reply_packet(cnbp->request_buf,NT_STATUS_CANCELLED);
+
+change_notify_reply_error(cnbp->request_buf,NT_STATUS_CANCELLED);
change_notify_remove(cnbp);
}
}
@@ -141,6 +398,34 @@
return cnotify->select_time;
}
+
+
+/****************************************************************************
+ Print out the contents of a file data record
+****************************************************************************/
+int change_notify_print_file_data(TDB_CONTEXT *tdb, TDB_DATA key,
+ TDB_DATA data, void *state) {
+
+ SMB_STRUCT_STAT *st = (SMB_STRUCT_STAT *)data.dptr;
+
+ DEBUG(0, ("file '%s': inode %d access %d modification %d change %d\n",
+ key.dptr, st->st_ino, st->st_atime, st->st_mtime,
+ st->st_ctime));
+
+ return 0;
+}
+
+change_notify_print_file_actions(struct change_notify *cnbp) {
+
+ struct file_action *p = cnbp->file_actions;
+
+ DEBUG(0, ("print file actions\n"));
+
+ while(p) {
+ DEBUG(0, ("file action: %s - %d\n", p->filename, p->action));
+ p = p->next;
+ }
+}
/****************************************************************************
Process the change notify queue. Note that this is only called as root.
Returns True if there are still outstanding change notify requests on the
@@ -157,9 +442,10 @@
vuid = (lp_security() == SEC_SHARE) ? UID_FIELD_INVALID :
SVAL(cnbp->request_buf,smb_uid);
+
if (cnotify->check_notify(cnbp->conn, vuid, cnbp->fsp->fsp_name,
cnbp->flags, cnbp->change_data, t)) {
DEBUG(10,("process_pending_change_notify_queue: dir %s changed
!\n", cnbp->fsp->fsp_name ));
-
change_notify_reply_packet(cnbp->request_buf,STATUS_NOTIFY_ENUM_DIR);
+ change_notify_reply_notify(cnbp);
change_notify_remove(cnbp);
}
}
@@ -168,6 +454,59 @@
}
/****************************************************************************
+ Setup the file_data field in the change_notify struct to contain
+ a tdb table of file stat data keyed by file name
+****************************************************************************/
+static void change_notify_setup_file_data(struct change_notify *cnbp) {
+
+ void *dp;
+ char *fname;
+ TDB_DATA tdb_key, tdb_data;
+ SMB_STRUCT_STAT st;
+ pstring full_name;
+ size_t fullname_len, remaining_len;
+ char *p;
+
+ if (!(cnbp->file_data = tdb_open("/dev/null", 0, TDB_INTERNAL,
+ O_RDWR | O_CREAT | O_TRUNC, 0600))) {
+ DEBUG(0, ("Failed to open file data db"));
+
+ }
+
+ if (!(dp = OpenDir(cnbp->conn, cnbp->fsp->fsp_name, True))) {
+ DEBUG(0, ("Failed to open directory '%s'", cnbp->fsp->fsp_name));
+ }
+
+
+ pstrcpy(full_name, cnbp->fsp->fsp_name);
+ pstrcat(full_name, "/");
+
+ fullname_len = strlen(full_name);
+ remaining_len = sizeof(full_name) - fullname_len - 1;
+ p = &full_name[fullname_len];
+
+ while (fname = ReadDirName(dp)) {
+ if (strequal(fname, ".") || strequal(fname, "..")) {
+ continue;
+ }
+
+ safe_strcpy(p, fname, remaining_len);
+
+ /*stat(full_name, (struct stat *)&st);*/
+ vfs_stat(cnbp->conn, full_name, &st);
+
+ tdb_key.dptr = (void *)fname;
+ tdb_key.dsize = strlen(fname) + 1;
+
+ tdb_data.dptr = (void *)&st;
+ tdb_data.dsize = (size_t)sizeof(st);
+
+ if (tdb_store(cnbp->file_data, tdb_key, tdb_data, 0)) {
+ DEBUG(0, ("Unable to store data for '%s': %s", full_name,
+tdb_error));
+ }
+ }
+}
+/****************************************************************************
Now queue an entry on the notify change list.
We only need to save smb_size bytes from this incoming packet
as we will always by returning a 'read the directory yourself'
@@ -196,6 +535,8 @@
return False;
}
+ change_notify_setup_file_data(cnbp);
+
DLIST_ADD(change_notify_list, cnbp);
return True;
