You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Greg Ames <gr...@remulak.net> on 2001/12/21 20:46:16 UTC

[PATCH] call first post_config hook earlier

This patch lets modules easily verify the relationship between related config directives
in the post_config hook.  The only core change is to make the first call to the
post_config hook before the logs are open.  This allows modules to write error messages
for bad configs to the console at startup time.  Any error messages generated during
restarts go to the error log.

I included a patch to prefork which compares ServerLimit and MaxClients to demonstrate its
use.  Notice how many lines of code it takes to deal with the physical order of the
directives in the config file: zero.  I chose not to log the warnings to the error_log
file during the second config pass at startup to keep the logging behavior the same as the
current code, but you certainly could log in the second pass if you want a more permanent
record.

Greg

Re: [PATCH] call first post_config hook earlier

Posted by Greg Ames <gr...@remulak.net>.
Bill Stoddard wrote:
> 
> Did you trash the notion of making a function to do swaps in the config tree?

I really prefer not to mess with the tree, unless there's something
wrong with this approach.

Greg

Re: [PATCH] call first post_config hook earlier

Posted by Bill Stoddard <bi...@wstoddard.com>.
Did you trash the notion of making a function to do swaps in the config tree?

Bill

----- Original Message -----
From: "Greg Ames" <gr...@remulak.net>
To: "httpd" <de...@httpd.apache.org>
Sent: Friday, December 21, 2001 2:46 PM
Subject: [PATCH] call first post_config hook earlier


> This patch lets modules easily verify the relationship between related config directives
> in the post_config hook.  The only core change is to make the first call to the
> post_config hook before the logs are open.  This allows modules to write error messages
> for bad configs to the console at startup time.  Any error messages generated during
> restarts go to the error log.
>
> I included a patch to prefork which compares ServerLimit and MaxClients to demonstrate
its
> use.  Notice how many lines of code it takes to deal with the physical order of the
> directives in the config file: zero.  I chose not to log the warnings to the error_log
> file during the second config pass at startup to keep the logging behavior the same as
the
> current code, but you certainly could log in the second pass if you want a more
permanent
> record.
>
> Greg


--------------------------------------------------------------------------------


> Index: server/main.c
> ===================================================================
> RCS file: /cvs/apache/httpd-2.0/server/main.c,v
> retrieving revision 1.111
> diff -u -d -b -r1.111 main.c
> --- main.c 2001/12/18 20:26:15 1.111
> +++ main.c 2001/12/21 18:55:07
> @@ -411,13 +411,13 @@
>   ap_log_error(APLOG_MARK, APLOG_STARTUP | APLOG_NOERRNO, 0, NULL, "Syntax OK\n");
>   destroy_and_exit_process(process, 0);
>      }
> +    if ( ap_run_post_config(pconf, plog, ptemp, server_conf) != OK) {
> +        ap_log_error(APLOG_MARK, APLOG_STARTUP |APLOG_ERR| APLOG_NOERRNO, 0, NULL,
"Configuration Failed\n");
> +        destroy_and_exit_process(process, 1);
> +    }
>      apr_pool_clear(plog);
>      if ( ap_run_open_logs(pconf, plog, ptemp, server_conf) != OK) {
>          ap_log_error(APLOG_MARK, APLOG_STARTUP |APLOG_ERR| APLOG_NOERRNO, 0, NULL,
"Unable to open logs\n");
> -        destroy_and_exit_process(process, 1);
> -    }
> -    if ( ap_run_post_config(pconf, plog, ptemp, server_conf) != OK) {
> -        ap_log_error(APLOG_MARK, APLOG_STARTUP |APLOG_ERR| APLOG_NOERRNO, 0, NULL,
"Configuration Failed\n");
>          destroy_and_exit_process(process, 1);
>      }
>      apr_pool_destroy(ptemp);
> Index: server/mpm/prefork/prefork.c
> ===================================================================
> RCS file: /cvs/apache/httpd-2.0/server/mpm/prefork/prefork.c,v
> retrieving revision 1.226
> diff -u -d -b -r1.226 prefork.c
> --- prefork.c 2001/12/19 14:49:22 1.226
> +++ prefork.c 2001/12/21 18:55:07
> @@ -1238,6 +1238,27 @@
>      apr_cpystrn(ap_coredump_dir, ap_server_root, sizeof(ap_coredump_dir));
>  }
>
> +static int prefork_post_config(apr_pool_t *pconf, apr_pool_t *plog, apr_pool_t *ptemp,
server_rec *s)
> +{
> +    static int call_count;
> +
> +    if (ap_daemons_limit > server_limit) {
> +        if (++call_count != 2)

> +            /* if the warnings already went to the console, don't log them */
> +            ap_log_error(APLOG_MARK, APLOG_STARTUP | APLOG_NOERRNO, 0, NULL,
> +                         "WARNING: MaxClients of %d exceeds ServerLimit value "
> +                         "of %d servers,", ap_daemons_limit, server_limit);
> +            ap_log_error(APLOG_MARK, APLOG_STARTUP | APLOG_NOERRNO, 0, NULL,
> +                         " lowering MaxClients to %d.  To increase, please "
> +                         "see the ServerLimit", server_limit);
> +            ap_log_error(APLOG_MARK, APLOG_STARTUP | APLOG_NOERRNO, 0, NULL,
> +                         " directive.");
> +        }
> +        ap_daemons_limit = server_limit;
> +    }
> +    return OK;
> +}
> +
>  static void prefork_hooks(apr_pool_t *p)
>  {
>  #ifdef AUX3
> @@ -1245,6 +1266,7 @@
>  #endif
>
>      ap_hook_pre_config(prefork_pre_config, NULL, NULL, APR_HOOK_MIDDLE);
> +    ap_hook_post_config(prefork_post_config, NULL, NULL, APR_HOOK_MIDDLE);
>  }
>
>  static const char *set_daemons_to_start(cmd_parms *cmd, void *dummy, const char *arg)
> @@ -1298,18 +1320,8 @@
>      }
>
>      ap_daemons_limit = atoi(arg);
> -    if (ap_daemons_limit > server_limit) {
> -       ap_log_error(APLOG_MARK, APLOG_STARTUP | APLOG_NOERRNO, 0, NULL,
> -                    "WARNING: MaxClients of %d exceeds ServerLimit value "
> -                    "of %d servers,", ap_daemons_limit, server_limit);
> -       ap_log_error(APLOG_MARK, APLOG_STARTUP | APLOG_NOERRNO, 0, NULL,
> -                    " lowering MaxClients to %d.  To increase, please "
> -                    "see the ServerLimit", server_limit);
> -       ap_log_error(APLOG_MARK, APLOG_STARTUP | APLOG_NOERRNO, 0, NULL,
> -                    " directive.");
> -       ap_daemons_limit = server_limit;
> -    }
> -    else if (ap_daemons_limit < 1) {
> +
> +    if (ap_daemons_limit < 1) {
>   ap_log_error(APLOG_MARK, APLOG_STARTUP | APLOG_NOERRNO, 0, NULL,
>                       "WARNING: Require MaxClients > 0, setting to 1");
>   ap_daemons_limit = 1;
>


Re: [PATCH] call first post_config hook earlier

Posted by Aaron Bannert <aa...@clove.org>.
On Fri, Dec 21, 2001 at 02:46:16PM -0500, Greg Ames wrote:
> This patch lets modules easily verify the relationship between related config directives
> in the post_config hook.  The only core change is to make the first call to the
> post_config hook before the logs are open.  This allows modules to write error messages
> for bad configs to the console at startup time.  Any error messages generated during
> restarts go to the error log.
> 
> I included a patch to prefork which compares ServerLimit and MaxClients to demonstrate its
> use.  Notice how many lines of code it takes to deal with the physical order of the
> directives in the config file: zero.  I chose not to log the warnings to the error_log
> file during the second config pass at startup to keep the logging behavior the same as the
> current code, but you certainly could log in the second pass if you want a more permanent
> record.

+1 in concept. I'll even volunteer to bring worker up to speed once
this is applied.

-aaron