mirror of
https://github.com/moby/moby.git
synced 2022-11-09 12:21:53 -05:00
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>
This commit is contained in:
parent
3a9ab941ad
commit
a450a575a6
1 changed files with 23 additions and 14 deletions
|
@ -181,9 +181,10 @@ func (d *Driver) String() string {
|
|||
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 @@ func (d *Driver) Remove(id string) error {
|
|||
}
|
||||
|
||||
// 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 @@ func (d *Driver) Get(id, mountLabel string) (containerfs.ContainerFS, error) {
|
|||
|
||||
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 @@ func (d *Driver) Put(id string) error {
|
|||
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
|
||||
}
|
||||
|
||||
|
|
Loading…
Add table
Reference in a new issue