[master] cf024ae Introduce a generic handler for memory pool parameters.

Poul-Henning Kamp phk at varnish-cache.org
Wed Dec 14 11:52:22 CET 2011


commit cf024ae0766179d693b6d5b0d74754134df54647
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Wed Dec 14 10:51:23 2011 +0000

    Introduce a generic handler for memory pool parameters.
    
    Minor improvements to a couple param-tweakers as consequence.

diff --git a/bin/varnishd/cache/cache_backend.c b/bin/varnishd/cache/cache_backend.c
index 333b9bf..c1bc055 100644
--- a/bin/varnishd/cache/cache_backend.c
+++ b/bin/varnishd/cache/cache_backend.c
@@ -45,12 +45,6 @@
 
 static struct mempool	*vbcpool;
 
-static struct poolparam vbcpp = {
-	.min_pool	= 10,
-	.max_pool	= 100,
-	.max_age	= 60,
-};
-
 static unsigned		vbcps = sizeof(struct vbc);
 
 /*--------------------------------------------------------------------
@@ -523,6 +517,6 @@ void
 VDI_Init(void)
 {
 
-	vbcpool = MPL_New("vbc", NULL, &vbcpp, &vbcps);
+	vbcpool = MPL_New("vbc", NULL, &cache_param->vbc_pool, &vbcps);
 	AN(vbcpool);
 }
diff --git a/bin/varnishd/common/params.h b/bin/varnishd/common/params.h
index 3f1821f..8198a78 100644
--- a/bin/varnishd/common/params.h
+++ b/bin/varnishd/common/params.h
@@ -200,4 +200,6 @@ struct params {
 	/* VSM dimensions */
 	ssize_t			vsm_space;
 	ssize_t			vsl_space;
+
+	struct poolparam	vbc_pool;
 };
diff --git a/bin/varnishd/mgt/mgt_param.c b/bin/varnishd/mgt/mgt_param.c
index cc14e41..fedc0d1 100644
--- a/bin/varnishd/mgt/mgt_param.c
+++ b/bin/varnishd/mgt/mgt_param.c
@@ -102,33 +102,49 @@ tweak_timeout(struct cli *cli, const struct parspec *par, const char *arg)
 	tweak_generic_timeout(cli, dest, arg);
 }
 
-static void
-tweak_timeout_double(struct cli *cli, const struct parspec *par,
-    const char *arg)
+/*--------------------------------------------------------------------*/
+
+static int
+tweak_generic_timeout_double(struct cli *cli, volatile double *dest,
+    const char *arg, double min, double max)
 {
-	volatile double *dest;
 	double u;
+	char *p;
 
-	dest = par->priv;
 	if (arg != NULL) {
-		u = strtod(arg, NULL);
-		if (u < par->min) {
+		p = NULL;
+		u = strtod(arg, &p);
+		if (*arg == '\0' || *p != '\0') {
+			VCLI_Out(cli, "Not a number(%s)\n", arg);
+			VCLI_SetResult(cli, CLIS_PARAM);
+			return (-1);
+		}
+		if (u < min) {
 			VCLI_Out(cli,
-			    "Timeout must be greater or equal to %.g\n",
-				 par->min);
+			    "Timeout must be greater or equal to %.g\n", min);
 			VCLI_SetResult(cli, CLIS_PARAM);
-			return;
+			return (-1);
 		}
-		if (u > par->max) {
+		if (u > max) {
 			VCLI_Out(cli,
-			    "Timeout must be less than or equal to %.g\n",
-				 par->max);
+			    "Timeout must be less than or equal to %.g\n", max);
 			VCLI_SetResult(cli, CLIS_PARAM);
-			return;
+			return (-1);
 		}
 		*dest = u;
 	} else
 		VCLI_Out(cli, "%.6f", *dest);
+	return (0);
+}
+
+static void
+tweak_timeout_double(struct cli *cli, const struct parspec *par,
+    const char *arg)
+{
+	volatile double *dest;
+
+	dest = par->priv;
+	(void)tweak_generic_timeout_double(cli, dest, arg, par->min, par->max);
 }
 
 /*--------------------------------------------------------------------*/
@@ -138,11 +154,19 @@ tweak_generic_double(struct cli *cli, const struct parspec *par,
     const char *arg)
 {
 	volatile double *dest;
+	char *p;
 	double u;
 
 	dest = par->priv;
 	if (arg != NULL) {
-		u = strtod(arg, NULL);
+		p = NULL;
+		u = strtod(arg, &p);
+		if (arg == '\0' || *p != '\0') {
+			VCLI_Out(cli,
+			    "Not a number (%s)\n", arg);
+			VCLI_SetResult(cli, CLIS_PARAM);
+			return;
+		}
 		if (u < par->min) {
 			VCLI_Out(cli,
 			    "Must be greater or equal to %.g\n",
@@ -206,26 +230,33 @@ tweak_bool(struct cli *cli, const struct parspec *par, const char *arg)
 
 /*--------------------------------------------------------------------*/
 
-void
+int
 tweak_generic_uint(struct cli *cli, volatile unsigned *dest, const char *arg,
     unsigned min, unsigned max)
 {
 	unsigned u;
+	char *p;
 
 	if (arg != NULL) {
+		p = NULL;
 		if (!strcasecmp(arg, "unlimited"))
 			u = UINT_MAX;
 		else
-			u = strtoul(arg, NULL, 0);
+			u = strtoul(arg, &p, 0);
+		if (*arg == '\0' || *p != '\0') {
+			VCLI_Out(cli, "Not a number (%s)\n", arg);
+			VCLI_SetResult(cli, CLIS_PARAM);
+			return (-1);
+		}
 		if (u < min) {
 			VCLI_Out(cli, "Must be at least %u\n", min);
 			VCLI_SetResult(cli, CLIS_PARAM);
-			return;
+			return (-1);
 		}
 		if (u > max) {
 			VCLI_Out(cli, "Must be no more than %u\n", max);
 			VCLI_SetResult(cli, CLIS_PARAM);
-			return;
+			return (-1);
 		}
 		*dest = u;
 	} else if (*dest == UINT_MAX) {
@@ -233,6 +264,7 @@ tweak_generic_uint(struct cli *cli, volatile unsigned *dest, const char *arg,
 	} else {
 		VCLI_Out(cli, "%u", *dest);
 	}
+	return (0);
 }
 
 /*--------------------------------------------------------------------*/
@@ -243,7 +275,8 @@ tweak_uint(struct cli *cli, const struct parspec *par, const char *arg)
 	volatile unsigned *dest;
 
 	dest = par->priv;
-	tweak_generic_uint(cli, dest, arg, (uint)par->min, (uint)par->max);
+	(void)tweak_generic_uint(cli, dest, arg,
+	    (uint)par->min, (uint)par->max);
 }
 
 /*--------------------------------------------------------------------*/
@@ -554,6 +587,56 @@ tweak_diag_bitmap(struct cli *cli, const struct parspec *par, const char *arg)
 
 /*--------------------------------------------------------------------*/
 
+static void
+tweak_poolparam(struct cli *cli, const struct parspec *par, const char *arg)
+{
+	volatile struct poolparam *pp, px;
+	char **av;
+
+	pp = par->priv;
+	if (arg == NULL) {
+		VCLI_Out(cli, "%u,%u,%g",
+		    pp->min_pool, pp->max_pool, pp->max_age);
+	} else {
+		av = VAV_Parse(arg, NULL, ARGV_COMMA);
+		do {
+			if (av[0] != NULL) {
+				VCLI_Out(cli, "Parse error: %s", av[0]);
+				VCLI_SetResult(cli, CLIS_PARAM);
+				break;
+			}
+			if (av[1] == NULL || av[2] == NULL || av[3] == NULL) {
+				VCLI_Out(cli,
+				    "Three fields required:"
+				    " min_pool, max_pool and max_age\n");
+				VCLI_SetResult(cli, CLIS_PARAM);
+				break;
+			}
+			px = *pp;
+			if (tweak_generic_uint(cli, &px.min_pool, av[1],
+			    (uint)par->min, (uint)par->max))
+				break;
+			if (tweak_generic_uint(cli, &px.max_pool, av[2],
+			    (uint)par->min, (uint)par->max))
+				break;
+			if (tweak_generic_timeout_double(cli, &px.max_age,
+			    av[3], 0, 1e6))
+				break;
+			if (px.min_pool > px.max_pool) {
+				VCLI_Out(cli,
+				    "min_pool cannot be larger"
+				    " than max_pool\n");
+				VCLI_SetResult(cli, CLIS_PARAM);
+				break;
+			}
+			*pp = px;
+		} while(0);
+		
+	}
+}
+
+/*--------------------------------------------------------------------*/
+
 /*
  * Make sure to end all lines with either a space or newline of the
  * formatting will go haywire.
@@ -1113,6 +1196,15 @@ static const struct parspec input_parspec[] = {
 		0,
 		"true", ""},
 
+	{ "vbc_pool", tweak_poolparam, &mgt_param.vbc_pool, 0, 10000,
+		"Parameters for backend connection memory pool.\n"
+		"The three numbers are:\n"
+		"   min_pool -- minimum size of free pool.\n"
+		"   max_pool -- maximum size of free pool.\n"
+		"   max_age -- max age of free element.\n",
+		0,
+		"10,100,10", ""},
+
 	{ NULL, NULL, NULL }
 };
 
diff --git a/bin/varnishd/mgt/mgt_param.h b/bin/varnishd/mgt/mgt_param.h
index 2d4a97f..275dfe6 100644
--- a/bin/varnishd/mgt/mgt_param.h
+++ b/bin/varnishd/mgt/mgt_param.h
@@ -49,7 +49,7 @@ struct parspec {
 	const char	*units;
 };
 
-void tweak_generic_uint(struct cli *cli,
+int tweak_generic_uint(struct cli *cli,
     volatile unsigned *dest, const char *arg, unsigned min, unsigned max);
 void tweak_uint(struct cli *cli, const struct parspec *par, const char *arg);
 void tweak_timeout(struct cli *cli,
diff --git a/bin/varnishd/mgt/mgt_pool.c b/bin/varnishd/mgt/mgt_pool.c
index badb3f4..e6320fd 100644
--- a/bin/varnishd/mgt/mgt_pool.c
+++ b/bin/varnishd/mgt/mgt_pool.c
@@ -59,7 +59,7 @@ tweak_thread_pool_min(struct cli *cli, const struct parspec *par,
     const char *arg)
 {
 
-	tweak_generic_uint(cli, &mgt_param.wthread_min, arg,
+	(void)tweak_generic_uint(cli, &mgt_param.wthread_min, arg,
 	    (unsigned)par->min, mgt_param.wthread_max);
 }
 
@@ -86,7 +86,7 @@ tweak_stack_size(struct cli *cli, const struct parspec *par,
 		arg = buf;
 	}
 
-	tweak_generic_uint(cli, &mgt_param.wthread_stacksize, arg,
+	(void)tweak_generic_uint(cli, &mgt_param.wthread_stacksize, arg,
 	    low, (uint)par->max);
 }
 
@@ -98,7 +98,7 @@ tweak_thread_pool_max(struct cli *cli, const struct parspec *par,
 {
 
 	(void)par;
-	tweak_generic_uint(cli, &mgt_param.wthread_max, arg,
+	(void)tweak_generic_uint(cli, &mgt_param.wthread_max, arg,
 	    mgt_param.wthread_min, UINT_MAX);
 }
 
diff --git a/bin/varnishtest/tests/b00034.vtc b/bin/varnishtest/tests/b00034.vtc
new file mode 100644
index 0000000..3579fcb
--- /dev/null
+++ b/bin/varnishtest/tests/b00034.vtc
@@ -0,0 +1,13 @@
+varnishtest "mempool param handling"
+
+server s1 {
+} -start
+
+varnish v1 -vcl+backend {}
+
+varnish v1 -cliok "param.set vbc_pool 1,10,1"
+varnish v1 -clierr 106 "param.set vbc_pool 10"
+varnish v1 -clierr 106 "param.set vbc_pool 10,1,1"
+varnish v1 -clierr 106 "param.set vbc_pool a,10,10"
+varnish v1 -clierr 106 "param.set vbc_pool 10,a,10"
+varnish v1 -clierr 106 "param.set vbc_pool 10,10,a"



More information about the varnish-commit mailing list