Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Commit

Permalink
virtcontainers: Remove the hypervisor init method
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Samuel Ortiz committed Jan 8, 2019
1 parent 142d3b4 commit 763bf18
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 62 deletions.
19 changes: 4 additions & 15 deletions virtcontainers/fc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand Down
4 changes: 1 addition & 3 deletions virtcontainers/hypervisor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 6 additions & 10 deletions virtcontainers/mock_hypervisor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
}
Expand All @@ -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
}

Expand Down
15 changes: 3 additions & 12 deletions virtcontainers/mock_hypervisor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"testing"
)

func TestMockHypervisorInit(t *testing.T) {
func TestMockHypervisorCreateSandbox(t *testing.T) {
var m *mockHypervisor

sandbox := &Sandbox{
Expand All @@ -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()
}

Expand All @@ -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)
}
}
Expand Down
18 changes: 11 additions & 7 deletions virtcontainers/qemu.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
24 changes: 19 additions & 5 deletions virtcontainers/qemu_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}

Expand All @@ -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)
}

Expand All @@ -101,7 +108,7 @@ func TestQemuInit(t *testing.T) {
}
}

func TestQemuInitMissingParentDirFail(t *testing.T) {
func TestQemuCreateSandboxMissingParentDirFail(t *testing.T) {
qemuConfig := newQemuConfig()
q := &qemu{}

Expand All @@ -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)
}
}

Expand Down
6 changes: 1 addition & 5 deletions virtcontainers/sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
6 changes: 1 addition & 5 deletions virtcontainers/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down

0 comments on commit 763bf18

Please sign in to comment.