From 36762c7cad212c362a90a312737463644832542f Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Thu, 17 Jan 2019 17:08:12 +0800 Subject: [PATCH 1/3] qemu: cleanup vm template path properly VM templates creates a symlink from `/run/vc/vm/sbid` to `/run/vc/vm/vmid`. We need to clean up both of them. Signed-off-by: Peng Tao --- virtcontainers/qemu.go | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 65419a83e..4ff64d492 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -8,10 +8,6 @@ package virtcontainers import ( "context" "fmt" - govmmQemu "github.com/intel/govmm/qemu" - "github.com/kata-containers/runtime/virtcontainers/pkg/uuid" - "github.com/opentracing/opentracing-go" - "github.com/sirupsen/logrus" "math" "os" "path/filepath" @@ -21,6 +17,11 @@ import ( "time" "unsafe" + govmmQemu "github.com/intel/govmm/qemu" + "github.com/kata-containers/runtime/virtcontainers/pkg/uuid" + "github.com/opentracing/opentracing-go" + "github.com/sirupsen/logrus" + "github.com/kata-containers/runtime/virtcontainers/device/config" "github.com/kata-containers/runtime/virtcontainers/types" "github.com/kata-containers/runtime/virtcontainers/utils" @@ -653,9 +654,25 @@ func (q *qemu) stopSandbox() error { return err } - err = os.RemoveAll(filepath.Join(RunVMStoragePath, q.id)) + // cleanup vm path + dir := filepath.Join(RunVMStoragePath, q.id) + + // If it's a symlink, remove both dir and the target. + // This can happen when vm template links a sandbox to a vm. + link, err := filepath.EvalSymlinks(dir) if err != nil { - q.Logger().WithError(err).Error("Fail to clean up vm directory") + // Well, it's just cleanup failure. Let's ignore it. + q.Logger().WithError(err).WithField("dir", dir).Warn("failed to resolve vm path") + } + q.Logger().WithField("link", link).WithField("dir", dir).Infof("cleanup vm path") + + if err := os.RemoveAll(dir); err != nil { + q.Logger().WithError(err).Warnf("failed to remove vm path %s", dir) + } + if link != dir && link != "" { + if err := os.RemoveAll(link); err != nil { + q.Logger().WithError(err).WithField("link", link).Warn("failed to remove resolved vm path") + } } return nil From d314e2d0b7c47d342a41ce231b34d476e4988278 Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Thu, 17 Jan 2019 17:09:48 +0800 Subject: [PATCH 2/3] agent: clean up share path created by the agent The agent code creates a directory at `/run/kata-containers/shared/sandboxes/sbid/` to hold shared data between host and guest. We need to clean it up when removing a sandbox. Fixes: #1138 Signed-off-by: Peng Tao --- virtcontainers/agent.go | 3 +++ virtcontainers/hyperstart_agent.go | 7 +++++++ virtcontainers/kata_agent.go | 8 ++++++++ virtcontainers/noop_agent.go | 3 +++ virtcontainers/sandbox.go | 2 ++ 5 files changed, 23 insertions(+) diff --git a/virtcontainers/agent.go b/virtcontainers/agent.go index cf0c71c50..9cfce0f34 100644 --- a/virtcontainers/agent.go +++ b/virtcontainers/agent.go @@ -254,4 +254,7 @@ type agent interface { // copyFile copies file from host to container's rootfs copyFile(src, dst string) error + + // cleanup removes all on disk information generated by the agent + cleanup(id string) } diff --git a/virtcontainers/hyperstart_agent.go b/virtcontainers/hyperstart_agent.go index feabc1f5a..8a0693a65 100644 --- a/virtcontainers/hyperstart_agent.go +++ b/virtcontainers/hyperstart_agent.go @@ -1012,3 +1012,10 @@ func (h *hyper) copyFile(src, dst string) error { // hyperstart-agent does not support copyFile return nil } + +func (h *hyper) cleanup(id string) { + path := h.getSharePath(id) + if err := os.RemoveAll(path); err != nil { + h.Logger().WithError(err).Errorf("failed to cleanup vm share path %s", path) + } +} diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 3f0abc423..3f725ac87 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -1835,3 +1835,11 @@ func (k *kataAgent) copyFile(src, dst string) error { return nil } + +func (k *kataAgent) cleanup(id string) { + path := k.getSharePath(id) + k.Logger().WithField("path", path).Infof("cleanup agent") + if err := os.RemoveAll(path); err != nil { + k.Logger().WithError(err).Errorf("failed to cleanup vm share path %s", path) + } +} diff --git a/virtcontainers/noop_agent.go b/virtcontainers/noop_agent.go index 1783454b9..6ba15d4b3 100644 --- a/virtcontainers/noop_agent.go +++ b/virtcontainers/noop_agent.go @@ -215,3 +215,6 @@ func (n *noopAgent) setGuestDateTime(time.Time) error { func (n *noopAgent) copyFile(src, dst string) error { return nil } + +func (n *noopAgent) cleanup(id string) { +} diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index a3794ab90..004ad3dc0 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -744,6 +744,8 @@ func (s *Sandbox) Delete() error { s.Logger().WithError(err).Error("failed to cleanup hypervisor") } + s.agent.cleanup(s.id) + return s.storage.deleteSandboxResources(s.id, nil) } From d75f26d7191eb23abdb6a407340e0a2cdc9080e7 Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Thu, 17 Jan 2019 23:50:52 +0800 Subject: [PATCH 3/3] vc: set detach flag when umounting rootfs docker might bind mount some files/dirs under container rootfs without notifying runtime. We need to unmount them otherwise docker will fail to clean up containers. man umount(2): MNT_DETACH (since Linux 2.4.11) Perform a lazy unmount: make the mount point unavailable for new accesses, immediately disconnect the filesystem and all filesystems mounted below it from each other and from the mount table, and actually perform the unmount when the mount point ceases to be busy. Signed-off-by: Peng Tao --- virtcontainers/mount.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/virtcontainers/mount.go b/virtcontainers/mount.go index 69567331f..bd279dfde 100644 --- a/virtcontainers/mount.go +++ b/virtcontainers/mount.go @@ -306,7 +306,7 @@ func bindUnmountContainerRootfs(ctx context.Context, sharedDir, sandboxID, cID s defer span.Finish() rootfsDest := filepath.Join(sharedDir, sandboxID, cID, rootfsDir) - syscall.Unmount(rootfsDest, 0) + syscall.Unmount(rootfsDest, syscall.MNT_DETACH) return nil }