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>
Reviewed-by: Andreas Schneider <asn@samba.org>

diff --git a/source3/printing/spoolssd.c b/source3/printing/spoolssd.c
index f181fcc3f2fd3d4af78cb97ae3e88547df8a6711..afb8f4fd2feb7a753984776b8322cd1b7a32519c 100644 (file)
--- a/source3/printing/spoolssd.c
+++ b/source3/printing/spoolssd.c
@@ -132,27 +132,6 @@ static void smb_conf_updated(struct messaging_context *msg,
        update_conf(ev_ctx, msg);
 }
 
-static void update_pcap(struct tevent_context *ev_ctx,
-                       struct messaging_context *msg_ctx)
-{
-       change_to_root_user();
-       delete_and_reload_printers(ev_ctx, msg_ctx);
-}
-
-static void 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;
-
-       ev_ctx = talloc_get_type_abort(private_data, struct tevent_context);
-
-       DEBUG(10, ("Got message that pcap updated. Reloading.\n"));
-       update_pcap(ev_ctx, msg);
-}
-
 static void spoolss_sig_term_handler(struct tevent_context *ev,
                                     struct tevent_signal *se,
                                     int signum,
@@ -318,8 +297,6 @@ static bool spoolss_child_init(struct tevent_context *ev_ctx,
 
        messaging_register(msg_ctx, ev_ctx,
                           MSG_SMB_CONF_UPDATED, smb_conf_updated);
-       messaging_register(msg_ctx, ev_ctx, MSG_PRINTER_PCAP,
-                          pcap_updated);
        messaging_register(msg_ctx, ev_ctx,
                           MSG_PREFORK_PARENT_EVENT, parent_ping);
 
@@ -739,15 +716,14 @@ pid_t start_spoolssd(struct tevent_context *ev_ctx,
                           MSG_SMB_CONF_UPDATED, smb_conf_updated);
        messaging_register(msg_ctx, NULL, MSG_PRINTER_UPDATE,
                           print_queue_forward);
-       messaging_register(msg_ctx, ev_ctx, MSG_PRINTER_PCAP,
-                          pcap_updated);
        messaging_register(msg_ctx, ev_ctx,
                           MSG_PREFORK_CHILD_EVENT, child_ping);
 
-       /* As soon as messaging is up check if pcap has been loaded already.
-        * If so then we probably missed a message and should load_printers()
-        * ourselves. If pcap has not been loaded yet, then ignore, we will get
-        * a message as soon as the bq process completes the reload. */
+       /*
+        * As soon as messaging is up check if pcap has been loaded already.
+        * If pcap has not been loaded yet, then ignore, as we will reload on
+        * client enumeration anyway.
+        */
        if (pcap_cache_loaded()) {
                load_printers(ev_ctx, msg_ctx);
        }

diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c
index 760c924d80f43ab2a75f329d350d9f596cf33390..b8cae89e9768c5f77ab9bbb8ef1cde8ef0fd3ff1 100644 (file)
--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c
+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c
@@ -4312,7 +4312,7 @@ 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;
@@ -4324,6 +4324,15 @@ static WERROR enum_all_printers_info_level(TALLOC_CTX *mem_ctx,
                return WERR_NOMEM;
        }
 
+       /*
+        * printer shares are only updated on client enumeration. The background
+        * printer process updates printer_list.tdb at regular intervals.
+        */
+       become_root();
+       delete_and_reload_printers(server_event_context(), 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 e030b98ad0ba319f80f151ff18fdafe341ed8514..11abc6c8385075640e3932b42b584b5e1b7d8d44 100644 (file)
--- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
+++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
@@ -548,6 +548,7 @@ static WERROR init_srv_share_info_ctr(struct pipes_struct *p,
 
        /* Ensure all the usershares are loaded. */
        become_root();
+       delete_and_reload_printers(server_event_context(), p->msg_ctx);
        load_usershare_shares(NULL, connections_snum_used);
        load_registry_shares();
        num_services = lp_numservices();

diff --git a/source3/smbd/lanman.c b/source3/smbd/lanman.c
index 66ab8a20130ff3df3655a06f05ba150734443090..b7c74e907b3ff7059f4247b6e4b9f566b70fbf29 100644 (file)
--- a/source3/smbd/lanman.c
+++ b/source3/smbd/lanman.c
@@ -2091,6 +2091,7 @@ static bool api_RNetShareEnum(struct smbd_server_connection *sconn,
 
        /* Ensure all the usershares are loaded. */
        become_root();
+       delete_and_reload_printers(sconn->ev_ctx, sconn->msg_ctx);
        load_registry_shares();
        count = load_usershare_shares(NULL, connections_snum_used);
        unbecome_root();

diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index ec9348c34c5c2ef0c912bb54646470353ec7dfd6..bc07c0f1ed95a04ba7d11d2cc7559f96b6a46111 100644 (file)
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -109,24 +109,6 @@ static void smbd_parent_conf_updated(struct messaging_context *msg,
        printing_subsystem_update(ev_ctx, msg, false);
 }
 
-/*******************************************************************
- 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();
-       delete_and_reload_printers(ev_ctx, msg);
-}
-
 /*******************************************************************
  Delete a statcache entry.
  ********************************************************************/
@@ -894,8 +876,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, ev_ctx, MSG_PRINTER_PCAP,
-                          smb_pcap_updated);
        messaging_register(msg_ctx, NULL, MSG_SMB_BRL_VALIDATE,
                           brl_revalidate);
        messaging_register(msg_ctx, NULL, MSG_SMB_FORCE_TDIS,