syz-ci: de-hardcode list of VMs that support overcommit

We currently have this list in multiple places (somewhat diverged).
Specify this "overcommit" property in VM implementations.
In particular, we also want to allow overcommit for "vmm" type.

Update #712

12 files changed, 43 insertions, 36 deletions

diff --git a/pkg/instance/instance.go b/pkg/instance/instance.go
index ad0a1ba76..c7c457ea6 100644
--- a/pkg/instance/instance.go
+++ b/pkg/instance/instance.go
@@ -31,10 +31,8 @@ type Env struct {
 }
 
 func NewEnv(cfg *mgrconfig.Config) (*Env, error) {
-	switch cfg.Type {
-	case "gce", "qemu", "gvisor":
-	default:
-		return nil, fmt.Errorf("test instances can only work with qemu/gce")
+	if !vm.AllowsOvercommit(cfg.Type) {
+		return nil, fmt.Errorf("test instances are not supported for %v VMs", cfg.Type)
 	}
 	if cfg.Workdir == "" {
 		return nil, fmt.Errorf("workdir path is empty")
diff --git a/syz-ci/jobs.go b/syz-ci/jobs.go
index 3002731ee..a8dfdd824 100644
--- a/syz-ci/jobs.go
+++ b/syz-ci/jobs.go
@@ -17,6 +17,7 @@ import (
 	"github.com/google/syzkaller/pkg/mgrconfig"
 	"github.com/google/syzkaller/pkg/osutil"
 	"github.com/google/syzkaller/pkg/vcs"
+	"github.com/google/syzkaller/vm"
 )
 
 type JobProcessor struct {
@@ -152,13 +153,7 @@ func (jp *JobProcessor) process(job *Job) *dashapi.JobDoneReq {
 			return job.resp
 		}
 	}
-	// TODO(dvyukov): this will only work for qemu/gce,
-	// because e.g. adb requires unique device IDs and we can't use what
-	// manager already uses. For qemu/gce this is also bad, because we
-	// override resource limits specified in config (e.g. can OOM), but works.
-	switch typ := mgr.managercfg.Type; typ {
-	case "gce", "qemu":
-	default:
+	if typ := mgr.managercfg.Type; !vm.AllowsOvercommit(typ) {
 		job.resp.Error = []byte(fmt.Sprintf("testing is not yet supported for %v machine type.", typ))
 		jp.Errorf("%s", job.resp.Error)
 		return job.resp
diff --git a/syz-ci/manager.go b/syz-ci/manager.go
index 7145fbcbd..7ad0617e7 100644
--- a/syz-ci/manager.go
+++ b/syz-ci/manager.go
@@ -20,6 +20,7 @@ import (
 	"github.com/google/syzkaller/pkg/osutil"
 	"github.com/google/syzkaller/pkg/report"
 	"github.com/google/syzkaller/pkg/vcs"
+	"github.com/google/syzkaller/vm"
 )
 
 // This is especially slightly longer than syzkaller rebuild period.
@@ -347,11 +348,8 @@ func (mgr *Manager) testImage(imageDir string, info *BuildInfo) error {
 		return fmt.Errorf("failed to create manager config: %v", err)
 	}
 	defer os.RemoveAll(mgrcfg.Workdir)
-	switch typ := mgrcfg.Type; typ {
-	case "gce", "qemu", "gvisor":
-	default:
-		// Other types don't support creating machines out of thin air.
-		return nil
+	if !vm.AllowsOvercommit(mgrcfg.Type) {
+		return nil // No support for creating machines out of thin air.
 	}
 	env, err := instance.NewEnv(mgrcfg)
 	if err != nil {
diff --git a/vm/adb/adb.go b/vm/adb/adb.go
index 8a7a0bb2e..86d3db32a 100644
--- a/vm/adb/adb.go
+++ b/vm/adb/adb.go
@@ -24,7 +24,7 @@ import (
 )
 
 func init() {
-	vmimpl.Register("adb", ctor)
+	vmimpl.Register("adb", ctor, false)
 }
 
 type Config struct {
diff --git a/vm/gce/gce.go b/vm/gce/gce.go
index 6386cef5c..52ea87a48 100644
--- a/vm/gce/gce.go
+++ b/vm/gce/gce.go
@@ -32,7 +32,7 @@ import (
 )
 
 func init() {
-	vmimpl.Register("gce", ctor)
+	vmimpl.Register("gce", ctor, true)
 }
 
 type Config struct {
diff --git a/vm/gvisor/gvisor.go b/vm/gvisor/gvisor.go
index f594efcf4..94081052a 100644
--- a/vm/gvisor/gvisor.go
+++ b/vm/gvisor/gvisor.go
@@ -23,7 +23,7 @@ import (
 )
 
 func init() {
-	vmimpl.Register("gvisor", ctor)
+	vmimpl.Register("gvisor", ctor, true)
 }
 
 type Config struct {
diff --git a/vm/isolated/isolated.go b/vm/isolated/isolated.go
index 20dfd0730..d55bced9f 100644
--- a/vm/isolated/isolated.go
+++ b/vm/isolated/isolated.go
@@ -20,7 +20,7 @@ import (
 )
 
 func init() {
-	vmimpl.Register("isolated", ctor)
+	vmimpl.Register("isolated", ctor, false)
 }
 
 type Config struct {
diff --git a/vm/kvm/kvm.go b/vm/kvm/kvm.go
index 68604ae98..ebcb9e545 100644
--- a/vm/kvm/kvm.go
+++ b/vm/kvm/kvm.go
@@ -25,7 +25,7 @@ const (
 )
 
 func init() {
-	vmimpl.Register("kvm", ctor)
+	vmimpl.Register("kvm", ctor, true)
 }
 
 type Config struct {
diff --git a/vm/qemu/qemu.go b/vm/qemu/qemu.go
index 501a6b02b..c643c6d8e 100644
--- a/vm/qemu/qemu.go
+++ b/vm/qemu/qemu.go
@@ -24,7 +24,7 @@ const (
 )
 
 func init() {
-	vmimpl.Register("qemu", ctor)
+	vmimpl.Register("qemu", ctor, true)
 }
 
 type Config struct {
diff --git a/vm/vm.go b/vm/vm.go
index 8d397194f..0a6114790 100644
--- a/vm/vm.go
+++ b/vm/vm.go
@@ -50,7 +50,24 @@ type BootErrorer interface {
 	BootError() (string, []byte)
 }
 
+// AllowsOvercommit returns if the instance type allows overcommit of instances
+// (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
+// 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.
+func AllowsOvercommit(typ string) bool {
+	return vmimpl.Types[typ].Overcommit
+}
+
+// Create creates a VM pool that can be used to create individual VMs.
 func Create(cfg *mgrconfig.Config, debug bool) (*Pool, error) {
+	typ, ok := vmimpl.Types[cfg.Type]
+	if !ok {
+		return nil, fmt.Errorf("unknown instance type '%v'", cfg.Type)
+	}
 	env := &vmimpl.Env{
 		Name:    cfg.Name,
 		OS:      cfg.TargetOS,
@@ -62,7 +79,7 @@ func Create(cfg *mgrconfig.Config, debug bool) (*Pool, error) {
 		Debug:   debug,
 		Config:  cfg.VM,
 	}
-	impl, err := vmimpl.Create(cfg.Type, env)
+	impl, err := typ.Ctor(env)
 	if err != nil {
 		return nil, err
 	}
diff --git a/vm/vmimpl/vmimpl.go b/vm/vmimpl/vmimpl.go
index 7d3411ee9..e9ac59dd6 100644
--- a/vm/vmimpl/vmimpl.go
+++ b/vm/vmimpl/vmimpl.go
@@ -81,30 +81,29 @@ func (err BootError) BootError() (string, []byte) {
 	return err.Title, err.Output
 }
 
-// Create creates a VM type that can be used to create individual VMs.
-func Create(typ string, env *Env) (Pool, error) {
-	ctor := ctors[typ]
-	if ctor == nil {
-		return nil, fmt.Errorf("unknown instance type '%v'", typ)
+// Register registers a new VM type within the package.
+func Register(typ string, ctor ctorFunc, allowsOvercommit bool) {
+	Types[typ] = Type{
+		Ctor:       ctor,
+		Overcommit: allowsOvercommit,
 	}
-	return ctor(env)
 }
 
-// Register registers a new VM type within the package.
-func Register(typ string, ctor ctorFunc) {
-	ctors[typ] = ctor
+type Type struct {
+	Ctor       ctorFunc
+	Overcommit bool
 }
 
+type ctorFunc func(env *Env) (Pool, error)
+
 var (
 	// Close to interrupt all pending operations in all VMs.
 	Shutdown   = make(chan struct{})
 	ErrTimeout = errors.New("timeout")
 
-	ctors = make(map[string]ctorFunc)
+	Types = make(map[string]Type)
 )
 
-type ctorFunc func(env *Env) (Pool, error)
-
 func Multiplex(cmd *exec.Cmd, merger *OutputMerger, console io.Closer, timeout time.Duration,
 	stop, closed <-chan bool, debug bool) (<-chan []byte, <-chan error, error) {
 	errc := make(chan error, 1)
diff --git a/vm/vmm/vmm.go b/vm/vmm/vmm.go
index 8d2ce5121..bbe820a6e 100644
--- a/vm/vmm/vmm.go
+++ b/vm/vmm/vmm.go
@@ -20,7 +20,7 @@ import (
 )
 
 func init() {
-	vmimpl.Register("vmm", ctor)
+	vmimpl.Register("vmm", ctor, true)
 }
 
 type Config struct {