diff --git a/virtcontainers/api.go b/virtcontainers/api.go index abaebb50e..8c75b095d 100644 --- a/virtcontainers/api.go +++ b/virtcontainers/api.go @@ -35,7 +35,12 @@ func SetLogger(logger logrus.FieldLogger) { // CreateSandbox is the virtcontainers sandbox creation entry point. // CreateSandbox creates a sandbox and its containers. It does not start them. func CreateSandbox(sandboxConfig SandboxConfig, factory Factory) (VCSandbox, error) { - return createSandboxFromConfig(sandboxConfig, factory) + s, err := createSandboxFromConfig(sandboxConfig, factory) + if err == nil { + s.Release() + } + + return s, err } func createSandboxFromConfig(sandboxConfig SandboxConfig, factory Factory) (*Sandbox, error) { @@ -86,6 +91,7 @@ func DeleteSandbox(sandboxID string) (VCSandbox, error) { if err != nil { return nil, err } + defer s.Release() // Delete it. if err := s.Delete(); err != nil { @@ -97,7 +103,8 @@ func DeleteSandbox(sandboxID string) (VCSandbox, error) { // FetchSandbox is the virtcontainers sandbox fetching entry point. // FetchSandbox will find out and connect to an existing sandbox and -// return the sandbox structure. +// return the sandbox structure. The caller is responsible of calling +// VCSandbox.Release() after done with it. func FetchSandbox(sandboxID string) (VCSandbox, error) { if sandboxID == "" { return nil, errNeedSandboxID @@ -110,21 +117,22 @@ func FetchSandbox(sandboxID string) (VCSandbox, error) { defer unlockSandbox(lockFile) // Fetch the sandbox from storage and create it. - sandbox, err := fetchSandbox(sandboxID) + s, err := fetchSandbox(sandboxID) if err != nil { return nil, err } // If the proxy is KataBuiltInProxyType type, it needs to restart the proxy to watch the // guest console if it hadn't been watched. - if isProxyBuiltIn(sandbox.config.ProxyType) { - err = sandbox.startProxy() + if isProxyBuiltIn(s.config.ProxyType) { + err = s.startProxy() if err != nil { + s.Release() return nil, err } } - return sandbox, nil + return s, nil } // StartSandbox is the virtcontainers sandbox starting entry point. @@ -147,6 +155,7 @@ func StartSandbox(sandboxID string) (VCSandbox, error) { if err != nil { return nil, err } + defer s.Release() return startSandbox(s) } @@ -184,6 +193,7 @@ func StopSandbox(sandboxID string) (VCSandbox, error) { if err != nil { return nil, err } + defer s.Release() // Stop it. err = s.stop() @@ -211,6 +221,7 @@ func RunSandbox(sandboxConfig SandboxConfig, factory Factory) (VCSandbox, error) if err != nil { return nil, err } + defer s.Release() lockFile, err := rwLockSandbox(s.id) if err != nil { @@ -269,6 +280,7 @@ func StatusSandbox(sandboxID string) (SandboxStatus, error) { unlockSandbox(lockFile) return SandboxStatus{}, err } + defer s.Release() // We need to potentially wait for a separate container.stop() routine // that needs to be terminated before we return from this function. @@ -320,6 +332,7 @@ func CreateContainer(sandboxID string, containerConfig ContainerConfig) (VCSandb if err != nil { return nil, nil, err } + defer s.Release() c, err := s.CreateContainer(containerConfig) if err != nil { @@ -351,6 +364,7 @@ func DeleteContainer(sandboxID, containerID string) (VCContainer, error) { if err != nil { return nil, err } + defer s.Release() return s.DeleteContainer(containerID) } @@ -376,6 +390,7 @@ func StartContainer(sandboxID, containerID string) (VCContainer, error) { if err != nil { return nil, err } + defer s.Release() c, err := s.StartContainer(containerID) if err != nil { @@ -406,6 +421,7 @@ func StopContainer(sandboxID, containerID string) (VCContainer, error) { if err != nil { return nil, err } + defer s.Release() // Fetch the container. c, err := s.findContainer(containerID) @@ -443,6 +459,7 @@ func EnterContainer(sandboxID, containerID string, cmd Cmd) (VCSandbox, VCContai if err != nil { return nil, nil, nil, err } + defer s.Release() c, process, err := s.EnterContainer(containerID, cmd) if err != nil { @@ -473,6 +490,7 @@ func StatusContainer(sandboxID, containerID string) (ContainerStatus, error) { unlockSandbox(lockFile) return ContainerStatus{}, err } + defer s.Release() // We need to potentially wait for a separate container.stop() routine // that needs to be terminated before we return from this function. @@ -559,6 +577,7 @@ func KillContainer(sandboxID, containerID string, signal syscall.Signal, all boo if err != nil { return err } + defer s.Release() // Fetch the container. c, err := s.findContainer(containerID) @@ -608,6 +627,7 @@ func ProcessListContainer(sandboxID, containerID string, options ProcessListOpti if err != nil { return nil, err } + defer s.Release() // Fetch the container. c, err := s.findContainer(containerID) @@ -639,6 +659,7 @@ func UpdateContainer(sandboxID, containerID string, resources specs.LinuxResourc if err != nil { return err } + defer s.Release() return s.UpdateContainer(containerID, resources) } @@ -664,6 +685,7 @@ func StatsContainer(sandboxID, containerID string) (ContainerStats, error) { if err != nil { return ContainerStats{}, err } + defer s.Release() return s.StatsContainer(containerID) } @@ -687,6 +709,7 @@ func togglePauseContainer(sandboxID, containerID string, pause bool) error { if err != nil { return err } + defer s.Release() // Fetch the container. c, err := s.findContainer(containerID) diff --git a/virtcontainers/api_test.go b/virtcontainers/api_test.go index c7a5ebe8d..40c1a9237 100644 --- a/virtcontainers/api_test.go +++ b/virtcontainers/api_test.go @@ -1811,6 +1811,7 @@ func TestStatusContainerStateReady(t *testing.T) { if err != nil { t.Fatal(err) } + defer p2.Release() expectedStatus := ContainerStatus{ ID: contID, @@ -1884,6 +1885,7 @@ func TestStatusContainerStateRunning(t *testing.T) { if err != nil { t.Fatal(err) } + defer p2.Release() expectedStatus := ContainerStatus{ ID: contID, diff --git a/virtcontainers/hypervisor.go b/virtcontainers/hypervisor.go index 81b27c509..1e8a04e12 100644 --- a/virtcontainers/hypervisor.go +++ b/virtcontainers/hypervisor.go @@ -547,5 +547,6 @@ type hypervisor interface { hotplugAddDevice(devInfo interface{}, devType deviceType) (interface{}, error) hotplugRemoveDevice(devInfo interface{}, devType deviceType) (interface{}, error) getSandboxConsole(sandboxID string) (string, error) + disconnect() capabilities() capabilities } diff --git a/virtcontainers/mock_hypervisor.go b/virtcontainers/mock_hypervisor.go index 38f9a44d7..85e599ebc 100644 --- a/virtcontainers/mock_hypervisor.go +++ b/virtcontainers/mock_hypervisor.go @@ -73,3 +73,6 @@ func (m *mockHypervisor) hotplugRemoveDevice(devInfo interface{}, devType device func (m *mockHypervisor) getSandboxConsole(sandboxID string) (string, error) { return "", nil } + +func (m *mockHypervisor) disconnect() { +} diff --git a/virtcontainers/mock_hypervisor_test.go b/virtcontainers/mock_hypervisor_test.go index 194939dc7..ccde7c2ab 100644 --- a/virtcontainers/mock_hypervisor_test.go +++ b/virtcontainers/mock_hypervisor_test.go @@ -104,3 +104,9 @@ func TestMockHypervisorSaveSandbox(t *testing.T) { t.Fatal(err) } } + +func TestMockHypervisorDisconnect(t *testing.T) { + var m *mockHypervisor + + m.disconnect() +} diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 1664da5f1..1d84fa88d 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -560,7 +560,6 @@ func (q *qemu) stopSandbox() error { if err != nil { return err } - defer q.qmpShutdown() err = q.qmpMonitorCh.qmp.ExecuteQuit(q.qmpMonitorCh.ctx) if err != nil { @@ -581,7 +580,6 @@ func (q *qemu) togglePauseSandbox(pause bool) error { if err != nil { return err } - defer q.qmpShutdown() if pause { err = q.qmpMonitorCh.qmp.ExecuteStop(q.qmpMonitorCh.ctx) @@ -663,7 +661,6 @@ func (q *qemu) hotplugBlockDevice(drive *deviceDrivers.Drive, op operation) erro if err != nil { return err } - defer q.qmpShutdown() devID := "virtio-" + drive.ID @@ -725,7 +722,6 @@ func (q *qemu) hotplugVFIODevice(device deviceDrivers.VFIODevice, op operation) if err != nil { return err } - defer q.qmpShutdown() devID := "vfio-" + device.DeviceInfo.ID @@ -800,7 +796,6 @@ func (q *qemu) hotplugCPUs(vcpus uint32, op operation) (uint32, error) { if err != nil { return 0, err } - defer q.qmpShutdown() if op == addDevice { return q.hotplugAddCPUs(vcpus) @@ -928,7 +923,6 @@ func (q *qemu) hotplugAddMemory(memDev *memoryDevice) error { if err != nil { return err } - defer q.qmpShutdown() err = q.qmpMonitorCh.qmp.ExecHotplugMemory(q.qmpMonitorCh.ctx, "memory-backend-ram", "mem"+strconv.Itoa(memDev.slot), "", memDev.sizeMB) if err != nil { @@ -989,7 +983,6 @@ func (q *qemu) saveSandbox() error { if err != nil { return err } - defer q.qmpShutdown() // BootToBeTemplate sets the VM to be a template that other VMs can clone from. We would want to // bypass shared memory when saving the VM to a local file through migration exec. @@ -1015,6 +1008,10 @@ func (q *qemu) saveSandbox() error { return nil } +func (q *qemu) disconnect() { + q.qmpShutdown() +} + // genericAppendBridges appends to devices the given bridges func genericAppendBridges(devices []govmmQemu.Device, bridges []Bridge, machineType string) []govmmQemu.Device { bus := defaultPCBridgeBus diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 316dbd72c..b175f2ef2 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -541,10 +541,12 @@ func (s *Sandbox) GetContainer(containerID string) VCContainer { // Release closes the agent connection and removes sandbox from internal list. func (s *Sandbox) Release() error { + s.Logger().Info("release sandbox") globalSandboxList.removeSandbox(s.id) if s.monitor != nil { s.monitor.stop() } + s.hypervisor.disconnect() return s.agent.disconnect() } @@ -808,6 +810,7 @@ func (s *Sandbox) storeSandbox() error { // fetchSandbox fetches a sandbox config from a sandbox ID and returns a sandbox. func fetchSandbox(sandboxID string) (sandbox *Sandbox, err error) { + virtLog.WithField("sandbox-id", sandboxID).Info("fetch sandbox") if sandboxID == "" { return nil, errNeedSandboxID } @@ -1388,6 +1391,7 @@ func togglePauseSandbox(sandboxID string, pause bool) (*Sandbox, error) { if err != nil { return nil, err } + defer s.Release() if pause { err = s.Pause()