Attachment 6190 Details for Bug 7836 – bso7836_34t.patch

[patch] bso7836_34t.patch

bso7836_34t.patch (text/plain), 11.67 KB, created by David Disseldorp on 2011-01-10 09:30:00 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Disseldorp

Created: 2011-01-10 09:30:00 UTC

Size: 11.67 KB

patch

obsolete

>From e8835d30653c38e31adcc4ec1f9e215f9e3664f3 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/2] 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       |    4 ++--
> source3/printing/load.c       |    5 ++---
> source3/printing/pcap.c       |   18 ++++++++++++++----
> source3/printing/print_cups.c |   33 ++++++++++++++++++++++++---------
> source3/smbd/server.c         |   12 ++++++------
> source3/web/swat.c            |    4 ++--
> 6 files changed, 50 insertions(+), 26 deletions(-)
>
>diff --git a/source3/include/proto.h b/source3/include/proto.h
>index 7c2893b..46ed4ab 100644
>--- a/source3/include/proto.h
>+++ b/source3/include/proto.h
>@@ -4869,7 +4869,7 @@ 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);
>-void pcap_cache_reload(void);
>+void pcap_cache_reload(void (*post_cache_fill_fn)(void));
> bool pcap_printername_ok(const char *printername);
> 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 *);
>@@ -4880,7 +4880,7 @@ bool aix_cache_reload(void);
> 
> /* The following definitions come from printing/print_cups.c  */
> 
>-bool cups_cache_reload(void);
>+bool cups_cache_reload(void (*post_cache_fill_fn)(void));
> bool cups_pull_comment_location(NT_PRINTER_INFO_LEVEL_2 *printer);
> 
> /* The following definitions come from printing/print_generic.c  */
>diff --git a/source3/printing/load.c b/source3/printing/load.c
>index 874f7f2..00da9cb 100644
>--- a/source3/printing/load.c
>+++ b/source3/printing/load.c
>@@ -53,12 +53,11 @@ static void add_auto_printers(void)
> }
> 
> /***************************************************************************
>-load automatic printer services
>+load automatic printer services from pre-populated pcap cache
> ***************************************************************************/
> void load_printers(void)
> {
>-	if (!pcap_cache_loaded())
>-		pcap_cache_reload();
>+	SMB_ASSERT(pcap_cache_loaded());
> 
> 	add_auto_printers();
> 
>diff --git a/source3/printing/pcap.c b/source3/printing/pcap.c
>index a6bf52a..0d6480c 100644
>--- a/source3/printing/pcap.c
>+++ b/source3/printing/pcap.c
>@@ -125,13 +125,14 @@ void pcap_cache_replace(const struct pcap_cache *pcache)
> 	}
> }
> 
>-void pcap_cache_reload(void)
>+void pcap_cache_reload(void (*post_cache_fill_fn)(void))
> {
> 	const char *pcap_name = lp_printcapname();
> 	bool pcap_reloaded = False;
> 	struct pcap_cache *tmp_cache = NULL;
> 	XFILE *pcap_file;
> 	char *pcap_line;
>+	bool post_cache_fill_fn_handled = false;
> 
> 	DEBUG(3, ("reloading printcap cache\n"));
> 
>@@ -146,7 +147,12 @@ void pcap_cache_reload(void)
> 
> #ifdef HAVE_CUPS
> 	if (strequal(pcap_name, "cups")) {
>-		pcap_reloaded = cups_cache_reload();
>+		pcap_reloaded = cups_cache_reload(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
>@@ -242,9 +248,13 @@ void pcap_cache_reload(void)
> done:
> 	DEBUG(3, ("reload status: %s\n", (pcap_reloaded) ? "ok" : "error"));
> 
>-	if (pcap_reloaded)
>+	if (pcap_reloaded) {
> 		pcap_cache_destroy_specific(&tmp_cache);
>-	else {
>+		if ((post_cache_fill_fn_handled == false)
>+		 && (post_cache_fill_fn != NULL)) {
>+			post_cache_fill_fn();
>+		}
>+	} else {
> 		pcap_cache_destroy_specific(&pcap_cache);
> 		pcap_cache = tmp_cache;
> 	}
>diff --git a/source3/printing/print_cups.c b/source3/printing/print_cups.c
>index e2c8b7e..a8cc538 100644
>--- a/source3/printing/print_cups.c
>+++ b/source3/printing/print_cups.c
>@@ -440,13 +440,19 @@ static bool cups_pcap_load_async(int *pfd)
> 	_exit(0);
> }
> 
>+struct cups_async_cb_args {
>+	int pipe_fd;
>+	void (*post_cache_fill_fn)(void);
>+};
>+
> 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. "
>@@ -540,27 +546,36 @@ 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) {
>+			cb_args->post_cache_fill_fn();
>+		}
> 	} 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(void)
>+bool cups_cache_reload(void (*post_cache_fill_fn)(void))
> {
>-	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) {
> 		return false;
> 	}
>-
>+	cb_args->post_cache_fill_fn = post_cache_fill_fn;
>+	p_pipe_fd = &cb_args->pipe_fd;
> 	*p_pipe_fd = -1;
> 
> 	/* Set up an async refresh. */
> 	if (!cups_pcap_load_async(p_pipe_fd)) {
>+		talloc_free(cb_args);
> 		return false;
> 	}
> 	if (!local_pcap_copy) {
>@@ -573,7 +588,7 @@ bool cups_cache_reload(void)
> 		cups_async_callback(smbd_event_context(),
> 					NULL,
> 					EVENT_FD_READ,
>-					(void *)p_pipe_fd);
>+					(void *)cb_args);
> 		if (!local_pcap_copy) {
> 			return false;
> 		}
>@@ -590,10 +605,10 @@ bool cups_cache_reload(void)
> 					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/server.c b/source3/smbd/server.c
>index 2c5ce40..72a15fc 100644
>--- a/source3/smbd/server.c
>+++ b/source3/smbd/server.c
>@@ -689,9 +689,9 @@ static void smbd_parent_loop(struct smbd_parent_context *parent)
> /* NOTREACHED	return True; */
> }
> 
>-/****************************************************************************
>- Reload printers
>-**************************************************************************/
>+/***************************************************************************
>+ purge stale printers and reload from pre-populated pcap cache
>+***************************************************************************/
> void reload_printers(void)
> {
> 	int snum;
>@@ -699,9 +699,9 @@ void reload_printers(void)
> 	int pnum = lp_servicenumber(PRINTERS_NAME);
> 	const char *pname;
> 
>-	pcap_cache_reload();
>+	SMB_ASSERT(pcap_cache_loaded());
> 
>-	/* remove stale printers */
>+	DEBUG(10, ("reloading printer services from pcap cache\n"));
> 	for (snum = 0; snum < n_services; snum++) {
> 		/* avoid removing PRINTERS_NAME or non-autoloaded printers */
> 		if (snum == pnum || !(lp_snum_ok(snum) && lp_print_ok(snum) &&
>@@ -748,7 +748,7 @@ bool reload_services(bool test)
> 
> 	ret = lp_load(get_dyn_CONFIGFILE(), False, False, True, True);
> 
>-	reload_printers();
>+	pcap_cache_reload(&reload_printers);
> 
> 	/* perhaps the config filename is now set */
> 	if (!test)
>diff --git a/source3/web/swat.c b/source3/web/swat.c
>index 4bfb731..6741082 100644
>--- a/source3/web/swat.c
>+++ b/source3/web/swat.c
>@@ -490,7 +490,7 @@ static int save_reload(int snum)
>                 return 0;
>         }
> 	iNumNonAutoPrintServices = lp_numservices();
>-	load_printers();
>+	pcap_cache_reload(&load_printers);
> 
> 	return 1;
> }
>@@ -1434,7 +1434,7 @@ const char *lang_msg_rotate(TALLOC_CTX *ctx, const char *msgid)
> 	load_config(True);
> 	load_interfaces();
> 	iNumNonAutoPrintServices = lp_numservices();
>-	load_printers();
>+	pcap_cache_reload(&load_printers);
> 
> 	cgi_setup(get_dyn_SWATDIR(), !demo_mode);
> 
>-- 
>1.7.1
>
>From 7141129bc411737b5088bc4df7d93799e6535df3 Mon Sep 17 00:00:00 2001
>From: David Disseldorp <ddiss@suse.de>
>Date: Mon, 10 Jan 2011 14:08:07 +0100
>Subject: [PATCH 2/2] s3-printing: update parent smbd pcap cache
>
>If a client connects to a samba share and while connected a printer is
>added, the client will see the new printer share after a maximum of
>'printcap cache time' seconds.
>
>smbd's forked for new client connections inherit printcap information
>from the parent (listener) smbd, which does not perform updates on
>printcap cache time expiry. Therefore newly connected clients may
>initially be presented with stale printer shares.
>
>Add a housekeeping function to the parent smbd to ensure newly connected
>clients see up to date printer shares.
>---
> source3/include/local.h |    1 +
> source3/smbd/process.c  |    4 ++--
> source3/smbd/server.c   |   16 ++++++++++++++++
> 3 files changed, 19 insertions(+), 2 deletions(-)
>
>diff --git a/source3/include/local.h b/source3/include/local.h
>index a88b17b..d65cc00 100644
>--- a/source3/include/local.h
>+++ b/source3/include/local.h
>@@ -148,6 +148,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/smbd/process.c b/source3/smbd/process.c
>index 2abea8f..4b2e6eb 100644
>--- a/source3/smbd/process.c
>+++ b/source3/smbd/process.c
>@@ -1854,7 +1854,7 @@ void check_reload(time_t t)
> 			|| (t-last_printer_reload_time  < 0) ) 
> 		{
> 			DEBUG( 3,( "Printcap cache time expired.\n"));
>-			reload_printers();
>+			pcap_cache_reload(&reload_printers);
> 			last_printer_reload_time = t;
> 		}
> 	}
>@@ -2123,7 +2123,7 @@ void smbd_process(void)
> 	}
> 
> 	if (!(event_add_idle(smbd_event_context(), NULL,
>-			     timeval_set(SMBD_SELECT_TIMEOUT, 0),
>+			     timeval_set(SMBD_HOUSEKEEPING_INTERVAL, 0),
> 			     "housekeeping", housekeeping_fn, NULL))) {
> 		DEBUG(0, ("Could not add housekeeping event\n"));
> 		exit(1);
>diff --git a/source3/smbd/server.c b/source3/smbd/server.c
>index 72a15fc..6147526 100644
>--- a/source3/smbd/server.c
>+++ b/source3/smbd/server.c
>@@ -498,6 +498,14 @@ static bool smbd_open_one_socket(struct smbd_parent_context *parent,
> 	return true;
> }
> 
>+static bool parent_housekeeping_fn(const struct timeval *now, void *private_data)
>+{
>+	DEBUG(5, ("houskeeping\n"));
>+	/* check if we need to reload services */
>+	check_reload(time(NULL));
>+	return true;
>+}
>+
> /****************************************************************************
>  Open the socket communication.
> ****************************************************************************/
>@@ -625,6 +633,14 @@ static bool open_sockets_smbd(struct smbd_parent_context *parent,
> 	claim_connection(NULL,"",
> 			 FLAG_MSG_GENERAL|FLAG_MSG_SMBD|FLAG_MSG_DBWRAP);
> 
>+	if (!(event_add_idle(smbd_event_context(), NULL,
>+			     timeval_set(SMBD_HOUSEKEEPING_INTERVAL, 0),
>+			     "parent_housekeeping", parent_housekeeping_fn,
>+			     parent))) {
>+		DEBUG(0, ("Could not add housekeeping event\n"));
>+		exit(1);
>+	}
>+
>         /* Listen to messages */
> 
> 	messaging_register(smbd_messaging_context(), NULL,
>-- 
>1.7.1
>

Flags:

asn: review+

Actions: View

Attachments on bug 7836: 6104 | 6105 | 6106 | 6107 | 6112 | 6113 | 6114 | 6115 | 6116 | 6117 | 6154 | 6170 | 6186 | 6188 | 6190 | 6191 | 6208 | 6209 | 6474 | 6475