From f1a459229724f5e8e440b49f058167c2eeeb2dc6 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 13 Feb 2018 16:10:53 -0800 Subject: [PATCH] devmapper.shutdown: optimize Move the "unmount and deactivate" code into a separate method, and optimize it a bit: 1. Do not use filepath.Walk() as there's no requirement to recursively go into every directory under home/mnt; a list of directories in mnt is sufficient. With filepath.Walk(), in case some container will fail to unmount, it'll go through the whole container filesystem which is excessive and useless. 2. Do not use GetMounts() and check if a directory is mounted; just unmount it and ignore "not mounted" error. Note the same error is returned in case of wrong flags set, but as flags are hardcoded we can safely ignore such case. While at it, promote "can't unmount" log level from debug to warning. Signed-off-by: Kir Kolyshkin --- daemon/graphdriver/devmapper/deviceset.go | 72 +++++++++++------------ 1 file changed, 33 insertions(+), 39 deletions(-) diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go index 3926648e29..ca6251023b 100644 --- a/daemon/graphdriver/devmapper/deviceset.go +++ b/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() - - // 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.unmountAndDeactivateAll(path.Join(home, "mnt")) devices.Unlock() info, _ := devices.lookupDeviceWithLock("")