From bf1a5ce0001a29db84cb0c1c5481f3a826e0709c Mon Sep 17 00:00:00 2001
From: Peng Tao <bergwolf@gmail.com>
Date: Wed, 19 Dec 2018 16:48:51 +0800
Subject: [PATCH] sandbox: cleanup sandbox if creation failed

This includes cleaning up the sandbox on disk resources,
and closing open fds when preparing the hypervisor.

Fixes: #1057

Signed-off-by: Peng Tao <bergwolf@gmail.com>
---
 virtcontainers/api.go             | 11 +++++++++--
 virtcontainers/fc.go              |  4 ++++
 virtcontainers/hypervisor.go      |  1 +
 virtcontainers/mock_hypervisor.go |  4 ++++
 virtcontainers/qemu.go            | 15 +++++++++++++++
 virtcontainers/qemu_test.go       | 11 +++++++++++
 virtcontainers/sandbox.go         |  4 ++++
 7 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/virtcontainers/api.go b/virtcontainers/api.go
index 4b9bee8d67..c5162488c9 100644
--- a/virtcontainers/api.go
+++ b/virtcontainers/api.go
@@ -74,6 +74,13 @@ func createSandboxFromConfig(ctx context.Context, sandboxConfig SandboxConfig, f
 		return nil, err
 	}
 
+	// cleanup sandbox resources in case of any failure
+	defer func() {
+		if err != nil {
+			s.Delete()
+		}
+	}()
+
 	// Create the sandbox network
 	if err = s.createNetwork(); err != nil {
 		return nil, err
@@ -112,7 +119,7 @@ func createSandboxFromConfig(ctx context.Context, sandboxConfig SandboxConfig, f
 		}
 	}()
 
-	if err := s.getAndStoreGuestDetails(); err != nil {
+	if err = s.getAndStoreGuestDetails(); err != nil {
 		return nil, err
 	}
 
@@ -127,7 +134,7 @@ func createSandboxFromConfig(ctx context.Context, sandboxConfig SandboxConfig, f
 	}
 
 	// Setup host cgroups
-	if err := s.setupCgroups(); err != nil {
+	if err = s.setupCgroups(); err != nil {
 		return nil, err
 	}
 
diff --git a/virtcontainers/fc.go b/virtcontainers/fc.go
index 279a054859..6ba67c001a 100644
--- a/virtcontainers/fc.go
+++ b/virtcontainers/fc.go
@@ -705,3 +705,7 @@ func (fc *firecracker) getThreadIDs() (*threadIDs, error) {
 	// of get /machine-config
 	return nil, nil
 }
+
+func (fc *firecracker) cleanup() error {
+	return nil
+}
diff --git a/virtcontainers/hypervisor.go b/virtcontainers/hypervisor.go
index f994128e62..b218b01694 100644
--- a/virtcontainers/hypervisor.go
+++ b/virtcontainers/hypervisor.go
@@ -606,4 +606,5 @@ type hypervisor interface {
 	capabilities() capabilities
 	hypervisorConfig() HypervisorConfig
 	getThreadIDs() (*threadIDs, error)
+	cleanup() error
 }
diff --git a/virtcontainers/mock_hypervisor.go b/virtcontainers/mock_hypervisor.go
index 0255278704..89b3687a8c 100644
--- a/virtcontainers/mock_hypervisor.go
+++ b/virtcontainers/mock_hypervisor.go
@@ -101,3 +101,7 @@ func (m *mockHypervisor) getThreadIDs() (*threadIDs, error) {
 	vcpus := []int{os.Getpid()}
 	return &threadIDs{vcpus}, nil
 }
+
+func (m *mockHypervisor) cleanup() error {
+	return nil
+}
diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go
index 9dbb57ee14..c02ecc5475 100644
--- a/virtcontainers/qemu.go
+++ b/virtcontainers/qemu.go
@@ -533,6 +533,7 @@ func (q *qemu) startSandbox() error {
 				q.Logger().WithError(err).Error("After launching Qemu")
 			}
 		}
+		q.fds = []*os.File{}
 	}()
 
 	vmPath := filepath.Join(RunVMStoragePath, q.id)
@@ -1474,3 +1475,17 @@ func (q *qemu) resizeVCPUs(reqVCPUs uint32) (currentVCPUs uint32, newVCPUs uint3
 	}
 	return currentVCPUs, newVCPUs, nil
 }
+
+func (q *qemu) cleanup() error {
+	span, _ := q.trace("cleanup")
+	defer span.Finish()
+
+	for _, fd := range q.fds {
+		if err := fd.Close(); err != nil {
+			q.Logger().WithError(err).Warn("failed closing fd")
+		}
+	}
+	q.fds = []*os.File{}
+
+	return nil
+}
diff --git a/virtcontainers/qemu_test.go b/virtcontainers/qemu_test.go
index 168c791253..019f9fe98e 100644
--- a/virtcontainers/qemu_test.go
+++ b/virtcontainers/qemu_test.go
@@ -374,3 +374,14 @@ func TestQMPSetupShutdown(t *testing.T) {
 	err := q.qmpSetup()
 	assert.Nil(err)
 }
+
+func TestQemuCleanup(t *testing.T) {
+	assert := assert.New(t)
+
+	q := &qemu{
+		config: newQemuConfig(),
+	}
+
+	err := q.cleanup()
+	assert.Nil(err)
+}
diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go
index 4f8d367388..589ef578e1 100644
--- a/virtcontainers/sandbox.go
+++ b/virtcontainers/sandbox.go
@@ -1001,6 +1001,10 @@ func (s *Sandbox) Delete() error {
 		s.monitor.stop()
 	}
 
+	if err := s.hypervisor.cleanup(); err != nil {
+		s.Logger().WithError(err).Error("failed to cleanup hypervisor")
+	}
+
 	return s.storage.deleteSandboxResources(s.id, nil)
 }