Refactored NewMachine and Start functions

Signed-off-by: David Son <[email protected]>

Author: sondavidb

docs/snapshotting.md

@@ -40,7 +40,7 @@ m.PauseVM(ctx)
 m.CreateSnapshot(ctx, memPath, snapPath)
 ```
 
-The snapshot can be loaded at any later time at startup of a machine via the machine's `Start()` function, using `WithSnapshot()` as an option. The VM must then be resumed before attempting to use it.
+The snapshot can be loaded at any later time at creation of a machine via the machine's `NewMachine()` function, using `WithSnapshot()` as an option. Upon starting, the VM loads the snapshot and must then be resumed before attempting to use it.
 
 ```
 ctx := context.Background()
@@ -49,9 +49,9 @@ cfg := sdk.Config{
     ...
 
 }
-m, _ := sdk.NewMachine(ctx, cfg)
+m, _ := sdk.NewMachine(ctx, cfg, sdk.WithSnapshot(memPath, snapPath))
 
-m.Start(ctx, sdk.WithSnapshot(memPath, snapPath))
+m.Start(ctx)
 m.ResumeVM(ctx)
 ```
 

examples/cmd/snapshotting/example_demo.go

@@ -305,13 +305,13 @@ func loadSnapshotSSH(ctx context.Context, socketPath, memPath, snapPath, ipToRes
 	// Use the firecracker binary
 	cmd := sdk.VMCommandBuilder{}.WithSocketPath(socketFile).WithBin(filepath.Join(dir, "firecracker")).Build(ctx)
 
-	m, err := sdk.NewMachine(ctx, cfg, sdk.WithProcessRunner(cmd))
+	m, err := sdk.NewMachine(ctx, cfg, sdk.WithProcessRunner(cmd), sdk.WithSnapshot(memPath, snapPath))
 	if err != nil {
 		log.Fatal(err)
 	}
 	defer os.Remove(socketFile)
 
-	err = m.Start(ctx, sdk.WithSnapshot(memPath, snapPath))
+	err = m.Start(ctx)
 	if err != nil {
 		log.Fatal(err)
 	}

examples/cmd/snapshotting/go.mod

@@ -39,3 +39,5 @@ require (
 	golang.org/x/net v0.0.0-20220728211354-c7608f3a8462 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 )
+
+replace github.com/firecracker-microvm/firecracker-go-sdk => ../../..

machine.go

@@ -339,10 +339,6 @@ func NewMachine(ctx context.Context, cfg Config, opts ...Opt) (*Machine, error)
 		m.cmd = configureBuilder(defaultFirecrackerVMMCommandBuilder, cfg).Build(ctx)
 	}
 
-	for _, opt := range opts {
-		opt(m)
-	}
-
 	if m.logger == nil {
 		logger := log.New()
 
@@ -370,6 +366,10 @@ func NewMachine(ctx context.Context, cfg Config, opts ...Opt) (*Machine, error)
 		m.Cfg.NetNS = m.defaultNetNSPath()
 	}
 
+	for _, opt := range opts {
+		opt(m)
+	}
+
 	m.logger.Debug("Called NewMachine()")
 	return m, nil
 }
@@ -382,7 +382,7 @@ func NewMachine(ctx context.Context, cfg Config, opts ...Opt) (*Machine, error)
 // handlers succeed, then this will start the VMM instance.
 // Start may only be called once per Machine.  Subsequent calls will return
 // ErrAlreadyStarted.
-func (m *Machine) Start(ctx context.Context, opts ...StartOpt) error {
+func (m *Machine) Start(ctx context.Context) error {
 	m.logger.Debug("Called Machine.Start()")
 	alreadyStarted := true
 	m.startOnce.Do(func() {
@@ -403,10 +403,6 @@ func (m *Machine) Start(ctx context.Context, opts ...StartOpt) error {
 		}
 	}()
 
-	for _, opt := range opts {
-		opt(m)
-	}
-
 	err = m.Handlers.Run(ctx, m)
 	if err != nil {
 		return err

machine_test.go

@@ -1945,10 +1945,10 @@ func TestLoadSnapshot(t *testing.T) {
 					// some unexported members
 					args := m.cmd.Args[1:]
 					m.cmd = exec.Command(getFirecrackerBinaryPath(), args...)
-				}, WithLogger(logrus.NewEntry(machineLogger)))
+				}, WithLogger(logrus.NewEntry(machineLogger)), WithSnapshot(memPath, snapPath))
 				require.NoError(t, err)
 
-				err = m.Start(ctx, WithSnapshot(memPath, snapPath))
+				err = m.Start(ctx)
 				require.NoError(t, err)
 
 				err = m.ResumeVM(ctx)
@@ -1971,10 +1971,10 @@ func TestLoadSnapshot(t *testing.T) {
 					// some unexported members
 					args := m.cmd.Args[1:]
 					m.cmd = exec.Command(getFirecrackerBinaryPath(), args...)
-				}, WithLogger(logrus.NewEntry(machineLogger)))
+				}, WithLogger(logrus.NewEntry(machineLogger)), WithSnapshot(memPath, snapPath))
 				require.NoError(t, err)
 
-				err = m.Start(ctx, WithSnapshot(memPath, snapPath))
+				err = m.Start(ctx)
 				require.Error(t, err)
 			},
 		},
@@ -2085,10 +2085,10 @@ func TestLoadSnapshot(t *testing.T) {
 
 				cmd := VMCommandBuilder{}.WithSocketPath(fmt.Sprintf("%s.load", socketPath)).WithBin(getFirecrackerBinaryPath()).Build(ctx)
 
-				m, err := NewMachine(ctx, cfg, WithProcessRunner(cmd))
+				m, err := NewMachine(ctx, cfg, WithProcessRunner(cmd), WithSnapshot(memPath, snapPath))
 				require.NoError(t, err)
 
-				err = m.Start(ctx, WithSnapshot(memPath, snapPath))
+				err = m.Start(ctx)
 				require.NoError(t, err)
 				defer m.StopVMM()
 

machineiface.go

@@ -23,7 +23,7 @@ var _ MachineIface = (*Machine)(nil)
 // MachineIface can be used for mocking and testing of the Machine. The Machine
 // is subject to change, meaning this interface would change.
 type MachineIface interface {
-	Start(context.Context, ...StartOpt) error
+	Start(context.Context) error
 	StopVMM() error
 	Shutdown(context.Context) error
 	Wait(context.Context) error

opts.go

@@ -21,7 +21,6 @@ import (
 
 // Opt represents a functional option to help modify functionality of a Machine.
 type Opt func(*Machine)
-type StartOpt func(*Machine)
 
 // WithClient will use the client in place rather than the client constructed
 // during bootstrapping of the machine. This option is useful for mocking out
@@ -54,7 +53,7 @@ func WithProcessRunner(cmd *exec.Cmd) Opt {
 type WithSnapshotOpt func(*SnapshotConfig)
 
 // WithSnapshot will allow for the machine to start using a given snapshot.
-func WithSnapshot(memFilePath, snapshotPath string, opts ...WithSnapshotOpt) StartOpt {
+func WithSnapshot(memFilePath, snapshotPath string, opts ...WithSnapshotOpt) Opt {
 	return func(m *Machine) {
 		m.Cfg.Snapshot.MemFilePath = memFilePath
 		m.Cfg.Snapshot.SnapshotPath = snapshotPath