From 1a53ae9189775273cc4f4ed8e7f38cab4abd54ed Mon Sep 17 00:00:00 2001
From: David Disseldorp <ddiss@suse.de>
Date: Sun, 19 Dec 2010 19:52:08 +0100
Subject: [PATCH 1/4] s3-printing: reload shares after pcap cache fill
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Since commit eada8f8a, updates to the cups pcap cache are performed
asynchronously - cups_cache_reload() forks a child process to request
cups printer information and notify the parent smbd on completion.

Currently printer shares are reloaded immediately following the call to
cups_cache_reload(), this occurs prior to smbd receiving new cups pcap
information from the child process. Such behaviour can result in stale
print shares as outlined in bug 7836.

This fix ensures print shares are only reloaded after new pcap data has
been received.

Pair-Programmed-With: Lars Müller <lars@samba.org>
---
 source3/include/proto.h       |    7 +++++--
 source3/printing/load.c       |    6 ++----
 source3/printing/pcap.c       |   17 +++++++++++++++--
 source3/printing/pcap.h       |    4 +++-
 source3/printing/print_cups.c |   40 ++++++++++++++++++++++++++++++++--------
 source3/smbd/process.c        |    3 ++-
 source3/smbd/server.c         |    3 ++-
 source3/smbd/server_reload.c  |   13 ++++++++-----
 source3/web/swat.c            |    6 ++++--
 9 files changed, 73 insertions(+), 26 deletions(-)

diff --git a/source3/include/proto.h b/source3/include/proto.h
index 0775acd..91d3ef1 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -4062,7 +4062,9 @@ void notify_printer_sepfile(struct tevent_context *ev,
 /* The following definitions come from printing/pcap.c  */
 
 void pcap_cache_reload(struct tevent_context *ev,
-		       struct messaging_context *msg_ctx);
+		       struct messaging_context *msg_ctx,
+		       void (*post_cache_fill_fn)(struct tevent_context *,
+						  struct messaging_context *));
 bool pcap_printername_ok(const char *printername);
 
 /* The following definitions come from printing/printing.c  */
@@ -5399,7 +5401,8 @@ void server_messaging_context_free(void);
 struct event_context *smbd_event_context(void);
 struct messaging_context *smbd_messaging_context(void);
 struct memcache *smbd_memcache(void);
-void reload_printers(struct messaging_context *msg_ctx);
+void reload_printers(struct tevent_context *ev,
+		     struct messaging_context *msg_ctx);
 bool reload_services(struct messaging_context *msg_ctx, int smb_sock,
 		     bool test);
 void exit_server(const char *const explanation);
diff --git a/source3/printing/load.c b/source3/printing/load.c
index 4f1bb88..66c3ffd 100644
--- a/source3/printing/load.c
+++ b/source3/printing/load.c
@@ -54,14 +54,12 @@ static void add_auto_printers(void)
 }
 
 /***************************************************************************
-load automatic printer services
+load automatic printer services from pre-populated pcap cache
 ***************************************************************************/
 void load_printers(struct tevent_context *ev,
 		   struct messaging_context *msg_ctx)
 {
-	if (!pcap_cache_loaded()) {
-		pcap_cache_reload(ev, msg_ctx);
-	}
+	SMB_ASSERT(pcap_cache_loaded());
 
 	add_auto_printers();
 
diff --git a/source3/printing/pcap.c b/source3/printing/pcap.c
index 1b8f46d..be267bd 100644
--- a/source3/printing/pcap.c
+++ b/source3/printing/pcap.c
@@ -107,11 +107,14 @@ void pcap_cache_replace(const struct pcap_cache *pcache)
 }
 
 void pcap_cache_reload(struct tevent_context *ev,
-		       struct messaging_context *msg_ctx)
+		       struct messaging_context *msg_ctx,
+		       void (*post_cache_fill_fn)(struct tevent_context *,
+						  struct messaging_context *))
 {
 	const char *pcap_name = lp_printcapname();
 	bool pcap_reloaded = False;
 	NTSTATUS status;
+	bool post_cache_fill_fn_handled = false;
 
 	DEBUG(3, ("reloading printcap cache\n"));
 
@@ -135,7 +138,13 @@ void pcap_cache_reload(struct tevent_context *ev,
 
 #ifdef HAVE_CUPS
 	if (strequal(pcap_name, "cups")) {
-		pcap_reloaded = cups_cache_reload(ev, msg_ctx);
+		pcap_reloaded = cups_cache_reload(ev, msg_ctx,
+						  post_cache_fill_fn);
+		/*
+		 * cups_cache_reload() is async and calls post_cache_fill_fn()
+		 * on successful completion
+		 */
+		post_cache_fill_fn_handled = true;
 		goto done;
 	}
 #endif
@@ -174,6 +183,10 @@ done:
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(0, ("Failed to cleanup printer list!\n"));
 		}
+		if ((post_cache_fill_fn_handled == false)
+		 && (post_cache_fill_fn != NULL)) {
+			post_cache_fill_fn(ev, msg_ctx);
+		}
 	}
 
 	return;
diff --git a/source3/printing/pcap.h b/source3/printing/pcap.h
index 67f36d6..7f8f7d2 100644
--- a/source3/printing/pcap.h
+++ b/source3/printing/pcap.h
@@ -35,7 +35,9 @@ bool aix_cache_reload(void);
 /* The following definitions come from printing/print_cups.c  */
 
 bool cups_cache_reload(struct tevent_context *ev,
-		       struct messaging_context *msg_ctx);
+		       struct messaging_context *msg_ctx,
+		       void (*post_cache_fill_fn)(struct tevent_context *,
+						  struct messaging_context *));
 bool cups_pull_comment_location(TALLOC_CTX *mem_ctx,
 				const char *printername,
 				char **comment,
diff --git a/source3/printing/print_cups.c b/source3/printing/print_cups.c
index a85fba8..8c023dd 100644
--- a/source3/printing/print_cups.c
+++ b/source3/printing/print_cups.c
@@ -449,13 +449,22 @@ static bool cups_pcap_load_async(struct tevent_context *ev,
 	_exit(0);
 }
 
+struct cups_async_cb_args {
+	int pipe_fd;
+	struct event_context *event_ctx;
+	struct messaging_context *msg_ctx;
+	void (*post_cache_fill_fn)(struct event_context *,
+				   struct messaging_context *);
+};
+
 static void cups_async_callback(struct event_context *event_ctx,
 				struct fd_event *event,
 				uint16 flags,
 				void *p)
 {
 	TALLOC_CTX *frame = talloc_stackframe();
-	int fd = *(int *)p;
+	struct cups_async_cb_args *cb_args = (struct cups_async_cb_args *)p;
+	int fd = cb_args->pipe_fd;
 	struct pcap_cache *tmp_pcap_cache = NULL;
 
 	DEBUG(5,("cups_async_callback: callback received for printer data. "
@@ -549,28 +558,43 @@ static void cups_async_callback(struct event_context *event_ctx,
 
 		/* And the systemwide pcap cache. */
 		pcap_cache_replace(local_pcap_copy);
+
+		/* Caller may have requested post cache fill callback */
+		if (cb_args->post_cache_fill_fn != NULL) {
+			cb_args->post_cache_fill_fn(cb_args->event_ctx,
+						    cb_args->msg_ctx);
+		}
 	} else {
 		DEBUG(2,("cups_async_callback: failed to read a new "
 			"printer list\n"));
 	}
 	close(fd);
-	TALLOC_FREE(p);
+	TALLOC_FREE(cb_args);
 	TALLOC_FREE(cache_fd_event);
 }
 
 bool cups_cache_reload(struct tevent_context *ev,
-		       struct messaging_context *msg_ctx)
+		       struct messaging_context *msg_ctx,
+		       void (*post_cache_fill_fn)(struct tevent_context *,
+						  struct messaging_context *))
 {
-	int *p_pipe_fd = TALLOC_P(NULL, int);
+	struct cups_async_cb_args *cb_args;
+	int *p_pipe_fd;
 
-	if (!p_pipe_fd) {
+	cb_args = TALLOC_P(NULL, struct cups_async_cb_args);
+	if (cb_args == NULL) {
 		return false;
 	}
 
+	cb_args->post_cache_fill_fn = post_cache_fill_fn;
+	cb_args->event_ctx = ev;
+	cb_args->msg_ctx = msg_ctx;
+	p_pipe_fd = &cb_args->pipe_fd;
 	*p_pipe_fd = -1;
 
 	/* Set up an async refresh. */
 	if (!cups_pcap_load_async(ev, msg_ctx, p_pipe_fd)) {
+		talloc_free(cb_args);
 		return false;
 	}
 	if (!local_pcap_copy) {
@@ -582,7 +606,7 @@ bool cups_cache_reload(struct tevent_context *ev,
 
 		cups_async_callback(ev, NULL,
 					EVENT_FD_READ,
-					(void *)p_pipe_fd);
+					(void *)cb_args);
 		if (!local_pcap_copy) {
 			return false;
 		}
@@ -599,10 +623,10 @@ bool cups_cache_reload(struct tevent_context *ev,
 					NULL, *p_pipe_fd,
 					EVENT_FD_READ,
 					cups_async_callback,
-					(void *)p_pipe_fd);
+					(void *)cb_args);
 		if (!cache_fd_event) {
 			close(*p_pipe_fd);
-			TALLOC_FREE(p_pipe_fd);
+			TALLOC_FREE(cb_args);
 			return false;
 		}
 	}
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index 150b2dd..2992576 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -2259,7 +2259,8 @@ static void check_reload(struct smbd_server_connection *sconn, time_t t)
 			|| (t-last_printer_reload_time  < 0) ) 
 		{
 			DEBUG( 3,( "Printcap cache time expired.\n"));
-			reload_printers(sconn->msg_ctx);
+			pcap_cache_reload(server_event_context(),
+					  sconn->msg_ctx, &reload_printers);
 			last_printer_reload_time = t;
 		}
 	}
diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index 1b9e793..16aa055 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -1238,7 +1238,8 @@ extern void build_options(bool screen);
 	}
 
 	/* Publish nt printers, this requires a working winreg pipe */
-	reload_printers(smbd_messaging_context());
+	pcap_cache_reload(server_event_context(), smbd_messaging_context(),
+			  &reload_printers);
 
 	/* only start the background queue daemon if we are 
 	   running as a daemon -- bad things will happen if
diff --git a/source3/smbd/server_reload.c b/source3/smbd/server_reload.c
index 38d1f3a..2b74e7a 100644
--- a/source3/smbd/server_reload.c
+++ b/source3/smbd/server_reload.c
@@ -25,11 +25,13 @@
 #include "smbd/globals.h"
 #include "librpc/gen_ndr/messaging.h"
 #include "nt_printing.h"
+#include "printing/pcap.h"
 
 /****************************************************************************
- Reload printers
+ purge stale printers and reload from pre-populated pcap cache
 **************************************************************************/
-void reload_printers(struct messaging_context *msg_ctx)
+void reload_printers(struct tevent_context *ev,
+		     struct messaging_context *msg_ctx)
 {
 	struct auth_serversupplied_info *server_info = NULL;
 	struct spoolss_PrinterInfo2 *pinfo2 = NULL;
@@ -40,7 +42,8 @@ void reload_printers(struct messaging_context *msg_ctx)
 	NTSTATUS status;
 	bool skip = false;
 
-	pcap_cache_reload(server_event_context(), msg_ctx);
+	SMB_ASSERT(pcap_cache_loaded());
+	DEBUG(10, ("reloading printer services from pcap cache\n"));
 
 	status = make_server_info_system(talloc_tos(), &server_info);
 	if (!NT_STATUS_IS_OK(status)) {
@@ -79,7 +82,7 @@ void reload_printers(struct messaging_context *msg_ctx)
 		}
 	}
 
-	load_printers(server_event_context(), msg_ctx);
+	load_printers(ev, msg_ctx);
 
 	TALLOC_FREE(server_info);
 }
@@ -111,7 +114,7 @@ bool reload_services(struct messaging_context *msg_ctx, int smb_sock,
 
 	ret = lp_load(get_dyn_CONFIGFILE(), False, False, True, True);
 
-	reload_printers(msg_ctx);
+	pcap_cache_reload(server_event_context(), msg_ctx, &reload_printers);
 
 	/* perhaps the config filename is now set */
 	if (!test)
diff --git a/source3/web/swat.c b/source3/web/swat.c
index bb3f3f9..1cbecd4 100644
--- a/source3/web/swat.c
+++ b/source3/web/swat.c
@@ -491,7 +491,8 @@ static int save_reload(int snum)
                 return 0;
         }
 	iNumNonAutoPrintServices = lp_numservices();
-	load_printers(server_event_context(), server_messaging_context());
+	pcap_cache_reload(server_event_context(), server_messaging_context(),
+			  &load_printers);
 
 	return 1;
 }
@@ -1435,7 +1436,8 @@ const char *lang_msg_rotate(TALLOC_CTX *ctx, const char *msgid)
 	reopen_logs();
 	load_interfaces();
 	iNumNonAutoPrintServices = lp_numservices();
-	load_printers(server_event_context(), server_messaging_context());
+	pcap_cache_reload(server_event_context(), server_messaging_context(),
+			  &load_printers);
 
 	cgi_setup(get_dyn_SWATDIR(), !demo_mode);
 
-- 
1.7.1

From 6225f7a550743eb01633a5c52780ed187bb3900e Mon Sep 17 00:00:00 2001
From: David Disseldorp <ddiss@suse.de>
Date: Thu, 23 Dec 2010 12:14:21 +0100
Subject: [PATCH 2/4] s3-printing: Initiate pcap reload from parent smbd
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Since commit 7022554, smbds share a printcap cache (printer_list.tdb),
therefore ordering of events between smbd processes is important when
updating printcap cache information. Consider the following two process
example:
1) smbd1 receives HUP or printcap cache time expiry
2) smbd1 checks whether pcap needs refresh, it does
3) smbd1 marks pcap as refreshed
4) smbd1 forks child1 to obtain cups printer info
5) smbd2 receives HUP or printcap cache time expiry
6) smbd2 checks whether pcap needs refresh, it does not (due to step 3)
7) smbd2 reloads printer shares prior to child1 completion (stale pcap)
8) child1 completion, pcap cache (printer_list.tdb) is updated by smbd1
9) smbd1 reloads printer shares based on new pcap information

In this case both smbd1 and smbd2 are reliant on the pcap update
performed on child1 completion.
The prior commit "reload shares after pcap cache fill" ensures that
smbd1 only reloads printer shares following pcap update, however smbd2
continues to present shares based on stale pcap data.

This commit addresses the above problem by driving pcap cache and
printer share updates from the parent smbd process.
1) smbd0 (parent) receives a HUP or printcap cache time expiry
2) smbd0 forks child0 to obtain cups printer info
3) child0 completion, pcap cache (printer_list.tdb) is updated by smbd0
4) smbd0 reloads printer shares
5) smbd0 notifies child smbds of pcap update via message_send_all()
6) child smbds read fresh pcap data and reload printer shares

This architecture has the additional advantage that only a single
process (the parent smbd) requests printer information from the printcap
backend.

Use time_mono in housekeeping functions As suggested by Björn Jacke.
---
 docs-xml/smbdotconf/printing/printcapcachetime.xml |    4 +-
 source3/include/local.h                            |    1 +
 source3/include/proto.h                            |    2 +
 source3/librpc/idl/messaging.idl                   |    1 +
 source3/smbd/globals.c                             |    1 -
 source3/smbd/globals.h                             |    1 -
 source3/smbd/process.c                             |   45 ++++---------------
 source3/smbd/server.c                              |   47 ++++++++++++++++++++
 source3/smbd/server_reload.c                       |   11 ++++-
 source3/web/swat.c                                 |   13 ++++--
 10 files changed, 79 insertions(+), 47 deletions(-)

diff --git a/docs-xml/smbdotconf/printing/printcapcachetime.xml b/docs-xml/smbdotconf/printing/printcapcachetime.xml
index 7dcd1b6..e9e0c98 100644
--- a/docs-xml/smbdotconf/printing/printcapcachetime.xml
+++ b/docs-xml/smbdotconf/printing/printcapcachetime.xml
@@ -5,9 +5,7 @@
                  xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
 <description>
     <para>This option specifies the number of seconds before the printing
-    subsystem is again asked for the known printers.  If the value
-    is greater than 60 the initial waiting time is set to 60 seconds
-    to allow an earlier first rescan of the printing subsystem.
+    subsystem is again asked for the known printers.
     </para>
 
     <para>Setting this parameter to 0 disables any rescanning for new 
diff --git a/source3/include/local.h b/source3/include/local.h
index a8889af..bb73840 100644
--- a/source3/include/local.h
+++ b/source3/include/local.h
@@ -139,6 +139,7 @@
 #define LPQ_LOCK_TIMEOUT (5)
 #define NMBD_INTERFACES_RELOAD (120)
 #define NMBD_UNEXPECTED_TIMEOUT (15)
+#define SMBD_HOUSEKEEPING_INTERVAL SMBD_SELECT_TIMEOUT
 
 /* the following are in milliseconds */
 #define LOCK_RETRY_TIMEOUT (100)
diff --git a/source3/include/proto.h b/source3/include/proto.h
index 91d3ef1..338c27d 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -5405,6 +5405,8 @@ void reload_printers(struct tevent_context *ev,
 		     struct messaging_context *msg_ctx);
 bool reload_services(struct messaging_context *msg_ctx, int smb_sock,
 		     bool test);
+void reload_pcap_change_notify(struct tevent_context *ev,
+			       struct messaging_context *msg_ctx);
 void exit_server(const char *const explanation);
 void exit_server_cleanly(const char *const explanation);
 void exit_server_fault(void);
diff --git a/source3/librpc/idl/messaging.idl b/source3/librpc/idl/messaging.idl
index 9041d22..faa9a6e 100644
--- a/source3/librpc/idl/messaging.idl
+++ b/source3/librpc/idl/messaging.idl
@@ -45,6 +45,7 @@ interface messaging
 		MSG_PRINTERDATA_INIT_RESET	= 0x0204,
 		MSG_PRINTER_UPDATE		= 0x0205,
 		MSG_PRINTER_MOD			= 0x0206,
+		MSG_PRINTER_PCAP		= 0x0207,
 
 		/* smbd messages */
 		MSG_SMB_CONF_UPDATED		= 0x0301,
diff --git a/source3/smbd/globals.c b/source3/smbd/globals.c
index aac30ea..ca97884 100644
--- a/source3/smbd/globals.c
+++ b/source3/smbd/globals.c
@@ -54,7 +54,6 @@ struct msg_state *smbd_msg_state = NULL;
 
 bool logged_ioctl_message = false;
 
-pid_t mypid = 0;
 time_t last_smb_conf_reload_time = 0;
 time_t last_printer_reload_time = 0;
 /****************************************************************************
diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index cb97cb5..7771049 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -61,7 +61,6 @@ extern bool logged_ioctl_message;
 
 extern int trans_num;
 
-extern pid_t mypid;
 extern time_t last_smb_conf_reload_time;
 extern time_t last_printer_reload_time;
 /****************************************************************************
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index 2992576..e5b6f68 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -943,6 +943,9 @@ static void smbd_sig_hup_handler(struct tevent_context *ev,
 	change_to_root_user();
 	DEBUG(1,("Reloading services after SIGHUP\n"));
 	reload_services(msg_ctx, smbd_server_conn->sock, False);
+	if (am_parent) {
+		pcap_cache_reload(ev, msg_ctx, &reload_pcap_change_notify);
+	}
 }
 
 void smbd_setup_sig_hup_handler(struct tevent_context *ev,
@@ -2222,48 +2225,15 @@ void chain_reply(struct smb_request *req)
 
 static void check_reload(struct smbd_server_connection *sconn, time_t t)
 {
-	time_t printcap_cache_time = (time_t)lp_printcap_cache_time();
 
-	if(last_smb_conf_reload_time == 0) {
+	if (last_smb_conf_reload_time == 0) {
 		last_smb_conf_reload_time = t;
-		/* Our printing subsystem might not be ready at smbd start up.
-		   Then no printer is available till the first printers check
-		   is performed.  A lower initial interval circumvents this. */
-		if ( printcap_cache_time > 60 )
-			last_printer_reload_time = t - printcap_cache_time + 60;
-		else
-			last_printer_reload_time = t;
-	}
-
-	if (mypid != getpid()) { /* First time or fork happened meanwhile */
-		/* randomize over 60 second the printcap reload to avoid all
-		 * process hitting cupsd at the same time */
-		int time_range = 60;
-
-		last_printer_reload_time += random() % time_range;
-		mypid = getpid();
 	}
 
 	if (t >= last_smb_conf_reload_time+SMBD_RELOAD_CHECK) {
 		reload_services(sconn->msg_ctx, sconn->sock, True);
 		last_smb_conf_reload_time = t;
 	}
-
-	/* 'printcap cache time = 0' disable the feature */
-
-	if ( printcap_cache_time != 0 )
-	{ 
-		/* see if it's time to reload or if the clock has been set back */
-
-		if ( (t >= last_printer_reload_time+printcap_cache_time) 
-			|| (t-last_printer_reload_time  < 0) ) 
-		{
-			DEBUG( 3,( "Printcap cache time expired.\n"));
-			pcap_cache_reload(server_event_context(),
-					  sconn->msg_ctx, &reload_printers);
-			last_printer_reload_time = t;
-		}
-	}
 }
 
 static bool fd_is_readable(int fd)
@@ -2493,13 +2463,16 @@ static bool housekeeping_fn(const struct timeval *now, void *private_data)
 {
 	struct smbd_server_connection *sconn = talloc_get_type_abort(
 		private_data, struct smbd_server_connection);
+
+	DEBUG(5, ("housekeeping\n"));
+
 	change_to_root_user();
 
 	/* update printer queue caches if necessary */
 	update_monitored_printq_cache(sconn->msg_ctx);
 
 	/* check if we need to reload services */
-	check_reload(sconn, time(NULL));
+	check_reload(sconn, time_mono(NULL));
 
 	/* Change machine password if neccessary. */
 	attempt_machine_password_change();
@@ -3106,7 +3079,7 @@ void smbd_process(struct smbd_server_connection *sconn)
 	}
 
 	if (!(event_add_idle(smbd_event_context(), NULL,
-			     timeval_set(SMBD_SELECT_TIMEOUT, 0),
+			     timeval_set(SMBD_HOUSEKEEPING_INTERVAL, 0),
 			     "housekeeping", housekeeping_fn, sconn))) {
 		DEBUG(0, ("Could not add housekeeping event\n"));
 		exit(1);
diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index 16aa055..d1c4eaf 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -102,8 +102,26 @@ static void smb_conf_updated(struct messaging_context *msg,
 		  "updated. Reloading.\n"));
 	change_to_root_user();
 	reload_services(msg, smbd_server_conn->sock, False);
+	if (am_parent) {
+		pcap_cache_reload(server_event_context(), msg,
+				  &reload_pcap_change_notify);
+	}
 }
 
+/*******************************************************************
+ What to do when printcap is updated.
+ ********************************************************************/
+
+static void smb_pcap_updated(struct messaging_context *msg,
+			     void *private_data,
+			     uint32_t msg_type,
+			     struct server_id server_id,
+			     DATA_BLOB *data)
+{
+	DEBUG(10,("Got message saying pcap was updated. Reloading.\n"));
+	change_to_root_user();
+	reload_printers(server_event_context(), msg);
+}
 
 /*******************************************************************
  Delete a statcache entry.
@@ -579,6 +597,26 @@ static bool smbd_open_one_socket(struct smbd_parent_context *parent,
 	return true;
 }
 
+static bool smbd_parent_housekeeping(const struct timeval *now, void *private_data)
+{
+	time_t printcap_cache_time = (time_t)lp_printcap_cache_time();
+	time_t t = time_mono(NULL);
+
+	DEBUG(5, ("parent housekeeping\n"));
+
+	/* if periodic printcap rescan is enabled, see if it's time to reload */
+	if ((printcap_cache_time != 0)
+	 && (t >= (last_printer_reload_time + printcap_cache_time))) {
+		DEBUG( 3,( "Printcap cache time expired.\n"));
+		pcap_cache_reload(server_event_context(),
+				  smbd_messaging_context(),
+				  &reload_pcap_change_notify);
+		last_printer_reload_time = t;
+	}
+
+	return true;
+}
+
 /****************************************************************************
  Open the socket communication.
 ****************************************************************************/
@@ -713,6 +751,14 @@ static bool open_sockets_smbd(struct smbd_parent_context *parent,
 		return false;
 	}
 
+	if (!(event_add_idle(smbd_event_context(), NULL,
+			     timeval_set(SMBD_HOUSEKEEPING_INTERVAL, 0),
+			     "parent_housekeeping", smbd_parent_housekeeping,
+			     NULL))) {
+		DEBUG(0, ("Could not add parent_housekeeping event\n"));
+		return false;
+	}
+
         /* Listen to messages */
 
 	messaging_register(msg_ctx, NULL, MSG_SMB_SAM_SYNC, msg_sam_sync);
@@ -724,6 +770,7 @@ static bool open_sockets_smbd(struct smbd_parent_context *parent,
 	messaging_register(msg_ctx, NULL, MSG_SMB_STAT_CACHE_DELETE,
 			   smb_stat_cache_delete);
 	messaging_register(msg_ctx, NULL, MSG_DEBUG, smbd_msg_debug);
+	messaging_register(msg_ctx, NULL, MSG_PRINTER_PCAP, smb_pcap_updated);
 	brl_register_msgs(msg_ctx);
 
 #ifdef CLUSTER_SUPPORT
diff --git a/source3/smbd/server_reload.c b/source3/smbd/server_reload.c
index 2b74e7a..bdca29d 100644
--- a/source3/smbd/server_reload.c
+++ b/source3/smbd/server_reload.c
@@ -114,8 +114,6 @@ bool reload_services(struct messaging_context *msg_ctx, int smb_sock,
 
 	ret = lp_load(get_dyn_CONFIGFILE(), False, False, True, True);
 
-	pcap_cache_reload(server_event_context(), msg_ctx, &reload_printers);
-
 	/* perhaps the config filename is now set */
 	if (!test)
 		reload_services(msg_ctx, smb_sock, True);
@@ -137,3 +135,12 @@ bool reload_services(struct messaging_context *msg_ctx, int smb_sock,
 
 	return(ret);
 }
+
+/****************************************************************************
+ Notify smbds of new printcap data
+**************************************************************************/
+void reload_pcap_change_notify(struct tevent_context *ev,
+			       struct messaging_context *msg_ctx)
+{
+	message_send_all(msg_ctx, MSG_PRINTER_PCAP, NULL, 0, NULL);
+}
diff --git a/source3/web/swat.c b/source3/web/swat.c
index 1cbecd4..93fe368 100644
--- a/source3/web/swat.c
+++ b/source3/web/swat.c
@@ -30,6 +30,7 @@
 #include "includes.h"
 #include "popt_common.h"
 #include "web/swat_proto.h"
+#include "printing/pcap.h"
 
 static int demo_mode = False;
 static int passwd_only = False;
@@ -491,8 +492,10 @@ static int save_reload(int snum)
                 return 0;
         }
 	iNumNonAutoPrintServices = lp_numservices();
-	pcap_cache_reload(server_event_context(), server_messaging_context(),
-			  &load_printers);
+	if (pcap_cache_loaded()) {
+		load_printers(server_event_context(),
+			      server_messaging_context());
+	}
 
 	return 1;
 }
@@ -1436,8 +1439,10 @@ const char *lang_msg_rotate(TALLOC_CTX *ctx, const char *msgid)
 	reopen_logs();
 	load_interfaces();
 	iNumNonAutoPrintServices = lp_numservices();
-	pcap_cache_reload(server_event_context(), server_messaging_context(),
-			  &load_printers);
+	if (pcap_cache_loaded()) {
+		load_printers(server_event_context(),
+			      server_messaging_context());
+	}
 
 	cgi_setup(get_dyn_SWATDIR(), !demo_mode);
 
-- 
1.7.1

From d1de99aa2960f4de3a5b5d7437abd6ce30bbebd2 Mon Sep 17 00:00:00 2001
From: David Disseldorp <ddiss@suse.de>
Date: Tue, 28 Dec 2010 14:55:01 +0100
Subject: [PATCH 3/4] s3-printing: remove old entries in pcap_cache_replace

Callers of pcap_cache_replace() assume the existing printcap cache is
replaced by the new values provided. This is not currently the case,
old entries should be removed.
---
 source3/printing/pcap.c       |   22 ++++++++++++++++++----
 source3/printing/pcap.h       |    2 +-
 source3/printing/print_cups.c |   11 +++++------
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/source3/printing/pcap.c b/source3/printing/pcap.c
index be267bd..7a733b1 100644
--- a/source3/printing/pcap.c
+++ b/source3/printing/pcap.c
@@ -97,13 +97,28 @@ bool pcap_cache_loaded(void)
 	return NT_STATUS_IS_OK(status);
 }
 
-void pcap_cache_replace(const struct pcap_cache *pcache)
+bool pcap_cache_replace(const struct pcap_cache *pcache)
 {
 	const struct pcap_cache *p;
+	NTSTATUS status;
+
+	status = printer_list_mark_reload();
+	if (!NT_STATUS_IS_OK(status)) {
+		DEBUG(0, ("Failed to mark printer list for reload!\n"));
+		return false;
+	}
 
 	for (p = pcache; p; p = p->next) {
 		pcap_cache_add(p->name, p->comment);
 	}
+
+	status = printer_list_clean_old();
+	if (!NT_STATUS_IS_OK(status)) {
+		DEBUG(0, ("Failed to cleanup printer list!\n"));
+		return false;
+	}
+
+	return true;
 }
 
 void pcap_cache_reload(struct tevent_context *ev,
@@ -175,7 +190,7 @@ void pcap_cache_reload(struct tevent_context *ev,
 done:
 	DEBUG(3, ("reload status: %s\n", (pcap_reloaded) ? "ok" : "error"));
 
-	if (pcap_reloaded) {
+	if ((pcap_reloaded) && (post_cache_fill_fn_handled == false)) {
 		/* cleanup old entries only if the operation was successful,
 		 * otherwise keep around the old entries until we can
 		 * successfuly reaload */
@@ -183,8 +198,7 @@ done:
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(0, ("Failed to cleanup printer list!\n"));
 		}
-		if ((post_cache_fill_fn_handled == false)
-		 && (post_cache_fill_fn != NULL)) {
+		if (post_cache_fill_fn != NULL) {
 			post_cache_fill_fn(ev, msg_ctx);
 		}
 	}
diff --git a/source3/printing/pcap.h b/source3/printing/pcap.h
index 7f8f7d2..4198be1 100644
--- a/source3/printing/pcap.h
+++ b/source3/printing/pcap.h
@@ -24,7 +24,7 @@ bool pcap_cache_add_specific(struct pcap_cache **ppcache, const char *name, cons
 void pcap_cache_destroy_specific(struct pcap_cache **ppcache);
 bool pcap_cache_add(const char *name, const char *comment);
 bool pcap_cache_loaded(void);
-void pcap_cache_replace(const struct pcap_cache *cache);
+bool pcap_cache_replace(const struct pcap_cache *cache);
 void pcap_printer_fn_specific(const struct pcap_cache *, void (*fn)(const char *, const char *, void *), void *);
 void pcap_printer_fn(void (*fn)(const char *, const char *, void *), void *);
 
diff --git a/source3/printing/print_cups.c b/source3/printing/print_cups.c
index 8c023dd..0e5dac5 100644
--- a/source3/printing/print_cups.c
+++ b/source3/printing/print_cups.c
@@ -552,15 +552,18 @@ static void cups_async_callback(struct event_context *event_ctx,
 
 	TALLOC_FREE(frame);
 	if (tmp_pcap_cache) {
+		bool ret;
 		/* We got a namelist, replace our local cache. */
 		pcap_cache_destroy_specific(&local_pcap_copy);
 		local_pcap_copy = tmp_pcap_cache;
 
 		/* And the systemwide pcap cache. */
-		pcap_cache_replace(local_pcap_copy);
+		ret = pcap_cache_replace(local_pcap_copy);
+		if (!ret)
+			DEBUG(0, ("failed to replace pcap cache\n"));
 
 		/* Caller may have requested post cache fill callback */
-		if (cb_args->post_cache_fill_fn != NULL) {
+		if (ret && cb_args->post_cache_fill_fn != NULL) {
 			cb_args->post_cache_fill_fn(cb_args->event_ctx,
 						    cb_args->msg_ctx);
 		}
@@ -611,10 +614,6 @@ bool cups_cache_reload(struct tevent_context *ev,
 			return false;
 		}
 	} else {
-		/* Replace the system cache with our
-		 * local copy. */
-		pcap_cache_replace(local_pcap_copy);
-
 		DEBUG(10,("cups_cache_reload: async read on fd %d\n",
 			*p_pipe_fd ));
 
-- 
1.7.1

From 082332b8cccb54563ad5ecea3edcf3813ea42511 Mon Sep 17 00:00:00 2001
From: David Disseldorp <ddiss@suse.de>
Date: Tue, 28 Dec 2010 15:54:54 +0100
Subject: [PATCH 4/4] s3-printing: remove printer_list_need_refresh

printer_list_need_refresh() was used previously to ensure one smbd
process did not attempt to update the printer_list tdb during or soon
after update by another smbd.

It is no longer needed, as pcap updates are now only performed by the
parent smbd process following startup, SIGHUP, config update or printcap
cache time expiry.
---
 source3/printing/pcap.c         |    6 ------
 source3/printing/printer_list.c |   26 --------------------------
 source3/printing/printer_list.h |    9 ---------
 3 files changed, 0 insertions(+), 41 deletions(-)

diff --git a/source3/printing/pcap.c b/source3/printing/pcap.c
index 7a733b1..7208f4b 100644
--- a/source3/printing/pcap.c
+++ b/source3/printing/pcap.c
@@ -139,12 +139,6 @@ void pcap_cache_reload(struct tevent_context *ev,
 		return;
 	}
 
-	if (!printer_list_need_refresh()) {
-		/* has been just refeshed, skip */
-		DEBUG(5, ("Refresh just happend, skipping.\n"));
-		return;
-	}
-
 	status = printer_list_mark_reload();
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(0, ("Failed to mark printer list for reload!\n"));
diff --git a/source3/printing/printer_list.c b/source3/printing/printer_list.c
index f3f00f0..d36a746 100644
--- a/source3/printing/printer_list.c
+++ b/source3/printing/printer_list.c
@@ -215,32 +215,6 @@ done:
 	return status;
 }
 
-bool printer_list_need_refresh(void)
-{
-	NTSTATUS status;
-	time_t last_refresh;
-	int timediff;
-
-	status = printer_list_get_last_refresh(&last_refresh);
-	if (!NT_STATUS_IS_OK(status)) {
-		return true;
-	}
-	timediff = time_mono(NULL) - last_refresh;
-
-	if (timediff > 1 ) {
-		/* if refresh occurred more than 1s (TODO:use lp_printcap_cache_time) ago,
-		 * then we need to refresh */
-		return true;
-	} else if (timediff < 0) {
-		/* last_refresh newer than now. Seems we have no monotonic
-		 * clock and the clock was adjusted backwards.
-		 * we need to refresh which also resets last_refresh */
-		return true;
-	}
-
-	return false;
-}
-
 NTSTATUS printer_list_mark_reload(void)
 {
 	struct db_context *db;
diff --git a/source3/printing/printer_list.h b/source3/printing/printer_list.h
index bdcf308..fce3e34 100644
--- a/source3/printing/printer_list.h
+++ b/source3/printing/printer_list.h
@@ -96,13 +96,4 @@ NTSTATUS printer_list_clean_old(void);
 
 NTSTATUS printer_list_run_fn(void (*fn)(const char *, const char *, void *),
 			     void *private_data);
-
-/**
- * @brief Check if the printer list needs to be refreshed.
- *
- * @return              True if the database needs to be refreshed, false if
- *                      not.
- */
-bool printer_list_need_refresh(void);
-
 #endif /* _PRINTER_LIST_H_ */
-- 
1.7.1