Re: [PATCH/heads up] mod_fcgid: checking for global-only directives in a vhost

2009-09-16 Thread Jeff Trawick
On Wed, Sep 16, 2009 at 11:42 AM, Rainer Jung wrote:

> On 16.09.2009 17:18, Jeff Trawick wrote:
> > This has the potential for breaking existing configs by forcing the
> > admin to remove some ignored directives they've coded in a vhost.
> >
> > The affected directives are BusyScanInterval,
> > DefaultMaxClassProcessCount, DefaultMinProcessCount, ErrorScanInterval,
> > IdleScanInterval, IdleTimeout, MaxProcessCount, PHP_Fix_Pathinfo_Enable,
> > ProcessLifetime, SharedmemPath, SocketPath, SpawnScore,
> > SpawnScoreUpLimit, TerminationScore, TimeScore, and ZombieScanInterval.
> >
> > I suppose that a couple of these might have seemed to the administrator
> > to be more flexible than global-only, though it wouldn't have changed
> > the behavior when they added any of these to a vhost.
>
> Does it make sense to correctly merge some of them? I didn't check right
> now. Maybe some things are singletons, like the scanners, but others
> moght make sense per vhost, like e.g. the IdleTimeout or the scores.
> Consider e.g. when various vhosts use different wrappers.
>

In the long term, maybe a few of them should support vhost-specific settings
(PHP_Fix_Pathinfo_Enable, IdleTimeout, or ProcessLifetime).  We'll see what
people ask for (or who jumps in to implement ;) ).

At the moment I'm more interested in fixing the settings that pretend to be
merged and/or aren't picked up from the proper vhost.


Re: [PATCH/heads up] mod_fcgid: checking for global-only directives in a vhost

2009-09-16 Thread Rainer Jung
On 16.09.2009 17:18, Jeff Trawick wrote:
> This has the potential for breaking existing configs by forcing the
> admin to remove some ignored directives they've coded in a vhost.
> 
> The affected directives are BusyScanInterval,
> DefaultMaxClassProcessCount, DefaultMinProcessCount, ErrorScanInterval,
> IdleScanInterval, IdleTimeout, MaxProcessCount, PHP_Fix_Pathinfo_Enable,
> ProcessLifetime, SharedmemPath, SocketPath, SpawnScore,
> SpawnScoreUpLimit, TerminationScore, TimeScore, and ZombieScanInterval.
> 
> I suppose that a couple of these might have seemed to the administrator
> to be more flexible than global-only, though it wouldn't have changed
> the behavior when they added any of these to a vhost.

Does it make sense to correctly merge some of them? I didn't check right
now. Maybe some things are singletons, like the scanners, but others
moght make sense per vhost, like e.g. the IdleTimeout or the scores.
Consider e.g. when various vhosts use different wrappers.



Re: [PATCH/heads up] mod_fcgid: checking for global-only directives in a vhost

2009-09-16 Thread William A. Rowe, Jr.
Jeff Trawick wrote:
> 
> Please object now if you want to allow affected existing configurations
> to continue to work.  We can probably change the hard failure to a warning.

As this is a beta, let's just break the config, we should put something very
clear in README about this.


[PATCH/heads up] mod_fcgid: checking for global-only directives in a vhost

2009-09-16 Thread Jeff Trawick
This has the potential for breaking existing configs by forcing the admin to
remove some ignored directives they've coded in a vhost.

The affected directives are BusyScanInterval, DefaultMaxClassProcessCount,
DefaultMinProcessCount, ErrorScanInterval, IdleScanInterval, IdleTimeout,
MaxProcessCount, PHP_Fix_Pathinfo_Enable, ProcessLifetime, SharedmemPath,
SocketPath, SpawnScore, SpawnScoreUpLimit, TerminationScore, TimeScore, and
ZombieScanInterval.

I suppose that a couple of these might have seemed to the administrator to
be more flexible than global-only, though it wouldn't have changed the
behavior when they added any of these to a vhost.

Please object now if you want to allow affected existing configurations to
continue to work.  We can probably change the hard failure to a warning.

(A case where I might want to issue a warning for an ignored directive is
with IPCConnectTimeout on Unix; it is ignored there, but it has entered the
web wisdom as something that can help certain problems regardless of
platform.  But that is independent of this patch.)
Index: fcgid_conf.c
===
--- fcgid_conf.c(revision 815491)
+++ fcgid_conf.c(working copy)
@@ -161,6 +161,12 @@
 server_rec *s = cmd->server;
 fcgid_server_conf *config =
 ap_get_module_config(s->module_config, &fcgid_module);
+const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+
+if (err != NULL) {
+return err;
+}
+
 config->idle_timeout = atol(arg);
 return NULL;
 }
@@ -178,6 +184,12 @@
 server_rec *s = cmd->server;
 fcgid_server_conf *config =
 ap_get_module_config(s->module_config, &fcgid_module);
+const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+
+if (err != NULL) {
+return err;
+}
+
 config->idle_scan_interval = atol(arg);
 return NULL;
 }
@@ -211,6 +223,12 @@
 server_rec *s = cmd->server;
 fcgid_server_conf *config =
 ap_get_module_config(s->module_config, &fcgid_module);
+const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+
+if (err != NULL) {
+return err;
+}
+
 config->busy_scan_interval = atol(arg);
 return NULL;
 }
@@ -229,6 +247,12 @@
 server_rec *s = cmd->server;
 fcgid_server_conf *config =
 ap_get_module_config(s->module_config, &fcgid_module);
+const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+
+if (err != NULL) {
+return err;
+}
+
 config->proc_lifetime = atol(arg);
 return NULL;
 }
@@ -246,6 +270,12 @@
 server_rec *s = cmd->server;
 fcgid_server_conf *config =
 ap_get_module_config(s->module_config, &fcgid_module);
+const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+
+if (err != NULL) {
+return err;
+}
+
 config->error_scan_interval = atol(arg);
 return NULL;
 }
@@ -264,6 +294,12 @@
 server_rec *s = cmd->server;
 fcgid_server_conf *config =
 ap_get_module_config(s->module_config, &fcgid_module);
+const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+
+if (err != NULL) {
+return err;
+}
+
 config->zombie_scan_interval = atol(arg);
 return NULL;
 }
@@ -281,6 +317,12 @@
 server_rec *s = cmd->server;
 fcgid_server_conf *config =
 ap_get_module_config(s->module_config, &fcgid_module);
+const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+
+if (err != NULL) {
+return err;
+}
+
 config->sockname_prefix = ap_server_root_relative(cmd->pool, arg);
 if (!config->sockname_prefix)
 return "Invalid socket path";
@@ -300,6 +342,12 @@
 server_rec *s = cmd->server;
 fcgid_server_conf *config =
 ap_get_module_config(s->module_config, &fcgid_module);
+const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+
+if (err != NULL) {
+return err;
+}
+
 config->shmname_path = ap_server_root_relative(cmd->pool, arg);
 if (!config->shmname_path)
 return "Invalid shmname path";
@@ -320,6 +368,12 @@
 server_rec *s = cmd->server;
 fcgid_server_conf *config =
 ap_get_module_config(s->module_config, &fcgid_module);
+const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+
+if (err != NULL) {
+return err;
+}
+
 config->spawnscore_uplimit = atol(arg);
 return NULL;
 }
@@ -372,6 +426,12 @@
 server_rec *s = cmd->server;
 fcgid_server_conf *config =
 ap_get_module_config(s->module_config, &fcgid_module);
+const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+
+if (err != NULL) {
+return err;
+}
+
 config->spawn_score = atol(arg);
 return NULL;
 }
@@ -388,6 +448,12 @@
 server_rec *s = cmd->server;
 fcgid_server_conf *config =
 ap_get_module_config(s->module_config, &fcgid_module);
+const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+
+if (err != NULL) {
+