persist: remove VCStore from container

Remove VCStore from container struct.

Signed-off-by: Wei Zhang <weizhang555.zw@gmail.com>

virtcontainers/api_test.go

@@ -139,7 +139,7 @@ func TestCreateSandboxNoopAgentSuccessful(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	sandboxDir := store.SandboxConfigurationRootPath(p.ID())
+	sandboxDir := store.SandboxRuntimeRootPath(p.ID())
 	_, err = os.Stat(sandboxDir)
 	assert.NoError(err)
 }
@@ -176,7 +176,7 @@ func TestCreateSandboxKataAgentSuccessful(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	sandboxDir := store.SandboxConfigurationRootPath(p.ID())
+	sandboxDir := store.SandboxRuntimeRootPath(p.ID())
 	_, err = os.Stat(sandboxDir)
 	assert.NoError(err)
 }
@@ -203,7 +203,7 @@ func TestDeleteSandboxNoopAgentSuccessful(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	sandboxDir := store.SandboxConfigurationRootPath(p.ID())
+	sandboxDir := store.SandboxRuntimeRootPath(p.ID())
 	_, err = os.Stat(sandboxDir)
 	assert.NoError(err)
 
@@ -248,7 +248,7 @@ func TestDeleteSandboxKataAgentSuccessful(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	sandboxDir := store.SandboxConfigurationRootPath(p.ID())
+	sandboxDir := store.SandboxRuntimeRootPath(p.ID())
 	_, err = os.Stat(sandboxDir)
 	assert.NoError(err)
 
@@ -264,7 +264,7 @@ func TestDeleteSandboxFailing(t *testing.T) {
 	defer cleanUp()
 	assert := assert.New(t)
 
-	sandboxDir := store.SandboxConfigurationRootPath(testSandboxID)
+	sandboxDir := store.SandboxRuntimeRootPath(testSandboxID)
 	os.Remove(sandboxDir)
 
 	p, err := DeleteSandbox(context.Background(), testSandboxID)
@@ -328,7 +328,7 @@ func TestStartSandboxFailing(t *testing.T) {
 	defer cleanUp()
 	assert := assert.New(t)
 
-	sandboxDir := store.SandboxConfigurationRootPath(testSandboxID)
+	sandboxDir := store.SandboxRuntimeRootPath(testSandboxID)
 	os.Remove(sandboxDir)
 
 	p, err := StartSandbox(context.Background(), testSandboxID)
@@ -395,7 +395,7 @@ func TestStopSandboxKataAgentSuccessful(t *testing.T) {
 func TestStopSandboxFailing(t *testing.T) {
 	defer cleanUp()
 
-	sandboxDir := store.SandboxConfigurationRootPath(testSandboxID)
+	sandboxDir := store.SandboxRuntimeRootPath(testSandboxID)
 	os.Remove(sandboxDir)
 
 	p, err := StopSandbox(context.Background(), testSandboxID, false)
@@ -413,7 +413,7 @@ func TestRunSandboxNoopAgentSuccessful(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	sandboxDir := store.SandboxConfigurationRootPath(p.ID())
+	sandboxDir := store.SandboxRuntimeRootPath(p.ID())
 	_, err = os.Stat(sandboxDir)
 	assert.NoError(err)
 }
@@ -451,7 +451,7 @@ func TestRunSandboxKataAgentSuccessful(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	sandboxDir := store.SandboxConfigurationRootPath(p.ID())
+	sandboxDir := store.SandboxRuntimeRootPath(p.ID())
 	_, err = os.Stat(sandboxDir)
 	assert.NoError(err)
 
@@ -680,7 +680,7 @@ func TestCreateContainerSuccessful(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	sandboxDir := store.SandboxConfigurationRootPath(p.ID())
+	sandboxDir := store.SandboxRuntimeRootPath(p.ID())
 	_, err = os.Stat(sandboxDir)
 	assert.NoError(err)
 
@@ -734,7 +734,7 @@ func TestDeleteContainerSuccessful(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	sandboxDir := store.SandboxConfigurationRootPath(p.ID())
+	sandboxDir := store.SandboxRuntimeRootPath(p.ID())
 	_, err = os.Stat(sandboxDir)
 	assert.NoError(err)
 
@@ -778,7 +778,7 @@ func TestDeleteContainerFailingNoContainer(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	sandboxDir := store.SandboxConfigurationRootPath(p.ID())
+	sandboxDir := store.SandboxRuntimeRootPath(p.ID())
 	_, err = os.Stat(sandboxDir)
 	assert.NoError(err)
 
@@ -835,7 +835,7 @@ func TestStartContainerFailingNoContainer(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	sandboxDir := store.SandboxConfigurationRootPath(p.ID())
+	sandboxDir := store.SandboxRuntimeRootPath(p.ID())
 	_, err = os.Stat(sandboxDir)
 	assert.NoError(err)
 
@@ -856,7 +856,7 @@ func TestStartContainerFailingSandboxNotStarted(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	sandboxDir := store.SandboxConfigurationRootPath(p.ID())
+	sandboxDir := store.SandboxRuntimeRootPath(p.ID())
 	_, err = os.Stat(sandboxDir)
 	assert.NoError(err)
 
@@ -936,7 +936,7 @@ func TestStopContainerFailingNoContainer(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	sandboxDir := store.SandboxConfigurationRootPath(p.ID())
+	sandboxDir := store.SandboxRuntimeRootPath(p.ID())
 	_, err = os.Stat(sandboxDir)
 	assert.NoError(err)
 
@@ -1040,7 +1040,7 @@ func TestEnterContainerFailingNoContainer(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	sandboxDir := store.SandboxConfigurationRootPath(p.ID())
+	sandboxDir := store.SandboxRuntimeRootPath(p.ID())
 	_, err = os.Stat(sandboxDir)
 	assert.NoError(err)
 
@@ -1093,7 +1093,7 @@ func TestStatusContainerSuccessful(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	sandboxDir := store.SandboxConfigurationRootPath(p.ID())
+	sandboxDir := store.SandboxRuntimeRootPath(p.ID())
 	_, err = os.Stat(sandboxDir)
 	assert.NoError(err)
 
@@ -1136,7 +1136,7 @@ func TestStatusContainerStateReady(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	sandboxDir := store.SandboxConfigurationRootPath(p.ID())
+	sandboxDir := store.SandboxRuntimeRootPath(p.ID())
 	_, err = os.Stat(sandboxDir)
 	assert.NoError(err)
 
@@ -1199,7 +1199,7 @@ func TestStatusContainerStateRunning(t *testing.T) {
 	assert.NoError(err)
 	assert.NotNil(p)
 
-	sandboxDir := store.SandboxConfigurationRootPath(p.ID())
+	sandboxDir := store.SandboxRuntimeRootPath(p.ID())
 	_, err = os.Stat(sandboxDir)
 	assert.NoError(err)
 
@@ -1414,7 +1414,7 @@ func createAndStartSandbox(ctx context.Context, config SandboxConfig) (sandbox V
 		return nil, "", err
 	}
 
-	sandboxDir = store.SandboxConfigurationRootPath(sandbox.ID())
+	sandboxDir = store.SandboxRuntimeRootPath(sandbox.ID())
 	_, err = os.Stat(sandboxDir)
 	if err != nil {
 		return nil, "", err
@@ -1699,7 +1699,7 @@ func TestCleanupContainer(t *testing.T) {
 		CleanupContainer(ctx, p.ID(), c.ID(), true)
 	}
 
-	sandboxDir := store.SandboxConfigurationRootPath(p.ID())
+	sandboxDir := store.SandboxRuntimeRootPath(p.ID())
 
 	_, err = os.Stat(sandboxDir)
 	if err == nil {

virtcontainers/container.go

@@ -338,8 +338,6 @@ type Container struct {
 	systemMountsInfo SystemMountsInfo
 
 	ctx context.Context
-
-	store *store.VCStore
 }
 
 // ID returns the container identifier string.
@@ -391,13 +389,6 @@ func (c *Container) GetPid() int {
 func (c *Container) setStateFstype(fstype string) error {
 	c.state.Fstype = fstype
 
-	if !c.sandbox.supportNewStore() {
-		// experimental runtime use "persist.json" which doesn't need "state.json" anymore
-		if err := c.storeState(); err != nil {
-			return err
-		}
-	}
-
 	return nil
 }
 
@@ -421,49 +412,11 @@ func (c *Container) GetPatchedOCISpec() *specs.Spec {
 
 // storeContainer stores a container config.
 func (c *Container) storeContainer() error {
-	if c.sandbox.supportNewStore() {
 	if err := c.sandbox.Save(); err != nil {
 		return err
 	}
 	return nil
 }
-	return c.store.Store(store.Configuration, *(c.config))
-}
-
-func (c *Container) storeProcess() error {
-	return c.store.Store(store.Process, c.process)
-}
-
-func (c *Container) storeMounts() error {
-	return c.store.Store(store.Mounts, c.mounts)
-}
-
-func (c *Container) storeDevices() error {
-	return c.store.Store(store.DeviceIDs, c.devices)
-}
-
-func (c *Container) storeState() error {
-	return c.store.Store(store.State, c.state)
-}
-
-func (c *Container) loadMounts() ([]Mount, error) {
-	var mounts []Mount
-	if err := c.store.Load(store.Mounts, &mounts); err != nil {
-		return []Mount{}, err
-	}
-
-	return mounts, nil
-}
-
-func (c *Container) loadDevices() ([]ContainerDevice, error) {
-	var devices []ContainerDevice
-
-	if err := c.store.Load(store.DeviceIDs, &devices); err != nil {
-		return []ContainerDevice{}, err
-	}
-
-	return devices, nil
-}
 
 // setContainerState sets both the in-memory and on-disk state of the
 // container.
@@ -476,18 +429,10 @@ func (c *Container) setContainerState(state types.StateString) error {
 	// update in-memory state
 	c.state.State = state
 
-	if c.sandbox.supportNewStore() {
 	// flush data to storage
 	if err := c.sandbox.Save(); err != nil {
 		return err
 	}
-	} else {
-		// experimental runtime use "persist.json" which doesn't need "state.json" anymore
-		// update on-disk state
-		if err := c.store.Store(store.State, c.state); err != nil {
-			return err
-		}
-	}
 
 	return nil
 }
@@ -571,13 +516,6 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) (
 			if err := c.sandbox.devManager.AttachDevice(m.BlockDeviceID, c.sandbox); err != nil {
 				return nil, nil, err
 			}
-
-			if !c.sandbox.supportNewStore() {
-				if err := c.sandbox.storeSandboxDevices(); err != nil {
-					//TODO: roll back?
-					return nil, nil, err
-				}
-			}
 			continue
 		}
 
@@ -620,12 +558,6 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) (
 		sharedDirMounts[sharedDirMount.Destination] = sharedDirMount
 	}
 
-	if !c.sandbox.supportNewStore() {
-		if err := c.storeMounts(); err != nil {
-			return nil, nil, err
-		}
-	}
-
 	return sharedDirMounts, ignoredMounts, nil
 }
 
@@ -751,23 +683,7 @@ func newContainer(sandbox *Sandbox, contConfig *ContainerConfig) (*Container, er
 		ctx:           sandbox.ctx,
 	}
 
-	storeAlreadyExists := store.VCContainerStoreExists(sandbox.ctx, c.sandboxID, c.id)
-	ctrStore, err := store.NewVCContainerStore(sandbox.ctx, c.sandboxID, c.id)
-	if err != nil {
-		return nil, err
-	}
-	defer func() {
-		if err != nil && !storeAlreadyExists {
-			if delerr := c.store.Delete(); delerr != nil {
-				c.Logger().WithError(delerr).WithField("cid", c.id).Error("delete store failed")
-			}
-		}
-	}()
-
-	c.store = ctrStore
-
 	// experimental runtime use "persist.json" instead of legacy "state.json" as storage
-	if c.sandbox.supportNewStore() {
 	err := c.Restore()
 	if err == nil {
 		//container restored
@@ -778,19 +694,8 @@ func newContainer(sandbox *Sandbox, contConfig *ContainerConfig) (*Container, er
 	if !os.IsNotExist(err) && err != errContainerPersistNotExist {
 		return nil, err
 	}
+
 	// Go to next step for first created container
-	} else {
-		state, err := c.store.LoadContainerState()
-		if err == nil {
-			c.state = state
-		}
-
-		var process Process
-		if err := c.store.Load(store.Process, &process); err == nil {
-			c.process = process
-		}
-	}
-
 	if err = c.createMounts(); err != nil {
 		return nil, err
 	}
@@ -803,17 +708,6 @@ func newContainer(sandbox *Sandbox, contConfig *ContainerConfig) (*Container, er
 }
 
 func (c *Container) createMounts() error {
-	// If sandbox supports "newstore", only newly created container can reach this function,
-	// so we don't call restore when `supportNewStore` is true
-	if !c.sandbox.supportNewStore() {
-		mounts, err := c.loadMounts()
-		if err == nil {
-			// restore mounts from disk
-			c.mounts = mounts
-			return nil
-		}
-	}
-
 	// Create block devices for newly created container
 	if err := c.createBlockDevices(); err != nil {
 		return err
@@ -823,18 +717,6 @@ func (c *Container) createMounts() error {
 }
 
 func (c *Container) createDevices(contConfig *ContainerConfig) error {
-	// If sandbox supports "newstore", only newly created container can reach this function,
-	// so we don't call restore when `supportNewStore` is true
-	if !c.sandbox.supportNewStore() {
-		// Devices will be found in storage after create stage has completed.
-		// We load devices from storage at all other stages.
-		storedDevices, err := c.loadDevices()
-		if err == nil {
-			c.devices = storedDevices
-			return nil
-		}
-	}
-
 	// If devices were not found in storage, create Device implementations
 	// from the configuration. This should happen at create.
 	var storedDevices []ContainerDevice
@@ -914,12 +796,6 @@ func (c *Container) create() (err error) {
 	// inside the VM
 	c.getSystemMountInfo()
 
-	if !c.sandbox.supportNewStore() {
-		if err = c.storeDevices(); err != nil {
-			return
-		}
-	}
-
 	process, err := c.sandbox.agent.createContainer(c.sandbox, c)
 	if err != nil {
 		return err
@@ -932,13 +808,6 @@ func (c *Container) create() (err error) {
 		}
 	}
 
-	if !c.sandbox.supportNewStore() {
-		// Store the container process returned by the agent.
-		if err = c.storeProcess(); err != nil {
-			return
-		}
-	}
-
 	if err = c.setContainerState(types.StateReady); err != nil {
 		return
 	}
@@ -964,7 +833,7 @@ func (c *Container) delete() error {
 		}
 	}
 
-	return c.store.Delete()
+	return c.sandbox.storeSandbox()
 }
 
 // checkSandboxRunning validates the container state.
@@ -1383,12 +1252,6 @@ func (c *Container) plugDevice(devicePath string) error {
 		if err := c.sandbox.devManager.AttachDevice(b.DeviceID(), c.sandbox); err != nil {
 			return err
 		}
-
-		if !c.sandbox.supportNewStore() {
-			if err := c.sandbox.storeSandboxDevices(); err != nil {
-				return err
-			}
-		}
 	}
 	return nil
 }
@@ -1419,12 +1282,6 @@ func (c *Container) removeDrive() (err error) {
 				return err
 			}
 		}
-
-		if !c.sandbox.supportNewStore() {
-			if err := c.sandbox.storeSandboxDevices(); err != nil {
-				return err
-			}
-		}
 	}
 
 	return nil
@@ -1439,12 +1296,6 @@ func (c *Container) attachDevices() error {
 			return err
 		}
 	}
-
-	if !c.sandbox.supportNewStore() {
-		if err := c.sandbox.storeSandboxDevices(); err != nil {
-			return err
-		}
-	}
 	return nil
 }
 
@@ -1467,12 +1318,6 @@ func (c *Container) detachDevices() error {
 			}
 		}
 	}
-
-	if !c.sandbox.supportNewStore() {
-		if err := c.sandbox.storeSandboxDevices(); err != nil {
-			return err
-		}
-	}
 	return nil
 }
 

virtcontainers/container_test.go

@@ -341,18 +341,6 @@ func TestContainerAddDriveDir(t *testing.T) {
 		rootFs:  RootFs{Target: fakeRootfs, Mounted: true},
 	}
 
-	containerStore, err := store.NewVCContainerStore(sandbox.ctx, sandbox.id, container.id)
-	assert.Nil(err)
-	container.store = containerStore
-
-	// create state file
-	path := store.ContainerRuntimeRootPath(testSandboxID, container.ID())
-	stateFilePath := filepath.Join(path, store.StateFile)
-	os.Remove(stateFilePath)
-
-	_, err = os.Create(stateFilePath)
-	assert.NoError(err)
-
 	// Make the checkStorageDriver func variable point to a fake check function
 	savedFunc := checkStorageDriver
 	checkStorageDriver = func(major, minor int) (bool, error) {
@@ -405,11 +393,6 @@ func TestContainerRootfsPath(t *testing.T) {
 		rootFs:       RootFs{Target: fakeRootfs, Mounted: true},
 		rootfsSuffix: "rootfs",
 	}
-	cvcstore, err := store.NewVCContainerStore(context.Background(),
-		sandbox.id,
-		container.id)
-	assert.Nil(t, err)
-	container.store = cvcstore
 
 	container.hotplugDrive()
 	assert.Empty(t, container.rootfsSuffix)

virtcontainers/kata_agent.go

@@ -1444,13 +1444,6 @@ func (k *kataAgent) handleBlockVolumes(c *Container) ([]*grpc.Storage, error) {
 		// device is detached with detachDevices() for a container.
 		c.devices = append(c.devices, ContainerDevice{ID: id, ContainerPath: m.Destination})
 
-		if !c.sandbox.supportNewStore() {
-			if err := c.storeDevices(); err != nil {
-				k.Logger().WithField("device", id).WithError(err).Error("store device failed")
-				return nil, err
-			}
-		}
-
 		vol := &grpc.Storage{}
 
 		device := c.sandbox.devManager.GetDeviceByID(id)

virtcontainers/persist/api/interface.go

@@ -13,5 +13,8 @@ type PersistDriver interface {
 	// We only support get data for one whole sandbox
 	FromDisk(sid string) (SandboxState, map[string]ContainerState, error)
 	// Destroy will remove everything from storage
-	Destroy() error
+	Destroy(sid string) error
+	// Lock locks the persist driver, "exclusive" decides whether the lock is exclusive or shared.
+	// It returns Unlock Function and errors
+	Lock(sid string, exclusive bool) (func() error, error)
 }

virtcontainers/persist/fs/fs.go

@@ -51,8 +51,6 @@ var RunStoragePath = func() string {
 type FS struct {
 	sandboxState   *persistapi.SandboxState
 	containerState map[string]persistapi.ContainerState
-
-	lockFile *os.File
 }
 
 var fsLog = logrus.WithField("source", "virtcontainers/persist/fs")
@@ -77,21 +75,21 @@ func Init() (persistapi.PersistDriver, error) {
 	}, nil
 }
 
-func (fs *FS) sandboxDir() (string, error) {
+func (fs *FS) sandboxDir(sandboxID string) (string, error) {
-	id := fs.sandboxState.SandboxContainer
+	return filepath.Join(RunStoragePath(), sandboxID), nil
-	if id == "" {
-		return "", fmt.Errorf("sandbox container id required")
-	}
-
-	return filepath.Join(RunStoragePath(), id), nil
 }
 
 // ToDisk sandboxState and containerState to disk
 func (fs *FS) ToDisk(ss persistapi.SandboxState, cs map[string]persistapi.ContainerState) (retErr error) {
+	id := ss.SandboxContainer
+	if id == "" {
+		return fmt.Errorf("sandbox container id required")
+	}
+
 	fs.sandboxState = &ss
 	fs.containerState = cs
 
-	sandboxDir, err := fs.sandboxDir()
+	sandboxDir, err := fs.sandboxDir(id)
 	if err != nil {
 		return err
 	}
@@ -100,15 +98,10 @@ func (fs *FS) ToDisk(ss persistapi.SandboxState, cs map[string]persistapi.Contai
 		return err
 	}
 
-	if err := fs.lock(); err != nil {
-		return err
-	}
-	defer fs.unlock()
-
 	// if error happened, destroy all dirs
 	defer func() {
 		if retErr != nil {
-			if err := fs.Destroy(); err != nil {
+			if err := fs.Destroy(id); err != nil {
 				fs.Logger().WithError(err).Errorf("failed to destroy dirs")
 			}
 		}
@@ -155,6 +148,27 @@ func (fs *FS) ToDisk(ss persistapi.SandboxState, cs map[string]persistapi.Contai
 		}
 	}
 
+	// Walk sandbox dir and find container.
+	files, err := ioutil.ReadDir(sandboxDir)
+	if err != nil {
+		return err
+	}
+
+	// Remove non-existing containers
+	for _, file := range files {
+		if !file.IsDir() {
+			continue
+		}
+		// Container dir exists.
+		cid := file.Name()
+
+		// Container should be removed when container id doesn't exist in cs.
+		if _, ok := cs[cid]; !ok {
+			if err := os.RemoveAll(filepath.Join(sandboxDir, cid)); err != nil {
+				return err
+			}
+		}
+	}
 	return nil
 }
 
@@ -165,18 +179,11 @@ func (fs *FS) FromDisk(sid string) (persistapi.SandboxState, map[string]persista
 		return ss, nil, fmt.Errorf("restore requires sandbox id")
 	}
 
-	fs.sandboxState.SandboxContainer = sid
+	sandboxDir, err := fs.sandboxDir(sid)
-
-	sandboxDir, err := fs.sandboxDir()
 	if err != nil {
 		return ss, nil, err
 	}
 
-	if err := fs.lock(); err != nil {
-		return ss, nil, err
-	}
-	defer fs.unlock()
-
 	// get sandbox configuration from persist data
 	sandboxFile := filepath.Join(sandboxDir, persistFile)
 	f, err := os.OpenFile(sandboxFile, os.O_RDONLY, fileMode)
@@ -224,8 +231,12 @@ func (fs *FS) FromDisk(sid string) (persistapi.SandboxState, map[string]persista
 }
 
 // Destroy removes everything from disk
-func (fs *FS) Destroy() error {
+func (fs *FS) Destroy(sandboxID string) error {
-	sandboxDir, err := fs.sandboxDir()
+	if sandboxID == "" {
+		return fmt.Errorf("sandbox container id required")
+	}
+
+	sandboxDir, err := fs.sandboxDir(sandboxID)
 	if err != nil {
 		return err
 	}
@@ -236,40 +247,43 @@ func (fs *FS) Destroy() error {
 	return nil
 }
 
-func (fs *FS) lock() error {
+func (fs *FS) Lock(sandboxID string, exclusive bool) (func() error, error) {
-	sandboxDir, err := fs.sandboxDir()
+	if sandboxID == "" {
+		return nil, fmt.Errorf("sandbox container id required")
+	}
+
+	sandboxDir, err := fs.sandboxDir(sandboxID)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	f, err := os.Open(sandboxDir)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
-	if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX|syscall.LOCK_NB); err != nil {
+	var lockType int
+	if exclusive {
+		lockType = syscall.LOCK_EX | syscall.LOCK_NB
+	} else {
+		lockType = syscall.LOCK_SH | syscall.LOCK_NB
+	}
+
+	if err := syscall.Flock(int(f.Fd()), lockType); err != nil {
 		f.Close()
-		return err
+		return nil, err
-	}
-	fs.lockFile = f
-
-	return nil
 	}
 
-func (fs *FS) unlock() error {
+	unlockFunc := func() error {
-	if fs.lockFile == nil {
+		defer f.Close()
-		return nil
+		if err := syscall.Flock(int(f.Fd()), syscall.LOCK_UN); err != nil {
-	}
-
-	lockFile := fs.lockFile
-	defer lockFile.Close()
-	fs.lockFile = nil
-	if err := syscall.Flock(int(lockFile.Fd()), syscall.LOCK_UN); err != nil {
 			return err
 		}
 
 		return nil
 	}
+	return unlockFunc, nil
+}
 
 // TestSetRunStoragePath set RunStoragePath to path
 // this function is only used for testing purpose

virtcontainers/persist/fs/fs_test.go

@@ -40,18 +40,25 @@ func TestFsLock(t *testing.T) {
 		os.RemoveAll(testDir)
 	}()
 
-	fs.sandboxState.SandboxContainer = "test-fs-driver"
+	sid := "test-fs-driver"
-	sandboxDir, err := fs.sandboxDir()
+	fs.sandboxState.SandboxContainer = sid
+	sandboxDir, err := fs.sandboxDir(sid)
 	assert.Nil(t, err)
 
 	err = os.MkdirAll(sandboxDir, dirMode)
 	assert.Nil(t, err)
 
-	assert.Nil(t, fs.lock())
+	unlockFunc, err := fs.Lock(sid, false)
-	assert.NotNil(t, fs.lock())
+	assert.Nil(t, err)
+	unlockFunc2, err := fs.Lock(sid, false)
+	assert.Nil(t, err)
+	_, err = fs.Lock(sid, true)
+	assert.NotNil(t, err)
 
-	assert.Nil(t, fs.unlock())
+	assert.Nil(t, unlockFunc())
-	assert.Nil(t, fs.unlock())
+	// double unlock should return error
+	assert.NotNil(t, unlockFunc())
+	assert.Nil(t, unlockFunc2())
 }
 
 func TestFsDriver(t *testing.T) {
@@ -88,7 +95,7 @@ func TestFsDriver(t *testing.T) {
 	assert.Equal(t, ss.SandboxContainer, id)
 	assert.Equal(t, ss.State, "")
 
-	// flush all to disk
+	// flush all to disk.
 	ss.State = "running"
 	assert.Nil(t, fs.ToDisk(ss, cs))
 	ss, cs, err = fs.FromDisk(id)
@@ -99,9 +106,31 @@ func TestFsDriver(t *testing.T) {
 	assert.Equal(t, ss.SandboxContainer, id)
 	assert.Equal(t, ss.State, "running")
 
-	assert.Nil(t, fs.Destroy())
+	// add new container state.
+	cs["test-container"] = persistapi.ContainerState{
+		State: "ready",
+	}
+	assert.Nil(t, fs.ToDisk(ss, cs))
+	ss, cs, err = fs.FromDisk(id)
+	assert.Nil(t, err)
+	assert.NotNil(t, ss)
+	assert.Equal(t, len(cs), 1)
+	c, ok := cs["test-container"]
+	assert.True(t, ok)
+	assert.Equal(t, c.State, "ready")
 
-	dir, err := fs.sandboxDir()
+	// clean all container.
+	cs = make(map[string]persistapi.ContainerState)
+	assert.Nil(t, fs.ToDisk(ss, cs))
+	ss, cs, err = fs.FromDisk(id)
+	assert.Nil(t, err)
+	assert.NotNil(t, ss)
+	assert.Equal(t, len(cs), 0)
+
+	// destroy whole sandbox dir.
+	assert.Nil(t, fs.Destroy(id))
+
+	dir, err := fs.sandboxDir(id)
 	assert.Nil(t, err)
 	assert.NotEqual(t, len(dir), 0)
 

virtcontainers/sandbox.go

@@ -597,24 +597,10 @@ func (s *Sandbox) storeSandbox() error {
 	span, _ := s.trace("storeSandbox")
 	defer span.Finish()
 
-	if s.supportNewStore() {
 	// flush data to storage
 	if err := s.Save(); err != nil {
 		return err
 	}
-	} else {
-		err := s.store.Store(store.Configuration, *(s.config))
-		if err != nil {
-			return err
-		}
-
-		for _, container := range s.containers {
-			err = container.store.Store(store.Configuration, *(container.config))
-			if err != nil {
-				return err
-			}
-		}
-	}
 	return nil
 }
 
@@ -777,7 +763,7 @@ func (s *Sandbox) Delete() error {
 	s.agent.cleanup(s)
 
 	if s.supportNewStore() {
-		if err := s.newStore.Destroy(); err != nil {
+		if err := s.newStore.Destroy(s.id); err != nil {
 			return err
 		}
 	}
@@ -1070,7 +1056,6 @@ func (s *Sandbox) stopVM() error {
 	if s.disableVMShutdown {
 		// Do not kill the VM - allow the agent to shut it down
 		// (only used to support static agent tracing).
-		s.Logger().Info("Not stopping VM")
 		return nil
 	}
 
@@ -1118,7 +1103,6 @@ func (s *Sandbox) fetchContainers() error {
 // This should be called only when the sandbox is already created.
 // It will add new container config to sandbox.config.Containers
 func (s *Sandbox) CreateContainer(contConfig ContainerConfig) (VCContainer, error) {
-	storeAlreadyExists := store.VCContainerStoreExists(s.ctx, s.id, contConfig.ID)
 	// Create the container.
 	c, err := newContainer(s, &contConfig)
 	if err != nil {
@@ -1134,11 +1118,6 @@ func (s *Sandbox) CreateContainer(contConfig ContainerConfig) (VCContainer, erro
 				// delete container config
 				s.config.Containers = s.config.Containers[:len(s.config.Containers)-1]
 			}
-			if !storeAlreadyExists {
-				if delerr := c.store.Delete(); delerr != nil {
-					c.Logger().WithError(delerr).WithField("cid", c.id).Error("delete store failed")
-				}
-			}
 		}
 	}()
 
@@ -1152,6 +1131,13 @@ func (s *Sandbox) CreateContainer(contConfig ContainerConfig) (VCContainer, erro
 		return nil, err
 	}
 
+	defer func() {
+		// Rollback if error happens.
+		if err != nil {
+			s.removeContainer(c.id)
+		}
+	}()
+
 	// Sandbox is reponsable to update VM resources needed by Containers
 	// Update resources after having added containers to the sandbox, since
 	// container status is requiered to know if more resources should be added.
@@ -1160,12 +1146,6 @@ func (s *Sandbox) CreateContainer(contConfig ContainerConfig) (VCContainer, erro
 		return nil, err
 	}
 
-	// Store it.
-	err = c.storeContainer()
-	if err != nil {
-		return nil, err
-	}
-
 	if err = s.cgroupsUpdate(); err != nil {
 		return nil, err
 	}

virtcontainers/sandbox_test.go

@@ -7,7 +7,6 @@ package virtcontainers
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -668,17 +667,6 @@ func TestContainerStateSetFstype(t *testing.T) {
 	cImpl, ok := c.(*Container)
 	assert.True(ok)
 
-	containerStore, err := store.NewVCContainerStore(sandbox.ctx, sandbox.id, c.ID())
-	assert.NoError(err)
-
-	cImpl.store = containerStore
-
-	path := store.ContainerRuntimeRootPath(testSandboxID, c.ID())
-	stateFilePath := filepath.Join(path, store.StateFile)
-
-	f, err := os.Create(stateFilePath)
-	assert.NoError(err)
-
 	state := types.ContainerState{
 		State:  "ready",
 		Fstype: "vfs",
@@ -686,34 +674,10 @@ func TestContainerStateSetFstype(t *testing.T) {
 
 	cImpl.state = state
 
-	stateData := `{
-		"state":"ready",
-		"fstype":"vfs",
-	}`
-
-	n, err := f.WriteString(stateData)
-	defer f.Close()
-	assert.NoError(err)
-	assert.Equal(n, len(stateData))
-
 	newFstype := "ext4"
 	err = cImpl.setStateFstype(newFstype)
 	assert.NoError(err)
 	assert.Equal(cImpl.state.Fstype, newFstype)
-
-	fileData, err := ioutil.ReadFile(stateFilePath)
-	assert.NoError(err)
-
-	// experimental features doesn't write state.json
-	if sandbox.supportNewStore() {
-		return
-	}
-
-	var res types.ContainerState
-	err = json.Unmarshal([]byte(string(fileData)), &res)
-	assert.NoError(err)
-	assert.Equal(res.Fstype, newFstype)
-	assert.Equal(res.State, state.State)
 }
 
 const vfioPath = "/dev/vfio/"
@@ -916,8 +880,6 @@ func TestCreateContainer(t *testing.T) {
 	_, err = s.CreateContainer(contConfig)
 	assert.NotNil(t, err, "Should failed to create a duplicated container")
 	assert.Equal(t, len(s.config.Containers), 1, "Container config list length from sandbox structure should be 1")
-	ret := store.VCContainerStoreExists(s.ctx, testSandboxID, contID)
-	assert.True(t, ret, "Should not delete container store that already existed")
 }
 
 func TestDeleteContainer(t *testing.T) {
@@ -1023,8 +985,6 @@ func TestDeleteStoreWhenCreateContainerFail(t *testing.T) {
 	s.state.CgroupPath = filepath.Join(testDir, "bad-cgroup")
 	_, err = s.CreateContainer(contConfig)
 	assert.NotNil(t, err, "Should fail to create container due to wrong cgroup")
-	ret := store.VCContainerStoreExists(s.ctx, testSandboxID, contID)
-	assert.False(t, ret, "Should delete configuration root after failed to create a container")
 }
 
 func TestDeleteStoreWhenNewContainerFail(t *testing.T) {
@@ -1307,10 +1267,6 @@ func TestPreAddDevice(t *testing.T) {
 	}
 	container.state.State = types.StateReady
 
-	containerStore, err := store.NewVCContainerStore(sandbox.ctx, sandbox.id, container.id)
-	assert.Nil(t, err)
-	container.store = containerStore
-
 	// create state file
 	path := store.ContainerRuntimeRootPath(testSandboxID, container.ID())
 	err = os.MkdirAll(path, store.DirMode)

virtcontainers/store/vc.go

@@ -334,9 +334,3 @@ func VCSandboxStoreExists(ctx context.Context, sandboxID string) bool {
 	s := stores.findStore(SandboxConfigurationRoot(sandboxID))
 	return s != nil
 }
-
-// VCContainerStoreExists returns true if a container store already exists.
-func VCContainerStoreExists(ctx context.Context, sandboxID string, containerID string) bool {
-	s := stores.findStore(ContainerConfigurationRoot(sandboxID, containerID))
-	return s != nil
-}

virtcontainers/virtcontainers_test.go

@@ -175,7 +175,7 @@ func TestMain(m *testing.M) {
 	// allow the tests to run without affecting the host system.
 	store.ConfigStoragePath = func() string { return filepath.Join(testDir, store.StoragePathSuffix, "config") }
 	store.RunStoragePath = func() string { return filepath.Join(testDir, store.StoragePathSuffix, "run") }
-	fs.TestSetRunStoragePath(filepath.Join(testDir, "vc", "sbs"))
+	fs.TestSetRunStoragePath(filepath.Join(testDir, "vc", "run"))
 
 	defer func() {
 		store.ConfigStoragePath = ConfigStoragePathSaved