openbsd: run on gce

* build/openbsd: minor cleanup (use tuples instead of maps)

* Grammar nits in comments.

* Simplify openbsd.Create, will defer when there's more than one error exit.

* pkg/build: Support copying kernel into GCE image

* Simple test for openbsd image copy build.

* Cleanup in case something failed before.

* Support multi-processor VMs on GCE.

* More debug

* Reformat

* OpenBSD gce image needs to be raw.

* GC

* Force format to GNU directly on Go 1.10 or newer.

* Use vmType passed as a parameter inside openbsd.go

* gofmt

* more fmt

* Can't use GENERIC.mp just yet.

* capitalize

* Copyright

pkg/build/build.go

@@ -71,7 +71,7 @@ func getBuilder(targetOS, targetArch, vmType string) (builder, error) {
 		return fuchsia{}, nil
 	case targetOS == "akaros" && targetArch == "amd64" && vmType == "qemu":
 		return akaros{}, nil
-	case targetOS == "openbsd" && targetArch == "amd64" && vmType == "vmm":
+	case targetOS == "openbsd" && targetArch == "amd64" && (vmType == "gce" || vmType == "vmm"):
 		return openbsd{}, nil
 	default:
 		return nil, fmt.Errorf("unsupported image type %v/%v/%v", targetOS, targetArch, vmType)

pkg/build/openbsd.go

@@ -10,6 +10,7 @@ import (
 	"strconv"
 	"time"
 
+	"github.com/google/syzkaller/pkg/log"
 	"github.com/google/syzkaller/pkg/osutil"
 )
 
@@ -21,25 +22,29 @@ func (ctx openbsd) build(targetArch, vmType, kernelDir, outputDir, compiler, use
 	confDir := fmt.Sprintf("%v/sys/arch/%v/conf", kernelDir, targetArch)
 	compileDir := fmt.Sprintf("%v/sys/arch/%v/compile/%v", kernelDir, targetArch, kernelName)
 
-	if err := ctx.configure(confDir, compileDir, kernelName); err != nil {
+	useGCE := vmType == "gce"
+	if err := ctx.configure(confDir, compileDir, kernelName, useGCE); err != nil {
 		return err
 	}
 
 	if err := ctx.make(compileDir, "all"); err != nil {
 		return err
 	}
-
-	for src, dst := range map[string]string{
-		filepath.Join(compileDir, "obj/bsd"):     "kernel",
-		filepath.Join(compileDir, "obj/bsd.gdb"): "obj/bsd.gdb",
-		filepath.Join(userspaceDir, "image"):     "image",
-		filepath.Join(userspaceDir, "key"):       "key",
+	for _, s := range []struct{ dir, src, dst string }{
+		{compileDir, "obj/bsd", "kernel"},
+		{compileDir, "obj/bsd.gdb", "obj/bsd.gdb"},
+		{userspaceDir, "image", "image"},
+		{userspaceDir, "key", "key"},
 	} {
-		fullDst := filepath.Join(outputDir, dst)
-		if err := osutil.CopyFile(src, fullDst); err != nil {
-			return fmt.Errorf("failed to copy %v -> %v: %v", src, fullDst, err)
+		fullSrc := filepath.Join(s.dir, s.src)
+		fullDst := filepath.Join(outputDir, s.dst)
+		if err := osutil.CopyFile(fullSrc, fullDst); err != nil {
+			return fmt.Errorf("failed to copy %v -> %v: %v", fullSrc, fullDst, err)
 		}
 	}
+	if useGCE {
+		return CopyKernelToImage(outputDir)
+	}
 	return nil
 }
 
@@ -47,11 +52,18 @@ func (ctx openbsd) clean(kernelDir string) error {
 	return ctx.make(kernelDir, "", "clean")
 }
 
-func (ctx openbsd) configure(confDir, compileDir, kernelName string) error {
-	conf := []byte(`
-include "arch/amd64/conf/GENERIC"
+func (ctx openbsd) configure(confDir, compileDir, kernelName string, useGCE bool) error {
+	baseConfig := "GENERIC"
+	if useGCE {
+		// GCE supports multiple CPUs.
+		// TODO(gnezdo): Switch to GENERIC.MP once kernel crash is solved.
+		// http://openbsd-archive.7691.n7.nabble.com/option-kcov-GENERIC-MP-gt-silent-crash-tc355807.html
+		baseConfig = "GENERIC"
+	}
+	conf := []byte(fmt.Sprintf(`
+include "arch/amd64/conf/%v"
 pseudo-device kcov 1
-`)
+`, baseConfig))
 	if err := osutil.WriteFile(filepath.Join(confDir, kernelName), conf); err != nil {
 		return err
 	}
@@ -78,3 +90,27 @@ func (ctx openbsd) make(kernelDir string, args ...string) error {
 	_, err := osutil.RunCmd(10*time.Minute, kernelDir, "make", args...)
 	return err
 }
+
+// The easiest way to make an openbsd image that boots the given
+// kernel on GCE is to simply overwrite it inside the disk image.
+// Ideally a user space tool capable of understanding FFS should
+// implement this directly, but vnd(4) device would do in a pinch.
+// Assumes that the outputDir contains the appropriately named files.
+func CopyKernelToImage(outputDir string) error {
+	script := `set -eux
+# Cleanup in case something failed before.
+doas umount /altroot || true
+doas vnconfig -u vnd0 || true
+
+doas /sbin/vnconfig vnd0 image
+doas mount /dev/vnd0a /altroot
+doas cp kernel /altroot/bsd
+doas umount /altroot
+doas vnconfig -u vnd0
+`
+	debugOut, err := osutil.RunCmd(10*time.Minute, outputDir, "/bin/sh", "-c", script)
+	if err != nil {
+		log.Logf(0, "Error copying kernel into image %v\n%v\n", outputDir, debugOut)
+	}
+	return err
+}

tools/create-openbsd-vmm-worker.sh

@@ -41,6 +41,10 @@ perl -i.bak -pne 's/^(ttyC.*)vt220.*/$1unknown off/' /etc/ttys
 touch root/.hushlogin home/syzkaller/.hushlogin
 EOF
 
+cat >etc/sysctl.conf <<EOF
+hw.smt=1
+EOF
+
 cat >etc/installurl <<EOF
 https://${MIRROR}/pub/OpenBSD
 EOF
@@ -108,15 +112,15 @@ growisofs -M "${ISO_PATCHED}" -l -R -graft-points \
   /etc/random.seed=random.seed
 
 # Initialize disk image.
-rm -f worker_disk.qcow2
-qemu-img create -f qcow2 worker_disk.qcow2 1G
+rm -f worker_disk.raw
+qemu-img create -f raw worker_disk.raw 1G
 
 # Run the installer to create the disk image.
 expect 2>&1 <<EOF | tee install_log
 set timeout 1800
 
 spawn qemu-system-x86_64 -nographic -smp 2 \
-  -drive if=virtio,file=worker_disk.qcow2,format=qcow2 -cdrom "${ISO_PATCHED}" \
+  -drive if=virtio,file=worker_disk.raw,format=raw -cdrom "${ISO_PATCHED}" \
   -net nic,model=virtio -net user -boot once=d -m 4000 -enable-kvm
 
 expect timeout { exit 1 } "boot>"
@@ -165,5 +169,5 @@ expect {
 EOF
 
 cat <<EOF
-Done: worker_disk.qcow2
+Done: worker_disk.raw
 EOF

vm/gce/gce.go

@@ -100,7 +100,7 @@ func ctor(env *vmimpl.Env) (vmimpl.Pool, error) {
 	if cfg.GCEImage == "" {
 		cfg.GCEImage = env.Name
 		gcsImage := filepath.Join(cfg.GCSPath, env.Name+"-image.tar.gz")
-		log.Logf(0, "uploading image to %v...", gcsImage)
+		log.Logf(0, "uploading image %v to %v...", env.Image, gcsImage)
 		if err := uploadImageToGCS(env.Image, gcsImage); err != nil {
 			return nil, err
 		}
@@ -436,15 +436,10 @@ func uploadImageToGCS(localImage, gcsImage string) error {
 		Mode:     0640,
 		Size:     localStat.Size(),
 		ModTime:  time.Now(),
-		// This is hacky but we actually need these large uids.
-		// GCE understands only the old GNU tar format and
-		// there is no direct way to force tar package to use GNU format.
-		// But these large numbers force tar to switch to GNU format.
-		Uid:   100000000,
-		Gid:   100000000,
-		Uname: "syzkaller",
-		Gname: "syzkaller",
+		Uname:    "syzkaller",
+		Gname:    "syzkaller",
 	}
+	setGNUFormat(tarHeader)
 	if err := tarWriter.WriteHeader(tarHeader); err != nil {
 		return fmt.Errorf("failed to write image tar header: %v", err)
 	}

vm/gce/tar_go1.10.go

@@ -0,0 +1,14 @@
+// Copyright 2018 syzkaller project authors. All rights reserved.
+// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
+
+// +build go1.10
+
+package gce
+
+import (
+	"archive/tar"
+)
+
+func setGNUFormat(hdr *tar.Header) {
+	hdr.Format = tar.FormatGNU
+}

vm/gce/tar_go1.9.go

@@ -0,0 +1,19 @@
+// Copyright 2018 syzkaller project authors. All rights reserved.
+// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
+
+// +build !go1.10
+
+package gce
+
+import (
+	"archive/tar"
+)
+
+func setGNUFormat(hdr *tar.Header) {
+	// This is hacky but we actually need these large uids.
+	// GCE understands only the old GNU tar format and prior to Go 1.10
+	// there is no direct way to force tar package to use GNU format.
+	// But these large numbers force tar to switch to GNU format.
+	hdr.Uid = 100000000
+	hdr.Gid = 100000000
+}

vm/vm.go

@@ -54,7 +54,7 @@ type BootErrorer interface {
 // (i.e. creation of instances out-of-thin-air). Overcommit is used during image
 // and patch testing in syz-ci when it just asks for more than specified in config
 // instances. Generally virtual machines (qemu, gce) support overcommit,
-// while physical machines (adb, isolated) do not. Strictly saying, we should
+// while physical machines (adb, isolated) do not. Strictly speaking, we should
 // never use overcommit and use only what's specified in config, because we
 // override resource limits specified in config (e.g. can OOM). But it works and
 // makes lots of things much simpler.

vm/vmimpl/vmimpl.go

@@ -33,8 +33,8 @@ type Instance interface {
 	// Copy copies a hostSrc file into VM and returns file name in VM.
 	Copy(hostSrc string) (string, error)
 
-	// Forward setups forwarding from within VM to host port port
-	// and returns address to use in VM.
+	// Forward sets up forwarding from within VM to the given tcp
+	// port on the host and returns the address to use in VM.
 	Forward(port int) (string, error)
 
 	// Run runs cmd inside of the VM (think of ssh cmd).

vm/vmm/vmm.go

@@ -125,18 +125,12 @@ func (pool *Pool) Create(workdir string, index int) (vmimpl.Instance, error) {
 		return nil, err
 	}
 
-	closeInst := inst
-	defer func() {
-		if closeInst != nil {
-			closeInst.Close()
-		}
-	}()
-
 	if err := inst.Boot(); err != nil {
+		// Cleans up if Boot fails.
+		inst.Close()
 		return nil, err
 	}
 
-	closeInst = nil
 	return inst, nil
 }