From 1404928c057d220da7c01866eee0a7280769b715 Mon Sep 17 00:00:00 2001
From: Sebastien Boeuf <sebastien.boeuf@intel.com>
Date: Tue, 3 Apr 2018 16:13:45 -0700
Subject: [PATCH] virtcontainers: Fix container creation rollback

The rollback does not work as expected because the error has to be
checked from the defer itself.

Fixes #178

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
---
 virtcontainers/container.go  | 26 ++++++++++++++------------
 virtcontainers/kata_agent.go | 22 ++++++++++++----------
 2 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/virtcontainers/container.go b/virtcontainers/container.go
index 3a23359025..16a95d2e05 100644
--- a/virtcontainers/container.go
+++ b/virtcontainers/container.go
@@ -420,17 +420,15 @@ func newContainer(pod *Pod, contConfig ContainerConfig) (*Container, error) {
 // been performed before the container creation failed.
 // - Unplug CPU and memory resources from the VM.
 // - Unplug devices from the VM.
-func rollbackFailingContainerCreation(c *Container, err error) {
-	if err != nil && c != nil {
-		if err2 := c.removeResources(); err2 != nil {
-			c.Logger().WithError(err2).Error("rollback failed removeResources()")
-		}
-		if err2 := c.detachDevices(); err2 != nil {
-			c.Logger().WithError(err2).Error("rollback failed detachDevices()")
-		}
-		if err2 := c.removeDrive(); err2 != nil {
-			c.Logger().WithError(err2).Error("rollback failed removeDrive()")
-		}
+func (c *Container) rollbackFailingContainerCreation() {
+	if err := c.removeResources(); err != nil {
+		c.Logger().WithError(err).Error("rollback failed removeResources()")
+	}
+	if err := c.detachDevices(); err != nil {
+		c.Logger().WithError(err).Error("rollback failed detachDevices()")
+	}
+	if err := c.removeDrive(); err != nil {
+		c.Logger().WithError(err).Error("rollback failed removeDrive()")
 	}
 }
 
@@ -452,7 +450,11 @@ func createContainer(pod *Pod, contConfig ContainerConfig) (c *Container, err er
 
 	// In case the container creation fails, the following takes care
 	// of rolling back all the actions previously performed.
-	defer rollbackFailingContainerCreation(c, err)
+	defer func() {
+		if err != nil {
+			c.rollbackFailingContainerCreation()
+		}
+	}()
 
 	if err = c.hotplugDrive(); err != nil {
 		return
diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go
index 0a207a90b0..905859b854 100644
--- a/virtcontainers/kata_agent.go
+++ b/virtcontainers/kata_agent.go
@@ -603,16 +603,14 @@ func (k *kataAgent) appendDevices(deviceList []*grpc.Device, devices []Device) [
 // been performed before the container creation failed.
 // - Unmount container volumes.
 // - Unmount container rootfs.
-func (k *kataAgent) rollbackFailingContainerCreation(c *Container, err error) {
-	if err != nil {
-		if c != nil {
-			if err2 := c.unmountHostMounts(); err2 != nil {
-				k.Logger().WithError(err2).Error("rollback failed unmountHostMounts()")
-			}
+func (k *kataAgent) rollbackFailingContainerCreation(c *Container) {
+	if c != nil {
+		if err2 := c.unmountHostMounts(); err2 != nil {
+			k.Logger().WithError(err2).Error("rollback failed unmountHostMounts()")
+		}
 
-			if err2 := bindUnmountContainerRootfs(kataHostSharedDir, c.pod.id, c.id); err2 != nil {
-				k.Logger().WithError(err2).Error("rollback failed bindUnmountContainerRootfs()")
-			}
+		if err2 := bindUnmountContainerRootfs(kataHostSharedDir, c.pod.id, c.id); err2 != nil {
+			k.Logger().WithError(err2).Error("rollback failed bindUnmountContainerRootfs()")
 		}
 	}
 }
@@ -639,7 +637,11 @@ func (k *kataAgent) createContainer(pod *Pod, c *Container) (p *Process, err err
 
 	// In case the container creation fails, the following defer statement
 	// takes care of rolling back actions previously performed.
-	defer k.rollbackFailingContainerCreation(c, err)
+	defer func() {
+		if err != nil {
+			k.rollbackFailingContainerCreation(c)
+		}
+	}()
 
 	if c.state.Fstype != "" {
 		// This is a block based device rootfs.