From 7342060b070df67481f8da4f394a57cac1671d56 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Mon, 18 Apr 2016 19:41:29 -0400 Subject: [PATCH] Add refcounts to graphdrivers that use fsdiff This makes sure fsdiff doesn't try to unmount things that shouldn't be. **Note**: This is intended as a temporary solution to have as minor a change as possible for 1.11.1. A bigger change will be required in order to support container re-attach. Signed-off-by: Brian Goff --- daemon/graphdriver/counter.go | 32 ++++++++++++++++++++++++++ daemon/graphdriver/devmapper/driver.go | 15 ++++++++++++ daemon/graphdriver/overlay/overlay.go | 16 +++++++++++++ daemon/graphdriver/zfs/zfs.go | 15 ++++++++++++ 4 files changed, 78 insertions(+) create mode 100644 daemon/graphdriver/counter.go diff --git a/daemon/graphdriver/counter.go b/daemon/graphdriver/counter.go new file mode 100644 index 0000000000..572fc9be47 --- /dev/null +++ b/daemon/graphdriver/counter.go @@ -0,0 +1,32 @@ +package graphdriver + +import "sync" + +// RefCounter is a generic counter for use by graphdriver Get/Put calls +type RefCounter struct { + counts map[string]int + mu sync.Mutex +} + +// NewRefCounter returns a new RefCounter +func NewRefCounter() *RefCounter { + return &RefCounter{counts: make(map[string]int)} +} + +// Increment increaes the ref count for the given id and returns the current count +func (c *RefCounter) Increment(id string) int { + c.mu.Lock() + c.counts[id]++ + count := c.counts[id] + c.mu.Unlock() + return count +} + +// Decrement decreases the ref count for the given id and returns the current count +func (c *RefCounter) Decrement(id string) int { + c.mu.Lock() + c.counts[id]-- + count := c.counts[id] + c.mu.Unlock() + return count +} diff --git a/daemon/graphdriver/devmapper/driver.go b/daemon/graphdriver/devmapper/driver.go index 100c4ba8a8..95990c7fee 100644 --- a/daemon/graphdriver/devmapper/driver.go +++ b/daemon/graphdriver/devmapper/driver.go @@ -28,6 +28,7 @@ type Driver struct { home string uidMaps []idtools.IDMap gidMaps []idtools.IDMap + ctr *graphdriver.RefCounter } // Init creates a driver with the given home and the set of options. @@ -46,6 +47,7 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap home: home, uidMaps: uidMaps, gidMaps: gidMaps, + ctr: graphdriver.NewRefCounter(), } return graphdriver.NewNaiveDiffDriver(d, uidMaps, gidMaps), nil @@ -157,26 +159,35 @@ func (d *Driver) Remove(id string) error { // Get mounts a device with given id into the root filesystem func (d *Driver) Get(id, mountLabel string) (string, error) { mp := path.Join(d.home, "mnt", id) + if count := d.ctr.Increment(id); count > 1 { + return mp, nil + } uid, gid, err := idtools.GetRootUIDGID(d.uidMaps, d.gidMaps) if err != nil { + d.ctr.Decrement(id) return "", err } + // Create the target directories if they don't exist if err := idtools.MkdirAllAs(path.Join(d.home, "mnt"), 0755, uid, gid); err != nil && !os.IsExist(err) { + d.ctr.Decrement(id) return "", err } if err := idtools.MkdirAs(mp, 0755, uid, gid); err != nil && !os.IsExist(err) { + d.ctr.Decrement(id) return "", err } // Mount the device if err := d.DeviceSet.MountDevice(id, mp, mountLabel); err != nil { + d.ctr.Decrement(id) return "", err } rootFs := path.Join(mp, "rootfs") if err := idtools.MkdirAllAs(rootFs, 0755, uid, gid); err != nil && !os.IsExist(err) { + d.ctr.Decrement(id) d.DeviceSet.UnmountDevice(id, mp) return "", err } @@ -186,6 +197,7 @@ func (d *Driver) Get(id, mountLabel string) (string, error) { // Create an "id" file with the container/image id in it to help reconstruct this in case // of later problems if err := ioutil.WriteFile(idFile, []byte(id), 0600); err != nil { + d.ctr.Decrement(id) d.DeviceSet.UnmountDevice(id, mp) return "", err } @@ -196,6 +208,9 @@ func (d *Driver) Get(id, mountLabel string) (string, error) { // Put unmounts a device and removes it. func (d *Driver) Put(id string) error { + if count := d.ctr.Decrement(id); count > 0 { + return nil + } mp := path.Join(d.home, "mnt", id) err := d.DeviceSet.UnmountDevice(id, mp) if err != nil { diff --git a/daemon/graphdriver/overlay/overlay.go b/daemon/graphdriver/overlay/overlay.go index bfe9a666de..afee12a61c 100644 --- a/daemon/graphdriver/overlay/overlay.go +++ b/daemon/graphdriver/overlay/overlay.go @@ -95,6 +95,7 @@ type Driver struct { pathCache map[string]string uidMaps []idtools.IDMap gidMaps []idtools.IDMap + ctr *graphdriver.RefCounter } var backingFs = "" @@ -150,6 +151,7 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap pathCache: make(map[string]string), uidMaps: uidMaps, gidMaps: gidMaps, + ctr: graphdriver.NewRefCounter(), } return NaiveDiffDriverWithApply(d, uidMaps, gidMaps), nil @@ -362,28 +364,39 @@ func (d *Driver) Get(id string, mountLabel string) (string, error) { workDir := path.Join(dir, "work") mergedDir := path.Join(dir, "merged") + if count := d.ctr.Increment(id); count > 1 { + return mergedDir, nil + } + opts := fmt.Sprintf("lowerdir=%s,upperdir=%s,workdir=%s", lowerDir, upperDir, workDir) // if it's mounted already, just return mounted, err := d.mounted(mergedDir) if err != nil { + d.ctr.Decrement(id) return "", err } if mounted { + d.ctr.Decrement(id) return mergedDir, nil } if err := syscall.Mount("overlay", mergedDir, "overlay", 0, label.FormatMountLabel(opts, mountLabel)); err != nil { + d.ctr.Decrement(id) return "", fmt.Errorf("error creating overlay mount to %s: %v", mergedDir, err) } // chown "workdir/work" to the remapped root UID/GID. Overlay fs inside a // user namespace requires this to move a directory from lower to upper. rootUID, rootGID, err := idtools.GetRootUIDGID(d.uidMaps, d.gidMaps) if err != nil { + d.ctr.Decrement(id) + syscall.Unmount(mergedDir, 0) return "", err } if err := os.Chown(path.Join(workDir, "work"), rootUID, rootGID); err != nil { + d.ctr.Decrement(id) + syscall.Unmount(mergedDir, 0) return "", err } @@ -400,6 +413,9 @@ func (d *Driver) mounted(dir string) (bool, error) { // Put unmounts the mount path created for the give id. func (d *Driver) Put(id string) error { + if count := d.ctr.Decrement(id); count > 0 { + return nil + } d.pathCacheLock.Lock() mountpoint, exists := d.pathCache[id] d.pathCacheLock.Unlock() diff --git a/daemon/graphdriver/zfs/zfs.go b/daemon/graphdriver/zfs/zfs.go index b979b2af90..ffde8a545f 100644 --- a/daemon/graphdriver/zfs/zfs.go +++ b/daemon/graphdriver/zfs/zfs.go @@ -105,6 +105,7 @@ func Init(base string, opt []string, uidMaps, gidMaps []idtools.IDMap) (graphdri filesystemsCache: filesystemsCache, uidMaps: uidMaps, gidMaps: gidMaps, + ctr: graphdriver.NewRefCounter(), } return graphdriver.NewNaiveDiffDriver(d, uidMaps, gidMaps), nil } @@ -161,6 +162,7 @@ type Driver struct { filesystemsCache map[string]bool uidMaps []idtools.IDMap gidMaps []idtools.IDMap + ctr *graphdriver.RefCounter } func (d *Driver) String() string { @@ -305,25 +307,35 @@ 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) (string, error) { mountpoint := d.mountPath(id) + if count := d.ctr.Increment(id); count > 1 { + return mountpoint, nil + } + filesystem := d.zfsPath(id) options := label.FormatMountLabel("", mountLabel) logrus.Debugf(`[zfs] mount("%s", "%s", "%s")`, filesystem, mountpoint, options) rootUID, rootGID, err := idtools.GetRootUIDGID(d.uidMaps, d.gidMaps) if err != nil { + d.ctr.Decrement(id) return "", err } // Create the target directories if they don't exist if err := idtools.MkdirAllAs(mountpoint, 0755, rootUID, rootGID); err != nil { + d.ctr.Decrement(id) return "", err } if err := mount.Mount(filesystem, mountpoint, "zfs", options); err != nil { + d.ctr.Decrement(id) return "", 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(id) return "", fmt.Errorf("error modifying zfs mountpoint (%s) directory ownership: %v", mountpoint, err) } @@ -332,6 +344,9 @@ func (d *Driver) Get(id, mountLabel string) (string, error) { // Put removes the existing mountpoint for the given id if it exists. func (d *Driver) Put(id string) error { + if count := d.ctr.Decrement(id); count > 0 { + return nil + } mountpoint := d.mountPath(id) mounted, err := graphdriver.Mounted(graphdriver.FsMagicZfs, mountpoint) if err != nil || !mounted {