From 36cb6efebc599900b691e206fb9e99d3aa2fb9a3 Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Fri, 30 Jun 2017 14:27:26 -0400
Subject: [PATCH] Wait for device removal if deferredRemoval=true and
 deferredDeletion=false

There have been some cases where umount, a device can be busy for a very
short duration. Maybe its udev rules, or maybe it is runc related races
or probably it is something else. We don't know yet.

If deferred removal is enabled but deferred deletion is not, then for the
case of "docker run -ti --rm fedora bash", a container will exit, device
will be deferred removed and then immediately a call will come to delete
the device. It is possible that deletion will fail if device was busy
at that time.

A device can't be deleted if it can't be removed/deactivated first. There
is only one exception and that is when deferred deletion is on. In that
case graph driver will keep track of deleted device and try to delete it
later and return success to caller.

Always make sure that device deactivation is synchronous when device is
being deleted (except the case when deferred deletion is enabled).

This should also take care of small races when device is busy for a short
duration and it is being deleted.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 33 +++++++++++++++++------
 pkg/devicemapper/devmapper.go             |  8 ++++++
 2 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index c0116c7993..3218e8a8cc 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -2088,7 +2088,16 @@ func (devices *DeviceSet) deleteDevice(info *devInfo, syncDelete bool) error {
 	}
 
 	// Try to deactivate device in case it is active.
-	if err := devices.deactivateDevice(info); err != nil {
+	// If deferred removal is enabled and deferred deletion is disabled
+	// then make sure device is removed synchronously. There have been
+	// some cases of device being busy for short duration and we would
+	// rather busy wait for device removal to take care of these cases.
+	deferredRemove := devices.deferredRemove
+	if !devices.deferredDelete {
+		deferredRemove = false
+	}
+
+	if err := devices.deactivateDeviceMode(info, deferredRemove); err != nil {
 		logrus.Debugf("devmapper: Error deactivating device: %s", err)
 		return err
 	}
@@ -2145,6 +2154,11 @@ func (devices *DeviceSet) deactivatePool() error {
 }
 
 func (devices *DeviceSet) deactivateDevice(info *devInfo) error {
+	return devices.deactivateDeviceMode(info, devices.deferredRemove)
+}
+
+func (devices *DeviceSet) deactivateDeviceMode(info *devInfo, deferredRemove bool) error {
+	var err error
 	logrus.Debugf("devmapper: deactivateDevice START(%s)", info.Hash)
 	defer logrus.Debugf("devmapper: deactivateDevice END(%s)", info.Hash)
 
@@ -2157,14 +2171,17 @@ func (devices *DeviceSet) deactivateDevice(info *devInfo) error {
 		return nil
 	}
 
-	if devices.deferredRemove {
-		if err := devicemapper.RemoveDeviceDeferred(info.Name()); err != nil {
-			return err
-		}
+	if deferredRemove {
+		err = devicemapper.RemoveDeviceDeferred(info.Name())
 	} else {
-		if err := devices.removeDevice(info.Name()); err != nil {
-			return err
-		}
+		err = devices.removeDevice(info.Name())
+	}
+
+	// This function's semantics is such that it does not return an
+	// error if device does not exist. So if device went away by
+	// the time we actually tried to remove it, do not return error.
+	if err != devicemapper.ErrEnxio {
+		return err
 	}
 	return nil
 }
diff --git a/pkg/devicemapper/devmapper.go b/pkg/devicemapper/devmapper.go
index 73433c0e5a..fa77fc6cb1 100644
--- a/pkg/devicemapper/devmapper.go
+++ b/pkg/devicemapper/devmapper.go
@@ -336,10 +336,14 @@ func RemoveDevice(name string) error {
 	defer UdevWait(cookie)
 
 	dmSawBusy = false // reset before the task is run
+	dmSawEnxio = false
 	if err = task.run(); err != nil {
 		if dmSawBusy {
 			return ErrBusy
 		}
+		if dmSawEnxio {
+			return ErrEnxio
+		}
 		return fmt.Errorf("devicemapper: Error running RemoveDevice %s", err)
 	}
 
@@ -380,7 +384,11 @@ func RemoveDeviceDeferred(name string) error {
 	// by udev, what UdevWait is just cleaning up the semaphore.
 	defer UdevWait(cookie)
 
+	dmSawEnxio = false
 	if err = task.run(); err != nil {
+		if dmSawEnxio {
+			return ErrEnxio
+		}
 		return fmt.Errorf("devicemapper: Error running RemoveDeviceDeferred %s", err)
 	}