From 341a988e0640373ff8719bfa6dec3c29a0185e6d Mon Sep 17 00:00:00 2001 From: Wei Zhang Date: Mon, 22 Apr 2019 19:08:33 +0800 Subject: [PATCH 1/3] persist: simplify persist api Fixes #803 Simplify new store API to make the code easier to understand and use. Signed-off-by: Wei Zhang --- virtcontainers/container.go | 4 +- virtcontainers/persist.go | 71 +++++++++++-------------- virtcontainers/persist/api/interface.go | 10 +--- virtcontainers/persist/fs/fs.go | 43 ++++++--------- virtcontainers/persist/fs/fs_test.go | 34 +++++------- virtcontainers/persist_test.go | 23 ++------ virtcontainers/sandbox.go | 10 +--- virtcontainers/types/sandbox.go | 5 ++ 8 files changed, 74 insertions(+), 126 deletions(-) diff --git a/virtcontainers/container.go b/virtcontainers/container.go index 6030d7355..d1408d915 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -392,7 +392,7 @@ func (c *Container) GetAnnotations() map[string]string { // storeContainer stores a container config. func (c *Container) storeContainer() error { if c.sandbox.supportNewStore() { - if err := c.sandbox.newStore.ToDisk(); err != nil { + if err := c.sandbox.Save(); err != nil { return err } } @@ -447,7 +447,7 @@ func (c *Container) setContainerState(state types.StateString) error { if c.sandbox.supportNewStore() { // flush data to storage - if err := c.sandbox.newStore.ToDisk(); err != nil { + if err := c.sandbox.Save(); err != nil { return err } } else { diff --git a/virtcontainers/persist.go b/virtcontainers/persist.go index b6bc5814e..ccb5e3153 100644 --- a/virtcontainers/persist.go +++ b/virtcontainers/persist.go @@ -19,7 +19,16 @@ var ( errContainerPersistNotExist = errors.New("container doesn't exist in persist data") ) -func (s *Sandbox) dumpState(ss *persistapi.SandboxState, cs map[string]persistapi.ContainerState) error { +func (s *Sandbox) dumpVersion(ss *persistapi.SandboxState) { + // New created sandbox has a uninitialized `PersistVersion` which should be set to current version when do the first saving; + // Old restored sandbox should keep its original version and shouldn't be modified any more after it's initialized. + ss.PersistVersion = s.state.PersistVersion + if ss.PersistVersion == 0 { + ss.PersistVersion = persistapi.CurPersistVersion + } +} + +func (s *Sandbox) dumpState(ss *persistapi.SandboxState, cs map[string]persistapi.ContainerState) { ss.SandboxContainer = s.id ss.GuestMemoryBlockSizeMB = s.state.GuestMemoryBlockSizeMB ss.State = string(s.state.State) @@ -45,13 +54,10 @@ func (s *Sandbox) dumpState(ss *persistapi.SandboxState, cs map[string]persistap delete(cs, id) } } - - return nil } -func (s *Sandbox) dumpHypervisor(ss *persistapi.SandboxState, cs map[string]persistapi.ContainerState) error { +func (s *Sandbox) dumpHypervisor(ss *persistapi.SandboxState, cs map[string]persistapi.ContainerState) { ss.HypervisorState.BlockIndex = s.state.BlockIndex - return nil } func deviceToDeviceState(devices []api.Device) (dss []persistapi.DeviceState) { @@ -61,7 +67,7 @@ func deviceToDeviceState(devices []api.Device) (dss []persistapi.DeviceState) { return } -func (s *Sandbox) dumpDevices(ss *persistapi.SandboxState, cs map[string]persistapi.ContainerState) error { +func (s *Sandbox) dumpDevices(ss *persistapi.SandboxState, cs map[string]persistapi.ContainerState) { ss.Devices = deviceToDeviceState(s.devManager.GetAllDevices()) for id, cont := range s.containers { @@ -90,48 +96,34 @@ func (s *Sandbox) dumpDevices(ss *persistapi.SandboxState, cs map[string]persist delete(cs, id) } } +} + +func (s *Sandbox) Save() error { + var ( + ss = persistapi.SandboxState{} + cs = make(map[string]persistapi.ContainerState) + ) + + s.dumpVersion(&ss) + s.dumpState(&ss, cs) + s.dumpHypervisor(&ss, cs) + s.dumpDevices(&ss, cs) + + if err := s.newStore.ToDisk(ss, cs); err != nil { + return err + } return nil } -// verSaveCallback 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 - }) -} - -// stateSaveCallback register hook to set sandbox and container state to persist -func (s *Sandbox) stateSaveCallback() { - s.newStore.AddSaveCallback("state", s.dumpState) -} - -// hvStateSaveCallback register hook to save hypervisor state to persist data -func (s *Sandbox) hvStateSaveCallback() { - s.newStore.AddSaveCallback("hypervisor", s.dumpHypervisor) -} - -// PersistDevices register hook to save device informations -func (s *Sandbox) devicesSaveCallback() { - s.newStore.AddSaveCallback("devices", s.dumpDevices) -} - -func (s *Sandbox) getSbxAndCntStates() (*persistapi.SandboxState, map[string]persistapi.ContainerState, error) { - if err := s.newStore.FromDisk(s.id); err != nil { - return nil, nil, err - } - - return s.newStore.GetStates() -} - // Restore will restore sandbox data from persist file on disk func (s *Sandbox) Restore() error { - ss, _, err := s.getSbxAndCntStates() + ss, _, err := s.newStore.FromDisk(s.id) if err != nil { return err } + s.state.PersistVersion = ss.PersistVersion s.state.GuestMemoryBlockSizeMB = ss.GuestMemoryBlockSizeMB s.state.BlockIndex = ss.HypervisorState.BlockIndex s.state.State = types.StateString(ss.State) @@ -141,8 +133,9 @@ func (s *Sandbox) Restore() error { } // Restore will restore container data from persist file on disk +// TODO: func (c *Container) Restore() error { - _, cs, err := c.sandbox.getSbxAndCntStates() + _, cs, err := c.sandbox.newStore.FromDisk(c.sandbox.id) if err != nil { return err } diff --git a/virtcontainers/persist/api/interface.go b/virtcontainers/persist/api/interface.go index 0b94a61a6..a14dbc5e1 100644 --- a/virtcontainers/persist/api/interface.go +++ b/virtcontainers/persist/api/interface.go @@ -8,16 +8,10 @@ package persistapi // PersistDriver is interface describing operations to save/restore persist data type PersistDriver interface { // ToDisk flushes data to disk(or other storage media such as a remote db) - ToDisk() error + ToDisk(SandboxState, map[string]ContainerState) error // FromDisk will restore all data for sandbox with `sid` from storage. // We only support get data for one whole sandbox - FromDisk(sid string) 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) + FromDisk(sid string) (SandboxState, map[string]ContainerState, error) // Destroy will remove everything from storage Destroy() error - // GetStates will return SandboxState and ContainerState(s) directly - GetStates() (*SandboxState, map[string]ContainerState, error) } diff --git a/virtcontainers/persist/fs/fs.go b/virtcontainers/persist/fs/fs.go index df3bc5b35..1e7552785 100644 --- a/virtcontainers/persist/fs/fs.go +++ b/virtcontainers/persist/fs/fs.go @@ -44,7 +44,6 @@ var runStoragePath = filepath.Join("/run", storagePathSuffix, sandboxPathSuffix) type FS struct { sandboxState *persistapi.SandboxState containerState map[string]persistapi.ContainerState - setFuncs map[string]persistapi.SetFunc lockFile *os.File } @@ -68,7 +67,6 @@ func Init() (persistapi.PersistDriver, error) { return &FS{ sandboxState: &persistapi.SandboxState{}, containerState: make(map[string]persistapi.ContainerState), - setFuncs: make(map[string]persistapi.SetFunc), }, nil } @@ -82,11 +80,9 @@ func (fs *FS) sandboxDir() (string, 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) - } +func (fs *FS) ToDisk(ss persistapi.SandboxState, cs map[string]persistapi.ContainerState) (retErr error) { + fs.sandboxState = &ss + fs.containerState = cs sandboxDir, err := fs.sandboxDir() if err != nil { @@ -146,20 +142,21 @@ func (fs *FS) ToDisk() (retErr error) { } // FromDisk restores state for sandbox with name sid -func (fs *FS) FromDisk(sid string) error { +func (fs *FS) FromDisk(sid string) (persistapi.SandboxState, map[string]persistapi.ContainerState, error) { + ss := persistapi.SandboxState{} if sid == "" { - return fmt.Errorf("restore requires sandbox id") + return ss, nil, fmt.Errorf("restore requires sandbox id") } fs.sandboxState.SandboxContainer = sid sandboxDir, err := fs.sandboxDir() if err != nil { - return err + return ss, nil, err } if err := fs.lock(); err != nil { - return err + return ss, nil, err } defer fs.unlock() @@ -167,18 +164,18 @@ func (fs *FS) FromDisk(sid string) error { sandboxFile := filepath.Join(sandboxDir, persistFile) f, err := os.OpenFile(sandboxFile, os.O_RDONLY, fileMode) if err != nil { - return err + return ss, nil, err } defer f.Close() if err := json.NewDecoder(f).Decode(fs.sandboxState); err != nil { - return err + return ss, nil, err } // walk sandbox dir and find container files, err := ioutil.ReadDir(sandboxDir) if err != nil { - return err + return ss, nil, err } for _, file := range files { @@ -194,18 +191,19 @@ func (fs *FS) FromDisk(sid string) error { if os.IsNotExist(err) { continue } - return err + return ss, nil, err } var cstate persistapi.ContainerState if err := json.NewDecoder(cf).Decode(&cstate); err != nil { - return err + return ss, nil, err } cf.Close() fs.containerState[cid] = cstate } - return nil + + return *fs.sandboxState, fs.containerState, nil } // Destroy removes everything from disk @@ -221,17 +219,6 @@ func (fs *FS) Destroy() error { return nil } -// GetStates returns SandboxState and ContainerState -func (fs *FS) GetStates() (*persistapi.SandboxState, map[string]persistapi.ContainerState, error) { - return fs.sandboxState, fs.containerState, nil -} - -// 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 -} - func (fs *FS) lock() error { sandboxDir, err := fs.sandboxDir() if err != nil { diff --git a/virtcontainers/persist/fs/fs_test.go b/virtcontainers/persist/fs/fs_test.go index e9ad01b34..1d618a7f4 100644 --- a/virtcontainers/persist/fs/fs_test.go +++ b/virtcontainers/persist/fs/fs_test.go @@ -51,31 +51,21 @@ func TestFsDriver(t *testing.T) { assert.Nil(t, err) assert.NotNil(t, fs) - fs.AddSaveCallback("test", func(ss *persistapi.SandboxState, cs map[string]persistapi.ContainerState) error { - return nil - }) + ss := persistapi.SandboxState{} + cs := make(map[string]persistapi.ContainerState) // missing sandbox container id - assert.NotNil(t, fs.ToDisk()) + assert.NotNil(t, fs.ToDisk(ss, cs)) id := "test-fs-driver" - // missing sandbox container id - fs.AddSaveCallback("test", func(ss *persistapi.SandboxState, cs map[string]persistapi.ContainerState) error { - ss.SandboxContainer = id - return nil - }) - assert.Nil(t, fs.ToDisk()) - - fs.AddSaveCallback("test1", func(ss *persistapi.SandboxState, cs map[string]persistapi.ContainerState) error { - ss.State = "running" - return nil - }) + ss.SandboxContainer = id + assert.Nil(t, fs.ToDisk(ss, cs)) // try non-existent dir - assert.NotNil(t, fs.FromDisk("test-fs")) + _, _, err = fs.FromDisk("test-fs") + assert.NotNil(t, err) - // since we didn't call ToDisk, Callbacks are not invoked, and state is still empty in disk file - assert.Nil(t, fs.FromDisk(id)) - ss, cs, err := fs.GetStates() + // since we didn't call ToDisk, state is still empty in disk file + ss, cs, err = fs.FromDisk(id) assert.Nil(t, err) assert.NotNil(t, ss) assert.Equal(t, len(cs), 0) @@ -84,9 +74,9 @@ func TestFsDriver(t *testing.T) { assert.Equal(t, ss.State, "") // flush all to disk - assert.Nil(t, fs.ToDisk()) - assert.Nil(t, fs.FromDisk(id)) - ss, cs, err = fs.GetStates() + ss.State = "running" + 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) diff --git a/virtcontainers/persist_test.go b/virtcontainers/persist_test.go index 425b33d6c..12f359842 100644 --- a/virtcontainers/persist_test.go +++ b/virtcontainers/persist_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/assert" + "github.com/kata-containers/runtime/virtcontainers/device/manager" exp "github.com/kata-containers/runtime/virtcontainers/experimental" "github.com/kata-containers/runtime/virtcontainers/persist" "github.com/kata-containers/runtime/virtcontainers/types" @@ -74,6 +75,7 @@ func TestSandboxRestore(t *testing.T) { sandbox := Sandbox{ id: "test-exp", containers: container, + devManager: manager.NewDeviceManager(manager.VirtioSCSI, nil), hypervisor: &mockHypervisor{}, ctx: context.Background(), config: &sconfig, @@ -89,7 +91,7 @@ func TestSandboxRestore(t *testing.T) { assert.True(t, os.IsNotExist(err)) // disk data are empty - err = sandbox.newStore.ToDisk() + err = sandbox.Save() assert.Nil(t, err) err = sandbox.Restore() @@ -98,14 +100,12 @@ func TestSandboxRestore(t *testing.T) { assert.Equal(t, sandbox.state.GuestMemoryBlockSizeMB, uint32(0)) assert.Equal(t, sandbox.state.BlockIndex, 0) - // register callback function - sandbox.stateSaveCallback() - + // set state data and save again sandbox.state.State = types.StateString("running") sandbox.state.GuestMemoryBlockSizeMB = uint32(1024) sandbox.state.BlockIndex = 2 // flush data to disk - err = sandbox.newStore.ToDisk() + err = sandbox.Save() assert.Nil(t, err) // empty the sandbox @@ -116,18 +116,5 @@ func TestSandboxRestore(t *testing.T) { assert.Nil(t, err) assert.Equal(t, sandbox.state.State, types.StateString("running")) assert.Equal(t, sandbox.state.GuestMemoryBlockSizeMB, uint32(1024)) - // require hvStateSaveCallback to make it savable - assert.Equal(t, sandbox.state.BlockIndex, 0) - - // after use hvStateSaveCallbck, BlockIndex can be saved now - sandbox.state.BlockIndex = 2 - sandbox.hvStateSaveCallback() - err = sandbox.newStore.ToDisk() - assert.Nil(t, err) - err = sandbox.Restore() - assert.Nil(t, err) - assert.Equal(t, sandbox.state.State, types.StateString("running")) - assert.Equal(t, sandbox.state.GuestMemoryBlockSizeMB, uint32(1024)) assert.Equal(t, sandbox.state.BlockIndex, 2) - } diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 3ade96204..e013426b0 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -481,18 +481,10 @@ func createSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Fac s.devManager = deviceManager.NewDeviceManager(sandboxConfig.HypervisorConfig.BlockDeviceDriver, devices) if s.supportNewStore() { - // 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 } - // if sandbox doesn't exist, set persist version to current version - // otherwise do nothing - s.verSaveCallback() } else { // We first try to fetch the sandbox state from storage. // If it exists, this means this is a re-creation, i.e. @@ -619,7 +611,7 @@ func (s *Sandbox) storeSandbox() error { if s.supportNewStore() { // flush data to storage - if err := s.newStore.ToDisk(); err != nil { + if err := s.Save(); err != nil { return err } } diff --git a/virtcontainers/types/sandbox.go b/virtcontainers/types/sandbox.go index 731736516..2259e86d5 100644 --- a/virtcontainers/types/sandbox.go +++ b/virtcontainers/types/sandbox.go @@ -43,6 +43,11 @@ type SandboxState struct { // CgroupPath is the cgroup hierarchy where sandbox's processes // including the hypervisor are placed. CgroupPath string `json:"cgroupPath,omitempty"` + + // PersistVersion indicates current storage api version. + // It's also known as ABI version of kata-runtime. + // Note: it won't be written to disk + PersistVersion uint `json:"-"` } // Valid checks that the sandbox state is valid. From 4c192139cfea6f57088c2d2cd930a71513ad7ead Mon Sep 17 00:00:00 2001 From: Wei Zhang Date: Wed, 24 Apr 2019 16:54:23 +0800 Subject: [PATCH 2/3] 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 --- virtcontainers/api.go | 4 ++ virtcontainers/container.go | 47 ++++++++++++------- virtcontainers/container_test.go | 1 + virtcontainers/device/api/interface.go | 14 +++++- virtcontainers/device/drivers/block.go | 30 ++++++++++-- virtcontainers/device/drivers/generic.go | 20 ++++++-- virtcontainers/device/drivers/vfio.go | 25 ++++++++-- .../device/drivers/vhost_user_blk.go | 26 ++++++++-- .../device/drivers/vhost_user_net.go | 26 ++++++++-- .../device/drivers/vhost_user_scsi.go | 26 ++++++++-- virtcontainers/device/manager/manager.go | 39 +++++++++++++++ virtcontainers/persist.go | 23 +++++---- virtcontainers/persist/api/device.go | 2 +- virtcontainers/sandbox.go | 21 +++++---- virtcontainers/sandbox_test.go | 1 + 15 files changed, 246 insertions(+), 59 deletions(-) diff --git a/virtcontainers/api.go b/virtcontainers/api.go index c3197c1c5..550a60b22 100644 --- a/virtcontainers/api.go +++ b/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 } diff --git a/virtcontainers/container.go b/virtcontainers/container.go index d1408d915..f5938d761 100644 --- a/virtcontainers/container.go +++ b/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 { - //TODO: roll back? - return nil, nil, err + if !c.sandbox.supportNewStore() { + if err := c.sandbox.storeSandboxDevices(); err != nil { + //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 { - return err + if !c.sandbox.supportNewStore() { + 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 { - return err + if !c.sandbox.supportNewStore() { + 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 { - return err + if !c.sandbox.supportNewStore() { + 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 { - return err + if !c.sandbox.supportNewStore() { + if err := c.sandbox.storeSandboxDevices(); err != nil { + return err + } } return nil } diff --git a/virtcontainers/container_test.go b/virtcontainers/container_test.go index 333d2e488..e820f613a 100644 --- a/virtcontainers/container_test.go +++ b/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) diff --git a/virtcontainers/device/api/interface.go b/virtcontainers/device/api/interface.go index 560a40d1b..828049c6f 100644 --- a/virtcontainers/device/api/interface.go +++ b/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 - Dump() persistapi.DeviceState + // Save converts Device to 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) } diff --git a/virtcontainers/device/drivers/block.go b/virtcontainers/device/drivers/block.go index 9897118ad..1e23bc75c 100644 --- a/virtcontainers/device/drivers/block.go +++ b/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 -func (device *BlockDevice) Dump() persistapi.DeviceState { - ds := device.GenericDevice.Dump() +// Save converts Device to DeviceState +func (device *BlockDevice) Save() persistapi.DeviceState { + 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 diff --git a/virtcontainers/device/drivers/generic.go b/virtcontainers/device/drivers/generic.go index d01014f67..32d91d462 100644 --- a/virtcontainers/device/drivers/generic.go +++ b/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 -func (device *GenericDevice) Dump() persistapi.DeviceState { +// Save converts Device to 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, + } +} diff --git a/virtcontainers/device/drivers/vfio.go b/virtcontainers/device/drivers/vfio.go index bea10dc2d..ade39c18b 100644 --- a/virtcontainers/device/drivers/vfio.go +++ b/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 -func (device *VFIODevice) Dump() persistapi.DeviceState { - ds := device.GenericDevice.Dump() +// Save converts Device to DeviceState +func (device *VFIODevice) Save() persistapi.DeviceState { + 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 diff --git a/virtcontainers/device/drivers/vhost_user_blk.go b/virtcontainers/device/drivers/vhost_user_blk.go index 588c46590..b1bb61d55 100644 --- a/virtcontainers/device/drivers/vhost_user_blk.go +++ b/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 -func (device *VhostUserBlkDevice) Dump() persistapi.DeviceState { - ds := device.GenericDevice.Dump() +// Save converts Device to DeviceState +func (device *VhostUserBlkDevice) Save() persistapi.DeviceState { + 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 diff --git a/virtcontainers/device/drivers/vhost_user_net.go b/virtcontainers/device/drivers/vhost_user_net.go index 8f5abb32e..9b89a241a 100644 --- a/virtcontainers/device/drivers/vhost_user_net.go +++ b/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 -func (device *VhostUserNetDevice) Dump() persistapi.DeviceState { - ds := device.GenericDevice.Dump() +// Save converts Device to DeviceState +func (device *VhostUserNetDevice) Save() persistapi.DeviceState { + 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 diff --git a/virtcontainers/device/drivers/vhost_user_scsi.go b/virtcontainers/device/drivers/vhost_user_scsi.go index 543949ceb..33d2210fa 100644 --- a/virtcontainers/device/drivers/vhost_user_scsi.go +++ b/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 -func (device *VhostUserSCSIDevice) Dump() persistapi.DeviceState { - ds := device.GenericDevice.Dump() +// Save converts Device to DeviceState +func (device *VhostUserSCSIDevice) Save() persistapi.DeviceState { + 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 diff --git a/virtcontainers/device/manager/manager.go b/virtcontainers/device/manager/manager.go index f2566432b..2e8a58c83 100644 --- a/virtcontainers/device/manager/manager.go +++ b/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") + } + + } +} diff --git a/virtcontainers/persist.go b/virtcontainers/persist.go index ccb5e3153..7ec6c9681 100644 --- a/virtcontainers/persist.go +++ b/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.state.GuestMemoryBlockSizeMB = ss.GuestMemoryBlockSizeMB - s.state.BlockIndex = ss.HypervisorState.BlockIndex - s.state.State = types.StateString(ss.State) - s.state.CgroupPath = ss.CgroupPath - + s.loadState(ss) + s.loadDevices(ss.Devices) 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 { diff --git a/virtcontainers/persist/api/device.go b/virtcontainers/persist/api/device.go index 42d9acb5f..b1b3c1a6f 100644 --- a/virtcontainers/persist/api/device.go +++ b/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 diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index e013426b0..ca7283533 100644 --- a/virtcontainers/sandbox.go +++ b/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 { - return nil, err + if !s.supportNewStore() { + if err := s.storeSandboxDevices(); err != nil { + return nil, err + } } return b, nil diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index 2188972d1..82c6dce6f 100644 --- a/virtcontainers/sandbox_test.go +++ b/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) From 297097779eec7f7ed133e2c0bbd6c712eb6af1fb Mon Sep 17 00:00:00 2001 From: Wei Zhang Date: Thu, 9 May 2019 14:39:04 +0800 Subject: [PATCH 3/3] persist: save/load `GuestMemoryHotplugProbe` Support saving/loading `GuestMemoryHotplugProbe` from sandbox state. Signed-off-by: Wei Zhang --- virtcontainers/api.go | 4 ++-- virtcontainers/device/manager/manager.go | 30 ++++++++++-------------- virtcontainers/persist.go | 2 ++ 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/virtcontainers/api.go b/virtcontainers/api.go index 550a60b22..cdbfb1ad1 100644 --- a/virtcontainers/api.go +++ b/virtcontainers/api.go @@ -591,8 +591,8 @@ func statusContainer(sandbox *Sandbox, containerID string) (ContainerStatus, err if !running { virtLog.WithFields(logrus.Fields{ - "state": container.state.State, - "process pid": container.process.Pid}). + "state": container.state.State, + "pid": container.process.Pid}). Info("container isn't running") if err := container.stop(); err != nil { return ContainerStatus{}, err diff --git a/virtcontainers/device/manager/manager.go b/virtcontainers/device/manager/manager.go index 2e8a58c83..6647b1c94 100644 --- a/virtcontainers/device/manager/manager.go +++ b/virtcontainers/device/manager/manager.go @@ -237,34 +237,28 @@ func (dm *deviceManager) LoadDevices(devStates []persistapi.DeviceState) { defer dm.Unlock() for _, ds := range devStates { + var dev api.Device + switch config.DeviceType(ds.Type) { case config.DeviceGeneric: - dev := &drivers.GenericDevice{} - dev.Load(ds) - dm.devices[dev.DeviceID()] = dev + dev = &drivers.GenericDevice{} case config.DeviceBlock: - dev := &drivers.BlockDevice{} - dev.Load(ds) - dm.devices[dev.DeviceID()] = dev + dev = &drivers.BlockDevice{} case config.DeviceVFIO: - dev := &drivers.VFIODevice{} - dev.Load(ds) - dm.devices[dev.DeviceID()] = dev + dev = &drivers.VFIODevice{} case config.VhostUserSCSI: - dev := &drivers.VhostUserSCSIDevice{} - dev.Load(ds) - dm.devices[dev.DeviceID()] = dev + dev = &drivers.VhostUserSCSIDevice{} case config.VhostUserBlk: - dev := &drivers.VhostUserBlkDevice{} - dev.Load(ds) - dm.devices[dev.DeviceID()] = dev + dev = &drivers.VhostUserBlkDevice{} case config.VhostUserNet: - dev := &drivers.VhostUserNetDevice{} - dev.Load(ds) - dm.devices[dev.DeviceID()] = dev + dev = &drivers.VhostUserNetDevice{} default: deviceLogger().WithField("device-type", ds.Type).Warning("unrecognized device type is detected") + // continue the for loop + continue } + dev.Load(ds) + dm.devices[dev.DeviceID()] = dev } } diff --git a/virtcontainers/persist.go b/virtcontainers/persist.go index 7ec6c9681..821240f88 100644 --- a/virtcontainers/persist.go +++ b/virtcontainers/persist.go @@ -31,6 +31,7 @@ func (s *Sandbox) dumpVersion(ss *persistapi.SandboxState) { func (s *Sandbox) dumpState(ss *persistapi.SandboxState, cs map[string]persistapi.ContainerState) { ss.SandboxContainer = s.id ss.GuestMemoryBlockSizeMB = s.state.GuestMemoryBlockSizeMB + ss.GuestMemoryHotplugProbe = s.state.GuestMemoryHotplugProbe ss.State = string(s.state.State) ss.CgroupPath = s.state.CgroupPath @@ -122,6 +123,7 @@ func (s *Sandbox) loadState(ss persistapi.SandboxState) { s.state.BlockIndex = ss.HypervisorState.BlockIndex s.state.State = types.StateString(ss.State) s.state.CgroupPath = ss.CgroupPath + s.state.GuestMemoryHotplugProbe = ss.GuestMemoryHotplugProbe } func (s *Sandbox) loadDevices(devStates []persistapi.DeviceState) {