newstore: remove file "devices.json"

When using experimental feature "newstore", we save and load devices
information from `persist.json` instead of `devices.json`, in such case,
file `devices.json` isn't needed anymore, so remove it.

Signed-off-by: Wei Zhang <zhangwei555@huawei.com>

virtcontainers/api.go

@@ -590,6 +590,10 @@ func statusContainer(sandbox *Sandbox, containerID string) (ContainerStatus, err
 				}
 
 				if !running {
+					virtLog.WithFields(logrus.Fields{
+						"state":       container.state.State,
+						"process pid": container.process.Pid}).
+						Info("container isn't running")
 					if err := container.stop(); err != nil {
 						return ContainerStatus{}, err
 					}

virtcontainers/container.go

@@ -540,9 +540,11 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) (
 				return nil, nil, err
 			}
 
-			if err := c.sandbox.storeSandboxDevices(); err != nil {
+			if !c.sandbox.supportNewStore() {
-				//TODO: roll back?
+				if err := c.sandbox.storeSandboxDevices(); err != nil {
-				return nil, nil, err
+					//TODO: roll back?
+					return nil, nil, err
+				}
 			}
 			continue
 		}
@@ -712,11 +714,6 @@ func newContainer(sandbox *Sandbox, contConfig ContainerConfig) (*Container, err
 		}
 	}
 
-	if err := c.Restore(); err != nil &&
-		!os.IsNotExist(err) && err != errContainerPersistNotExist {
-		return nil, err
-	}
-
 	var process Process
 	if err := c.store.Load(store.Process, &process); err == nil {
 		c.process = process
@@ -1012,6 +1009,16 @@ func (c *Container) stop() error {
 		return err
 	}
 
+	defer func() {
+		// Save device and drive data.
+		// TODO: can we merge this saving with setContainerState()?
+		if c.sandbox.supportNewStore() {
+			if err := c.sandbox.Save(); err != nil {
+				c.Logger().WithError(err).Info("save container state failed")
+			}
+		}
+	}()
+
 	if err := c.sandbox.agent.stopContainer(c.sandbox, *c); err != nil {
 		return err
 	}
@@ -1273,8 +1280,10 @@ func (c *Container) plugDevice(devicePath string) error {
 			return err
 		}
 
-		if err := c.sandbox.storeSandboxDevices(); err != nil {
+		if !c.sandbox.supportNewStore() {
-			return err
+			if err := c.sandbox.storeSandboxDevices(); err != nil {
+				return err
+			}
 		}
 	}
 	return nil
@@ -1307,8 +1316,10 @@ func (c *Container) removeDrive() (err error) {
 			}
 		}
 
-		if err := c.sandbox.storeSandboxDevices(); err != nil {
+		if !c.sandbox.supportNewStore() {
-			return err
+			if err := c.sandbox.storeSandboxDevices(); err != nil {
+				return err
+			}
 		}
 	}
 
@@ -1325,8 +1336,10 @@ func (c *Container) attachDevices() error {
 		}
 	}
 
-	if err := c.sandbox.storeSandboxDevices(); err != nil {
+	if !c.sandbox.supportNewStore() {
-		return err
+		if err := c.sandbox.storeSandboxDevices(); err != nil {
+			return err
+		}
 	}
 	return nil
 }
@@ -1351,8 +1364,10 @@ func (c *Container) detachDevices() error {
 		}
 	}
 
-	if err := c.sandbox.storeSandboxDevices(); err != nil {
+	if !c.sandbox.supportNewStore() {
-		return err
+		if err := c.sandbox.storeSandboxDevices(); err != nil {
+			return err
+		}
 	}
 	return nil
 }

virtcontainers/container_test.go

@@ -94,6 +94,7 @@ func TestContainerRemoveDrive(t *testing.T) {
 		ctx:        context.Background(),
 		id:         "sandbox",
 		devManager: manager.NewDeviceManager(manager.VirtioSCSI, nil),
+		config:     &SandboxConfig{},
 	}
 
 	vcStore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)

virtcontainers/device/api/interface.go

@@ -45,28 +45,37 @@ type DeviceReceiver interface {
 type Device interface {
 	Attach(DeviceReceiver) error
 	Detach(DeviceReceiver) error
+
 	// ID returns device identifier
 	DeviceID() string
+
 	// DeviceType indicates which kind of device it is
 	// e.g. block, vfio or vhost user
 	DeviceType() config.DeviceType
+
 	// GetMajorMinor returns major and minor numbers
 	GetMajorMinor() (int64, int64)
+
 	// GetDeviceInfo returns device specific data used for hotplugging by hypervisor
 	// Caller could cast the return value to device specific struct
 	// e.g. Block device returns *config.BlockDrive and
 	// vfio device returns []*config.VFIODev
 	GetDeviceInfo() interface{}
+
 	// GetAttachCount returns how many times the device has been attached
 	GetAttachCount() uint
 
 	// Reference adds one reference to device then returns final ref count
 	Reference() uint
+
 	// Dereference removes one reference to device then returns final ref count
 	Dereference() uint
 
-	// Persist convert and return data in persist format
+	// Save converts Device to DeviceState
-	Dump() persistapi.DeviceState
+	Save() persistapi.DeviceState
+
+	// Load loads DeviceState and converts it to specific device
+	Load(persistapi.DeviceState)
 }
 
 // DeviceManager can be used to create a new device, this can be used as single
@@ -79,4 +88,5 @@ type DeviceManager interface {
 	IsDeviceAttached(string) bool
 	GetDeviceByID(string) Device
 	GetAllDevices() []Device
+	LoadDevices([]persistapi.DeviceState)
 }

virtcontainers/device/drivers/block.go

@@ -1,5 +1,5 @@
 // Copyright (c) 2017-2018 Intel Corporation
-// Copyright (c) 2018 Huawei Corporation
+// Copyright (c) 2018-2019 Huawei Corporation
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -147,9 +147,9 @@ func (device *BlockDevice) GetDeviceInfo() interface{} {
 	return device.BlockDrive
 }
 
-// Dump convert and return data in persist format
+// Save converts Device to DeviceState
-func (device *BlockDevice) Dump() persistapi.DeviceState {
+func (device *BlockDevice) Save() persistapi.DeviceState {
-	ds := device.GenericDevice.Dump()
+	ds := device.GenericDevice.Save()
 	ds.Type = string(device.DeviceType())
 
 	drive := device.BlockDrive
@@ -169,5 +169,27 @@ func (device *BlockDevice) Dump() persistapi.DeviceState {
 	return ds
 }
 
+// Load loads DeviceState and converts it to specific device
+func (device *BlockDevice) Load(ds persistapi.DeviceState) {
+	device.GenericDevice = &GenericDevice{}
+	device.GenericDevice.Load(ds)
+
+	bd := ds.BlockDrive
+	if bd == nil {
+		return
+	}
+	device.BlockDrive = &config.BlockDrive{
+		File:     bd.File,
+		Format:   bd.Format,
+		ID:       bd.ID,
+		Index:    bd.Index,
+		MmioAddr: bd.MmioAddr,
+		PCIAddr:  bd.PCIAddr,
+		SCSIAddr: bd.SCSIAddr,
+		NvdimmID: bd.NvdimmID,
+		VirtPath: bd.VirtPath,
+	}
+}
+
 // It should implement GetAttachCount() and DeviceID() as api.Device implementation
 // here it shares function from *GenericDevice so we don't need duplicate codes

virtcontainers/device/drivers/generic.go

@@ -1,5 +1,5 @@
 // Copyright (c) 2017-2018 Intel Corporation
-// Copyright (c) 2018 Huawei Corporation
+// Copyright (c) 2018-2019 Huawei Corporation
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -117,8 +117,8 @@ func (device *GenericDevice) bumpAttachCount(attach bool) (skip bool, err error)
 	}
 }
 
-// Dump convert and return data in persist format
+// Save converts Device to DeviceState
-func (device *GenericDevice) Dump() persistapi.DeviceState {
+func (device *GenericDevice) Save() persistapi.DeviceState {
 	dss := persistapi.DeviceState{
 		ID:          device.ID,
 		Type:        string(device.DeviceType()),
@@ -135,3 +135,17 @@ func (device *GenericDevice) Dump() persistapi.DeviceState {
 	}
 	return dss
 }
+
+// Load loads DeviceState and converts it to specific device
+func (device *GenericDevice) Load(ds persistapi.DeviceState) {
+	device.ID = ds.ID
+	device.RefCount = ds.RefCount
+	device.AttachCount = ds.AttachCount
+
+	device.DeviceInfo = &config.DeviceInfo{
+		DevType:       ds.DevType,
+		Major:         ds.Major,
+		Minor:         ds.Minor,
+		DriverOptions: ds.DriverOptions,
+	}
+}

virtcontainers/device/drivers/vfio.go

@@ -1,5 +1,5 @@
 // Copyright (c) 2017-2018 Intel Corporation
-// Copyright (c) 2018 Huawei Corporation
+// Copyright (c) 2018-2019 Huawei Corporation
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -140,9 +140,9 @@ func (device *VFIODevice) GetDeviceInfo() interface{} {
 	return device.VfioDevs
 }
 
-// Dump convert and return data in persist format
+// Save converts Device to DeviceState
-func (device *VFIODevice) Dump() persistapi.DeviceState {
+func (device *VFIODevice) Save() persistapi.DeviceState {
-	ds := device.GenericDevice.Dump()
+	ds := device.GenericDevice.Save()
 	ds.Type = string(device.DeviceType())
 
 	devs := device.VfioDevs
@@ -150,7 +150,7 @@ func (device *VFIODevice) Dump() persistapi.DeviceState {
 		if dev != nil {
 			ds.VFIODevs = append(ds.VFIODevs, &persistapi.VFIODev{
 				ID:       dev.ID,
-				Type:     string(dev.Type),
+				Type:     uint32(dev.Type),
 				BDF:      dev.BDF,
 				SysfsDev: dev.SysfsDev,
 			})
@@ -159,6 +159,21 @@ func (device *VFIODevice) Dump() persistapi.DeviceState {
 	return ds
 }
 
+// Load loads DeviceState and converts it to specific device
+func (device *VFIODevice) Load(ds persistapi.DeviceState) {
+	device.GenericDevice = &GenericDevice{}
+	device.GenericDevice.Load(ds)
+
+	for _, dev := range ds.VFIODevs {
+		device.VfioDevs = append(device.VfioDevs, &config.VFIODev{
+			ID:       dev.ID,
+			Type:     config.VFIODeviceType(dev.Type),
+			BDF:      dev.BDF,
+			SysfsDev: dev.SysfsDev,
+		})
+	}
+}
+
 // It should implement GetAttachCount() and DeviceID() as api.Device implementation
 // here it shares function from *GenericDevice so we don't need duplicate codes
 

virtcontainers/device/drivers/vhost_user_blk.go

@@ -1,5 +1,5 @@
 // Copyright (c) 2017-2018 Intel Corporation
-// Copyright (c) 2018 Huawei Corporation
+// Copyright (c) 2018-2019 Huawei Corporation
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -72,9 +72,9 @@ func (device *VhostUserBlkDevice) GetDeviceInfo() interface{} {
 	return &device.VhostUserDeviceAttrs
 }
 
-// Dump convert and return data in persist format
+// Save converts Device to DeviceState
-func (device *VhostUserBlkDevice) Dump() persistapi.DeviceState {
+func (device *VhostUserBlkDevice) Save() persistapi.DeviceState {
-	ds := device.GenericDevice.Dump()
+	ds := device.GenericDevice.Save()
 	ds.Type = string(device.DeviceType())
 	ds.VhostUserDev = &persistapi.VhostUserDeviceAttrs{
 		DevID:      device.DevID,
@@ -85,5 +85,23 @@ func (device *VhostUserBlkDevice) Dump() persistapi.DeviceState {
 	return ds
 }
 
+// Load loads DeviceState and converts it to specific device
+func (device *VhostUserBlkDevice) Load(ds persistapi.DeviceState) {
+	device.GenericDevice = &GenericDevice{}
+	device.GenericDevice.Load(ds)
+
+	dev := ds.VhostUserDev
+	if dev == nil {
+		return
+	}
+
+	device.VhostUserDeviceAttrs = config.VhostUserDeviceAttrs{
+		DevID:      dev.DevID,
+		SocketPath: dev.SocketPath,
+		Type:       config.DeviceType(dev.Type),
+		MacAddress: dev.MacAddress,
+	}
+}
+
 // It should implement GetAttachCount() and DeviceID() as api.Device implementation
 // here it shares function from *GenericDevice so we don't need duplicate codes

virtcontainers/device/drivers/vhost_user_net.go

@@ -1,5 +1,5 @@
 // Copyright (c) 2017-2018 Intel Corporation
-// Copyright (c) 2018 Huawei Corporation
+// Copyright (c) 2018-2019 Huawei Corporation
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -73,9 +73,9 @@ func (device *VhostUserNetDevice) GetDeviceInfo() interface{} {
 	return &device.VhostUserDeviceAttrs
 }
 
-// Dump convert and return data in persist format
+// Save converts Device to DeviceState
-func (device *VhostUserNetDevice) Dump() persistapi.DeviceState {
+func (device *VhostUserNetDevice) Save() persistapi.DeviceState {
-	ds := device.GenericDevice.Dump()
+	ds := device.GenericDevice.Save()
 	ds.Type = string(device.DeviceType())
 	ds.VhostUserDev = &persistapi.VhostUserDeviceAttrs{
 		DevID:      device.DevID,
@@ -86,5 +86,23 @@ func (device *VhostUserNetDevice) Dump() persistapi.DeviceState {
 	return ds
 }
 
+// Load loads DeviceState and converts it to specific device
+func (device *VhostUserNetDevice) Load(ds persistapi.DeviceState) {
+	device.GenericDevice = &GenericDevice{}
+	device.GenericDevice.Load(ds)
+
+	dev := ds.VhostUserDev
+	if dev == nil {
+		return
+	}
+
+	device.VhostUserDeviceAttrs = config.VhostUserDeviceAttrs{
+		DevID:      dev.DevID,
+		SocketPath: dev.SocketPath,
+		Type:       config.DeviceType(dev.Type),
+		MacAddress: dev.MacAddress,
+	}
+}
+
 // It should implement GetAttachCount() and DeviceID() as api.Device implementation
 // here it shares function from *GenericDevice so we don't need duplicate codes

virtcontainers/device/drivers/vhost_user_scsi.go

@@ -1,5 +1,5 @@
 // Copyright (c) 2017-2018 Intel Corporation
-// Copyright (c) 2018 Huawei Corporation
+// Copyright (c) 2018-2019 Huawei Corporation
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -73,9 +73,9 @@ func (device *VhostUserSCSIDevice) GetDeviceInfo() interface{} {
 	return &device.VhostUserDeviceAttrs
 }
 
-// Dump convert and return data in persist format
+// Save converts Device to DeviceState
-func (device *VhostUserSCSIDevice) Dump() persistapi.DeviceState {
+func (device *VhostUserSCSIDevice) Save() persistapi.DeviceState {
-	ds := device.GenericDevice.Dump()
+	ds := device.GenericDevice.Save()
 	ds.Type = string(device.DeviceType())
 	ds.VhostUserDev = &persistapi.VhostUserDeviceAttrs{
 		DevID:      device.DevID,
@@ -86,5 +86,23 @@ func (device *VhostUserSCSIDevice) Dump() persistapi.DeviceState {
 	return ds
 }
 
+// Load loads DeviceState and converts it to specific device
+func (device *VhostUserSCSIDevice) Load(ds persistapi.DeviceState) {
+	device.GenericDevice = &GenericDevice{}
+	device.GenericDevice.Load(ds)
+
+	dev := ds.VhostUserDev
+	if dev == nil {
+		return
+	}
+
+	device.VhostUserDeviceAttrs = config.VhostUserDeviceAttrs{
+		DevID:      dev.DevID,
+		SocketPath: dev.SocketPath,
+		Type:       config.DeviceType(dev.Type),
+		MacAddress: dev.MacAddress,
+	}
+}
+
 // It should implement GetAttachCount() and DeviceID() as api.Device implementation
 // here it shares function from *GenericDevice so we don't need duplicate codes

virtcontainers/device/manager/manager.go

@@ -16,6 +16,7 @@ import (
 	"github.com/kata-containers/runtime/virtcontainers/device/api"
 	"github.com/kata-containers/runtime/virtcontainers/device/config"
 	"github.com/kata-containers/runtime/virtcontainers/device/drivers"
+	persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api"
 	"github.com/kata-containers/runtime/virtcontainers/utils"
 )
 
@@ -229,3 +230,41 @@ func (dm *deviceManager) IsDeviceAttached(id string) bool {
 	}
 	return d.GetAttachCount() > 0
 }
+
+// NewDevice creates a device based on specified DeviceInfo
+func (dm *deviceManager) LoadDevices(devStates []persistapi.DeviceState) {
+	dm.Lock()
+	defer dm.Unlock()
+
+	for _, ds := range devStates {
+		switch config.DeviceType(ds.Type) {
+		case config.DeviceGeneric:
+			dev := &drivers.GenericDevice{}
+			dev.Load(ds)
+			dm.devices[dev.DeviceID()] = dev
+		case config.DeviceBlock:
+			dev := &drivers.BlockDevice{}
+			dev.Load(ds)
+			dm.devices[dev.DeviceID()] = dev
+		case config.DeviceVFIO:
+			dev := &drivers.VFIODevice{}
+			dev.Load(ds)
+			dm.devices[dev.DeviceID()] = dev
+		case config.VhostUserSCSI:
+			dev := &drivers.VhostUserSCSIDevice{}
+			dev.Load(ds)
+			dm.devices[dev.DeviceID()] = dev
+		case config.VhostUserBlk:
+			dev := &drivers.VhostUserBlkDevice{}
+			dev.Load(ds)
+			dm.devices[dev.DeviceID()] = dev
+		case config.VhostUserNet:
+			dev := &drivers.VhostUserNetDevice{}
+			dev.Load(ds)
+			dm.devices[dev.DeviceID()] = dev
+		default:
+			deviceLogger().WithField("device-type", ds.Type).Warning("unrecognized device type is detected")
+		}
+
+	}
+}

virtcontainers/persist.go

@@ -62,7 +62,7 @@ func (s *Sandbox) dumpHypervisor(ss *persistapi.SandboxState, cs map[string]pers
 
 func deviceToDeviceState(devices []api.Device) (dss []persistapi.DeviceState) {
 	for _, dev := range devices {
-		dss = append(dss, dev.Dump())
+		dss = append(dss, dev.Save())
 	}
 	return
 }
@@ -116,6 +116,18 @@ func (s *Sandbox) Save() error {
 	return nil
 }
 
+func (s *Sandbox) loadState(ss persistapi.SandboxState) {
+	s.state.PersistVersion = ss.PersistVersion
+	s.state.GuestMemoryBlockSizeMB = ss.GuestMemoryBlockSizeMB
+	s.state.BlockIndex = ss.HypervisorState.BlockIndex
+	s.state.State = types.StateString(ss.State)
+	s.state.CgroupPath = ss.CgroupPath
+}
+
+func (s *Sandbox) loadDevices(devStates []persistapi.DeviceState) {
+	s.devManager.LoadDevices(devStates)
+}
+
 // Restore will restore sandbox data from persist file on disk
 func (s *Sandbox) Restore() error {
 	ss, _, err := s.newStore.FromDisk(s.id)
@@ -123,17 +135,12 @@ func (s *Sandbox) Restore() error {
 		return err
 	}
 
-	s.state.PersistVersion = ss.PersistVersion
+	s.loadState(ss)
-	s.state.GuestMemoryBlockSizeMB = ss.GuestMemoryBlockSizeMB
+	s.loadDevices(ss.Devices)
-	s.state.BlockIndex = ss.HypervisorState.BlockIndex
-	s.state.State = types.StateString(ss.State)
-	s.state.CgroupPath = ss.CgroupPath
-
 	return nil
 }
 
 // Restore will restore container data from persist file on disk
-// TODO:
 func (c *Container) Restore() error {
 	_, cs, err := c.sandbox.newStore.FromDisk(c.sandbox.id)
 	if err != nil {

virtcontainers/persist/api/device.go

@@ -46,7 +46,7 @@ type VFIODev struct {
 	ID string
 
 	// Type of VFIO device
-	Type string
+	Type uint32
 
 	// BDF (Bus:Device.Function) of the PCI address
 	BDF string

virtcontainers/sandbox.go

@@ -474,18 +474,20 @@ func createSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Fac
 		s.networkNS = networkNS
 	}
 
-	devices, err := s.store.LoadDevices()
-	if err != nil {
-		s.Logger().WithError(err).WithField("sandboxid", s.id).Warning("load sandbox devices failed")
-	}
-	s.devManager = deviceManager.NewDeviceManager(sandboxConfig.HypervisorConfig.BlockDeviceDriver, devices)
-
 	if s.supportNewStore() {
+		s.devManager = deviceManager.NewDeviceManager(sandboxConfig.HypervisorConfig.BlockDeviceDriver, nil)
+
 		if err := s.Restore(); err == nil && s.state.State != "" {
 			return s, nil
 		}
 
 	} else {
+		devices, err := s.store.LoadDevices()
+		if err != nil {
+			s.Logger().WithError(err).WithField("sandboxid", s.id).Warning("load sandbox devices failed")
+		}
+		s.devManager = deviceManager.NewDeviceManager(sandboxConfig.HypervisorConfig.BlockDeviceDriver, devices)
+
 		// We first try to fetch the sandbox state from storage.
 		// If it exists, this means this is a re-creation, i.e.
 		// we don't need to talk to the guest's agent, but only
@@ -1142,6 +1144,7 @@ func (s *Sandbox) StartContainer(containerID string) (VCContainer, error) {
 		return nil, err
 	}
 
+	s.Logger().Info("Container is started")
 	//Fixme Container delete from sandbox, need to update resources
 
 	return c, nil
@@ -1750,8 +1753,10 @@ func (s *Sandbox) AddDevice(info config.DeviceInfo) (api.Device, error) {
 		return nil, err
 	}
 
-	if err := s.storeSandboxDevices(); err != nil {
+	if !s.supportNewStore() {
-		return nil, err
+		if err := s.storeSandboxDevices(); err != nil {
+			return nil, err
+		}
 	}
 
 	return b, nil

virtcontainers/sandbox_test.go

@@ -897,6 +897,7 @@ func TestSandboxAttachDevicesVFIO(t *testing.T) {
 		hypervisor: &mockHypervisor{},
 		devManager: dm,
 		ctx:        context.Background(),
+		config:     &SandboxConfig{},
 	}
 
 	store, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id)