From 52946338a123add3691ac0a6eb9949b71b8cb21c Mon Sep 17 00:00:00 2001 From: David Son Date: Tue, 9 Aug 2022 23:31:49 +0000 Subject: [PATCH] Removed unnecessary requirements for loading snapshots Signed-off-by: David Son --- examples/cmd/snapshotting/example_demo.go | 20 +++++++++++++- examples/cmd/snapshotting/go.mod | 2 +- handlers.go | 22 ++++++++++++--- machine.go | 27 +++++++++++++++++++ machine_test.go | 33 +++++++++++++++++++---- opts.go | 1 + 6 files changed, 95 insertions(+), 10 deletions(-) diff --git a/examples/cmd/snapshotting/example_demo.go b/examples/cmd/snapshotting/example_demo.go index e720760b..d62f7c3e 100644 --- a/examples/cmd/snapshotting/example_demo.go +++ b/examples/cmd/snapshotting/example_demo.go @@ -299,8 +299,26 @@ func loadSnapshotSSH(ctx context.Context, socketPath, memPath, snapPath, ipToRes }, } + driveID := "root" + isRootDevice := true + isReadOnly := false + rootfsPath := "root-drive-with-ssh.img" + socketFile := fmt.Sprintf("%s.load", socketPath) - cfg := createNewConfig(socketFile, withNetworkInterface(networkInterface)) + cfg := sdk.Config{ + SocketPath: socketPath + ".load", + Drives: []models.Drive{ + { + DriveID: &driveID, + IsRootDevice: &isRootDevice, + IsReadOnly: &isReadOnly, + PathOnHost: &rootfsPath, + }, + }, + NetworkInterfaces: []sdk.NetworkInterface{ + networkInterface, + }, + } // Use the firecracker binary cmd := sdk.VMCommandBuilder{}.WithSocketPath(socketFile).WithBin(filepath.Join(dir, "firecracker")).Build(ctx) diff --git a/examples/cmd/snapshotting/go.mod b/examples/cmd/snapshotting/go.mod index 7e64278e..739c41dc 100644 --- a/examples/cmd/snapshotting/go.mod +++ b/examples/cmd/snapshotting/go.mod @@ -40,4 +40,4 @@ require ( gopkg.in/yaml.v2 v2.4.0 // indirect ) -replace github.com/firecracker-microvm/firecracker-go-sdk => ../../.. \ No newline at end of file +replace github.com/firecracker-microvm/firecracker-go-sdk => ../../.. diff --git a/handlers.go b/handlers.go index 66906f99..874084c4 100644 --- a/handlers.go +++ b/handlers.go @@ -37,9 +37,10 @@ const ( CreateBalloonHandlerName = "fcinit.CreateBalloon" LoadSnapshotHandlerName = "fcinit.LoadSnapshot" - ValidateCfgHandlerName = "validate.Cfg" - ValidateJailerCfgHandlerName = "validate.JailerCfg" - ValidateNetworkCfgHandlerName = "validate.NetworkCfg" + ValidateCfgHandlerName = "validate.Cfg" + ValidateJailerCfgHandlerName = "validate.JailerCfg" + ValidateNetworkCfgHandlerName = "validate.NetworkCfg" + ValidateLoadSnapshotCfgHandlerName = "validate.LoadSnapshotCfg" ) // HandlersAdapter is an interface used to modify a given set of handlers. @@ -57,6 +58,16 @@ var ConfigValidationHandler = Handler{ }, } +// LoadSnapshotConfigValidationHandler is used to validate that required +// fields are present. +var LoadSnapshotConfigValidationHandler = Handler{ + Name: ValidateLoadSnapshotCfgHandlerName, + Fn: func(ctx context.Context, m *Machine) error { + // ensure that the configuration is valid for the FcInit handlers. + return m.Cfg.ValidateLoadSnapshot() + }, +} + // JailerConfigValidationHandler is used to validate that required fields are // present. var JailerConfigValidationHandler = Handler{ @@ -317,6 +328,11 @@ var defaultValidationHandlerList = HandlerList{}.Append( NetworkConfigValidationHandler, ) +var loadSnapshotValidationHandlerList = HandlerList{}.Append( + NetworkConfigValidationHandler, + LoadSnapshotConfigValidationHandler, +) + var defaultHandlers = Handlers{ Validation: defaultValidationHandlerList, FcInit: defaultFcInitHandlerList, diff --git a/machine.go b/machine.go index 4746c39d..83f11895 100644 --- a/machine.go +++ b/machine.go @@ -204,6 +204,33 @@ func (cfg *Config) Validate() error { return nil } +func (cfg *Config) ValidateLoadSnapshot() error { + if cfg.DisableValidation { + return nil + } + + for _, drive := range cfg.Drives { + rootPath := StringValue(drive.PathOnHost) + if _, err := os.Stat(rootPath); err != nil { + return fmt.Errorf("failed to stat drive path, %q: %v", rootPath, err) + } + } + + if _, err := os.Stat(cfg.SocketPath); err == nil { + return fmt.Errorf("socket %s already exists", cfg.SocketPath) + } + + if _, err := os.Stat(cfg.Snapshot.MemFilePath); err != nil { + return err + } + + if _, err := os.Stat(cfg.Snapshot.SnapshotPath); err != nil { + return err + } + + return nil +} + func (cfg *Config) ValidateNetwork() error { if cfg.DisableValidation { return nil diff --git a/machine_test.go b/machine_test.go index ac478e5f..fa2b33d2 100644 --- a/machine_test.go +++ b/machine_test.go @@ -1939,7 +1939,19 @@ func TestLoadSnapshot(t *testing.T) { }, loadSnapshot: func(ctx context.Context, machineLogger *logrus.Logger, socketPath, memPath, snapPath string) { - cfg := createValidConfig(t, socketPath+".load") + // Note that many fields are not necessary when loading a snapshot + cfg := Config{ + SocketPath: socketPath + ".load", + Drives: []models.Drive{ + { + DriveID: String("root"), + IsRootDevice: Bool(true), + IsReadOnly: Bool(true), + PathOnHost: String(testRootfs), + }, + }, + } + m, err := NewMachine(ctx, cfg, func(m *Machine) { // Rewriting m.cmd partially wouldn't work since Cmd has // some unexported members @@ -2078,10 +2090,21 @@ func TestLoadSnapshot(t *testing.T) { VMIfName: "eth0", }, } - cfg := createValidConfig(t, fmt.Sprintf("%s.load", socketPath), - withRootDrive(rootfsPath), - withNetworkInterface(networkInterface), - ) + + cfg := Config{ + SocketPath: socketPath + ".load", + Drives: []models.Drive{ + { + DriveID: String("root"), + IsRootDevice: Bool(true), + IsReadOnly: Bool(true), + PathOnHost: String(rootfsPath), + }, + }, + NetworkInterfaces: []NetworkInterface{ + networkInterface, + }, + } cmd := VMCommandBuilder{}.WithSocketPath(fmt.Sprintf("%s.load", socketPath)).WithBin(getFirecrackerBinaryPath()).Build(ctx) diff --git a/opts.go b/opts.go index 38f6ebdb..b6d9c1d0 100644 --- a/opts.go +++ b/opts.go @@ -62,6 +62,7 @@ func WithSnapshot(memFilePath, snapshotPath string, opts ...WithSnapshotOpt) Opt opt(&m.Cfg.Snapshot) } + m.Handlers.Validation = loadSnapshotValidationHandlerList m.Handlers.FcInit = loadSnapshotHandlerList } }