From 286878d0b04a702a11d616cdc0284d042ef465e3 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Tue, 12 Feb 2013 18:57:53 +0100 Subject: [PATCH 1/5] smbd: fix cups printcap cache updates on startup On startup the parent smbd process currently calls pcap_cache_reload(), which is done immediately before the background queue process is forked. pcap_cache_reload() is asynchronous with cups, in that it forks a separate process to obtain the printer listing. The cache_fd_event print_cups.c global variable is used to track when a cups printer listing is in progress. cache_fd_event is set when the background queue process is forked, due to smbd's pcap_cache_reload() call immediately prior. As a result, the background queue process assumes an existing pcap_cache_reload() call is indefinitely outstanding, causing the printcap cache to remain stale thereafter. --- source3/printing/printing.c | 5 +++++ source3/smbd/server.c | 26 ++++++++++++++------------ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/source3/printing/printing.c b/source3/printing/printing.c index a5b36c7..376fc7c 100644 --- a/source3/printing/printing.c +++ b/source3/printing/printing.c @@ -1751,6 +1751,11 @@ void start_background_queue(struct tevent_context *ev, smb_panic("tevent_add_fd() failed for pause_pipe"); } + /* reload on startup to ensure parent smbd is refreshed */ + pcap_cache_reload(server_event_context(), + smbd_messaging_context(), + &reload_pcap_change_notify); + if (!(event_add_idle(ev, NULL, timeval_set(SMBD_HOUSEKEEPING_INTERVAL, 0), "printer_housekeeping", diff --git a/source3/smbd/server.c b/source3/smbd/server.c index 4b6114a..4a4a006 100644 --- a/source3/smbd/server.c +++ b/source3/smbd/server.c @@ -108,10 +108,7 @@ 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(ev_ctx, msg, - &reload_pcap_change_notify); - } + reload_printers(ev_ctx, msg); } /******************************************************************* @@ -772,8 +769,6 @@ 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, server_event_context(), MSG_PRINTER_PCAP, - smb_pcap_updated); brl_register_msgs(msg_ctx); msg_idmap_register_msgs(msg_ctx); @@ -1234,10 +1229,6 @@ extern void build_options(bool screen); if (!print_backend_init(smbd_messaging_context())) exit(1); - /* Publish nt printers, this requires a working winreg pipe */ - 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 smbd is launched via inetd and we fork a copy of @@ -1245,8 +1236,19 @@ extern void build_options(bool screen); if (is_daemon && !interactive && lp_parm_bool(-1, "smbd", "backgroundqueue", true)) { - start_background_queue(smbd_event_context(), - smbd_messaging_context()); + /* background queue is responsible for printcap cache updates */ + messaging_register(smbd_server_conn->msg_ctx, + smbd_event_context(), + MSG_PRINTER_PCAP, smb_pcap_updated); + start_background_queue(server_event_context(), + smbd_server_conn->msg_ctx); + } else { + DEBUG(3, ("running without background printer process, dynamic " + "printer updates disabled\n")); + /* Publish nt printers, this requires a working winreg pipe */ + pcap_cache_reload(server_event_context(), + smbd_messaging_context(), + &reload_printers); } if (is_daemon && !_lp_disable_spoolss()) { -- 1.8.1.4 From 5c3fb810eeebee4c8a3db551f1e163374c52316a Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Thu, 14 Feb 2013 14:42:21 +0100 Subject: [PATCH 2/5] printing: move pcap change notifier to bg process The background print queue process is responsible for printcap cache updates, and should be the only process to send notifications. --- source3/printing/printing.c | 57 ++++++++++++++++++++++++++++++++++++++++++-- source3/smbd/process.c | 3 --- source3/smbd/proto.h | 4 ---- source3/smbd/server_reload.c | 18 -------------- 4 files changed, 55 insertions(+), 27 deletions(-) diff --git a/source3/printing/printing.c b/source3/printing/printing.c index 376fc7c..21f318c 100644 --- a/source3/printing/printing.c +++ b/source3/printing/printing.c @@ -1656,6 +1656,24 @@ static void add_child_pid(pid_t pid) num_children += 1; } +/**************************************************************************** + Notify smbds of new printcap data +**************************************************************************/ +static void reload_pcap_change_notify(struct tevent_context *ev, + struct messaging_context *msg_ctx) +{ + /* + * Reload the printers first in the background process so that + * newly added printers get default values created in the registry. + * + * This will block the process for some time (~1 sec per printer), but + * it doesn't block smbd's servering clients. + */ + reload_printers(ev, msg_ctx); + + message_send_all(msg_ctx, MSG_PRINTER_PCAP, NULL, 0, NULL); +} + static bool printer_housekeeping_fn(const struct timeval *now, void *private_data) { @@ -1678,6 +1696,30 @@ static bool printer_housekeeping_fn(const struct timeval *now, return true; } +static void printing_sig_term_handler(struct tevent_context *ev, + struct tevent_signal *se, + int signum, + int count, + void *siginfo, + void *private_data) +{ + exit_server_cleanly("termination signal"); +} + +static void printing_sig_hup_handler(struct tevent_context *ev, + struct tevent_signal *se, + int signum, + int count, + void *siginfo, + void *private_data) +{ + struct messaging_context *msg_ctx = talloc_get_type_abort( + private_data, struct messaging_context); + + DEBUG(1,("Reloading printers after SIGHUP\n")); + reload_pcap_change_notify(ev, msg_ctx); +} + static pid_t background_lpq_updater_pid = -1; /**************************************************************************** @@ -1713,6 +1755,7 @@ void start_background_queue(struct tevent_context *ev, struct tevent_fd *fde; int ret; NTSTATUS status; + struct tevent_signal *se; /* Child. */ DEBUG(5,("start_background_queue: background LPQ thread started\n")); @@ -1727,8 +1770,18 @@ void start_background_queue(struct tevent_context *ev, smb_panic("reinit_after_fork() failed"); } - smbd_setup_sig_term_handler(); - smbd_setup_sig_hup_handler(ev, msg_ctx); + se = tevent_add_signal(ev, ev, SIGTERM, 0, + printing_sig_term_handler, + NULL); + if (se == NULL) { + smb_panic("failed to setup SIGTERM handler"); + } + se = tevent_add_signal(ev, ev, SIGHUP, 0, + printing_sig_hup_handler, + msg_ctx); + if (se == NULL) { + smb_panic("failed to setup SIGHUP handler"); + } if (!serverid_register(procid_self(), FLAG_MSG_GENERAL|FLAG_MSG_SMBD diff --git a/source3/smbd/process.c b/source3/smbd/process.c index 358d051..b93b959 100644 --- a/source3/smbd/process.c +++ b/source3/smbd/process.c @@ -957,9 +957,6 @@ 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, diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index d6f7511..2baa58d 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -770,8 +770,6 @@ NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx, /* The following definitions come from smbd/process.c */ void smbd_setup_sig_term_handler(void); -void smbd_setup_sig_hup_handler(struct tevent_context *ev, - struct messaging_context *msg_ctx); bool srv_send_smb(struct smbd_server_connection *sconn, char *buffer, bool no_signing, uint32_t seqnum, bool do_encrypt, @@ -982,8 +980,6 @@ 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/smbd/server_reload.c b/source3/smbd/server_reload.c index bda5d08..f4c15f8 100644 --- a/source3/smbd/server_reload.c +++ b/source3/smbd/server_reload.c @@ -160,21 +160,3 @@ 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) -{ - /* - * Reload the printers first in the background process so that - * newly added printers get default values created in the registry. - * - * This will block the process for some time (~1 sec per printer), but - * it doesn't block smbd's servering clients. - */ - reload_printers(ev, msg_ctx); - - message_send_all(msg_ctx, MSG_PRINTER_PCAP, NULL, 0, NULL); -} -- 1.8.1.4 From 930125e4a8626da5cb8765eac93b74f21fcf5878 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Thu, 14 Feb 2013 17:02:08 +0100 Subject: [PATCH 3/5] printing: add sighup and conf change handlers The background printing process is now responsible for all printcap cache updates, which should be done on SIGHUP and configuration change. --- source3/printing/printing.c | 17 ++++++++++++++- source3/smbd/process.c | 50 ------------------------------------------ source3/smbd/proto.h | 1 - source3/smbd/server.c | 53 ++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 68 insertions(+), 53 deletions(-) diff --git a/source3/printing/printing.c b/source3/printing/printing.c index 21f318c..780afcf 100644 --- a/source3/printing/printing.c +++ b/source3/printing/printing.c @@ -1717,9 +1717,22 @@ static void printing_sig_hup_handler(struct tevent_context *ev, private_data, struct messaging_context); DEBUG(1,("Reloading printers after SIGHUP\n")); - reload_pcap_change_notify(ev, msg_ctx); + pcap_cache_reload(ev, msg_ctx, + &reload_pcap_change_notify); } +static void printing_conf_updated(struct messaging_context *msg, + void *private_data, + uint32_t msg_type, + struct server_id server_id, + DATA_BLOB *data) +{ + DEBUG(5,("Reloading printers after conf change\n")); + pcap_cache_reload(messaging_event_context(msg), msg, + &reload_pcap_change_notify); +} + + static pid_t background_lpq_updater_pid = -1; /**************************************************************************** @@ -1795,6 +1808,8 @@ void start_background_queue(struct tevent_context *ev, messaging_register(msg_ctx, NULL, MSG_PRINTER_UPDATE, print_queue_receive); + messaging_register(msg_ctx, NULL, MSG_SMB_CONF_UPDATED, + printing_conf_updated); fde = tevent_add_fd(ev, ev, pause_pipe[1], TEVENT_FD_READ, printing_pause_fd_handler, diff --git a/source3/smbd/process.c b/source3/smbd/process.c index b93b959..f43005d 100644 --- a/source3/smbd/process.c +++ b/source3/smbd/process.c @@ -921,56 +921,6 @@ struct idle_event *event_add_idle(struct event_context *event_ctx, return result; } -static void smbd_sig_term_handler(struct tevent_context *ev, - struct tevent_signal *se, - int signum, - int count, - void *siginfo, - void *private_data) -{ - exit_server_cleanly("termination signal"); -} - -void smbd_setup_sig_term_handler(void) -{ - struct tevent_signal *se; - - se = tevent_add_signal(smbd_event_context(), - smbd_event_context(), - SIGTERM, 0, - smbd_sig_term_handler, - NULL); - if (!se) { - exit_server("failed to setup SIGTERM handler"); - } -} - -static void smbd_sig_hup_handler(struct tevent_context *ev, - struct tevent_signal *se, - int signum, - int count, - void *siginfo, - void *private_data) -{ - struct messaging_context *msg_ctx = talloc_get_type_abort( - private_data, struct messaging_context); - change_to_root_user(); - DEBUG(1,("Reloading services after SIGHUP\n")); - reload_services(msg_ctx, smbd_server_conn->sock, False); -} - -void smbd_setup_sig_hup_handler(struct tevent_context *ev, - struct messaging_context *msg_ctx) -{ - struct tevent_signal *se; - - se = tevent_add_signal(ev, ev, SIGHUP, 0, smbd_sig_hup_handler, - msg_ctx); - if (!se) { - exit_server("failed to setup SIGHUP handler"); - } -} - static NTSTATUS smbd_server_connection_loop_once(struct smbd_server_connection *conn) { int timeout; diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 2baa58d..3b0f817 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -769,7 +769,6 @@ NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx, /* The following definitions come from smbd/process.c */ -void smbd_setup_sig_term_handler(void); bool srv_send_smb(struct smbd_server_connection *sconn, char *buffer, bool no_signing, uint32_t seqnum, bool do_encrypt, diff --git a/source3/smbd/server.c b/source3/smbd/server.c index 4a4a006..a08131f 100644 --- a/source3/smbd/server.c +++ b/source3/smbd/server.c @@ -108,7 +108,7 @@ static void smb_conf_updated(struct messaging_context *msg, "updated. Reloading.\n")); change_to_root_user(); reload_services(msg, smbd_server_conn->sock, False); - reload_printers(ev_ctx, msg); + /* printer reload triggered by background printing process */ } /******************************************************************* @@ -129,6 +129,57 @@ static void smb_pcap_updated(struct messaging_context *msg, reload_printers(ev_ctx, msg); } +static void smbd_sig_term_handler(struct tevent_context *ev, + struct tevent_signal *se, + int signum, + int count, + void *siginfo, + void *private_data) +{ + exit_server_cleanly("termination signal"); +} + +static void smbd_setup_sig_term_handler(void) +{ + struct tevent_signal *se; + + se = tevent_add_signal(smbd_event_context(), + smbd_event_context(), + SIGTERM, 0, + smbd_sig_term_handler, + NULL); + if (!se) { + exit_server("failed to setup SIGTERM handler"); + } +} + +static void smbd_sig_hup_handler(struct tevent_context *ev, + struct tevent_signal *se, + int signum, + int count, + void *siginfo, + void *private_data) +{ + struct messaging_context *msg_ctx = talloc_get_type_abort( + private_data, struct messaging_context); + change_to_root_user(); + DEBUG(1,("Reloading services after SIGHUP\n")); + reload_services(msg_ctx, smbd_server_conn->sock, false); +} + +static void smbd_setup_sig_hup_handler(struct tevent_context *ev, + struct messaging_context *msg_ctx) +{ + struct tevent_signal *se; + + se = tevent_add_signal(ev, ev, SIGHUP, 0, smbd_sig_hup_handler, + msg_ctx); + if (!se) { + exit_server("failed to setup SIGHUP handler"); + } +} + + /******************************************************************* Delete a statcache entry. ********************************************************************/ -- 1.8.1.4 From 23afd7e3ba8d07ee93c4638f6072c630a5cb652c Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Fri, 15 Feb 2013 12:17:53 +0100 Subject: [PATCH 4/5] spoolss: only reload printers on pcap update message Printcap cache updates are the responsibility of the background printing process, which after doing so broadcasts a MSG_PRINTER_PCAP message. Spoolssd should only reload printers after receiving such a message. --- source3/printing/spoolssd.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/source3/printing/spoolssd.c b/source3/printing/spoolssd.c index cb90a9f..83727df 100644 --- a/source3/printing/spoolssd.c +++ b/source3/printing/spoolssd.c @@ -71,10 +71,23 @@ static void smb_conf_updated(struct messaging_context *msg, DEBUG(10, ("Got message saying smb.conf was updated. Reloading.\n")); change_to_root_user(); - reload_printers(ev_ctx, 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, @@ -111,7 +124,6 @@ static void spoolss_sig_hup_handler(struct tevent_context *ev, change_to_root_user(); DEBUG(1,("Reloading printers after SIGHUP\n")); - reload_printers(ev, msg_ctx); spoolss_reopen_logs(); } @@ -198,6 +210,8 @@ void start_spoolssd(struct tevent_context *ev_ctx, 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. -- 1.8.1.4 From f1fbf9758047ec1f09f6586e4801c3d79c79e62f Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Fri, 15 Mar 2013 16:54:06 +0100 Subject: [PATCH 5/5] printing: update registry and publish in background Currently all smbd processes unnecessarily access each printer registry TDB entry following printcap cache reload. This change moves responsibility for this to the background print queue process. This and the last four commits address bug 9650: New or delete cups printerqueues are not recognized by the samba. --- source3/printing/printing.c | 2 +- source3/smbd/proto.h | 2 ++ source3/smbd/server.c | 2 +- source3/smbd/server_reload.c | 62 ++++++++++++++++++++++++++++++++++---------- 4 files changed, 53 insertions(+), 15 deletions(-) diff --git a/source3/printing/printing.c b/source3/printing/printing.c index 780afcf..16821ae 100644 --- a/source3/printing/printing.c +++ b/source3/printing/printing.c @@ -1669,7 +1669,7 @@ static void reload_pcap_change_notify(struct tevent_context *ev, * This will block the process for some time (~1 sec per printer), but * it doesn't block smbd's servering clients. */ - reload_printers(ev, msg_ctx); + reload_printers_full(ev, msg_ctx); message_send_all(msg_ctx, MSG_PRINTER_PCAP, NULL, 0, NULL); } diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 3b0f817..b4c5cb4 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -977,6 +977,8 @@ struct messaging_context *smbd_messaging_context(void); struct memcache *smbd_memcache(void); void reload_printers(struct tevent_context *ev, struct messaging_context *msg_ctx); +void reload_printers_full(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/smbd/server.c b/source3/smbd/server.c index a08131f..a26dbc4 100644 --- a/source3/smbd/server.c +++ b/source3/smbd/server.c @@ -1299,7 +1299,7 @@ extern void build_options(bool screen); /* Publish nt printers, this requires a working winreg pipe */ pcap_cache_reload(server_event_context(), smbd_messaging_context(), - &reload_printers); + &reload_printers_full); } if (is_daemon && !_lp_disable_spoolss()) { diff --git a/source3/smbd/server_reload.c b/source3/smbd/server_reload.c index f4c15f8..9e28a72 100644 --- a/source3/smbd/server_reload.c +++ b/source3/smbd/server_reload.c @@ -36,8 +36,51 @@ void reload_printers(struct tevent_context *ev, struct messaging_context *msg_ctx) { + int n_services; + int pnum; + int snum; + const char *pname; + + n_services = lp_numservices(); + pnum = lp_servicenumber(PRINTERS_NAME); + + DEBUG(10, ("reloading printer services from pcap cache\n")); + + /* + * Add default config for printers added to smb.conf file and remove + * stale printers + */ + for (snum = 0; snum < n_services; snum++) { + /* avoid removing PRINTERS_NAME */ + if (snum == pnum) { + continue; + } + + /* skip no-printer services */ + if (!(lp_snum_ok(snum) && lp_print_ok(snum))) { + continue; + } + + pname = lp_printername(snum); + + /* check printer, but avoid removing non-autoloaded printers */ + if (lp_autoloaded(snum) && !pcap_printername_ok(pname)) { + DEBUG(3, ("removing stale printer %s\n", pname)); + lp_killservice(snum); + } + } + + /* Make sure deleted printers are gone */ + load_printers(ev, msg_ctx); +} + +/**************************************************************************** + purge stale printers and reload from pre-populated pcap cache +**************************************************************************/ +void reload_printers_full(struct tevent_context *ev, + struct messaging_context *msg_ctx) +{ struct auth_serversupplied_info *session_info = NULL; - struct spoolss_PrinterInfo2 *pinfo2 = NULL; int n_services; int pnum; int snum; @@ -45,17 +88,12 @@ void reload_printers(struct tevent_context *ev, const char *sname; NTSTATUS status; - load_printers(ev, msg_ctx); - n_services = lp_numservices(); pnum = lp_servicenumber(PRINTERS_NAME); - DEBUG(10, ("reloading printer services from pcap cache\n")); - - status = make_session_info_system(talloc_tos(), &session_info); + status = make_session_info_system(talloc_new(NULL), &session_info); if (!NT_STATUS_IS_OK(status)) { - DEBUG(3, ("reload_printers: " - "Could not create system session_info\n")); + DEBUG(3, ("Could not create system session_info\n")); /* can't remove stale printers before we * are fully initilized */ return; @@ -81,8 +119,7 @@ void reload_printers(struct tevent_context *ev, /* check printer, but avoid removing non-autoloaded printers */ if (lp_autoloaded(snum) && !pcap_printername_ok(pname)) { - DEBUG(3, ("removing stale printer %s\n", pname)); - + struct spoolss_PrinterInfo2 *pinfo2 = NULL; if (is_printer_published(session_info, session_info, msg_ctx, NULL, lp_servicename(snum), @@ -96,7 +133,6 @@ void reload_printers(struct tevent_context *ev, } nt_printer_remove(session_info, session_info, msg_ctx, pname); - lp_killservice(snum); } else { DEBUG(8, ("Adding default registry entry for printer " "[%s], if it doesn't exist.\n", sname)); @@ -105,8 +141,8 @@ void reload_printers(struct tevent_context *ev, } } - /* Make sure deleted printers are gone */ - load_printers(ev, msg_ctx); + /* finally, purge old snums */ + reload_printers(ev, msg_ctx); TALLOC_FREE(session_info); } -- 1.8.1.4