The Samba-Bugzilla – Attachment 10191 Details for
Bug 10652
Samba 4 consuming a lot of CPU when re-reading printcap info
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
patch series for v3-6 branch
bso10652_pcap_cpu_usage_36s.patch (text/plain), 31.17 KB, created by
David Disseldorp
on 2014-08-11 13:38:23 UTC
(
hide
)
Description:
patch series for v3-6 branch
Filename:
MIME Type:
Creator:
David Disseldorp
Created:
2014-08-11 13:38:23 UTC
Size:
31.17 KB
patch
obsolete
>From 2d3cb6a7b2607b067e0b875fedaded7259fdb7c1 Mon Sep 17 00:00:00 2001 >From: David Disseldorp <ddiss@samba.org> >Date: Thu, 10 Jul 2014 00:18:10 +0200 >Subject: [PATCH 1/7] printing: traverse_read the printer list for share > updates >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >The printcap update procedure involves the background printer process >obtaining the printcap information from the printing backend, writing >this to printer_list.tdb, and then notifying all smbd processes of the >new list. The processes then all attempt to simultaneously traverse >printer_list.tdb, in order to update their local share lists. > >With a large number of printers, and a large number of per-client smbd >processes, this traversal results in significant lock contention, mostly >due to the fact that the traversal is unnecessarily done with an >exclusive (write) lock on the printer_list.tdb database. > >This commit changes the share update code path to perform a read-only >traversal. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=10652 > >Reported-by: Alex K <korobkin+samba@gmail.com> >Reported-by: Franz Pförtsch <franz.pfoertsch@brose.com> >Signed-off-by: David Disseldorp <ddiss@samba.org> >--- > source3/printing/load.c | 2 +- > source3/printing/pcap.c | 4 ++-- > source3/printing/pcap.h | 2 +- > source3/printing/printer_list.c | 17 +++++++++++------ > source3/printing/printer_list.h | 4 ++-- > 5 files changed, 17 insertions(+), 12 deletions(-) > >diff --git a/source3/printing/load.c b/source3/printing/load.c >index 829c3e3..0a3de73 100644 >--- a/source3/printing/load.c >+++ b/source3/printing/load.c >@@ -70,5 +70,5 @@ void load_printers(struct tevent_context *ev, > > /* load all printcap printers */ > if (lp_load_printers() && lp_servicenumber(PRINTERS_NAME) >= 0) >- pcap_printer_fn(lp_add_one_printer, NULL); >+ pcap_printer_read_fn(lp_add_one_printer, NULL); > } >diff --git a/source3/printing/pcap.c b/source3/printing/pcap.c >index 62db4f5..6ad8e33 100644 >--- a/source3/printing/pcap.c >+++ b/source3/printing/pcap.c >@@ -229,11 +229,11 @@ void pcap_printer_fn_specific(const struct pcap_cache *pc, > return; > } > >-void pcap_printer_fn(void (*fn)(const char *, const char *, const char *, void *), void *pdata) >+void pcap_printer_read_fn(void (*fn)(const char *, const char *, const char *, void *), void *pdata) > { > NTSTATUS status; > >- status = printer_list_run_fn(fn, pdata); >+ status = printer_list_read_run_fn(fn, pdata); > if (!NT_STATUS_IS_OK(status)) { > DEBUG(3, ("Failed to run fn for all printers!\n")); > } >diff --git a/source3/printing/pcap.h b/source3/printing/pcap.h >index 7056213..6c062c3 100644 >--- a/source3/printing/pcap.h >+++ b/source3/printing/pcap.h >@@ -39,7 +39,7 @@ bool pcap_cache_add(const char *name, const char *comment, const char *location) > bool pcap_cache_loaded(void); > bool pcap_cache_replace(const struct pcap_cache *cache); > void pcap_printer_fn_specific(const struct pcap_cache *, void (*fn)(const char *, const char *, const char *, void *), void *); >-void pcap_printer_fn(void (*fn)(const char *, const char *, const char *, void *), void *); >+void pcap_printer_read_fn(void (*fn)(const char *, const char *, const char *, void *), void *); > > void pcap_cache_reload(struct tevent_context *ev, > struct messaging_context *msg_ctx, >diff --git a/source3/printing/printer_list.c b/source3/printing/printer_list.c >index 8f196a5..ef15a65 100644 >--- a/source3/printing/printer_list.c >+++ b/source3/printing/printer_list.c >@@ -279,7 +279,8 @@ done: > typedef int (printer_list_trv_fn_t)(struct db_record *, void *); > > static NTSTATUS printer_list_traverse(printer_list_trv_fn_t *fn, >- void *private_data) >+ void *private_data, >+ bool read_only) > { > struct db_context *db; > int ret; >@@ -289,7 +290,11 @@ static NTSTATUS printer_list_traverse(printer_list_trv_fn_t *fn, > return NT_STATUS_INTERNAL_DB_CORRUPTION; > } > >- ret = db->traverse(db, fn, private_data); >+ if (read_only) { >+ ret = db->traverse_read(db, fn, private_data); >+ } else { >+ ret = db->traverse(db, fn, private_data); >+ } > if (ret < 0) { > return NT_STATUS_UNSUCCESSFUL; > } >@@ -356,7 +361,7 @@ NTSTATUS printer_list_clean_old(void) > > state.status = NT_STATUS_OK; > >- status = printer_list_traverse(printer_list_clean_fn, &state); >+ status = printer_list_traverse(printer_list_clean_fn, &state, false); > if (NT_STATUS_EQUAL(status, NT_STATUS_UNSUCCESSFUL) && > !NT_STATUS_IS_OK(state.status)) { > status = state.status; >@@ -403,8 +408,8 @@ static int printer_list_exec_fn(struct db_record *rec, void *private_data) > return 0; > } > >-NTSTATUS printer_list_run_fn(void (*fn)(const char *, const char *, const char *, void *), >- void *private_data) >+NTSTATUS printer_list_read_run_fn(void (*fn)(const char *, const char *, const char *, void *), >+ void *private_data) > { > struct printer_list_exec_state state; > NTSTATUS status; >@@ -413,7 +418,7 @@ NTSTATUS printer_list_run_fn(void (*fn)(const char *, const char *, const char * > state.private_data = private_data; > state.status = NT_STATUS_OK; > >- status = printer_list_traverse(printer_list_exec_fn, &state); >+ status = printer_list_traverse(printer_list_exec_fn, &state, true); > if (NT_STATUS_EQUAL(status, NT_STATUS_UNSUCCESSFUL) && > !NT_STATUS_IS_OK(state.status)) { > status = state.status; >diff --git a/source3/printing/printer_list.h b/source3/printing/printer_list.h >index fb2e007..b12c192 100644 >--- a/source3/printing/printer_list.h >+++ b/source3/printing/printer_list.h >@@ -100,6 +100,6 @@ NTSTATUS printer_list_mark_reload(void); > */ > NTSTATUS printer_list_clean_old(void); > >-NTSTATUS printer_list_run_fn(void (*fn)(const char *, const char *, const char *, void *), >- void *private_data); >+NTSTATUS printer_list_read_run_fn(void (*fn)(const char *, const char *, const char *, void *), >+ void *private_data); > #endif /* _PRINTER_LIST_H_ */ >-- >1.8.4.5 > > >From 237c92dd9ca1943cf8daeb4f432863c19ac62738 Mon Sep 17 00:00:00 2001 >From: David Disseldorp <ddiss@samba.org> >Date: Fri, 11 Jul 2014 17:00:05 +0200 >Subject: [PATCH 2/7] printing: only reload printer shares on client enum > >Currently, automatic printer share updates are handled in the following >way: >- Background printer process (BPP) forked on startup >- Parent smbd and per-client children await MSG_PRINTER_PCAP messages >- BPP periodically polls the printing backend for printcap data > - printcap data written to printer_list.tdb > - MSG_PRINTER_PCAP sent to all smbd processes following update >- smbd processes all read the latest printer_list.tdb data, and update > their share listings > >This procedure is not scalable, as all smbd processes hit >printer_list.tdb in parallel, resulting in a large spike in CPU usage. > >This change sees smbd processes only update their printer share lists >only when a client asks for this information, e.g. via NetShareEnum or >EnumPrinters. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=10652 > >Suggested-by: Volker Lendecke <vl@samba.org> >Signed-off-by: David Disseldorp <ddiss@samba.org> >--- > source3/printing/spoolssd.c | 17 +---------------- > source3/rpc_server/spoolss/srv_spoolss_nt.c | 11 ++++++++++- > source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 1 + > source3/smbd/lanman.c | 3 +++ > source3/smbd/server.c | 27 +++++---------------------- > 5 files changed, 20 insertions(+), 39 deletions(-) > >diff --git a/source3/printing/spoolssd.c b/source3/printing/spoolssd.c >index 83727df..7953237 100644 >--- a/source3/printing/spoolssd.c >+++ b/source3/printing/spoolssd.c >@@ -74,20 +74,6 @@ static void smb_conf_updated(struct messaging_context *msg, > spoolss_reopen_logs(); > } > >-static void spoolss_pcap_updated(struct messaging_context *msg, >- void *private_data, >- uint32_t msg_type, >- struct server_id server_id, >- DATA_BLOB *data) >-{ >- struct tevent_context *ev_ctx = talloc_get_type_abort(private_data, >- struct tevent_context); >- >- DEBUG(10, ("Got message saying pcap was updated. Reloading.\n")); >- change_to_root_user(); >- reload_printers(ev_ctx, msg); >-} >- > static void spoolss_sig_term_handler(struct tevent_context *ev, > struct tevent_signal *se, > int signum, >@@ -206,12 +192,11 @@ void start_spoolssd(struct tevent_context *ev_ctx, > exit(1); > } > >+ /* printer shares updated from printer_list.tdb on client enumeration */ > messaging_register(msg_ctx, NULL, > MSG_PRINTER_UPDATE, print_queue_receive); > messaging_register(msg_ctx, ev_ctx, > MSG_SMB_CONF_UPDATED, smb_conf_updated); >- messaging_register(msg_ctx, ev_ctx, >- MSG_PRINTER_PCAP, spoolss_pcap_updated); > > /* > * Initialize spoolss with an init function to convert printers first. >diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c >index 8372c43..b82dd9d 100644 >--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c >+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c >@@ -4290,12 +4290,21 @@ static WERROR enum_all_printers_info_level(TALLOC_CTX *mem_ctx, > uint32_t *count_p) > { > int snum; >- int n_services = lp_numservices(); >+ int n_services; > union spoolss_PrinterInfo *info = NULL; > uint32_t count = 0; > WERROR result = WERR_OK; > struct dcerpc_binding_handle *b = NULL; > >+ /* >+ * printer shares are only updated on client enumeration. The background >+ * printer process updates printer_list.tdb at regular intervals. >+ */ >+ become_root(); >+ reload_printers(messaging_event_context(msg_ctx), msg_ctx); >+ unbecome_root(); >+ >+ n_services = lp_numservices(); > *count_p = 0; > *info_p = NULL; > >diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c >index b9345d6..4600da3 100644 >--- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c >+++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c >@@ -568,6 +568,7 @@ static WERROR init_srv_share_info_ctr(struct pipes_struct *p, > > /* Ensure all the usershares are loaded. */ > become_root(); >+ reload_printers(messaging_event_context(p->msg_ctx), p->msg_ctx); > load_usershare_shares(); > load_registry_shares(); > num_services = lp_numservices(); >diff --git a/source3/smbd/lanman.c b/source3/smbd/lanman.c >index 0f5d6da..20e22e3 100644 >--- a/source3/smbd/lanman.c >+++ b/source3/smbd/lanman.c >@@ -43,6 +43,7 @@ > #include "passdb/machine_sid.h" > #include "auth.h" > #include "rpc_server/rpc_ncacn_np.h" >+#include "messages.h" > > #ifdef CHECK_TYPES > #undef CHECK_TYPES >@@ -2091,6 +2092,8 @@ static bool api_RNetShareEnum(struct smbd_server_connection *sconn, > > /* Ensure all the usershares are loaded. */ > become_root(); >+ reload_printers(messaging_event_context(sconn->msg_ctx), >+ sconn->msg_ctx); > load_registry_shares(); > count = load_usershare_shares(); > unbecome_root(); >diff --git a/source3/smbd/server.c b/source3/smbd/server.c >index a26dbc4..102e8dd 100644 >--- a/source3/smbd/server.c >+++ b/source3/smbd/server.c >@@ -111,24 +111,6 @@ static void smb_conf_updated(struct messaging_context *msg, > /* printer reload triggered by background printing process */ > } > >-/******************************************************************* >- 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) >-{ >- struct tevent_context *ev_ctx = >- talloc_get_type_abort(private_data, struct tevent_context); >- >- DEBUG(10,("Got message saying pcap was updated. Reloading.\n")); >- change_to_root_user(); >- reload_printers(ev_ctx, msg); >-} >- > static void smbd_sig_term_handler(struct tevent_context *ev, > struct tevent_signal *se, > int signum, >@@ -1287,10 +1269,11 @@ extern void build_options(bool screen); > > if (is_daemon && !interactive > && lp_parm_bool(-1, "smbd", "backgroundqueue", true)) { >- /* background queue is responsible for printcap cache updates */ >- messaging_register(smbd_server_conn->msg_ctx, >- smbd_event_context(), >- MSG_PRINTER_PCAP, smb_pcap_updated); >+ /* >+ * background queue is responsible for printcap cache updates. >+ * Other smbd processes only reload printers when a client >+ * issues an enumeration request. >+ */ > start_background_queue(server_event_context(), > smbd_server_conn->msg_ctx); > } else { >-- >1.8.4.5 > > >From c33876d2b5b40117bfe86a75607bcd1f06871102 Mon Sep 17 00:00:00 2001 >From: David Disseldorp <ddiss@samba.org> >Date: Tue, 22 Jul 2014 20:17:38 +0200 >Subject: [PATCH 3/7] printing: reload printer_list.tdb from in memory list > >This will allow in future for a single atomic printer_list.tdb update. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=10652 > >Signed-off-by: David Disseldorp <ddiss@samba.org> >--- > source3/printing/pcap.c | 26 +++++++++++--------------- > source3/printing/pcap.h | 8 ++++---- > source3/printing/print_aix.c | 17 ++++++++++++++--- > source3/printing/print_iprint.c | 16 ++++++++++------ > source3/printing/print_standard.c | 8 ++++++-- > source3/printing/print_svid.c | 11 +++++++---- > 6 files changed, 52 insertions(+), 34 deletions(-) > >diff --git a/source3/printing/pcap.c b/source3/printing/pcap.c >index 6ad8e33..5173fc9 100644 >--- a/source3/printing/pcap.c >+++ b/source3/printing/pcap.c >@@ -83,7 +83,7 @@ void pcap_cache_destroy_specific(struct pcap_cache **pp_cache) > *pp_cache = NULL; > } > >-bool pcap_cache_add(const char *name, const char *comment, const char *location) >+static bool pcap_cache_add(const char *name, const char *comment, const char *location) > { > NTSTATUS status; > time_t t = time_mono(NULL); >@@ -132,8 +132,8 @@ void pcap_cache_reload(struct tevent_context *ev, > { > const char *pcap_name = lp_printcapname(); > bool pcap_reloaded = False; >- NTSTATUS status; > bool post_cache_fill_fn_handled = false; >+ struct pcap_cache *pcache = NULL; > > DEBUG(3, ("reloading printcap cache\n")); > >@@ -143,12 +143,6 @@ void pcap_cache_reload(struct tevent_context *ev, > return; > } > >- status = printer_list_mark_reload(); >- if (!NT_STATUS_IS_OK(status)) { >- DEBUG(0, ("Failed to mark printer list for reload!\n")); >- return; >- } >- > #ifdef HAVE_CUPS > if (strequal(pcap_name, "cups")) { > pcap_reloaded = cups_cache_reload(ev, msg_ctx, >@@ -164,26 +158,26 @@ void pcap_cache_reload(struct tevent_context *ev, > > #ifdef HAVE_IPRINT > if (strequal(pcap_name, "iprint")) { >- pcap_reloaded = iprint_cache_reload(); >+ pcap_reloaded = iprint_cache_reload(&pcache); > goto done; > } > #endif > > #if defined(SYSV) || defined(HPUX) > if (strequal(pcap_name, "lpstat")) { >- pcap_reloaded = sysv_cache_reload(); >+ pcap_reloaded = sysv_cache_reload(&pcache); > goto done; > } > #endif > > #ifdef AIX > if (strstr_m(pcap_name, "/qconfig") != NULL) { >- pcap_reloaded = aix_cache_reload(); >+ pcap_reloaded = aix_cache_reload(&pcache); > goto done; > } > #endif > >- pcap_reloaded = std_pcap_cache_reload(pcap_name); >+ pcap_reloaded = std_pcap_cache_reload(pcap_name, &pcache); > > done: > DEBUG(3, ("reload status: %s\n", (pcap_reloaded) ? "ok" : "error")); >@@ -192,14 +186,16 @@ done: > /* cleanup old entries only if the operation was successful, > * otherwise keep around the old entries until we can > * successfuly reaload */ >- status = printer_list_clean_old(); >- if (!NT_STATUS_IS_OK(status)) { >- DEBUG(0, ("Failed to cleanup printer list!\n")); >+ >+ if (!pcap_cache_replace(pcache)) { >+ DEBUG(0, ("Failed to replace printer list!\n")); > } >+ > if (post_cache_fill_fn != NULL) { > post_cache_fill_fn(ev, msg_ctx); > } > } >+ pcap_cache_destroy_specific(&pcache); > > return; > } >diff --git a/source3/printing/pcap.h b/source3/printing/pcap.h >index 6c062c3..d388d7d 100644 >--- a/source3/printing/pcap.h >+++ b/source3/printing/pcap.h >@@ -49,7 +49,7 @@ bool pcap_printername_ok(const char *printername); > > /* The following definitions come from printing/print_aix.c */ > >-bool aix_cache_reload(void); >+bool aix_cache_reload(struct pcap_cache **_pcache); > > /* The following definitions come from printing/print_cups.c */ > >@@ -60,13 +60,13 @@ bool cups_cache_reload(struct tevent_context *ev, > > /* The following definitions come from printing/print_iprint.c */ > >-bool iprint_cache_reload(void); >+bool iprint_cache_reload(struct pcap_cache **_pcache); > > /* The following definitions come from printing/print_svid.c */ > >-bool sysv_cache_reload(void); >+bool sysv_cache_reload(struct pcap_cache **_pcache); > > /* The following definitions come from printing/print_standard.c */ >-bool std_pcap_cache_reload(const char *pcap_name); >+bool std_pcap_cache_reload(const char *pcap_name, struct pcap_cache **_pcache); > > #endif /* _PRINTING_PCAP_H_ */ >diff --git a/source3/printing/print_aix.c b/source3/printing/print_aix.c >index 23d9a86..927a71b 100644 >--- a/source3/printing/print_aix.c >+++ b/source3/printing/print_aix.c >@@ -29,12 +29,13 @@ > #include "printing/pcap.h" > > #ifdef AIX >-bool aix_cache_reload(void) >+bool aix_cache_reload(struct pcap_cache **_pcache) > { > int iEtat; > XFILE *pfile; > char *line = NULL, *p; > char *name = NULL; >+ struct pcap_cache *pcache = NULL; > TALLOC_CTX *ctx = talloc_init("aix_cache_reload"); > > if (!ctx) { >@@ -52,6 +53,8 @@ bool aix_cache_reload(void) > iEtat = 0; > /* scan qconfig file for searching <printername>: */ > for (;(line = fgets_slash(NULL, 1024, pfile)); free(line)) { >+ bool ok; >+ > if (*line == '*' || *line == 0) > continue; > >@@ -67,6 +70,7 @@ bool aix_cache_reload(void) > if (strcmp(p, "bsh") != 0) { > name = talloc_strdup(ctx, p); > if (!name) { >+ pcap_cache_destroy_specific(&pcache); > SAFE_FREE(line); > x_fclose(pfile); > TALLOC_FREE(ctx); >@@ -86,7 +90,10 @@ bool aix_cache_reload(void) > /* name is found without stanza device */ > /* probably a good printer ??? */ > iEtat = 0; >- if (!pcap_cache_add(name, NULL, NULL)) { >+ ok = pcap_cache_add_specific(&pcache, >+ name, NULL, NULL); >+ if (!ok) { >+ pcap_cache_destroy_specific(&pcache); > SAFE_FREE(line); > x_fclose(pfile); > TALLOC_FREE(ctx); >@@ -101,7 +108,10 @@ bool aix_cache_reload(void) > } else if (strstr_m(line, "device")) { > /* it's a good virtual printer */ > iEtat = 0; >- if (!pcap_cache_add(name, NULL, NULL)) { >+ ok = pcap_cache_add_specific(&pcache, >+ name, NULL, NULL); >+ if (!ok) { >+ pcap_cache_destroy_specific(&pcache); > SAFE_FREE(line); > x_fclose(pfile); > TALLOC_FREE(ctx); >@@ -113,6 +123,7 @@ bool aix_cache_reload(void) > } > } > >+ *_pcache = pcache; > x_fclose(pfile); > TALLOC_FREE(ctx); > return true; >diff --git a/source3/printing/print_iprint.c b/source3/printing/print_iprint.c >index 529f0dd..6e91747 100644 >--- a/source3/printing/print_iprint.c >+++ b/source3/printing/print_iprint.c >@@ -204,7 +204,8 @@ static int iprint_get_server_version(http_t *http, char* serviceUri) > > static int iprint_cache_add_printer(http_t *http, > int reqId, >- char* url) >+ char *url, >+ struct pcap_cache **pcache) > { > ipp_t *request = NULL, /* IPP Request */ > *response = NULL; /* IPP Response */ >@@ -340,7 +341,7 @@ static int iprint_cache_add_printer(http_t *http, > */ > > if (name != NULL && !secure && smb_enabled) >- pcap_cache_add(name, info, NULL); >+ pcap_cache_add_specific(pcache, name, info, NULL); > } > > out: >@@ -349,7 +350,7 @@ static int iprint_cache_add_printer(http_t *http, > return(0); > } > >-bool iprint_cache_reload(void) >+bool iprint_cache_reload(struct pcap_cache **_pcache) > { > http_t *http = NULL; /* HTTP connection to server */ > ipp_t *request = NULL, /* IPP Request */ >@@ -357,7 +358,8 @@ bool iprint_cache_reload(void) > ipp_attribute_t *attr; /* Current attribute */ > cups_lang_t *language = NULL; /* Default language */ > int i; >- bool ret = False; >+ bool ret = false; >+ struct pcap_cache *pcache = NULL; > > DEBUG(5, ("reloading iprint printcap cache\n")); > >@@ -439,14 +441,16 @@ bool iprint_cache_reload(void) > char *url = ippGetString(attr, i, NULL); > if (!url || !strlen(url)) > continue; >- iprint_cache_add_printer(http, i+2, url); >+ iprint_cache_add_printer(http, i+2, url, >+ &pcache); > } > } > attr = ippNextAttribute(response); > } > } > >- ret = True; >+ ret = true; >+ *_pcache = pcache; > > out: > if (response) >diff --git a/source3/printing/print_standard.c b/source3/printing/print_standard.c >index c4f9c5b..b5f1056 100644 >--- a/source3/printing/print_standard.c >+++ b/source3/printing/print_standard.c >@@ -59,10 +59,11 @@ > #include "printing/pcap.h" > > /* handle standard printcap - moved from pcap_printer_fn() */ >-bool std_pcap_cache_reload(const char *pcap_name) >+bool std_pcap_cache_reload(const char *pcap_name, struct pcap_cache **_pcache) > { > XFILE *pcap_file; > char *pcap_line; >+ struct pcap_cache *pcache = NULL; > > if ((pcap_file = x_fopen(pcap_name, O_RDONLY, 0)) == NULL) { > DEBUG(0, ("Unable to open printcap file %s for read!\n", pcap_name)); >@@ -117,12 +118,15 @@ bool std_pcap_cache_reload(const char *pcap_name) > } > } > >- if (*name && !pcap_cache_add(name, comment, NULL)) { >+ if ((*name != '\0') >+ && !pcap_cache_add_specific(&pcache, name, comment, NULL)) { > x_fclose(pcap_file); >+ pcap_cache_destroy_specific(&pcache); > return false; > } > } > > x_fclose(pcap_file); >+ *_pcache = pcache; > return true; > } >diff --git a/source3/printing/print_svid.c b/source3/printing/print_svid.c >index 2226493..879661b 100644 >--- a/source3/printing/print_svid.c >+++ b/source3/printing/print_svid.c >@@ -35,10 +35,11 @@ > #include "printing/pcap.h" > > #if defined(SYSV) || defined(HPUX) >-bool sysv_cache_reload(void) >+bool sysv_cache_reload(struct pcap_cache **_pcache) > { > char **lines; > int i; >+ struct pcap_cache *pcache = NULL; > > #if defined(HPUX) > DEBUG(5, ("reloading hpux printcap cache\n")); >@@ -111,14 +112,16 @@ bool sysv_cache_reload(void) > *tmp = '\0'; > > /* add it to the cache */ >- if (!pcap_cache_add(name, NULL, NULL)) { >+ if (!pcap_cache_add_specific(&pcache, name, NULL, NULL)) { > TALLOC_FREE(lines); >- return False; >+ pcap_cache_destroy_specific(&pcache); >+ return false; > } > } > > TALLOC_FREE(lines); >- return True; >+ *_pcache = pcache; >+ return true; > } > > #else >-- >1.8.4.5 > > >From 10f8bf31e9457072367df0bd3d6f183e96c74b23 Mon Sep 17 00:00:00 2001 >From: David Disseldorp <ddiss@samba.org> >Date: Fri, 25 Jul 2014 12:18:54 +0200 >Subject: [PATCH 4/7] printing: remove pcap_cache_add() > >All print list updates are now done via pcap_cache_replace(), which can >call into the print_list code directly. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=10652 > >Signed-off-by: David Disseldorp <ddiss@samba.org> >--- > source3/printing/pcap.c | 16 ++++++---------- > source3/printing/pcap.h | 1 - > 2 files changed, 6 insertions(+), 11 deletions(-) > >diff --git a/source3/printing/pcap.c b/source3/printing/pcap.c >index 5173fc9..5059f20 100644 >--- a/source3/printing/pcap.c >+++ b/source3/printing/pcap.c >@@ -83,15 +83,6 @@ void pcap_cache_destroy_specific(struct pcap_cache **pp_cache) > *pp_cache = NULL; > } > >-static bool pcap_cache_add(const char *name, const char *comment, const char *location) >-{ >- NTSTATUS status; >- time_t t = time_mono(NULL); >- >- status = printer_list_set_printer(talloc_tos(), name, comment, location, t); >- return NT_STATUS_IS_OK(status); >-} >- > bool pcap_cache_loaded(void) > { > NTSTATUS status; >@@ -105,6 +96,7 @@ bool pcap_cache_replace(const struct pcap_cache *pcache) > { > const struct pcap_cache *p; > NTSTATUS status; >+ time_t t = time_mono(NULL); > > status = printer_list_mark_reload(); > if (!NT_STATUS_IS_OK(status)) { >@@ -113,7 +105,11 @@ bool pcap_cache_replace(const struct pcap_cache *pcache) > } > > for (p = pcache; p; p = p->next) { >- pcap_cache_add(p->name, p->comment, p->location); >+ status = printer_list_set_printer(talloc_tos(), p->name, >+ p->comment, p->location, t); >+ if (!NT_STATUS_IS_OK(status)) { >+ return false; >+ } > } > > status = printer_list_clean_old(); >diff --git a/source3/printing/pcap.h b/source3/printing/pcap.h >index d388d7d..7dccf84 100644 >--- a/source3/printing/pcap.h >+++ b/source3/printing/pcap.h >@@ -35,7 +35,6 @@ struct pcap_cache; > > bool pcap_cache_add_specific(struct pcap_cache **ppcache, const char *name, const char *comment, const char *location); > void pcap_cache_destroy_specific(struct pcap_cache **ppcache); >-bool pcap_cache_add(const char *name, const char *comment, const char *location); > bool pcap_cache_loaded(void); > bool pcap_cache_replace(const struct pcap_cache *cache); > void pcap_printer_fn_specific(const struct pcap_cache *, void (*fn)(const char *, const char *, const char *, void *), void *); >-- >1.8.4.5 > > >From 60a71852d8f085578591ef86061d4850530e4755 Mon Sep 17 00:00:00 2001 >From: David Disseldorp <ddiss@samba.org> >Date: Wed, 23 Jul 2014 12:12:34 +0200 >Subject: [PATCH 5/7] printing: return last change time with > pcap_cache_loaded() > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=10652 > >Signed-off-by: David Disseldorp <ddiss@samba.org> >--- > source3/printing/load.c | 2 +- > source3/printing/pcap.c | 10 ++++++++-- > source3/printing/pcap.h | 2 +- > source3/web/swat.c | 4 ++-- > 4 files changed, 12 insertions(+), 6 deletions(-) > >diff --git a/source3/printing/load.c b/source3/printing/load.c >index 0a3de73..83f1095 100644 >--- a/source3/printing/load.c >+++ b/source3/printing/load.c >@@ -64,7 +64,7 @@ load automatic printer services from pre-populated pcap cache > void load_printers(struct tevent_context *ev, > struct messaging_context *msg_ctx) > { >- SMB_ASSERT(pcap_cache_loaded()); >+ SMB_ASSERT(pcap_cache_loaded(NULL)); > > add_auto_printers(); > >diff --git a/source3/printing/pcap.c b/source3/printing/pcap.c >index 5059f20..027c1b2 100644 >--- a/source3/printing/pcap.c >+++ b/source3/printing/pcap.c >@@ -83,13 +83,19 @@ void pcap_cache_destroy_specific(struct pcap_cache **pp_cache) > *pp_cache = NULL; > } > >-bool pcap_cache_loaded(void) >+bool pcap_cache_loaded(time_t *_last_change) > { > NTSTATUS status; > time_t last; > > status = printer_list_get_last_refresh(&last); >- return NT_STATUS_IS_OK(status); >+ if (!NT_STATUS_IS_OK(status)) { >+ return false; >+ } >+ if (_last_change != NULL) { >+ *_last_change = last; >+ } >+ return true; > } > > bool pcap_cache_replace(const struct pcap_cache *pcache) >diff --git a/source3/printing/pcap.h b/source3/printing/pcap.h >index 7dccf84..8fc9e9d 100644 >--- a/source3/printing/pcap.h >+++ b/source3/printing/pcap.h >@@ -35,7 +35,7 @@ struct pcap_cache; > > bool pcap_cache_add_specific(struct pcap_cache **ppcache, const char *name, const char *comment, const char *location); > void pcap_cache_destroy_specific(struct pcap_cache **ppcache); >-bool pcap_cache_loaded(void); >+bool pcap_cache_loaded(time_t *_last_change); > bool pcap_cache_replace(const struct pcap_cache *cache); > void pcap_printer_fn_specific(const struct pcap_cache *, void (*fn)(const char *, const char *, const char *, void *), void *); > void pcap_printer_read_fn(void (*fn)(const char *, const char *, const char *, void *), void *); >diff --git a/source3/web/swat.c b/source3/web/swat.c >index f8933d2..a1a035c 100644 >--- a/source3/web/swat.c >+++ b/source3/web/swat.c >@@ -586,7 +586,7 @@ static int save_reload(int snum) > return 0; > } > iNumNonAutoPrintServices = lp_numservices(); >- if (pcap_cache_loaded()) { >+ if (pcap_cache_loaded(NULL)) { > load_printers(server_event_context(), > server_messaging_context()); > } >@@ -1572,7 +1572,7 @@ const char *lang_msg_rotate(TALLOC_CTX *ctx, const char *msgid) > reopen_logs(); > load_interfaces(); > iNumNonAutoPrintServices = lp_numservices(); >- if (pcap_cache_loaded()) { >+ if (pcap_cache_loaded(NULL)) { > load_printers(server_event_context(), > server_messaging_context()); > } >-- >1.8.4.5 > > >From 83f4882672eca499b0088302b5d8c17b34d23849 Mon Sep 17 00:00:00 2001 >From: David Disseldorp <ddiss@samba.org> >Date: Wed, 23 Jul 2014 14:42:00 +0200 >Subject: [PATCH 6/7] smbd: only reprocess printer_list.tdb if it changed > >The per-client smbd printer share inventory is currently updated from >printer_list.tdb when a client enumerates printers, via EnumPrinters or >NetShareEnum. >printer_list.tdb is populated by the background print process, based on >the latest printcap values retrieved from the printing backend (e.g. >CUPS) at regular intervals. >This change ensures that per-client smbd processes don't reparse >printer_list.tdb if it hasn't been updated since the last enumeration. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=10652 > >Suggested-by: Volker Lendecke <vl@samba.org> >Signed-off-by: David Disseldorp <ddiss@samba.org> >--- > source3/smbd/server_reload.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > >diff --git a/source3/smbd/server_reload.c b/source3/smbd/server_reload.c >index c4c5a8d..57f7972 100644 >--- a/source3/smbd/server_reload.c >+++ b/source3/smbd/server_reload.c >@@ -30,6 +30,13 @@ > #include "auth.h" > #include "messages.h" > >+/* >+ * The persistent pcap cache is populated by the background print process. Per >+ * client smbds should only reload their printer share inventories if this >+ * information has changed. Use last_reload_time to detect this. >+ */ >+static time_t reload_last_pcap_time = 0; >+ > /**************************************************************************** > purge stale printers and reload from pre-populated pcap cache > **************************************************************************/ >@@ -40,6 +47,20 @@ void reload_printers(struct tevent_context *ev, > int pnum; > int snum; > const char *pname; >+ bool ok; >+ time_t pcap_last_update; >+ >+ ok = pcap_cache_loaded(&pcap_last_update); >+ if (!ok) { >+ DEBUG(1, ("pcap cache not loaded\n")); >+ return; >+ } >+ >+ if (reload_last_pcap_time == pcap_last_update) { >+ DEBUG(5, ("skipping printer reload, already up to date.\n")); >+ return; >+ } >+ reload_last_pcap_time = pcap_last_update; > > n_services = lp_numservices(); > pnum = lp_servicenumber(PRINTERS_NAME); >-- >1.8.4.5 > > >From 2a3f3c3a5a35078f5afc97215b1b029293c2e01f Mon Sep 17 00:00:00 2001 >From: David Disseldorp <ddiss@samba.org> >Date: Wed, 6 Aug 2014 14:33:02 +0200 >Subject: [PATCH 7/7] printing: reload printer shares on OpenPrinter > >The printer share inventory should be reloaded on open _and_ >enumeration, as there are some clients, such as cupsaddsmb, that do not >perform an enumeration prior to access. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=10652 > >Signed-off-by: David Disseldorp <ddiss@samba.org> >--- > source3/rpc_server/spoolss/srv_spoolss_nt.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > >diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c >index b82dd9d..fad992a 100644 >--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c >+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c >@@ -1737,6 +1737,16 @@ WERROR _spoolss_OpenPrinterEx(struct pipes_struct *p, > return WERR_INVALID_PARAM; > } > >+ /* >+ * The printcap printer share inventory is updated on client >+ * enumeration. For clients that do not perform enumeration prior to >+ * access, such as cupssmbadd, we reinitialise the printer share >+ * inventory on open as well. >+ */ >+ become_root(); >+ reload_printers(messaging_event_context(p->msg_ctx), p->msg_ctx); >+ unbecome_root(); >+ > /* some sanity check because you can open a printer or a print server */ > /* aka: \\server\printer or \\server */ > >@@ -4297,7 +4307,7 @@ static WERROR enum_all_printers_info_level(TALLOC_CTX *mem_ctx, > struct dcerpc_binding_handle *b = NULL; > > /* >- * printer shares are only updated on client enumeration. The background >+ * printer shares are updated on client enumeration. The background > * printer process updates printer_list.tdb at regular intervals. > */ > become_root(); >-- >1.8.4.5 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=10191">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Actions:
View
Attachments on
bug 10652
:
10008
|
10009
|
10010
|
10026
|
10027
|
10028
|
10029
|
10097
|
10098
|
10149
|
10150
|
10173
|
10174
|
10187
|
10190
| 10191 |
10295