zfs: fix ebusy on umount
This commit is a set of fixes and improvement for zfs graph driver,
in particular:
1. Remove mount point after umount in `Get()` error path, as well
as in `Put()` (with `MNT_DETACH` flag). This should solve "failed
to remove root filesystem for <ID> .... dataset is busy" error
reported in Moby issue 35642.
To reproduce the issue:
- start dockerd with zfs
- docker run -d --name c1 --rm busybox top
- docker run -d --name c2 --rm busybox top
- docker stop c1
- docker rm c1
Output when the bug is present:
```
Error response from daemon: driver "zfs" failed to remove root
filesystem for XXX : exit status 1: "/sbin/zfs zfs destroy -r
scratch/docker/YYY" => cannot destroy 'scratch/docker/YYY':
dataset is busy
```
Output when the bug is fixed:
```
Error: No such container: c1
```
(as the container has been successfully autoremoved on stop)
2. Fix/improve error handling in `Get()` -- do not try to umount
if `refcount` > 0
3. Simplifies unmount in `Get()`. Specifically, remove call to
`graphdriver.Mounted()` (which checks if fs is mounted using
`statfs()` and check for fs type) and `mount.Unmount()` (which
parses `/proc/self/mountinfo`). Calling `unix.Unmount()` is
simple and sufficient.
4. Add unmounting of driver's home to `Cleanup()`.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
diff --git a/daemon/graphdriver/zfs/zfs.go b/daemon/graphdriver/zfs/zfs.go
index 51d31bc..8b3f78d 100644
--- a/daemon/graphdriver/zfs/zfs.go
+++ b/daemon/graphdriver/zfs/zfs.go
@@ -181,9 +181,10 @@
return "zfs"
}
-// Cleanup is used to implement graphdriver.ProtoDriver. There is no cleanup required for this driver.
+// Cleanup is called on daemon shutdown. It unmounts the bind mount
+// created by mount.MakePrivate() in Init().
func (d *Driver) Cleanup() error {
- return nil
+ return mount.Unmount(d.options.mountPath)
}
// Status returns information about the ZFS filesystem. It returns a two dimensional array of information
@@ -357,11 +358,24 @@
}
// Get returns the mountpoint for the given id after creating the target directories if necessary.
-func (d *Driver) Get(id, mountLabel string) (containerfs.ContainerFS, error) {
+func (d *Driver) Get(id, mountLabel string) (_ containerfs.ContainerFS, retErr error) {
mountpoint := d.mountPath(id)
if count := d.ctr.Increment(mountpoint); count > 1 {
return containerfs.NewLocalContainerFS(mountpoint), nil
}
+ defer func() {
+ if retErr != nil {
+ if c := d.ctr.Decrement(mountpoint); c <= 0 {
+ if mntErr := unix.Unmount(mountpoint, 0); mntErr != nil {
+ logrus.Errorf("Error unmounting %v: %v", mountpoint, mntErr)
+ }
+ if rmErr := unix.Rmdir(mountpoint); rmErr != nil && !os.IsNotExist(rmErr) {
+ logrus.Debugf("Failed to remove %s: %v", id, rmErr)
+ }
+
+ }
+ }
+ }()
filesystem := d.zfsPath(id)
options := label.FormatMountLabel("", mountLabel)
@@ -369,25 +383,20 @@
rootUID, rootGID, err := idtools.GetRootUIDGID(d.uidMaps, d.gidMaps)
if err != nil {
- d.ctr.Decrement(mountpoint)
return nil, err
}
// Create the target directories if they don't exist
if err := idtools.MkdirAllAndChown(mountpoint, 0755, idtools.IDPair{rootUID, rootGID}); err != nil {
- d.ctr.Decrement(mountpoint)
return nil, err
}
if err := mount.Mount(filesystem, mountpoint, "zfs", options); err != nil {
- d.ctr.Decrement(mountpoint)
return nil, fmt.Errorf("error creating zfs mount of %s to %s: %v", filesystem, mountpoint, err)
}
// this could be our first mount after creation of the filesystem, and the root dir may still have root
// permissions instead of the remapped root uid:gid (if user namespaces are enabled):
if err := os.Chown(mountpoint, rootUID, rootGID); err != nil {
- mount.Unmount(mountpoint)
- d.ctr.Decrement(mountpoint)
return nil, fmt.Errorf("error modifying zfs mountpoint (%s) directory ownership: %v", mountpoint, err)
}
@@ -400,16 +409,16 @@
if count := d.ctr.Decrement(mountpoint); count > 0 {
return nil
}
- mounted, err := graphdriver.Mounted(graphdriver.FsMagicZfs, mountpoint)
- if err != nil || !mounted {
- return err
- }
logrus.Debugf(`[zfs] unmount("%s")`, mountpoint)
- if err := mount.Unmount(mountpoint); err != nil {
- return fmt.Errorf("error unmounting to %s: %v", mountpoint, err)
+ if err := unix.Unmount(mountpoint, unix.MNT_DETACH); err != nil {
+ logrus.Warnf("Failed to unmount %s mount %s: %v", id, mountpoint, err)
}
+ if err := unix.Rmdir(mountpoint); err != nil && !os.IsNotExist(err) {
+ logrus.Debugf("Failed to remove %s mount point %s: %v", id, mountpoint, err)
+ }
+
return nil
}
