virtcontainers: Remove the hypervisor init method

We always combine the hypervisor init and createSandbox, because what
we're trying to do is simply that: Set the hypervisor and have it create
a sandbox.

Instead of keeping a method with vague semantics, remove init and
integrate the actual hypervisor setup phase into the createSandbox one.

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>

virtcontainers/fc.go

@@ -126,15 +126,12 @@ func (fc *firecracker) trace(name string) (opentracing.Span, context.Context) {
 	return span, ctx
 }
 
-//
+// For firecracker this call only sets the internal structure up.
-// init: initialize the firecracker hypervisor's structure. Doesn't
+// The sandbox will be created and started through startSandbox().
-//  actually do anything with firecracker itself, rather it just parses
+func (fc *firecracker) createSandbox(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error {
-//  through and provides necessary details for its structs...
-//
-func (fc *firecracker) init(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error {
 	fc.ctx = ctx
 
-	span, _ := fc.trace("init")
+	span, _ := fc.trace("createSandbox")
 	defer span.Finish()
 
 	//TODO: check validity of the hypervisor config provided
@@ -154,14 +151,6 @@ func (fc *firecracker) init(ctx context.Context, id string, hypervisorConfig *Hy
 	return nil
 }
 
-// for firecracker this call isn't necessary
-func (fc *firecracker) createSandbox() error {
-	span, _ := fc.trace("createSandbox")
-	defer span.Finish()
-
-	return nil
-}
-
 func (fc *firecracker) newFireClient() *client.Firecracker {
 	span, _ := fc.trace("newFireClient")
 	defer span.Finish()

virtcontainers/hypervisor.go

@@ -590,9 +590,7 @@ func RunningOnVMM(cpuInfoPath string) (bool, error) {
 // hypervisor is the virtcontainers hypervisor interface.
 // The default hypervisor implementation is Qemu.
 type hypervisor interface {
-	init(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error
+	createSandbox(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error
-
-	createSandbox() error
 	startSandbox() error
 	waitSandbox(timeout int) error
 	stopSandbox() error

virtcontainers/mock_hypervisor.go

@@ -13,15 +13,6 @@ import (
 type mockHypervisor struct {
 }
 
-func (m *mockHypervisor) init(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error {
-	err := hypervisorConfig.valid()
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
 func (m *mockHypervisor) capabilities() capabilities {
 	return capabilities{}
 }
@@ -30,7 +21,12 @@ func (m *mockHypervisor) hypervisorConfig() HypervisorConfig {
 	return HypervisorConfig{}
 }
 
-func (m *mockHypervisor) createSandbox() error {
+func (m *mockHypervisor) createSandbox(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error {
+	err := hypervisorConfig.valid()
+	if err != nil {
+		return err
+	}
+
 	return nil
 }
 

virtcontainers/mock_hypervisor_test.go

@@ -11,7 +11,7 @@ import (
 	"testing"
 )
 
-func TestMockHypervisorInit(t *testing.T) {
+func TestMockHypervisorCreateSandbox(t *testing.T) {
 	var m *mockHypervisor
 
 	sandbox := &Sandbox{
@@ -29,7 +29,7 @@ func TestMockHypervisorInit(t *testing.T) {
 	ctx := context.Background()
 
 	// wrong config
-	if err := m.init(ctx, sandbox.config.ID, &sandbox.config.HypervisorConfig, sandbox.storage); err == nil {
+	if err := m.createSandbox(ctx, sandbox.config.ID, &sandbox.config.HypervisorConfig, sandbox.storage); err == nil {
 		t.Fatal()
 	}
 
@@ -39,16 +39,7 @@ func TestMockHypervisorInit(t *testing.T) {
 		HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor),
 	}
 
-	// right config
+	if err := m.createSandbox(ctx, sandbox.config.ID, &sandbox.config.HypervisorConfig, sandbox.storage); err != nil {
-	if err := m.init(ctx, sandbox.config.ID, &sandbox.config.HypervisorConfig, sandbox.storage); err != nil {
-		t.Fatal(err)
-	}
-}
-
-func TestMockHypervisorCreateSandbox(t *testing.T) {
-	var m *mockHypervisor
-
-	if err := m.createSandbox(); err != nil {
 		t.Fatal(err)
 	}
 }

virtcontainers/qemu.go

@@ -208,12 +208,9 @@ func (q *qemu) trace(name string) (opentracing.Span, context.Context) {
 	return span, ctx
 }
 
-// init intializes the Qemu structure.
+// setup sets the Qemu structure up.
-func (q *qemu) init(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error {
+func (q *qemu) setup(id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error {
-	// save
+	span, _ := q.trace("setup")
-	q.ctx = ctx
-
-	span, _ := q.trace("init")
 	defer span.Finish()
 
 	err := hypervisorConfig.valid()
@@ -418,10 +415,17 @@ func (q *qemu) setupTemplate(knobs *govmmQemu.Knobs, memory *govmmQemu.Memory) g
 }
 
 // createSandbox is the Hypervisor sandbox creation implementation for govmmQemu.
-func (q *qemu) createSandbox() error {
+func (q *qemu) createSandbox(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error {
+	// Save the tracing context
+	q.ctx = ctx
+
 	span, _ := q.trace("createSandbox")
 	defer span.Finish()
 
+	if err := q.setup(id, hypervisorConfig, storage); err != nil {
+		return err
+	}
+
 	machine, err := q.getQemuMachine()
 	if err != nil {
 		return err

virtcontainers/qemu_test.go

@@ -70,7 +70,7 @@ func TestQemuKernelParameters(t *testing.T) {
 	testQemuKernelParameters(t, params, expectedOut, false)
 }
 
-func TestQemuInit(t *testing.T) {
+func TestQemuCreateSandbox(t *testing.T) {
 	qemuConfig := newQemuConfig()
 	q := &qemu{}
 
@@ -82,13 +82,20 @@ func TestQemuInit(t *testing.T) {
 		},
 	}
 
+	// Create the hypervisor fake binary
+	testQemuPath := filepath.Join(testDir, testHypervisor)
+	_, err := os.Create(testQemuPath)
+	if err != nil {
+		t.Fatalf("Could not create hypervisor file %s: %v", testQemuPath, err)
+	}
+
 	// Create parent dir path for hypervisor.json
 	parentDir := filepath.Join(runStoragePath, sandbox.id)
 	if err := os.MkdirAll(parentDir, dirMode); err != nil {
 		t.Fatalf("Could not create parent directory %s: %v", parentDir, err)
 	}
 
-	if err := q.init(context.Background(), sandbox.id, &sandbox.config.HypervisorConfig, sandbox.storage); err != nil {
+	if err := q.createSandbox(context.Background(), sandbox.id, &sandbox.config.HypervisorConfig, sandbox.storage); err != nil {
 		t.Fatal(err)
 	}
 
@@ -101,7 +108,7 @@ func TestQemuInit(t *testing.T) {
 	}
 }
 
-func TestQemuInitMissingParentDirFail(t *testing.T) {
+func TestQemuCreateSandboxMissingParentDirFail(t *testing.T) {
 	qemuConfig := newQemuConfig()
 	q := &qemu{}
 
@@ -113,14 +120,21 @@ func TestQemuInitMissingParentDirFail(t *testing.T) {
 		},
 	}
 
+	// Create the hypervisor fake binary
+	testQemuPath := filepath.Join(testDir, testHypervisor)
+	_, err := os.Create(testQemuPath)
+	if err != nil {
+		t.Fatalf("Could not create hypervisor file %s: %v", testQemuPath, err)
+	}
+
 	// Ensure parent dir path for hypervisor.json does not exist.
 	parentDir := filepath.Join(runStoragePath, sandbox.id)
 	if err := os.RemoveAll(parentDir); err != nil {
 		t.Fatal(err)
 	}
 
-	if err := q.init(context.Background(), sandbox.id, &sandbox.config.HypervisorConfig, sandbox.storage); err != nil {
+	if err := q.createSandbox(context.Background(), sandbox.id, &sandbox.config.HypervisorConfig, sandbox.storage); err != nil {
-		t.Fatalf("Qemu init() is not expected to fail because of missing parent directory for storage: %v", err)
+		t.Fatalf("Qemu createSandbox() is not expected to fail because of missing parent directory for storage: %v", err)
 	}
 }
 

virtcontainers/sandbox.go

@@ -597,11 +597,7 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor
 		}
 	}()
 
-	if err = s.hypervisor.init(ctx, s.id, &sandboxConfig.HypervisorConfig, s.storage); err != nil {
+	if err = s.hypervisor.createSandbox(ctx, s.id, &sandboxConfig.HypervisorConfig, s.storage); err != nil {
-		return nil, err
-	}
-
-	if err = s.hypervisor.createSandbox(); err != nil {
 		return nil, err
 	}
 

virtcontainers/vm.go

@@ -118,11 +118,7 @@ func NewVM(ctx context.Context, config VMConfig) (*VM, error) {
 		}
 	}()
 
-	if err = hypervisor.init(ctx, id, &config.HypervisorConfig, &filesystem{}); err != nil {
+	if err = hypervisor.createSandbox(ctx, id, &config.HypervisorConfig, &filesystem{}); err != nil {
-		return nil, err
-	}
-
-	if err = hypervisor.createSandbox(); err != nil {
 		return nil, err
 	}