From fd7611ff1f1d61d5b4b45b2c0bd83976cbccf174 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 11 Oct 2018 23:16:10 -0700 Subject: [PATCH 1/3] pkg/mount: simplify ensureMountedAs 1. There is no need to specify rw argument -- bind mounts are read-write by default. 2. There is no point in parsing /proc/self/mountinfo after performing a mount, especially if we don't check whether the fs is mounted or not -- the only outcome from it could be an error from our mountinfo parser, which makes no sense in this context. Signed-off-by: Kir Kolyshkin (cherry picked from commit f01297d1ae352bc2bf01ebf62e879c1c83cdbee4) --- pkg/mount/sharedsubtree_linux.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/mount/sharedsubtree_linux.go b/pkg/mount/sharedsubtree_linux.go index 538f6637a043c..30a3f5b8f025f 100644 --- a/pkg/mount/sharedsubtree_linux.go +++ b/pkg/mount/sharedsubtree_linux.go @@ -55,13 +55,10 @@ func ensureMountedAs(mountPoint, options string) error { } if !mounted { - if err := Mount(mountPoint, mountPoint, "none", "bind,rw"); err != nil { + if err := Mount(mountPoint, mountPoint, "none", "bind"); err != nil { return err } } - if _, err = Mounted(mountPoint); err != nil { - return err - } return ForceMount("", mountPoint, "none", options) } From 2199ada691dc635cac5cdd065d909a539dd0b793 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 11 Oct 2018 23:22:11 -0700 Subject: [PATCH 2/3] pkg/mount: add MakeMount() This function ensures the argument is the mount point (i.e. if it's not, it bind mounts it to itself). Signed-off-by: Kir Kolyshkin (cherry picked from commit 8abadb36fa8149cd44e76b0e7fdedd6f1f2eccd0) --- pkg/mount/sharedsubtree_linux.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/pkg/mount/sharedsubtree_linux.go b/pkg/mount/sharedsubtree_linux.go index 30a3f5b8f025f..8a100f0bc85a3 100644 --- a/pkg/mount/sharedsubtree_linux.go +++ b/pkg/mount/sharedsubtree_linux.go @@ -48,16 +48,23 @@ func MakeRUnbindable(mountPoint string) error { return ensureMountedAs(mountPoint, "runbindable") } -func ensureMountedAs(mountPoint, options string) error { - mounted, err := Mounted(mountPoint) +// MakeMount ensures that the file or directory given is a mount point, +// bind mounting it to itself it case it is not. +func MakeMount(mnt string) error { + mounted, err := Mounted(mnt) if err != nil { return err } + if mounted { + return nil + } + + return Mount(mnt, mnt, "none", "bind") +} - if !mounted { - if err := Mount(mountPoint, mountPoint, "none", "bind"); err != nil { - return err - } +func ensureMountedAs(mountPoint, options string) error { + if err := MakeMount(mountPoint); err != nil { + return err } return ForceMount("", mountPoint, "none", options) From fa8ac946165b8004a15e85744e774ed6ba99fd38 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 11 Oct 2018 23:37:28 -0700 Subject: [PATCH 3/3] btrfs: ensure graphdriver home is bind mount For some reason, shared mount propagation between the host and a container does not work for btrfs, unless container root directory (i.e. graphdriver home) is a bind mount. The above issue was reproduced on SLES 12sp3 + btrfs using the following script: #!/bin/bash set -eux -o pipefail # DIR should not be under a subvolume DIR=${DIR:-/lib} MNT=$DIR/my-mnt FILE=$MNT/file ID=$(docker run -d --privileged -v $DIR:$DIR:rshared ubuntu sleep 24h) docker exec $ID mkdir -p $MNT docker exec $ID mount -t tmpfs tmpfs $MNT docker exec $ID touch $FILE ls -l $FILE umount $MNT docker rm -f $ID which fails this way: + ls -l /lib/my-mnt/file ls: cannot access '/lib/my-mnt/file': No such file or directory meaning the mount performed inside a priviledged container is not propagated back to the host (even if all the mounts have "shared" propagation mode). The remedy to the above is to make graphdriver home a bind mount. Signed-off-by: Kir Kolyshkin (cherry picked from commit 16d822bba8ac5ab22c8697750f700403bca3dbf3) --- daemon/graphdriver/btrfs/btrfs.go | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/daemon/graphdriver/btrfs/btrfs.go b/daemon/graphdriver/btrfs/btrfs.go index 4c14191d46774..7ce7edef36e91 100644 --- a/daemon/graphdriver/btrfs/btrfs.go +++ b/daemon/graphdriver/btrfs/btrfs.go @@ -29,10 +29,12 @@ import ( "github.com/docker/docker/daemon/graphdriver" "github.com/docker/docker/pkg/containerfs" "github.com/docker/docker/pkg/idtools" + "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/parsers" "github.com/docker/docker/pkg/system" "github.com/docker/go-units" "github.com/opencontainers/selinux/go-selinux/label" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" ) @@ -81,6 +83,15 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap return nil, err } + // For some reason shared mount propagation between a container + // and the host does not work for btrfs, and a remedy is to bind + // mount graphdriver home to itself (even without changing the + // propagation mode). + err = mount.MakeMount(home) + if err != nil { + return nil, errors.Wrapf(err, "failed to make %s a mount", home) + } + driver := &Driver{ home: home, uidMaps: uidMaps, @@ -158,7 +169,19 @@ func (d *Driver) GetMetadata(id string) (map[string]string, error) { // Cleanup unmounts the home directory. func (d *Driver) Cleanup() error { - return d.subvolDisableQuota() + err := d.subvolDisableQuota() + umountErr := mount.Unmount(d.home) + + // in case we have two errors, prefer the one from disableQuota() + if err != nil { + return err + } + + if umountErr != nil { + return errors.Wrapf(umountErr, "error unmounting %s", d.home) + } + + return nil } func free(p *C.char) {