diff --git a/virtcontainers/fc.go b/virtcontainers/fc.go index 573ad88d40..bc5e3bd376 100644 --- a/virtcontainers/fc.go +++ b/virtcontainers/fc.go @@ -126,15 +126,12 @@ func (fc *firecracker) trace(name string) (opentracing.Span, context.Context) { return span, ctx } -// -// init: initialize the firecracker hypervisor's structure. Doesn't -// actually do anything with firecracker itself, rather it just parses -// through and provides necessary details for its structs... -// -func (fc *firecracker) init(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error { +// For firecracker this call only sets the internal structure up. +// The sandbox will be created and started through startSandbox(). +func (fc *firecracker) createSandbox(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() diff --git a/virtcontainers/hypervisor.go b/virtcontainers/hypervisor.go index bff95df32f..0e2289beeb 100644 --- a/virtcontainers/hypervisor.go +++ b/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() error + createSandbox(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error startSandbox() error waitSandbox(timeout int) error stopSandbox() error diff --git a/virtcontainers/mock_hypervisor.go b/virtcontainers/mock_hypervisor.go index 89b3687a8c..d5ca20c2d0 100644 --- a/virtcontainers/mock_hypervisor.go +++ b/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 } diff --git a/virtcontainers/mock_hypervisor_test.go b/virtcontainers/mock_hypervisor_test.go index 2be8d30d16..e6007380fb 100644 --- a/virtcontainers/mock_hypervisor_test.go +++ b/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.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 { + if err := m.createSandbox(ctx, sandbox.config.ID, &sandbox.config.HypervisorConfig, sandbox.storage); err != nil { t.Fatal(err) } } diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 60b6476890..97e9857c97 100644 --- a/virtcontainers/qemu.go +++ b/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. -func (q *qemu) init(ctx context.Context, id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error { - // save - q.ctx = ctx - - span, _ := q.trace("init") +// setup sets the Qemu structure up. +func (q *qemu) setup(id string, hypervisorConfig *HypervisorConfig, storage resourceStorage) error { + span, _ := q.trace("setup") 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 diff --git a/virtcontainers/qemu_test.go b/virtcontainers/qemu_test.go index 3bd09e3cd4..73ad6adfc9 100644 --- a/virtcontainers/qemu_test.go +++ b/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 { - t.Fatalf("Qemu init() is not expected to fail because of missing parent directory for storage: %v", err) + if err := q.createSandbox(context.Background(), sandbox.id, &sandbox.config.HypervisorConfig, sandbox.storage); err != nil { + t.Fatalf("Qemu createSandbox() is not expected to fail because of missing parent directory for storage: %v", err) } } diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index a34203b128..90f1ea015b 100644 --- a/virtcontainers/sandbox.go +++ b/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 { - return nil, err - } - - if err = s.hypervisor.createSandbox(); err != nil { + if err = s.hypervisor.createSandbox(ctx, s.id, &sandboxConfig.HypervisorConfig, s.storage); err != nil { return nil, err } diff --git a/virtcontainers/vm.go b/virtcontainers/vm.go index 147a876d5f..e22fc1e691 100644 --- a/virtcontainers/vm.go +++ b/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 { - return nil, err - } - - if err = hypervisor.createSandbox(); err != nil { + if err = hypervisor.createSandbox(ctx, id, &config.HypervisorConfig, &filesystem{}); err != nil { return nil, err }