from flexelint review

 Conflicts:
	lib/libvmod_directors/shard_dir.h

We implement weights by scaling the number of replicas of each backend.
The replicas parameter of .reconfigure() remains a minimum.

For existing vtcs, the Debug hashcircle output has been compared
before/after this change to ensure that behaviour is exactly equivalent.

For for wighted backends, it has been checked that the number of
instances per host on the hashcircle matches the expectation.

Also refactor and clean up some of the code:

- consistently make the number of ring points a uint32_t
- some constification

Ref #3276

 Conflicts:
	lib/libvmod_directors/shard_cfg.c
	lib/libvmod_directors/shard_dir.h
	lib/libvmod_directors/vmod.vcc

 Conflicts:
	lib/libvmod_directors/shard_dir.h
	lib/libvmod_directors/vmod_shard.c

dd28fc73Co-authored-by: Dridi Boukelmoune <dridi.boukelmoune@gmail.com>

Thank you, @Dridi

- yes, allocations outside the lock were motivated by minimizing the
  critical section, but n_backend could actually change, so this was
  wrong. As we use an RW lock, doing more work under it should only have
  marginal impact.

- n_backend == 0 is probably best handled as a special case

With that, (almost) only libvgz carries goto statements from zlib.

This isn't changing any of the previous semantics, in particular the
AN(be) assertion from the "ok:" jump is honored by all code paths
formerly leading to it.

Previously, the bitmap was allocated on the stack prior to the magic
check of shardd, which is now fixed at the expense of a potential code
style violation. But more importantly, we currently read the number of
backends prior to acquiring the read lock, but there is no evidence
that this was done on purpose and not overlooked, besides moving a
possibly expensive state initialization outside of the critical
section.

If that was on purpose, please document it.

ebbbad96Co-authored-by: Poul-Henning Kamp <phk@FreeBSD.org>

This reverts commit 0e32f166.

 Conflicts:
	bin/varnishd/http2/cache_http2_deliver.c

This came from the H2 backport c3902d57
Fixes #3382

For VCL_TIME values that would convert to a year element that can not fit
in an int, gmtime_r would fail, and VTIM_format() would use random stack
values when picking weekday and month strings.

This patch changes VTIM_format to return "" when gmtime_r reports
failures. This way the API is not changed. Callers can test for empty
string to catch the failure if needed.

VRT_TIME_string is patched to catch the VTIM_format error, and return NULL
on failure.

Fixes: #3308

since d4b6228e the busyobj might also
gain one, so depending on who deref's first, there may be one or zero
references left.

With this patch fetch threads will for completely cached request bodies
keep a reference to it for the entire duration of the fetch. This extends
the retry window of backend requests with request body beyond the
BOS_REQ_DONE point.

Patch by: Poul-Henning Kamp

Currently we allow fetch retries with body even after we have released the
request that initiated the fetch, and the request body with it. The
attached test case demonstrates this, where s2 on the retry attempt gets
stuck waiting for 3 bytes of body data that is never sent.

Fix this by keeping track of what the initial request body status was, and
failing the retry attempt if the request was already released
(BOS_REQ_DONE) and the request body was not cached.

Conflicts:
    bin/varnishd/cache/cache.h

This reverts commit b9050923.

 Conflicts:
	bin/varnishd/cache/cache_esi_fetch.c
	bin/varnishd/cache/cache_fetch.c

Previously, we only checked `v1l->deadline` (which gets initialized
from the `send_timeout` session attribute or parameter) only for short
writes, so for successful "dripping" http1 writes (streaming from a
backend busy object with small chunks), we did not respect the
timeout.

This patch restructures `V1L_Flush()` to always check the deadline
before a write by turning a `while() { ... }` into a `do { ... }
while` with the same continuation criteria: `while (i != v1l->liov)`
is turned into `if (i == v1l->liov) break;` and `while (i > 0 || errno
== EWOULDBLOCK)` is kept to retry short writes.

This also reduces the `writev()` call sites to one.

Fixes #3189

 Conflicts:
	bin/varnishd/http1/cache_http1_line.c
	bin/varnishtest/tests/s00011.vtc

When retrying after a rollback, we might have unread bytes of the
backend response body. As we need to finalize the director for rollback,
all we can do in this case is to close the connection.

Spotted by reza, who also provided the test case, thank you.

Closes #3353

 Conflicts:
        bin/varnishd/cache/cache_fetch.c

 Conflicts:
	bin/varnishd/cache/cache_fetch.c
	bin/varnishd/cache/cache_req.c

I had previously overlooked that there exists a more complete regression
test by him in #3010

Integrate that vtc, polished and modified to account for the backend 503
for no restart after rollback, into m17, making r3009 obsolete again

Fixes #3083

to get there, also centralize req->resp setup for deliver and synth:

No semantic changes other than some reordering, which also fixes an odd
log line ordering as shown by the change to c00018.vtc

 Conflicts:
	bin/varnishd/cache/cache_req_fsm.c

by properly cleaning up the busyobj

Also move the relevant code from cache_vrt.c to cache_fetch.c

As we fini the director during cleanup, we now also need to handle the
backend connection gone missing in vbf_stp_fetch(). The hypothetical
alternative would be to not fini the director, but I believe this is not
safe in case it also used some workspace.

Fixes #3009

 Conflicts:
	bin/varnishd/cache/cache_fetch.c
	bin/varnishd/cache/cache_varnishd.h
	bin/varnishd/cache/cache_vrt.c

imples the following changes:

* VDI_Finish() is now always conditional on bo->director_state !=
  DIR_S_NULL, making it idempotent

* introduces additional calls to VFP_Close() from startfetch and
  for the filter_list / VCL_StackVFP error in vbf_stp_fetch(),
  but VFP_Close() is idempotent.

* adds VFP_Close() for VFP_Open() failure in vbf_stp_fetch() which
  I think is actually missing (for the case that some VFPs could
  get opened before the open failure)

* calls VDI_Finish() earlier in vbf_stp_fetchend: I checked the
  code and can not see any issue with this.

motivated by #3009

 Conflicts:
	bin/varnishd/cache/cache_fetch.c

required for #3009

VGZ_NewGzip will either assert or succeed.

Turn assertion into VFP error

The vtc is based upon r02645.vtc and reliably reproduces the panic
without the patch by sweeping through possible amounts of free workspace
ranging from 4 to 400 bytes.

Fixes #3253

This reverts commit 79536662.

It shouldn't be exposed in libvarnishapi the way it is in this change
because it creates two diverging symbol versions (for systems with
linker support for symbol versioning) without changing the soname,
which of course we want to avoid in the first place in a stable branch.

If the requirement comes up we can revisit this topic and ensure both
binary stability and compatility (upgrade path).

Refs #3268

Conflicts:
	doc/sphinx/reference/vcl_var.rst