[mike@mwxm4]

One dialect: -j, and nothing older than v2.5.0 is talked to.

The text road is gone. It said the same things in a sentence and exited 0 while
refusing, and keeping it meant keeping a second way to be wrong for the sake of
versions nobody is running.

An older helper is not silently ignored. "There is none" and "the one you have
is too old" send somebody to two different places, so the helper carries why it
cannot be used and both the refusal and `gvm config` say it:

    /usr/local/bin/dns is 2.4.4, and gvm wants 2.5.0 or newer — 'dns --update' fetches it

A version that cannot be read at all is a third reason, said as itself: gvm
would rather say so than talk to something whose answers it cannot predict.

And the version check found a bug in itself while being tested. The line -v
prints carries two versions —

    dns - infoblox helper (v2.5.0 (1282), toolbox v0.5.0, mwx'2026)

— and taking whichever came first read the toolbox's 0.5.0 the moment the
tool's own could not be read. Which is exactly the quiet wrong answer the check
exists to prevent. It is anchored on the bracket now, and the trap is a test
case of its own.

Checked read-only against the real v2.5.0: read as 2.5.0, usable, and a host
lookup coming back with its address.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

README.md

@@ -444,6 +444,16 @@ nothing but `y`: Enter finishes a name, it never takes a snapshot. Afterwards th
 name is on the status line, and a sheet that is open jumps to its snapshot
 section so the new one is there to see.
 
+Snapshots are taken **without memory and without quiescing**, and both are
+deliberate. Memory would keep the running machine's RAM too, so that a rollback
+came back mid-flight — at the price of writing all of it to the datastore every
+time, and of a rollback that restores a process tree along with the disks. What
+these are for is the moment before a patch, where coming back to a machine that
+*boots* is the point. Leaving quiescing off makes the disk state
+crash-consistent — what a machine finds after the plug is pulled, which a
+journalling filesystem handles and a database may not — and means a snapshot
+neither depends on VMware Tools running nor stops when they are not.
+
 ### Making a machine from a template
 
 `p` in a template's action menu, `gvm new` on the command line. It is the one
@@ -580,14 +590,27 @@ writes down:
 
 Three more things about it are deliberate:
 
-* **What it says is believed before how it exits.** `dns -s` on a name that is
-  not there prints `ERROR: host '...' not found` and exits 0 all the same, so a
-  refusal is read out of the output. gvm's first version read the exit status,
-  which took every refusal for an agreement and — the other way about — made a
-  free name look like an answer nobody could parse. An appliance that cannot be
-  reached is an error and not a free name: concluding "nobody has this name"
-  from a server that is down is the last thing to do before asking it for an
-  address.
+* **It is asked with `-j`, and nothing older than v2.5.0 is asked at all.**
+  From that version the helper answers in an envelope: `ok`, the reason in an
+  `error` field of its own, the record — and an exit status that agrees with it.
+
+      {"ok":true, "action":"showhost","name":"v308.fhi.mpg.de","record":{...}}
+      {"ok":false,"action":"showhost","error":"host '...' not found"}
+
+  What came before said the same things in a sentence and **exited 0 while
+  refusing**, which is what an earlier version of this got wrong by believing
+  exit statuses. Rather than keep reading both, gvm asks the helper its version
+  once and uses it only from 2.5.0 up. An older one is not silently ignored —
+  "there is none" and "the one you have is too old" send somebody to two
+  different places, so it says which:
+
+      /usr/local/bin/dns is 2.4.4, and gvm wants 2.5.0 or newer — 'dns --update' fetches it
+
+  `gvm config` says the same thing when nothing is being deployed.
+
+  An appliance that cannot be reached is an error and not a free name:
+  concluding "nobody has this name" from a server that is down is the last
+  thing to do before asking it for an address.
 * **The record is read back**, not taken from the sentence the helper prints.
   `OK: host 'web05.fhi.mpg.de' added with IP '141.14.140.182'` is written for a
   person and gets reworded between versions; the record is the appliance's own
@@ -1100,9 +1123,11 @@ wrong quietly:
   line they share
 * that a name already in the appliance is refused before anything is added to
   it, that an appliance which cannot be reached is not mistaken for a free
-  name, and that the helper's habits are the real ones — the stand-in says what
-  is wrong in its output and exits 0 while doing so, which is what the first
-  version of this got wrong
+  name, and that a helper older than 2.5.0 is refused by name and version
+  rather than talked to — against a stand-in, since a suite that called the
+  real one would be editing the institute's network every time it ran
+* that the version is read off the tool's own, and not off the toolbox version
+  in the same line — which is the wrong answer that check exists to prevent
 * that an address fetched from the site's tool is read back from the record
   rather than from its printed sentence, that the tool's own words come back
   when it refuses, that a fetched address is given back when the deployment

actions.go

@@ -722,7 +722,7 @@ func (b *browser) deployAddress(r vmRow, src deploySource, t deployTarget, name
 		leave = "empty to leave it to " + opts.spec
 	}
 	ask := SF("address for %s, %s: ", name, leave)
-	if opts.dns != "" {
+	if opts.dns.there() {
 		ask = SF("address for %s, %q for one from dns, %s: ", name, autoIP, leave)
 	}
 

browse.go

@@ -759,9 +759,9 @@ type browser struct {
 	events   []eventLine
 	eventsOf string // the id of the machine they are of
 
-	ssh  string // the command `h` runs, from ~/.gvmrc; empty means plain ssh
-	site site   // what a machine made from a template is told about the network
-	dns  string // the site's address helper, or "" where this machine has none
+	ssh  string    // the command `h` runs, from ~/.gvmrc; empty means plain ssh
+	site site      // what a machine made from a template is told about the network
+	dns  dnsHelper // the site's address helper, where this machine has one
 
 	// Live mode (live.go): the list re-reading itself on a timer rather than on
 	// a keystroke. hist is what the trend column is drawn from, kept here

config.go

@@ -685,7 +685,8 @@ func writeConfigTemplate(path string) {
 	b.WriteString("# timezone  = Europe/Berlin\n")
 	b.WriteString("# An address can also be fetched rather than typed, where the site has a\n")
 	b.WriteString("# tool for it: 'gvm new ... --ip auto'. Unset, gvm looks for 'dns' on the\n")
-	b.WriteString("# path and offers the option only where it finds it.\n")
+	b.WriteString("# path, and uses it from v2.5.0 up — older ones are said to be too old\n")
+	b.WriteString("# rather than read.\n")
 	b.WriteString("# dnstool   = /usr/local/bin/dns\n\n")
 	b.WriteString("# --- how the sheet's 'h' logs in to a guest ---\n")
 	b.WriteString("# %h is where the machine's name or address goes; appended when it is\n")

deploy.go

@@ -52,11 +52,11 @@ type deployOpts struct {
 
 	// What the guest is told about itself, and where that comes from.
 	how      custom
-	spec     string // which one, when how is customSpec
-	hostname string // empty means the machine's own name
-	ip       string // empty leaves the adapter on DHCP; autoIP fetches one
-	st       site   // the site's own answers, from the configuration
-	dns      string // the site's address helper, or "" where there is none
+	spec     string    // which one, when how is customSpec
+	hostname string    // empty means the machine's own name
+	ip       string    // empty leaves the adapter on DHCP; autoIP fetches one
+	st       site      // the site's own answers, from the configuration
+	dns      dnsHelper // the site's address helper, where there is one
 }
 
 // autoAddress reports whether the address is to be fetched rather than typed.
@@ -346,9 +346,10 @@ func fetchAddress(opts deployOpts, name string) (deployOpts, dnsHost, func(), er
 	if !opts.autoAddress() {
 		return opts, dnsHost{}, nothing, nil
 	}
-	if opts.dns == "" {
-		return opts, dnsHost{}, nothing, errf("there is no address tool on this machine — put one "+
-			"on the path as 'dns', name it as dnstool in %s, or give --ip an address", configFile())
+	if !opts.dns.there() {
+		// Why, rather than that: a tool that is too old and no tool at all are
+		// two different things to go and do something about.
+		return opts, dnsHost{}, nothing, errf("%s — or give --ip an address", opts.dns.why())
 	}
 
 	host := strings.TrimSpace(opts.hostname)
@@ -745,7 +746,7 @@ func deployCLI(vc VCenter, template, name string, opts deployOpts, yes bool) err
 		facts = append(facts, [2]string{"customise", how}, [2]string{"hostname", hostNameOf(custom)})
 		address := addressOf(custom)
 		if rec.Name != "" {
-			address += SF("  (%s, from %s)", rec.Name, opts.dns)
+			address += SF("  (%s, from %s)", rec.Name, opts.dns.path)
 		}
 		facts = append(facts, [2]string{"address", address})
 		if opts.building() && opts.ip != "" && gatewayOffSubnet(opts.ip, opts.st) {

dns.go

@@ -23,6 +23,8 @@ import (
 	"context"
 	"encoding/json"
 	"os/exec"
+	"regexp"
+	"strconv"
 	"strings"
 	"time"
 )
@@ -38,23 +40,127 @@ const dnsWait = 30 * time.Second
 // different question.
 const autoIP = "auto"
 
-// dnsTool is the helper to use, or "" where this machine has none.
+// dnsHelper is the tool, where there is one gvm can use.
+//
+// Only v2.5.0 and up: from there it answers in JSON when asked with -j, with
+// the reason for a refusal in a field of its own and an exit status that agrees
+// with it. What came before said the same things in a sentence and exited 0
+// while refusing, which is a shape worth reading only as long as somebody is
+// still running one — and nobody here is.
+//
+// A tool that is too old is not the same as no tool at all, and why() keeps the
+// difference: "there is none" and "the one you have is too old to ask this of"
+// send somebody to two different places.
+type dnsHelper struct {
+	path    string // "" where none was found at all
+	version [3]int // what it says it is
+	usable  bool   // found, and new enough
+}
+
+func (h dnsHelper) there() bool { return h.usable }
+
+// dnsJSONFrom is the first version that answers in JSON, and so the first that
+// gvm will talk to.
+var dnsJSONFrom = [3]int{2, 5, 0}
+
+func versionString(v [3]int) string { return SF("%d.%d.%d", v[0], v[1], v[2]) }
+
+// why says what is in the way, for the refusal and for `gvm config`. Empty
+// where nothing is.
+func (h dnsHelper) why() string {
+	switch {
+	case h.usable:
+		return ""
+	case h.path == "":
+		return SF("there is no address tool on this machine — put one on the path as 'dns', "+
+			"or name it as dnstool in %s", configFile())
+	case h.version == [3]int{}:
+		return SF("%s does not say which version it is, and gvm wants %s or newer",
+			h.path, versionString(dnsJSONFrom))
+	}
+	return SF("%s is %s, and gvm wants %s or newer — 'dns --update' fetches it",
+		h.path, versionString(h.version), versionString(dnsJSONFrom))
+}
+
+// dnsTool is the helper to use, or one that says why it cannot be used.
 //
 // The configuration may name it outright, for a machine where it is not on the
 // path; otherwise it is looked up by name, which is how it is found on the
-// machines it is installed on.
-func dnsTool(configured string) string {
+// machines it is installed on. Either way it is then asked how old it is —
+// once, here, because a probe per call would be three of them for one
+// deployment.
+func dnsTool(configured string) dnsHelper {
+	path := ""
 	if c := strings.TrimSpace(configured); c != "" {
-		if path, err := exec.LookPath(c); err == nil {
-			return path
+		// Named but not there: nothing is offered, rather than quietly using a
+		// different tool of the same name from somewhere on the path.
+		if p, err := exec.LookPath(c); err == nil {
+			path = p
 		}
-		return "" // named but not there: the option is not offered, and says so
+	} else if p, err := exec.LookPath("dns"); err == nil {
+		path = p
 	}
-	path, err := exec.LookPath("dns")
+	if path == "" {
+		return dnsHelper{}
+	}
+
+	h := dnsHelper{path: path}
+	if v, ok := dnsAskVersion(path); ok {
+		h.version = v
+		h.usable = !olderThan(v, dnsJSONFrom)
+	}
+	return h
+}
+
+// dnsAskVersion asks the helper how old it is. A version that cannot be read is
+// not a version: gvm would rather say so than talk to something whose answers
+// it cannot predict.
+func dnsAskVersion(path string) ([3]int, bool) {
+	ctx, cancel := context.WithTimeout(context.Background(), dnsWait)
+	defer cancel()
+
+	out, err := exec.CommandContext(ctx, path, "-v").CombinedOutput()
 	if err != nil {
-		return ""
+		return [3]int{}, false
 	}
-	return path
+	return dnsVersion(string(out))
+}
+
+// dnsVersion picks the version out of what -v says, which is a sentence about
+// itself:
+//
+//	dns - infoblox helper (v2.5.0 (1282), toolbox v0.5.0, mwx'2026)
+//
+// Anchored on the bracket, because there are two versions in that line and only
+// the first is the tool's own. Taking whichever came first instead read the
+// toolbox's 0.5.0 the moment the tool's own could not be read — which is the
+// quiet wrong answer this whole check exists to avoid, and it turned up in a
+// test rather than in front of somebody.
+var dnsVersionRe = regexp.MustCompile(`\(v(\d+)\.(\d+)\.(\d+)`)
+
+func dnsVersion(said string) ([3]int, bool) {
+	m := dnsVersionRe.FindStringSubmatch(said)
+	if m == nil {
+		return [3]int{}, false
+	}
+	var v [3]int
+	for i := range v {
+		n, err := strconv.Atoi(m[i+1])
+		if err != nil {
+			return [3]int{}, false
+		}
+		v[i] = n
+	}
+	return v, true
+}
+
+func olderThan(v, than [3]int) bool {
+	for i := range v {
+		if v[i] != than[i] {
+			return v[i] < than[i]
+		}
+	}
+	return false
 }
 
 // dnsHost is the part of a host record gvm reads. The helper answers in the
@@ -86,44 +192,55 @@ func dnsName(host string) string {
 	return name
 }
 
-// dnsRun is one call to the helper. -y throughout: it asks before it changes
-// anything when it has a terminal, and gvm has already asked.
+// dnsAnswer is one reply: whether the helper did the thing, what it said if it
+// did not, and the record where there is one.
 //
-// The output is believed before the exit status, because that is how this tool
-// reports: `dns -s` on a name that is not there prints "ERROR: host '...' not
-// found" and exits 0 all the same. Reading only the status took every refusal
-// for an agreement — and, the other way round, made a name that was free look
-// like an answer nobody could parse.
-func dnsRun(tool string, args ...string) (string, error) {
+// It is the envelope v2.5.0 and up put every reply in, kept as gvm's own type
+// so that the three calls above it read the same four fields rather than each
+// one unwrapping a reply for itself:
+//
+//	{"ok":true, "action":"showhost","name":"v308.fhi.mpg.de","record":{...}}
+//	{"ok":false,"action":"showhost","error":"host '...' not found"}
+type dnsAnswer struct {
+	ok     bool
+	err    string          // the helper's own words, where it refused
+	record json.RawMessage // the host record, where the call has one
+}
+
+// notFound reports whether the refusal is the ordinary one: no such name. That
+// is an answer to "is this name free", not a failure to report.
+func (a dnsAnswer) notFound() bool {
+	return !a.ok && strings.Contains(strings.ToLower(a.err), "not found")
+}
+
+// dnsAsk is one call to the helper. -j because that is the only way gvm talks
+// to it, and -y throughout: it asks before it changes anything when it has a
+// terminal, and gvm has already asked.
+func dnsAsk(h dnsHelper, args ...string) (dnsAnswer, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), dnsWait)
 	defer cancel()
 
-	out, err := exec.CommandContext(ctx, tool, append(args, "-y")...).CombinedOutput()
+	argv := append(append([]string{}, args...), "-j", "-y")
+	out, err := exec.CommandContext(ctx, h.path, argv...).CombinedOutput()
 	text := strings.TrimSpace(string(out))
 
-	switch {
-	case dnsSaidError(text):
-		// The helper's own words, which say what is wrong far better than any
-		// status: "host already exists", "no free address in the network".
-		return text, errf("%s", firstLine(text))
-	case err != nil:
-		if text == "" {
-			return "", errf("%s %s: %w", tool, strings.Join(args, " "), err)
-		}
-		return text, errf("%s", firstLine(text))
+	var env struct {
+		OK     bool            `json:"ok"`
+		Error  string          `json:"error"`
+		Record json.RawMessage `json:"record"`
+	}
+	if jerr := json.Unmarshal([]byte(text), &env); jerr != nil {
+		if err != nil && text == "" {
+			return dnsAnswer{}, errf("%s: %w", h.path, err)
+		}
+		return dnsAnswer{}, errf("%s answered something that is not a reply: %s", h.path, firstLine(text))
 	}
-	return text, nil
-}
 
-// dnsSaidError and dnsNotFound read the two things the helper says about
-// itself. "not found" is an answer rather than a failure — it is what a free
-// name looks like — and everything else beginning with ERROR is a refusal.
-func dnsSaidError(out string) bool {
-	return strings.HasPrefix(strings.TrimSpace(out), "ERROR")
-}
-
-func dnsNotFound(out string) bool {
-	return dnsSaidError(out) && strings.Contains(out, "not found")
+	a := dnsAnswer{ok: env.OK, err: env.Error, record: env.Record}
+	if !a.ok && a.err == "" {
+		a.err = "it refused, without saying why"
+	}
+	return a, nil
 }
 
 // dnsShow reads a host record. A name that is not there comes back as an empty
@@ -133,18 +250,24 @@ func dnsNotFound(out string) bool {
 // Anything else that goes wrong is an error and stays one. Swallowing those
 // would turn an appliance nobody can reach into "the name is free", which is
 // the last thing to conclude before asking it for an address.
-func dnsShow(tool, host string) (dnsHost, error) {
-	out, err := dnsRun(tool, "-s", dnsName(host))
+func dnsShow(h dnsHelper, host string) (dnsHost, error) {
+	a, err := dnsAsk(h, "-s", dnsName(host))
 	if err != nil {
-		if dnsNotFound(out) {
-			return dnsHost{}, nil
-		}
 		return dnsHost{}, err
 	}
+	switch {
+	case a.notFound():
+		return dnsHost{}, nil
+	case !a.ok:
+		return dnsHost{}, errf("%s", a.err)
+	case len(a.record) == 0:
+		return dnsHost{}, errf("%s said nothing about %s", h.path, dnsName(host))
+	}
 
 	var rec dnsHost
-	if err := json.Unmarshal([]byte(out), &rec); err != nil {
-		return dnsHost{}, errf("%s answered something that is not a host record: %s", tool, firstLine(out))
+	if err := json.Unmarshal(a.record, &rec); err != nil {
+		return dnsHost{}, errf("%s answered something that is not a host record: %s",
+			h.path, firstLine(string(a.record)))
 	}
 	return rec, nil
 }
@@ -156,7 +279,7 @@ func dnsShow(tool, host string) (dnsHost, error) {
 // a sentence for a person, and a sentence is a thing that gets reworded between
 // versions; the record is the appliance's own answer to the same question. If
 // the two ever disagree, the record is what the machine will actually be given.
-func dnsAdd(tool, host string) (dnsHost, error) {
+func dnsAdd(h dnsHelper, host string) (dnsHost, error) {
 	name := dnsName(host)
 	if name == "" {
 		return dnsHost{}, errf("there is no name to ask for an address for")
@@ -168,7 +291,7 @@ func dnsAdd(tool, host string) (dnsHost, error) {
 	// second address on somebody else's host record, which is worse than
 	// either. It is also the same question gvm asks the vCenter about the
 	// machine's name (nameTaken), one answer short of the same answer.
-	if rec, err := dnsShow(tool, name); err != nil {
+	if rec, err := dnsShow(h, name); err != nil {
 		return dnsHost{}, err
 	} else if rec.Name != "" {
 		taken := rec.Name
@@ -179,17 +302,24 @@ func dnsAdd(tool, host string) (dnsHost, error) {
 			"or free that one with 'dns -d %s'", name, taken, name)
 	}
 
-	said, err := dnsRun(tool, "-a", name)
+	a, err := dnsAsk(h, "-a", name)
 	if err != nil {
 		return dnsHost{}, err
 	}
+	if !a.ok {
+		return dnsHost{}, errf("%s", a.err)
+	}
 
-	rec, err := dnsShow(tool, name)
+	// Read back rather than taken from what the reply carried. Both dialects
+	// say something about what they just made, and neither has to be trusted
+	// for it: the record is the appliance's own answer to the same question,
+	// asked after the fact, and it is what the machine will actually have.
+	rec, err := dnsShow(h, name)
 	if err != nil {
 		return dnsHost{}, err
 	}
 	if rec.address() == "" {
-		return dnsHost{}, errf("%s said %q, but the record cannot be read back", tool, firstLine(said))
+		return dnsHost{}, errf("%s made %s, but the record has no address in it", h.path, name)
 	}
 	return rec, nil
 }
@@ -198,9 +328,15 @@ func dnsAdd(tool, host string) (dnsHost, error) {
 // happen after all, so its failure is worth saying and not worth stopping for:
 // the machine was not made either way, and what is left behind is a record
 // somebody can delete by hand.
-func dnsRemove(tool, host string) error {
-	_, err := dnsRun(tool, "-d", dnsName(host))
-	return err
+func dnsRemove(h dnsHelper, host string) error {
+	a, err := dnsAsk(h, "-d", dnsName(host))
+	if err != nil {
+		return err
+	}
+	if !a.ok {
+		return errf("%s", a.err)
+	}
+	return nil
 }
 
 // firstLine keeps a message from a tool to one line, for a status line that has

dns_test.go

@@ -7,28 +7,33 @@ import (
 	"testing"
 )
 
-// fakeDNS writes a stand-in for the site's helper and returns it with the file
-// it writes down what it was asked in.
-//
-// Nothing in this file may touch the real one. `dns -a` takes an address out of
-// the site's Infoblox and `dns -d` gives one back, and a test suite that did
+// Nothing in this file may touch the real helper. `dns -a` takes an address out
+// of the site's Infoblox and `dns -d` gives one back, and a test suite that did
 // either would be editing the institute's network every time somebody ran it.
-// So the thing under test is everything around the call — what gvm asks for,
-// what it makes of the answer, and what it does when the answer is no.
+// So what is under test is everything around the call: what gvm asks for, what
+// it makes of the answer, and what it does when the answer is no.
 //
-// It keeps the two habits of the real tool that gvm has to live with, both
-// measured rather than assumed: it says what is wrong in its output, beginning
-// with ERROR, and it exits 0 while doing so — a name that is not there is
-// "ERROR: host '...' not found" and a successful run at the same time. A
-// stand-in that answered in exit statuses would have let gvm's first version
-// pass, which believed them.
-func fakeDNS(t *testing.T, body string) (tool, log string) {
+// The stand-in answers the way v2.5.0 does, measured rather than assumed: an
+// envelope with ok, the reason in an error field of its own, and exit 1 where
+// it refused.
+//
+//	{"ok":true, "action":"showhost","record":{...}}
+//	{"ok":false,"action":"showhost","error":"host ... not found"}
+func fakeDNS(t *testing.T, version, body string) (tool, log string) {
 	t.Helper()
 	dir := t.TempDir()
 	tool = filepath.Join(dir, "dns")
 	log = filepath.Join(dir, "asked")
 
-	script := "#!/bin/sh\necho \"$@\" >> " + log + "\n" + body + "\nexit 0\n"
+	script := `#!/bin/sh
+echo "$@" >> ` + log + `
+if [ "$1" = "-v" ]; then
+  echo "dns - infoblox helper (v` + version + ` (1282), toolbox v0.5.0, mwx'2026)"
+  exit 0
+fi
+` + body + `
+exit 0
+`
 	if err := os.WriteFile(tool, []byte(script), 0o755); err != nil {
 		t.Fatal(err)
 	}
@@ -36,49 +41,117 @@ func fakeDNS(t *testing.T, body string) (tool, log string) {
 }
 
 // fakeInfoblox is that stand-in with a memory: a name is not there until it has
-// been added, and is gone again once it has been deleted. Which is the whole
+// been added, and is gone again once it has been deleted — which is the whole
 // shape of what gvm does with it.
-func fakeInfoblox(t *testing.T) (tool, log string) {
+func fakeInfoblox(t *testing.T) (dnsHelper, string) {
 	t.Helper()
 	state := filepath.Join(t.TempDir(), "records")
-	return fakeDNS(t, `
+
+	tool, log := fakeDNS(t, "2.5.0", `
 state=`+state+`
+rec() { printf '{"name":"%s.fhi.mpg.de","ipv4addrs":[{"ipv4addr":"141.14.140.182"}]}' "$1"; }
+ok()  { printf '{"ok":true,"action":"%s","record":%s}\n' "$1" "$(rec $2)"; exit 0; }
+no()  { printf '{"ok":false,"action":"%s","error":"%s"}\n' "$1" "$2"; exit 1; }
+done_() { printf '{"ok":true,"action":"%s"}\n' "$1"; exit 0; }
+
 case "$1" in
-  -a) if grep -qx "$2" $state 2>/dev/null; then
-        echo "ERROR: host '$2.fhi.mpg.de' already exists"
-      else
-        echo "$2" >> $state
-        echo "OK: host '$2.fhi.mpg.de' added with IP '141.14.140.182'"
-      fi ;;
-  -s) if grep -qx "$2" $state 2>/dev/null; then
-        printf '{"name":"%s.fhi.mpg.de","ipv4addrs":[{"ipv4addr":"141.14.140.182"}]}\n' "$2"
-      else
-        echo "ERROR: host '$2.fhi.mpg.de' not found"
-      fi ;;
+  -s) if grep -qx "$2" $state 2>/dev/null; then ok showhost "$2"
+      else no showhost "host $2.fhi.mpg.de not found"; fi ;;
+  -a) if grep -qx "$2" $state 2>/dev/null; then no addhost "host $2.fhi.mpg.de already exists"
+      else echo "$2" >> $state; done_ addhost; fi ;;
   -d) grep -vx "$2" $state > $state.tmp 2>/dev/null
       mv $state.tmp $state 2>/dev/null
-      echo "OK: host '$2.fhi.mpg.de' deleted" ;;
+      done_ delhost ;;
 esac`)
+
+	return dnsTool(tool), log
 }
 
-// asked is what the helper was called with, one call per line.
+// asked is what the helper was called with, one call per line, without the
+// version probe — that one is gvm working out which dialect it is talking to,
+// not part of what any of these tests is about.
 func asked(t *testing.T, log string) []string {
 	t.Helper()
 	out, err := os.ReadFile(log)
 	if err != nil {
 		return nil
 	}
-	return strings.Split(strings.TrimSpace(string(out)), "\n")
+	var calls []string
+	for _, l := range strings.Split(strings.TrimSpace(string(out)), "\n") {
+		if l != "" && !strings.HasPrefix(l, "-v") {
+			calls = append(calls, l)
+		}
+	}
+	return calls
 }
 
-// The record is read back rather than taken from the sentence the helper prints
-// on its way past: "OK: host '...' added with IP '...'" is written for a person
-// and gets reworded between versions, while the record is the appliance's own
-// answer to the same question.
-func TestAnAddressIsReadBackFromTheRecord(t *testing.T) {
-	tool, log := fakeInfoblox(t)
+// Which versions gvm will talk to. Below 2.5.0 the helper answers in sentences
+// and exits 0 while refusing, and gvm no longer reads that — so it says what is
+// in the way rather than talking to it anyway.
+func TestOnlyANewEnoughHelperIsUsed(t *testing.T) {
+	for _, c := range []struct {
+		version string
+		usable  bool
+	}{
+		{"2.5.0", true},
+		{"2.5.1", true},
+		{"2.6.0", true},
+		{"3.0.0", true},
+		{"2.4.4", false},
+		{"1.9.9", false},
+	} {
+		tool, _ := fakeDNS(t, c.version, `echo '{"ok":true,"action":"noop"}'`)
+		h := dnsTool(tool)
+		if h.there() != c.usable {
+			t.Errorf("v%s: usable = %v, want %v", c.version, h.there(), c.usable)
+		}
+		if c.usable {
+			continue
+		}
+		// And says which of the two things is wrong: a tool that is too old is
+		// not the same as no tool, and they send somebody to different places.
+		why := h.why()
+		if !strings.Contains(why, c.version) || !strings.Contains(why, "2.5.0") {
+			t.Errorf("v%s: it says %q, which does not name both versions", c.version, why)
+		}
+		if !strings.Contains(why, "dns --update") {
+			t.Errorf("v%s: it does not say what to do about it: %q", c.version, why)
+		}
+	}
 
-	rec, err := dnsAdd(tool, "web05")
+	// There are two versions in that line and only the first is the tool's own:
+	// a helper whose own version cannot be read must not be taken for its
+	// toolbox's.
+	if v, ok := dnsVersion("dns - infoblox helper (v2.5.0 (1282), toolbox v0.5.0, mwx'2026)"); !ok ||
+		v != [3]int{2, 5, 0} {
+		t.Errorf("the tool's own version read as %v (%v)", v, ok)
+	}
+	if v, ok := dnsVersion("dns - infoblox helper (vnonsense (1282), toolbox v0.5.0, mwx'2026)"); ok {
+		t.Errorf("the toolbox's version was taken for the tool's: %v", v)
+	}
+
+	// A helper that will not say which version it is, and one that is not there
+	// at all: two more reasons, each said as itself.
+	tool, _ := fakeDNS(t, "no-version-here", `echo hello`)
+	h := dnsTool(tool)
+	if h.there() {
+		t.Error("a helper that does not say its version was used anyway")
+	}
+	if !strings.Contains(h.why(), "does not say which version") {
+		t.Errorf("it says %q", h.why())
+	}
+	if none := (dnsHelper{}); !strings.Contains(none.why(), "no address tool") {
+		t.Errorf("no tool at all says %q", none.why())
+	}
+}
+
+// The record is read back rather than taken from whatever the reply carried:
+// the helper says something about what it just made, and it does not have to be
+// trusted for it.
+func TestAnAddressIsReadBackFromTheRecord(t *testing.T) {
+	h, log := fakeInfoblox(t)
+
+	rec, err := dnsAdd(h, "web05")
 	if err != nil {
 		t.Fatalf("dnsAdd: %v", err)
 	}
@@ -90,47 +163,32 @@ func TestAnAddressIsReadBackFromTheRecord(t *testing.T) {
 	}
 
 	// Asked whether the name is free, then to add it, then what it made — and
-	// never without -y, which is what keeps it from stopping to ask a question
-	// nobody is there to answer.
+	// never without -j and -y: the one is the only dialect gvm reads, the other
+	// is what keeps it from stopping to ask a question nobody is there to
+	// answer.
 	calls := asked(t, log)
 	if len(calls) != 3 || !strings.HasPrefix(calls[0], "-s web05") ||
 		!strings.HasPrefix(calls[1], "-a web05") || !strings.HasPrefix(calls[2], "-s web05") {
-		t.Errorf("it was asked %v", calls)
+		t.Fatalf("it was asked %v", calls)
 	}
 	for _, c := range calls {
-		if !strings.Contains(c, "-y") {
-			t.Errorf("a call was made that could stop and ask: %q", c)
+		if !strings.Contains(c, "-y") || !strings.Contains(c, "-j") {
+			t.Errorf("a call went out without -j and -y: %q", c)
 		}
 	}
 }
 
-// A helper that refuses says why in its own words, which are better than any
-// gvm could put there: "host already exists", "no free address in the network".
-func TestTheHelpersOwnWordsComeBack(t *testing.T) {
-	tool, _ := fakeDNS(t, `echo "ERROR: no free address in the network 141.14.128.0/20"`)
-
-	_, err := dnsAdd(tool, "web05")
-	if err == nil {
-		t.Fatal("a helper that refused was taken for one that agreed")
-	}
-	if !strings.Contains(err.Error(), "no free address") {
-		t.Errorf("its words did not come back: %v", err)
-	}
-}
-
-// A name that is already in the appliance is refused before anything is added
-// to it — and the refusal says what is in the way and how to clear it. What
-// `dns -a` would do with it is either refuse in worse words, or hang a second
-// address on somebody else's host record.
+// A name already in the appliance is refused before anything is added to it,
+// with what is in the way and how to clear it.
 func TestANameThatIsTakenIsRefused(t *testing.T) {
-	tool, log := fakeInfoblox(t)
+	h, log := fakeInfoblox(t)
 
-	if _, err := dnsAdd(tool, "web05"); err != nil {
+	if _, err := dnsAdd(h, "web05"); err != nil {
 		t.Fatalf("the first one went wrong: %v", err)
 	}
 	before := len(asked(t, log))
 
-	_, err := dnsAdd(tool, "web05")
+	_, err := dnsAdd(h, "web05")
 	if err == nil {
 		t.Fatal("a name that is already there was taken again")
 	}
@@ -140,47 +198,58 @@ func TestANameThatIsTakenIsRefused(t *testing.T) {
 		}
 	}
 
-	// Refused before anything was asked of it beyond the question.
 	calls := asked(t, log)
 	if len(calls) != before+1 || !strings.HasPrefix(calls[before], "-s web05") {
 		t.Errorf("more than a look was taken: %v", calls[before:])
 	}
 }
 
-// A helper that cannot be reached is not a free name. Swallowing that would
-// turn an appliance that is down into "nobody has this name", which is the last
-// thing to conclude before asking it for an address.
-func TestAnApplianceThatIsDownIsNotAFreeName(t *testing.T) {
-	tool, _ := fakeDNS(t, `echo "ERROR: cannot reach infoblox.fhi.mpg.de"`)
+// The helper's own words come back when it refuses for any other reason.
+func TestTheHelpersOwnWordsComeBack(t *testing.T) {
+	tool, _ := fakeDNS(t, "2.5.0",
+		`echo '{"ok":false,"action":"addhost","error":"no free address in the network"}'; exit 1`)
 
-	if _, err := dnsShow(tool, "web05"); err == nil {
-		t.Error("an appliance that could not be reached reported a free name")
+	_, err := dnsAdd(dnsTool(tool), "web05")
+	if err == nil {
+		t.Fatal("a helper that refused was taken for one that agreed")
 	}
-	if _, err := dnsAdd(tool, "web05"); err == nil {
-		t.Error("an address was asked for over an appliance that could not be reached")
+	if !strings.Contains(err.Error(), "no free address") {
+		t.Errorf("its words did not come back: %v", err)
 	}
 }
 
-// A helper that agrees but leaves no record is not an answer either: a
-// deployment would then be given an empty address and the machine would come up
-// on nothing.
-func TestAnAddressThatCannotBeReadBackIsNoAddress(t *testing.T) {
-	tool, _ := fakeDNS(t, `case "$1" in
-  -a) echo "OK: host 'web05.fhi.mpg.de' added with IP '141.14.140.182'" ;;
-  -s) echo '{"name": "web05.fhi.mpg.de", "ipv4addrs": []}' ;;
-esac`)
+// An appliance that cannot be reached is not a free name. Concluding "nobody
+// has this name" from a server that is down is the last thing to do before
+// asking it for an address.
+func TestAnApplianceThatIsDownIsNotAFreeName(t *testing.T) {
+	tool, _ := fakeDNS(t, "2.5.0",
+		`echo '{"ok":false,"action":"showhost","error":"cannot reach infoblox"}'; exit 1`)
+	h := dnsTool(tool)
 
-	if _, err := dnsAdd(tool, "web05"); err == nil {
+	if _, err := dnsShow(h, "web05"); err == nil {
+		t.Error("an appliance that could not be reached reported a free name")
+	}
+	if _, err := dnsAdd(h, "web05"); err == nil {
+		t.Error("an address was asked for over an appliance that is down")
+	}
+}
+
+// A helper that agrees but leaves no record is not an answer either: the
+// deployment would be given an empty address and the machine would come up on
+// nothing.
+func TestAnAddressThatCannotBeReadBackIsNoAddress(t *testing.T) {
+	tool, _ := fakeDNS(t, "2.5.0", `case "$1" in
+  -s) echo '{"ok":true,"action":"showhost","record":{"name":"web05.fhi.mpg.de","ipv4addrs":[]}}' ;;
+  -a) echo '{"ok":true,"action":"addhost"}' ;;
+esac`)
+	if _, err := dnsAdd(dnsTool(tool), "web05"); err == nil {
 		t.Fatal("an empty record was taken for an address")
 	}
 
-	// And something that is not a record at all.
-	tool, _ = fakeDNS(t, `case "$1" in
-  -a) echo "OK" ;;
-  -s) echo 'not json at all' ;;
-esac`)
-	if _, err := dnsAdd(tool, "web05"); err == nil {
-		t.Fatal("a page of prose was taken for a host record")
+	// And something that is not a reply at all.
+	tool, _ = fakeDNS(t, "2.5.0", `echo 'not json at all'`)
+	if _, err := dnsShow(dnsTool(tool), "web05"); err == nil {
+		t.Fatal("a page of prose was taken for a reply")
 	}
 }
 
@@ -200,13 +269,13 @@ func TestOnlyTheNameIsAskedFor(t *testing.T) {
 	}
 }
 
-// An address that was fetched and then not used is given back. A deployment
-// somebody abandons at the confirmation must not leave a record behind for a
-// machine that was never made.
+// An address that was fetched and then not used is given back — a deployment
+// abandoned at the confirmation must not leave a record behind for a machine
+// that was never made — and the name is free again afterwards.
 func TestAFetchedAddressIsGivenBack(t *testing.T) {
-	tool, log := fakeInfoblox(t)
+	h, log := fakeInfoblox(t)
 
-	opts := deployOpts{how: customSite, st: testSite(), ip: autoIP, dns: tool}
+	opts := deployOpts{how: customSite, st: testSite(), ip: autoIP, dns: h}
 	opts, rec, give, err := fetchAddress(opts, "web05")
 	if err != nil {
 		t.Fatalf("fetchAddress: %v", err)
@@ -219,21 +288,17 @@ func TestAFetchedAddressIsGivenBack(t *testing.T) {
 	}
 
 	give()
-	calls := asked(t, log)
-	if n := len(calls); n == 0 || !strings.HasPrefix(calls[n-1], "-d web05") {
+	if calls := asked(t, log); len(calls) == 0 || !strings.HasPrefix(calls[len(calls)-1], "-d web05") {
 		t.Errorf("the address was not given back: %v", calls)
 	}
-
-	// And having given it back, the name is free again — which is the whole
-	// point of giving it back.
-	if rec, err := dnsShow(tool, "web05"); err != nil || rec.Name != "" {
+	if rec, err := dnsShow(h, "web05"); err != nil || rec.Name != "" {
 		t.Errorf("the name is still taken after being released: %+v (%v)", rec, err)
 	}
 }
 
-// Everything above happens only where the site has such a tool. Without one,
-// "auto" is refused with what to do about it, and — the point of the whole
-// arrangement — an ordinary address is untouched either way.
+// All of it happens only where the site has such a tool. Without one, "auto" is
+// refused with what to do about it, and an ordinary address is untouched either
+// way.
 func TestWithoutAToolAutoIsRefusedAndNothingElseChanges(t *testing.T) {
 	_, _, _, err := fetchAddress(deployOpts{ip: autoIP}, "web05")
 	if err == nil {
@@ -246,8 +311,8 @@ func TestWithoutAToolAutoIsRefusedAndNothingElseChanges(t *testing.T) {
 	}
 
 	// A typed address is never handed to a tool, whether there is one or not.
-	tool, log := fakeDNS(t, `echo "this should never run"; exit 1`)
-	opts, rec, give, err := fetchAddress(deployOpts{ip: "10.0.0.55", dns: tool}, "web05")
+	h, log := fakeInfoblox(t)
+	opts, rec, give, err := fetchAddress(deployOpts{ip: "10.0.0.55", dns: h}, "web05")
 	if err != nil {
 		t.Fatalf("a typed address went wrong: %v", err)
 	}
@@ -260,7 +325,7 @@ func TestWithoutAToolAutoIsRefusedAndNothingElseChanges(t *testing.T) {
 	}
 
 	// And no address at all asks nothing either.
-	if _, _, _, err := fetchAddress(deployOpts{dns: tool}, "web05"); err != nil {
+	if _, _, _, err := fetchAddress(deployOpts{dns: h}, "web05"); err != nil {
 		t.Errorf("a deployment with no address at all went wrong: %v", err)
 	}
 	if calls := asked(t, log); len(calls) != 0 {
@@ -268,26 +333,25 @@ func TestWithoutAToolAutoIsRefusedAndNothingElseChanges(t *testing.T) {
 	}
 }
 
-// Which helper to use: the one named in the configuration, or "dns" on the
-// path, or none — and a configuration that names one that is not there offers
-// nothing rather than quietly using a different one.
+// Which helper is used: the one named in the configuration, or "dns" on the
+// path, or none — and one that is named but not there offers nothing rather
+// than quietly using a different tool of the same name from somewhere else.
 func TestWhichHelperIsUsed(t *testing.T) {
-	tool, _ := fakeDNS(t, `echo hello`)
+	tool, _ := fakeDNS(t, "2.5.0", `echo hello`)
 
-	if got := dnsTool(tool); got != tool {
-		t.Errorf("a named helper resolved to %q", got)
+	if got := dnsTool(tool); got.path != tool || !got.there() {
+		t.Errorf("a named helper resolved to %+v", got)
 	}
-	if got := dnsTool(filepath.Join(t.TempDir(), "not-there")); got != "" {
-		t.Errorf("a named helper that is not there resolved to %q", got)
+	if got := dnsTool(filepath.Join(t.TempDir(), "not-there")); got.there() {
+		t.Errorf("a named helper that is not there resolved to %+v", got)
 	}
 
-	// Found by name on the path, and not found where it is not.
 	t.Setenv("PATH", filepath.Dir(tool))
-	if got := dnsTool(""); got != tool {
-		t.Errorf("the helper on the path resolved to %q", got)
+	if got := dnsTool(""); got.path != tool {
+		t.Errorf("the helper on the path resolved to %+v", got)
 	}
 	t.Setenv("PATH", t.TempDir())
-	if got := dnsTool(""); got != "" {
-		t.Errorf("a path with no helper on it resolved to %q", got)
+	if got := dnsTool(""); got.there() {
+		t.Errorf("a path with no helper on it resolved to %+v", got)
 	}
 }

gvm.go

@@ -429,11 +429,15 @@ func showConfig(cfg Config) error {
 	// Said whether it is there or not: "why was I not offered an address" is
 	// the question that follows an option quietly not being there, and this is
 	// where it is answered.
+	// The helper, or what is in the way of using it. "There is none" and "the
+	// one you have is too old" send somebody to two different places, and this
+	// is where the difference is worth the line.
 	tool := dnsTool(cfg.DNSTool)
-	if tool == "" {
-		tool = "none found — 'gvm new --ip auto' wants one, as 'dns' on the path or dnstool here"
+	said := tool.path + SF("  (v%s)", versionString(tool.version))
+	if !tool.there() {
+		said = tool.why()
 	}
-	PF("dns tool   %s\n", tool)
+	PF("dns tool   %s\n", said)
 	PF("completion %s\n", inventoryAge())
 	PF("version    %s\n", version)
 	return nil

gvmrc.example

@@ -90,8 +90,10 @@ telemetry = http://monitor.rz-berlin.mpg.de/telemetry.php
 #
 #   gvm new --from ubuntu-tpl --name web05 --ip auto
 #
-# gvm looks for "dns" on the path and offers that only where it finds one; name
-# it here where it lives somewhere else. It is asked for an address under the
+# gvm looks for "dns" on the path and offers that only where it finds one, and
+# only from v2.5.0 — older ones answer in sentences rather than JSON and are
+# said to be too old rather than read. Name it here where it lives somewhere
+# else. It is asked for an address under the
 # machine's own name, and the address is given back if the deployment does not
 # happen after all.
 # dnstool   = /usr/local/bin/dns

snap.go

@@ -82,6 +82,21 @@ func snapNew(vc VCenter, vmname string) error {
 func snapshotNow(s *session, ref types.ManagedObjectReference, name, desc string) error {
 	vm := object.NewVirtualMachine(s.client.Client, ref)
 
+	// Without memory and without quiescing, and both are deliberate.
+	//
+	// Memory would keep the running machine's RAM as well, so that a rollback
+	// came back mid-flight — at the price of writing the whole of it to the
+	// datastore every time, and of a rollback that restores a process tree
+	// along with the disks. What these snapshots are for is the moment before
+	// a patch or an upgrade, where coming back to a machine that boots is the
+	// point and coming back to one that is still half way through the thing
+	// that went wrong is not.
+	//
+	// Quiescing would have VMware Tools still the guest's filesystems first.
+	// Leaving it off makes the disk state crash-consistent — what a machine
+	// would find after the plug was pulled — which a journalling filesystem
+	// handles and a database may not. It also means the snapshot does not
+	// depend on Tools running, and does not stop when they are not.
 	task, err := vm.CreateSnapshot(s.ctx, name, desc, false, false)
 	if err != nil {
 		return fmt.Errorf("%s: cannot start the snapshot %s: %w", s.vc.Name, name, err)

version.txt

@@ -1 +1 @@
-1.3.3
+1.3.7