[master] 32574ab46 vsm: Pass VARNISH_DEFAULT_N to VSM_Arg()

Dridi Boukelmoune dridi at varni.sh
Sun May 26 18:12:57 UTC 2024


On Sun, May 26, 2024 at 11:11 AM Nils Goroll <nils.goroll at uplex.de> wrote:
>
> I still don't get it.
>
> 1d7cdadda changed this in, I think, a wrong way, and now we add another wrong
> place. The default should be set for vut->n_arg. The default should be set for
> wd->wdname in VSM_Init(). What is the problem with that?

In 8f8d26c0c1fec91aec9a7f79380c93eb4d266b7e you introduced a fallback
to the environment variable in 4 locations:

- varnishadm
- varnishd
- VSM_New in libvarnishapi
- VUT_Init in libvarnishapi

By the way, thank you for your efforts on the docs aspect of this
change. You may also want to amend the "Invalid instance name" error
message to say "Invalid working directory" instead.

To me there should be only 2 locations needed:

- varnishd where we produce working directories
- libvarnishapi where we consume them

Neither the VUT API nor non-VUT programs like varnishadm should
*duplicate* this logic, they both ultimately converge to attaching to
a VSM before doing what they need to do (for the most part to find
what they need to operate). This is done by the VIN code, not exposed
by libvarnishapi.

In 1d7cdaddab67d156f6a99b2d2a1089b3a1913275 I placed the new fallback
next to the existing fallback that exists in VSM_Attach (not in
VSM_New) and I indeed got it slightly wrong and this is partly because
I lazily reused your initial REPLACE logic, partly because I didn't
pay enough attention. So eventually I realized that I wasn't washing
VARNISH_DEFAULT_N through VSM_Arg like the ultimate empty string
fallback.

And if you are now thinking the rationale should have been expanded in
the terse commit message, you are correct.

Dridi


More information about the varnish-commit mailing list