[PATCH 5/5] Honor the persisted GONE flag on reload of bans, and keep the duplicate counters consistent on reloads.

[Martin Blix Grydeland]

Add a test case for this.
---
 bin/varnishd/cache/cache_ban.c   |   14 +++++-----
 bin/varnishtest/tests/p00010.vtc |   56 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 7 deletions(-)
 create mode 100644 bin/varnishtest/tests/p00010.vtc

diff --git a/bin/varnishd/cache/cache_ban.c b/bin/varnishd/cache/cache_ban.c
index a06fd98..5d4cef0 100644
--- a/bin/varnishd/cache/cache_ban.c
+++ b/bin/varnishd/cache/cache_ban.c
@@ -636,7 +636,7 @@ static void
 ban_reload(const uint8_t *ban, unsigned len)
 {
 	struct ban *b, *b2;
-	int gone = 0;
+	int dup = 0;
 	double t0, t1, t2 = 9e99;
 
 	ASSERT_CLI();
@@ -653,11 +653,8 @@ ban_reload(const uint8_t *ban, unsigned len)
 			return;
 		if (t1 < t0)
 			break;
-		if (ban_equal(b->spec, ban)) {
-			gone |= BAN_F_GONE;
-			VSC_C_main->bans_dups++;
-			VSC_C_main->bans_gone++;
-		}
+		if (ban_equal(b->spec, ban))
+			dup = 1;
 	}
 
 	VSC_C_main->bans++;
@@ -668,11 +665,14 @@ ban_reload(const uint8_t *ban, unsigned len)
 	b2->spec = malloc(len);
 	AN(b2->spec);
 	memcpy(b2->spec, ban, len);
-	b2->flags |= gone;
 	if (ban[BANS_FLAGS] & BANS_FLAG_REQ) {
 		VSC_C_main->bans_req++;
 		b2->flags |= BAN_F_REQ;
 	}
+	if (dup)
+		VSC_C_main->bans_dups++;
+	if (dup || ban[BANS_FLAGS] & BANS_FLAG_GONE)
+		ban_mark_gone(b2);
 	if (b == NULL)
 		VTAILQ_INSERT_TAIL(&ban_head, b2, list);
 	else
diff --git a/bin/varnishtest/tests/p00010.vtc b/bin/varnishtest/tests/p00010.vtc
new file mode 100644
index 0000000..36196d7
--- /dev/null
+++ b/bin/varnishtest/tests/p00010.vtc
@@ -0,0 +1,56 @@
+varnishtest "Check that reloaded bans with gone flag are really gone on restart"
+
+shell "rm -f ${tmpdir}/_.per[12]"
+
+server s1 {
+	rxreq
+	txresp -hdr "x-foo: foo"
+
+	accept
+	rxreq
+	txresp -hdr "x-foo: bar"
+} -start
+
+varnish v1 \
+	-arg "-pfeature=+wait_silo" \
+	-arg "-pban_lurker_sleep=0" \
+	-storage "-sper1=persistent,${tmpdir}/_.per1,10m -sper2=persistent,${tmpdir}/_.per2,10m" \
+	-vcl+backend {
+	}
+varnish v1 -start
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.http.x-foo == "foo"
+} -run
+
+varnish v1 -cliok "ban req.url == /test"
+varnish v1 -cliok "ban req.url == /test"
+varnish v1 -cliok "ban.list"
+
+# Expect ban_magic plus the 2 we added
+varnish v1 -expect bans == 3
+
+# Expect 1 of the 2 added to be marked dup
+varnish v1 -expect bans_dups == 1
+
+# Expect ban_magic plus our 1 dup to be marked gone
+varnish v1 -expect bans_gone == 2
+
+# Restart 
+varnish v1 -stop
+varnish v1 -start
+varnish v1 -cliok "ban.list"
+
+# Check that our object is still there
+client c1 {
+	txreq
+	rxresp
+	expect resp.http.x-foo == "foo"
+} -run
+
+# Expect our duplicate
+varnish v1 -expect bans_dups == 1
+# One more than before restart due to the new ban_magic
+varnish v1 -expect bans_gone == 3
-- 
1.7.10.4