[master] 3199e69 Introduce `$ABI [strict|vrt]` for VMOD descriptors
Dridi Boukelmoune
dridi.boukelmoune at gmail.com
Mon Aug 14 14:38:10 CEST 2017
commit 3199e694965217cfa5415b0a2cb3d4770f612b23
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date: Wed May 17 10:18:27 2017 +0200
Introduce `$ABI [strict|vrt]` for VMOD descriptors
When versioning appeared in the VRT API, the goal was to allow loose
ABI compliance on loaded VMODs based on the major/minor revision against
which it was built. Strict checking was performed if Varnish was built
from the master branch in the VCC code, but omitted by the child.
This however has two flaws:
1) Release management might go wrong like it happened in 5.1.2 that got
released from the master branch.
2) This doesn't solve the original problem that some VMODs might rely
on supported symbols encompassed by the VRT major/minor while others
may choose to integrate deeper with Varnish and lose guarantees.
This patch retires the `VCS_Branch` macro that is no longer needed and
provides a new `$ABI` stanza that defaults to strict when omitted. To
help discovery, in-tree modules advertise a strict match.
To indicate that a VMOD needs the exact Varnish build to be loaded,
the VMOD's metadata contains 0.0 for the VRT major/minor revision.
In addition, both the VCC and child now perform the full ABI compliance
check, picking the strict or vrt option depending on the VMOD's metadata.
Closes #2330
diff --git a/bin/varnishd/cache/cache_vrt_vmod.c b/bin/varnishd/cache/cache_vrt_vmod.c
index 65e7074..719df9f 100644
--- a/bin/varnishd/cache/cache_vrt_vmod.c
+++ b/bin/varnishd/cache/cache_vrt_vmod.c
@@ -38,6 +38,7 @@
#include <stdlib.h>
#include "vcli_serve.h"
+#include "vmod_abi.h"
#include "vrt.h"
/*--------------------------------------------------------------------
@@ -65,6 +66,16 @@ struct vmod {
static VTAILQ_HEAD(,vmod) vmods = VTAILQ_HEAD_INITIALIZER(vmods);
+static unsigned
+vmod_abi_mismatch(const struct vmod_data *d)
+{
+
+ if (d->vrt_major == 0 && d->vrt_minor == 0)
+ return (d->abi == NULL || strcmp(d->abi, VMOD_ABI_Version));
+
+ return (d->vrt_major != VRT_MAJOR_VERSION ||
+ d->vrt_minor > VRT_MINOR_VERSION);
+}
int
VRT_Vmod_Init(VRT_CTX, struct vmod **hdl, void *ptr, int len, const char *nm,
@@ -112,15 +123,13 @@ VRT_Vmod_Init(VRT_CTX, struct vmod **hdl, void *ptr, int len, const char *nm,
FREE_OBJ(v);
return (1);
}
- if (d->vrt_major != VRT_MAJOR_VERSION ||
- d->vrt_minor > VRT_MINOR_VERSION ||
+ if (vmod_abi_mismatch(d) ||
d->name == NULL ||
strcmp(d->name, nm) ||
d->func == NULL ||
d->func_len <= 0 ||
d->proto == NULL ||
- d->spec == NULL ||
- d->abi == NULL) {
+ d->spec == NULL) {
VSB_printf(ctx->msg,
"Loading VMOD %s from %s:\n", nm, path);
VSB_printf(ctx->msg, "VMOD data is mangled.\n");
diff --git a/config.phk b/config.phk
index 7356757..b54257a 100644
--- a/config.phk
+++ b/config.phk
@@ -107,25 +107,23 @@ fi
VCSF=include/vcs_version.h
VMAV=include/vmod_abi.h
+V=NOGIT
if [ -d ./.git ] ; then
V=`git show -s --pretty=format:%h`
- B=`git rev-parse --abbrev-ref HEAD`
-else
- V="NOGIT"
- B="NOGIT"
fi
-(
-echo "/* $V */"
-echo "/*"
-echo " * NB: This file is machine generated, DO NOT EDIT!"
-echo " *"
-echo " * make(1) updates this when necessary"
-echo " *"
-echo " */"
-echo "#define VCS_Version \"$V\""
-echo "#define VCS_Branch \"$B\""
-) > ${VCSF}_
+
+cat > ${VCSF}_ <<EOF
+/* $V */
+/*
+ * NB: This file is machine generated, DO NOT EDIT!
+ *
+ * make(1) updates this when necessary
+ *
+ */
+#define VCS_Version "$V"
+EOF
+
if [ ! -f ${VCSF} ] ; then
mv ${VCSF}_ ${VCSF}
rm -f ${VMAV}
diff --git a/lib/libvcc/generate.py b/lib/libvcc/generate.py
index ed31ae9..7365d98 100755
--- a/lib/libvcc/generate.py
+++ b/lib/libvcc/generate.py
@@ -1417,7 +1417,6 @@ if i != "/* " + v + " */":
fo = open(vcsfn, "w")
file_header(fo)
fo.write('#define VCS_Version "%s"\n' % v)
- fo.write('#define VCS_Branch "%s"\n' % b)
fo.close()
for i in open(os.path.join(buildroot, "Makefile")):
diff --git a/lib/libvcc/vcc_vmod.c b/lib/libvcc/vcc_vmod.c
index d46e496..06ca562 100644
--- a/lib/libvcc/vcc_vmod.c
+++ b/lib/libvcc/vcc_vmod.c
@@ -155,7 +155,7 @@ vcc_ParseImport(struct vcc *tl)
vcc_ErrWhere(tl, mod);
return;
}
- if (strcmp(VCS_Branch, "master") == 0 &&
+ if (vmd->vrt_major == 0 && vmd->vrt_minor == 0 &&
strcmp(vmd->abi, VMOD_ABI_Version) != 0) {
VSB_printf(tl->sb, "Incompatible VMOD %.*s\n", PF(mod));
VSB_printf(tl->sb, "\tFile name: %s\n", fnp);
@@ -164,8 +164,8 @@ vcc_ParseImport(struct vcc *tl)
vcc_ErrWhere(tl, mod);
return;
}
- if (vmd->vrt_major != VRT_MAJOR_VERSION ||
- vmd->vrt_minor > VRT_MINOR_VERSION) {
+ if (vmd->vrt_major != 0 && (vmd->vrt_major != VRT_MAJOR_VERSION ||
+ vmd->vrt_minor > VRT_MINOR_VERSION)) {
VSB_printf(tl->sb, "Incompatible VMOD %.*s\n", PF(mod));
VSB_printf(tl->sb, "\tFile name: %s\n", fnp);
VSB_printf(tl->sb, "\tVMOD version %u.%u\n",
diff --git a/lib/libvcc/vmodtool.py b/lib/libvcc/vmodtool.py
index 90b888e..ab78cff 100755
--- a/lib/libvcc/vmodtool.py
+++ b/lib/libvcc/vmodtool.py
@@ -44,6 +44,7 @@ import unittest
import random
rstfmt = False
+strict_abi = True
ctypes = {
'ACL': "VCL_ACL",
@@ -434,6 +435,13 @@ class s_module(stanza):
fo.write("* :ref:`%s`\n" % i[1])
fo.write("\n")
+class s_abi(stanza):
+ def parse(self):
+ if self.line[1] not in ('strict', 'vrt'):
+ err("Valid ABI types are 'strict' or 'vrt', got '%s'\n" %
+ self.line[1])
+ strict_abi = self.line[1] == 'strict'
+
class s_event(stanza):
def parse(self):
self.event_func = self.line[1]
@@ -651,6 +659,8 @@ class vcc(object):
err("$Module must be first stanze")
if c[0] == "Module":
s_module(c, b[1:], self)
+ elif c[0] == "ABI":
+ s_abi(c, b[1:], self)
elif c[0] == "Event":
s_event(c, b[1:], self)
elif c[0] == "Function":
@@ -734,8 +744,12 @@ class vcc(object):
fo.write("\n/*lint -esym(%d, Vmod_%s_Data) */\n" % (i, self.modname))
fo.write("const struct vmod_data Vmod_%s_Data = {\n" %
self.modname)
- fo.write("\t.vrt_major =\tVRT_MAJOR_VERSION,\n")
- fo.write("\t.vrt_minor =\tVRT_MINOR_VERSION,\n")
+ if strict_abi:
+ fo.write("\t.vrt_major =\t0,\n")
+ fo.write("\t.vrt_minor =\t0,\n")
+ else:
+ fo.write("\t.vrt_major =\tVRT_MAJOR_VERSION,\n")
+ fo.write("\t.vrt_minor =\tVRT_MINOR_VERSION,\n")
fo.write('\t.name =\t\t"%s",\n' % self.modname)
fo.write('\t.func =\t\t&Vmod_Func,\n')
fo.write('\t.func_len =\tsizeof(Vmod_Func),\n')
diff --git a/lib/libvmod_debug/vmod.vcc b/lib/libvmod_debug/vmod.vcc
index 9323488..1b7889d 100644
--- a/lib/libvmod_debug/vmod.vcc
+++ b/lib/libvmod_debug/vmod.vcc
@@ -26,6 +26,7 @@
# SUCH DAMAGE.
$Module debug 3 Development, test and debug
+$ABI strict
DESCRIPTION
===========
diff --git a/lib/libvmod_directors/vmod.vcc b/lib/libvmod_directors/vmod.vcc
index cce8acb..9282ba3 100644
--- a/lib/libvmod_directors/vmod.vcc
+++ b/lib/libvmod_directors/vmod.vcc
@@ -33,6 +33,7 @@
# SUCH DAMAGE.
$Module directors 3 Varnish Directors Module
+$ABI strict
DESCRIPTION
===========
diff --git a/lib/libvmod_std/vmod.vcc b/lib/libvmod_std/vmod.vcc
index 0923fb7..c98eb26 100644
--- a/lib/libvmod_std/vmod.vcc
+++ b/lib/libvmod_std/vmod.vcc
@@ -26,6 +26,7 @@
# SUCH DAMAGE.
$Module std 3 Varnish Standard Module
+$ABI strict
DESCRIPTION
===========
More information about the varnish-commit
mailing list