[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