storage: address comments

Address some comments:
* fix persist driver func names for better understanding
* modify some logic, add some returned error etc

Signed-off-by: Wei Zhang <[email protected]>

virtcontainers/container.go

@@ -384,7 +384,7 @@ func (c *Container) GetAnnotations() map[string]string {
 
 // storeContainer stores a container config.
 func (c *Container) storeContainer() error {
-	if err := c.sandbox.newStore.Dump(); err != nil {
+	if err := c.sandbox.newStore.ToDisk(); err != nil {
 		return err
 	}
 	return c.store.Store(store.Configuration, *(c.config))
@@ -442,7 +442,8 @@ func (c *Container) setContainerState(state types.StateString) error {
 		return err
 	}
 
-	if err = c.sandbox.newStore.Dump(); err != nil {
+	// flush data to storage
+	if err = c.sandbox.newStore.ToDisk(); err != nil {
 		return err
 	}
 	return nil

virtcontainers/persist.go

@@ -14,7 +14,6 @@ import (
 )
 
 var (
-	errSandboxPersistNotExist   = errors.New("sandbox doesn't exist in persist data")
 	errContainerPersistNotExist = errors.New("container doesn't exist in persist data")
 )
 
@@ -93,50 +92,35 @@ func (s *Sandbox) dumpDevices(ss *persistapi.SandboxState, cs map[string]persist
 	return nil
 }
 
-// PersistVersion set persist data version to current version in runtime
-func (s *Sandbox) persistVersion() {
-	s.newStore.RegisterHook("version", func(ss *persistapi.SandboxState, cs map[string]persistapi.ContainerState) error {
+// versionCallback set persist data version to current version in runtime
+func (s *Sandbox) verSaveCallback() {
+	s.newStore.AddSaveCallback("version", func(ss *persistapi.SandboxState, cs map[string]persistapi.ContainerState) error {
 		ss.PersistVersion = persistapi.CurPersistVersion
 		return nil
 	})
 }
 
 // PersistState register hook to set sandbox and container state to persist
-func (s *Sandbox) persistState() {
-	s.newStore.RegisterHook("state", s.dumpState)
+func (s *Sandbox) stateSaveCallback() {
+	s.newStore.AddSaveCallback("state", s.dumpState)
 }
 
 // PersistHvState register hook to save hypervisor state to persist data
-func (s *Sandbox) persistHvState() {
-	s.newStore.RegisterHook("hypervisor", s.dumpHypervisor)
+func (s *Sandbox) hvStateSaveCallback() {
+	s.newStore.AddSaveCallback("hypervisor", s.dumpHypervisor)
 }
 
 // PersistDevices register hook to save device informations
-func (s *Sandbox) persistDevices() {
-	s.newStore.RegisterHook("devices", s.dumpDevices)
+func (s *Sandbox) devicesSaveCallback() {
+	s.newStore.AddSaveCallback("devices", s.dumpDevices)
 }
 
 func (s *Sandbox) getSbxAndCntStates() (*persistapi.SandboxState, map[string]persistapi.ContainerState, error) {
-	ss, cs, err := s.newStore.GetStates()
-	if err != nil {
+	if err := s.newStore.Restore(s.id); err != nil {
 		return nil, nil, err
 	}
 
-	if len(cs) == 0 {
-		if err := s.newStore.Restore(s.id); err != nil {
-			return nil, nil, err
-		}
-
-		ss, cs, err = s.newStore.GetStates()
-		if err != nil {
-			return nil, nil, err
-		}
-
-		if len(cs) == 0 {
-			return nil, nil, errSandboxPersistNotExist
-		}
-	}
-	return ss, cs, nil
+	return s.newStore.GetStates()
 }
 
 // Restore will restore sandbox data from persist file on disk

virtcontainers/persist/api/interface.go

@@ -7,10 +7,17 @@ package persistapi
 
 // PersistDriver is interface describing operations to save/restore persist data
 type PersistDriver interface {
-	// Dump persist data to
-	Dump() error
+	// ToDisk flushes data to disk(or other storage media such as a remote db)
+	ToDisk() error
+	// AddSaveCallback addes callback function named `name` to driver storage list
+	// The callback functions will be invoked when calling `ToDisk()`, notice that
+	// callback functions are not order guaranteed,
+	AddSaveCallback(name string, f SetFunc)
+	// Restore will restore all data for sandbox with `sid` from storage.
+	// We only support get data for one whole sandbox
 	Restore(sid string) error
+	// Destroy will remove everything from storage
 	Destroy() error
+	// GetStates will return SandboxState and ContainerState(s) directly
 	GetStates() (*SandboxState, map[string]ContainerState, error)
-	RegisterHook(name string, f SetFunc)
 }

virtcontainers/persist/fs/fs.go

@@ -70,8 +70,8 @@ func (fs *FS) sandboxDir() (string, error) {
 	return filepath.Join(runStoragePath, fs.sandboxState.SandboxContainer), nil
 }
 
-// Dump sandboxState and containerState to disk
-func (fs *FS) Dump() (retErr error) {
+// ToDisk sandboxState and containerState to disk
+func (fs *FS) ToDisk() (retErr error) {
 	// call registered hooks to set sandboxState and containerState
 	for _, fun := range fs.setFuncs {
 		fun(fs.sandboxState, fs.containerState)
@@ -214,8 +214,8 @@ func (fs *FS) GetStates() (*persistapi.SandboxState, map[string]persistapi.Conta
 	return fs.sandboxState, fs.containerState, nil
 }
 
-// RegisterHook registers processing hooks for Dump
-func (fs *FS) RegisterHook(name string, f persistapi.SetFunc) {
+// AddSaveCallback registers processing hooks for Dump
+func (fs *FS) AddSaveCallback(name string, f persistapi.SetFunc) {
 	// only accept last registered hook with same name
 	fs.setFuncs[name] = f
 }

virtcontainers/persist/manager.go

@@ -16,9 +16,9 @@ type initFunc (func() (persistapi.PersistDriver, error))
 
 var (
 	supportedDrivers = map[string]initFunc{
+
 		"fs": fs.Init,
 	}
-	defaultDriver = "fs"
 )
 
 // GetDriver returns new PersistDriver according to driver name

virtcontainers/sandbox.go

@@ -477,10 +477,10 @@ func createSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Fac
 	}
 	s.devManager = deviceManager.NewDeviceManager(sandboxConfig.HypervisorConfig.BlockDeviceDriver, devices)
 
-	// register persist hook for now, data will be written to disk by Dump()
-	s.persistState()
-	s.persistHvState()
-	s.persistDevices()
+	// register persist hook for now, data will be written to disk by ToDisk()
+	s.stateSaveCallback()
+	s.hvStateSaveCallback()
+	s.devicesSaveCallback()
 
 	if err := s.Restore(); err == nil && s.state.State != "" {
 		return s, nil
@@ -498,7 +498,7 @@ func createSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Fac
 
 	// if sandbox doesn't exist, set persist version to current version
 	// otherwise do nothing
-	s.persistVersion()
+	s.verSaveCallback()
 
 	// Below code path is called only during create, because of earlier check.
 	if err := s.agent.createSandbox(s); err != nil {
@@ -608,7 +608,8 @@ func (s *Sandbox) storeSandbox() error {
 		}
 	}
 
-	if err = s.newStore.Dump(); err != nil {
+	// flush data to storage
+	if err = s.newStore.ToDisk(); err != nil {
 		return err
 	}