[master] 9947307d3 cli: Teach vcl.discard to operate on multiple VCLs

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Mon Nov 23 05:15:15 UTC 2020


commit 9947307d357247823bd8b48f034f5b3d253e6664
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Tue Oct 13 10:45:23 2020 +0200

    cli: Teach vcl.discard to operate on multiple VCLs
    
    To put it simply, let's take a simple CLI script:
    
        vcl.discard vcl1
        vcl.discard vcl2
        [...]
        vcl.discard vclX
    
    We can now achieve the same with a single command:
    
        vcl.discard vcl1 vcl2 ... vclX
    
    But there is slighty more to it, because vcl.discard operates on both
    VCLs and VCL labels, and dependencies can exist between both. So in
    addition to operate on multiple VCLs it also does so in the correct
    order.

diff --git a/bin/varnishd/mgt/mgt_vcl.c b/bin/varnishd/mgt/mgt_vcl.c
index 37ca013e9..af4cffc11 100644
--- a/bin/varnishd/mgt/mgt_vcl.c
+++ b/bin/varnishd/mgt/mgt_vcl.c
@@ -678,28 +678,15 @@ mgt_vcl_discard(struct cli *cli, struct vclprog *vp)
 	mgt_vcl_del(vp);
 }
 
-static struct vclprog *
-mgt_vcl_can_discard(struct cli *cli, const char * const *av)
+static void
+mgt_vcl_discard_depfail(struct cli *cli, struct vclprog *vp)
 {
-	struct vclprog *vp;
 	struct vcldep *vd;
 	int n;
 
 	AN(cli);
-	AN(av);
-	AN(*av);
-
-	vp = mcf_find_vcl(cli, *av);
-	if (vp == NULL)
-		return (NULL);
-	if (vp == active_vcl) {
-		VCLI_SetResult(cli, CLIS_CANT);
-		VCLI_Out(cli, "Cannot discard active VCL program %s\n",
-		    vp->name);
-		return (NULL);
-	}
-	if (VTAILQ_EMPTY(&vp->dto))
-		return (vp);
+	AN(vp);
+	assert(!VTAILQ_EMPTY(&vp->dto));
 
 	VCLI_SetResult(cli, CLIS_CANT);
 	AN(vp->warm);
@@ -718,19 +705,114 @@ mgt_vcl_can_discard(struct cli *cli, const char * const *av)
 		}
 		VCLI_Out(cli, "\t%s\n", vd->from->name);
 	}
-	return (NULL);
+}
+
+static struct vclprog **
+mgt_vcl_discard_depcheck(struct cli *cli, const char * const *names,
+    unsigned *lp)
+{
+	struct vclprog **res, *vp;
+	struct vcldep *vd;
+	const char * const *s;
+	unsigned i, j, l;
+
+	l = 0;
+	s = names;
+	while (*s != NULL) {
+		l++;
+		s++;
+	}
+	AN(l);
+
+	res = calloc(l, sizeof *res);
+	AN(res);
+
+	/* NB: Build a list of VCL programs and labels to discard. A null
+	 * pointer in the array is a VCL program that no longer needs to be
+	 * discarded.
+	 */
+	for (i = 0; i < l; i++) {
+		vp = mcf_find_vcl(cli, names[i]);
+		if (vp == NULL)
+			break;
+		if (vp == active_vcl) {
+			VCLI_SetResult(cli, CLIS_CANT);
+			VCLI_Out(cli, "Cannot discard active VCL program %s\n",
+			    vp->name);
+			break;
+		}
+		for (j = 0; j < i; j++)
+			if (vp == res[j])
+				break;
+		if (j < i)
+			continue; /* skip duplicates */
+		res[i] = vp;
+	}
+
+	if (i < l) {
+		free(res);
+		return (NULL);
+	}
+
+	/* NB: Check that all direct dependent VCL programs and labels belong
+	 * to the list of VCLs to be discarded. This mechanically ensures that
+	 * indirect dependent VCLs also belong.
+	 */
+	for (i = 0; i < l; i++) {
+		if (res[i] == NULL || VTAILQ_EMPTY(&res[i]->dto))
+			continue;
+		VTAILQ_FOREACH(vd, &res[i]->dto, lto) {
+			for (j = 0; j < l; j++)
+				if (res[j] == vd->from)
+					break;
+			if (j == l)
+				break;
+		}
+		if (vd != NULL)
+			break;
+	}
+
+	if (i < l) {
+		mgt_vcl_discard_depfail(cli, res[i]);
+		free(res);
+		return (NULL);
+	}
+
+	*lp = l;
+	return (res);
 }
 
 static void v_matchproto_(cli_func_t)
 mcf_vcl_discard(struct cli *cli, const char * const *av, void *priv)
 {
-	struct vclprog *vp;
+	struct vclprog **vp;
+	unsigned i, l, done;
 
 	(void)priv;
-	vp = mgt_vcl_can_discard(cli, av + 2);
+	vp = mgt_vcl_discard_depcheck(cli, av + 2, &l);
 	if (vp == NULL)
 		return;
-	mgt_vcl_discard(cli, vp);
+
+	/* NB: discard VCLs in topological order. At this point it
+	 * can only succeed, but the loop ensures that it eventually
+	 * completes even if the dependency check is ever broken.
+	 */
+	AN(l);
+	do {
+		done = 0;
+		for (i = 0; i < l; i++) {
+			if (vp[i] == NULL || vp[i]->nto > 0)
+				continue;
+			mgt_vcl_discard(cli, vp[i]);
+			vp[i] = NULL;
+			done++;
+		}
+	} while (done > 0);
+
+	for (i = 0; i < l; i++)
+		if (vp[i] != NULL)
+			WRONG(vp[i]->name);
+	free(vp);
 }
 
 static void v_matchproto_(cli_func_t)
diff --git a/bin/varnishtest/tests/c00077.vtc b/bin/varnishtest/tests/c00077.vtc
index c25d9e4c5..f176bc948 100644
--- a/bin/varnishtest/tests/c00077.vtc
+++ b/bin/varnishtest/tests/c00077.vtc
@@ -64,15 +64,5 @@ varnish v1 -clierr 106 "vcl.label vclA vcl3"
 
 varnish v1 -cliok "vcl.symtab"
 
-varnish v1 -cliok "vcl.discard vcl3"
-varnish v1 -cliok "vcl.discard vcl4"
-varnish v1 -cliok "vcl.discard vcl5"
-varnish v1 -cliok "vcl.discard vcl6"
-varnish v1 -cliok "vcl.discard vcl7"
-
-varnish v1 -cliok "vcl.discard vclB"
-varnish v1 -cliok "vcl.discard vcl2"
-varnish v1 -cliok "vcl.discard vclA"
-varnish v1 -cliok "vcl.discard vcl1"
-
-
+varnish v1 -clierr 300 "vcl.discard vcl1 vcl2 vcl3 vcl4 vcl5 vcl6 vcl7"
+varnish v1 -cliok "vcl.discard vcl1 vcl2 vcl3 vcl4 vcl5 vcl6 vcl7 vclA vclB"
diff --git a/include/tbl/cli_cmds.h b/include/tbl/cli_cmds.h
index 3e737c641..6fef4be1f 100644
--- a/include/tbl/cli_cmds.h
+++ b/include/tbl/cli_cmds.h
@@ -99,10 +99,12 @@ CLI_CMD(VCL_STATE,
 
 CLI_CMD(VCL_DISCARD,
 	"vcl.discard",
-	"vcl.discard <configname|label>",
-	"Unload the named configuration (when possible).",
-	"",
-	1, 1
+	"vcl.discard <configname|label>...", /* XXX: allow globs */
+	"Unload the named configurations (if possible).",
+	" If more than one named configuration is specified the command"
+	" is equivalent to individual commands in the right order with"
+	" respect to configurations dependencies.",
+	1, -1
 )
 
 CLI_CMD(VCL_LIST,


More information about the varnish-commit mailing list