Merge pull request #36307 from kolyshkin/dm-misc

devmapper cleanup improvements

daemon/graphdriver/devmapper/deviceset.go

@@ -2223,6 +2223,38 @@ func (devices *DeviceSet) cancelDeferredRemoval(info *devInfo) error {
 	return err
 }
 
+func (devices *DeviceSet) unmountAndDeactivateAll(dir string) {
+	files, err := ioutil.ReadDir(dir)
+	if err != nil {
+		logrus.Warnf("devmapper: unmountAndDeactivate: %s", err)
+		return
+	}
+
+	for _, d := range files {
+		if !d.IsDir() {
+			continue
+		}
+
+		name := d.Name()
+		fullname := path.Join(dir, name)
+
+		// We use MNT_DETACH here in case it is still busy in some running
+		// container. This means it'll go away from the global scope directly,
+		// and the device will be released when that container dies.
+		if err := unix.Unmount(fullname, unix.MNT_DETACH); err != nil && err != unix.EINVAL {
+			logrus.Warnf("devmapper: Shutdown unmounting %s, error: %s", fullname, err)
+		}
+
+		if devInfo, err := devices.lookupDevice(name); err != nil {
+			logrus.Debugf("devmapper: Shutdown lookup device %s, error: %s", name, err)
+		} else {
+			if err := devices.deactivateDevice(devInfo); err != nil {
+				logrus.Debugf("devmapper: Shutdown deactivate %s, error: %s", devInfo.Hash, err)
+			}
+		}
+	}
+}
+
 // Shutdown shuts down the device by unmounting the root.
 func (devices *DeviceSet) Shutdown(home string) error {
 	logrus.Debugf("devmapper: [deviceset %s] Shutdown()", devices.devicePrefix)
@@ -2244,45 +2276,7 @@ func (devices *DeviceSet) Shutdown(home string) error {
 	// will be killed and we will not get a chance to save deviceset
 	// metadata. Hence save this early before trying to deactivate devices.
 	devices.saveDeviceSetMetaData()
-
+	devices.unmountAndDeactivateAll(path.Join(home, "mnt"))
-	// ignore the error since it's just a best effort to not try to unmount something that's mounted
-	mounts, _ := mount.GetMounts()
-	mounted := make(map[string]bool, len(mounts))
-	for _, mnt := range mounts {
-		mounted[mnt.Mountpoint] = true
-	}
-
-	if err := filepath.Walk(path.Join(home, "mnt"), func(p string, info os.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
-		if !info.IsDir() {
-			return nil
-		}
-
-		if mounted[p] {
-			// We use MNT_DETACH here in case it is still busy in some running
-			// container. This means it'll go away from the global scope directly,
-			// and the device will be released when that container dies.
-			if err := unix.Unmount(p, unix.MNT_DETACH); err != nil {
-				logrus.Debugf("devmapper: Shutdown unmounting %s, error: %s", p, err)
-			}
-		}
-
-		if devInfo, err := devices.lookupDevice(path.Base(p)); err != nil {
-			logrus.Debugf("devmapper: Shutdown lookup device %s, error: %s", path.Base(p), err)
-		} else {
-			if err := devices.deactivateDevice(devInfo); err != nil {
-				logrus.Debugf("devmapper: Shutdown deactivate %s , error: %s", devInfo.Hash, err)
-			}
-		}
-
-		return nil
-	}); err != nil && !os.IsNotExist(err) {
-		devices.Unlock()
-		return err
-	}
-
 	devices.Unlock()
 
 	info, _ := devices.lookupDeviceWithLock("")

daemon/graphdriver/devmapper/driver.go

@@ -16,6 +16,7 @@ import (
 	"github.com/docker/docker/pkg/locker"
 	"github.com/docker/docker/pkg/mount"
 	units "github.com/docker/go-units"
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 )
@@ -121,12 +122,18 @@ func (d *Driver) GetMetadata(id string) (map[string]string, error) {
 // Cleanup unmounts a device.
 func (d *Driver) Cleanup() error {
 	err := d.DeviceSet.Shutdown(d.home)
+	umountErr := mount.RecursiveUnmount(d.home)
 
-	if err2 := mount.RecursiveUnmount(d.home); err == nil {
+	// in case we have two errors, prefer the one from Shutdown()
-		err = err2
+	if err != nil {
+		return err
 	}
 
-	return err
+	if umountErr != nil {
+		return errors.Wrapf(umountErr, "error unmounting %s", d.home)
+	}
+
+	return nil
 }
 
 // CreateReadWrite creates a layer that is writable for use as a container