From a97e7a41c8d13153397f4c557b5b54c4c53b6b9f Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 19 Jan 2018 11:56:32 -0800 Subject: [PATCH 1/7] pkg/mount: implement/use filter for mountinfo parsing Functions `GetMounts()` and `parseMountTable()` return all the entries as read and parsed from /proc/self/mountinfo. In many cases the caller is only interested only one or a few entries, not all of them. One good example is `Mounted()` function, which looks for a specific entry only. Another example is `RecursiveUnmount()` which is only interested in mount under a specific path. This commit adds `filter` argument to `GetMounts()` to implement two things: 1. filter out entries a caller is not interested in 2. stop processing if a caller is found what it wanted `nil` can be passed to get a backward-compatible behavior, i.e. return all the entries. A few filters are implemented: - `PrefixFilter`: filters out all entries not under `prefix` - `SingleEntryFilter`: looks for a specific entry Finally, `Mounted()` is modified to use `SingleEntryFilter()`, and `RecursiveUnmount()` is using `PrefixFilter()`. Unit tests are added to check filters are working. [v2: ditch NoFilter, use nil] [v3: ditch GetMountsFiltered()] [v4: add unit test for filters] [v5: switch to gotestyourself] Signed-off-by: Kir Kolyshkin Upstream-commit: bb934c6aca3e77541dd4fd51b9ab2706294dadda Component: engine --- components/engine/daemon/daemon_linux.go | 2 +- .../engine/daemon/graphdriver/zfs/zfs.go | 2 +- components/engine/daemon/oci_linux.go | 2 +- .../docker_cli_daemon_plugins_test.go | 2 +- components/engine/pkg/mount/mount.go | 49 +++++++++++++------ .../engine/pkg/mount/mount_unix_test.go | 2 +- .../engine/pkg/mount/mounter_linux_test.go | 2 +- .../engine/pkg/mount/mountinfo_freebsd.go | 16 +++++- .../engine/pkg/mount/mountinfo_linux.go | 19 +++++-- .../engine/pkg/mount/mountinfo_linux_test.go | 34 +++++++++++-- .../engine/pkg/mount/mountinfo_unsupported.go | 2 +- .../engine/pkg/mount/mountinfo_windows.go | 2 +- components/engine/volume/local/local.go | 2 +- components/engine/volume/local/local_test.go | 2 +- 14 files changed, 104 insertions(+), 34 deletions(-) diff --git a/components/engine/daemon/daemon_linux.go b/components/engine/daemon/daemon_linux.go index d27e7333b7..bbac8a4fbc 100644 --- a/components/engine/daemon/daemon_linux.go +++ b/components/engine/daemon/daemon_linux.go @@ -74,7 +74,7 @@ func (daemon *Daemon) cleanupMounts() error { return err } - infos, err := mount.GetMounts() + infos, err := mount.GetMounts(nil) if err != nil { return errors.Wrap(err, "error reading mount table for cleanup") } diff --git a/components/engine/daemon/graphdriver/zfs/zfs.go b/components/engine/daemon/graphdriver/zfs/zfs.go index 7183e69421..c5054531a5 100644 --- a/components/engine/daemon/graphdriver/zfs/zfs.go +++ b/components/engine/daemon/graphdriver/zfs/zfs.go @@ -145,7 +145,7 @@ func lookupZfsDataset(rootdir string) (string, error) { } wantedDev := stat.Dev - mounts, err := mount.GetMounts() + mounts, err := mount.GetMounts(nil) if err != nil { return "", err } diff --git a/components/engine/daemon/oci_linux.go b/components/engine/daemon/oci_linux.go index a3638ace21..5f8b9e5aea 100644 --- a/components/engine/daemon/oci_linux.go +++ b/components/engine/daemon/oci_linux.go @@ -398,7 +398,7 @@ func getSourceMount(source string) (string, string, error) { return "", "", err } - mountinfos, err := mount.GetMounts() + mountinfos, err := mount.GetMounts(nil) if err != nil { return "", "", err } diff --git a/components/engine/integration-cli/docker_cli_daemon_plugins_test.go b/components/engine/integration-cli/docker_cli_daemon_plugins_test.go index 6d47fb1e03..2f66bc05e2 100644 --- a/components/engine/integration-cli/docker_cli_daemon_plugins_test.go +++ b/components/engine/integration-cli/docker_cli_daemon_plugins_test.go @@ -260,7 +260,7 @@ func (s *DockerDaemonSuite) TestPluginVolumeRemoveOnRestart(c *check.C) { } func existsMountpointWithPrefix(mountpointPrefix string) (bool, error) { - mounts, err := mount.GetMounts() + mounts, err := mount.GetMounts(nil) if err != nil { return false, err } diff --git a/components/engine/pkg/mount/mount.go b/components/engine/pkg/mount/mount.go index 7bdbf8a837..68000c38b1 100644 --- a/components/engine/pkg/mount/mount.go +++ b/components/engine/pkg/mount/mount.go @@ -9,26 +9,48 @@ import ( "github.com/sirupsen/logrus" ) -// GetMounts retrieves a list of mounts for the current running process. -func GetMounts() ([]*Info, error) { - return parseMountTable() +// FilterFunc is a type defining a callback function +// to filter out unwanted entries. It takes a pointer +// to an Info struct (not fully populated, currently +// only Mountpoint is filled in), and returns two booleans: +// - skip: true if the entry should be skipped +// - stop: true if parsing should be stopped after the entry +type FilterFunc func(*Info) (skip, stop bool) + +// PrefixFilter discards all entries whose mount points +// do not start with a prefix specified +func PrefixFilter(prefix string) FilterFunc { + return func(m *Info) (bool, bool) { + skip := !strings.HasPrefix(m.Mountpoint, prefix) + return skip, false + } +} + +// SingleEntryFilter looks for a specific entry +func SingleEntryFilter(mp string) FilterFunc { + return func(m *Info) (bool, bool) { + if m.Mountpoint == mp { + return false, true // don't skip, stop now + } + return true, false // skip, keep going + } +} + +// GetMounts retrieves a list of mounts for the current running process, +// with an optional filter applied (use nil for no filter). +func GetMounts(f FilterFunc) ([]*Info, error) { + return parseMountTable(f) } // Mounted determines if a specified mountpoint has been mounted. // On Linux it looks at /proc/self/mountinfo. func Mounted(mountpoint string) (bool, error) { - entries, err := parseMountTable() + entries, err := GetMounts(SingleEntryFilter(mountpoint)) if err != nil { return false, err } - // Search the table for the mountpoint - for _, e := range entries { - if e.Mountpoint == mountpoint { - return true, nil - } - } - return false, nil + return len(entries) > 0, nil } // Mount will mount filesystem according to the specified configuration, on the @@ -66,7 +88,7 @@ func Unmount(target string) error { // RecursiveUnmount unmounts the target and all mounts underneath, starting with // the deepsest mount first. func RecursiveUnmount(target string) error { - mounts, err := GetMounts() + mounts, err := parseMountTable(PrefixFilter(target)) if err != nil { return err } @@ -77,9 +99,6 @@ func RecursiveUnmount(target string) error { }) for i, m := range mounts { - if !strings.HasPrefix(m.Mountpoint, target) { - continue - } logrus.Debugf("Trying to unmount %s", m.Mountpoint) err = unmount(m.Mountpoint, mntDetach) if err != nil { diff --git a/components/engine/pkg/mount/mount_unix_test.go b/components/engine/pkg/mount/mount_unix_test.go index 84699eee5e..befff9d50c 100644 --- a/components/engine/pkg/mount/mount_unix_test.go +++ b/components/engine/pkg/mount/mount_unix_test.go @@ -129,7 +129,7 @@ func TestMountReadonly(t *testing.T) { } func TestGetMounts(t *testing.T) { - mounts, err := GetMounts() + mounts, err := GetMounts(nil) if err != nil { t.Fatal(err) } diff --git a/components/engine/pkg/mount/mounter_linux_test.go b/components/engine/pkg/mount/mounter_linux_test.go index e5da438efe..15f10593ac 100644 --- a/components/engine/pkg/mount/mounter_linux_test.go +++ b/components/engine/pkg/mount/mounter_linux_test.go @@ -121,7 +121,7 @@ func ensureUnmount(t *testing.T, mnt string) { // validateMount checks that mnt has the given options func validateMount(t *testing.T, mnt string, opts, optional, vfs string) { - info, err := GetMounts() + info, err := GetMounts(nil) if err != nil { t.Fatal(err) } diff --git a/components/engine/pkg/mount/mountinfo_freebsd.go b/components/engine/pkg/mount/mountinfo_freebsd.go index b86f4a97f6..36c89dc1a2 100644 --- a/components/engine/pkg/mount/mountinfo_freebsd.go +++ b/components/engine/pkg/mount/mountinfo_freebsd.go @@ -15,7 +15,7 @@ import ( // Parse /proc/self/mountinfo because comparing Dev and ino does not work from // bind mounts. -func parseMountTable() ([]*Info, error) { +func parseMountTable(filter FilterFunc) ([]*Info, error) { var rawEntries *C.struct_statfs count := int(C.getmntinfo(&rawEntries, C.MNT_WAIT)) @@ -32,10 +32,24 @@ func parseMountTable() ([]*Info, error) { var out []*Info for _, entry := range entries { var mountinfo Info + var skip, stop bool mountinfo.Mountpoint = C.GoString(&entry.f_mntonname[0]) + + if filter != nil { + // filter out entries we're not interested in + skip, stop = filter(p) + if skip { + continue + } + } + mountinfo.Source = C.GoString(&entry.f_mntfromname[0]) mountinfo.Fstype = C.GoString(&entry.f_fstypename[0]) + out = append(out, &mountinfo) + if stop { + break + } } return out, nil } diff --git a/components/engine/pkg/mount/mountinfo_linux.go b/components/engine/pkg/mount/mountinfo_linux.go index 4afa8d538d..8842c0040e 100644 --- a/components/engine/pkg/mount/mountinfo_linux.go +++ b/components/engine/pkg/mount/mountinfo_linux.go @@ -28,17 +28,17 @@ const ( // Parse /proc/self/mountinfo because comparing Dev and ino does not work from // bind mounts -func parseMountTable() ([]*Info, error) { +func parseMountTable(filter FilterFunc) ([]*Info, error) { f, err := os.Open("/proc/self/mountinfo") if err != nil { return nil, err } defer f.Close() - return parseInfoFile(f) + return parseInfoFile(f, filter) } -func parseInfoFile(r io.Reader) ([]*Info, error) { +func parseInfoFile(r io.Reader, filter FilterFunc) ([]*Info, error) { var ( s = bufio.NewScanner(r) out = []*Info{} @@ -53,6 +53,7 @@ func parseInfoFile(r io.Reader) ([]*Info, error) { p = &Info{} text = s.Text() optionalFields string + skip, stop bool ) if _, err := fmt.Sscanf(text, mountinfoFormat, @@ -60,6 +61,13 @@ func parseInfoFile(r io.Reader) ([]*Info, error) { &p.Root, &p.Mountpoint, &p.Opts, &optionalFields); err != nil { return nil, fmt.Errorf("Scanning '%s' failed: %s", text, err) } + if filter != nil { + // filter out entries we're not interested in + skip, stop = filter(p) + if skip { + continue + } + } // Safe as mountinfo encodes mountpoints with spaces as \040. index := strings.Index(text, " - ") postSeparatorFields := strings.Fields(text[index+3:]) @@ -75,6 +83,9 @@ func parseInfoFile(r io.Reader) ([]*Info, error) { p.Source = postSeparatorFields[1] p.VfsOpts = strings.Join(postSeparatorFields[2:], " ") out = append(out, p) + if stop { + break + } } return out, nil } @@ -89,5 +100,5 @@ func PidMountInfo(pid int) ([]*Info, error) { } defer f.Close() - return parseInfoFile(f) + return parseInfoFile(f, nil) } diff --git a/components/engine/pkg/mount/mountinfo_linux_test.go b/components/engine/pkg/mount/mountinfo_linux_test.go index 771b353823..2010ccb719 100644 --- a/components/engine/pkg/mount/mountinfo_linux_test.go +++ b/components/engine/pkg/mount/mountinfo_linux_test.go @@ -5,6 +5,8 @@ package mount // import "github.com/docker/docker/pkg/mount" import ( "bytes" "testing" + + "github.com/gotestyourself/gotestyourself/assert" ) const ( @@ -424,7 +426,7 @@ const ( func TestParseFedoraMountinfo(t *testing.T) { r := bytes.NewBuffer([]byte(fedoraMountinfo)) - _, err := parseInfoFile(r) + _, err := parseInfoFile(r, nil) if err != nil { t.Fatal(err) } @@ -432,7 +434,7 @@ func TestParseFedoraMountinfo(t *testing.T) { func TestParseUbuntuMountinfo(t *testing.T) { r := bytes.NewBuffer([]byte(ubuntuMountInfo)) - _, err := parseInfoFile(r) + _, err := parseInfoFile(r, nil) if err != nil { t.Fatal(err) } @@ -440,7 +442,7 @@ func TestParseUbuntuMountinfo(t *testing.T) { func TestParseGentooMountinfo(t *testing.T) { r := bytes.NewBuffer([]byte(gentooMountinfo)) - _, err := parseInfoFile(r) + _, err := parseInfoFile(r, nil) if err != nil { t.Fatal(err) } @@ -448,7 +450,7 @@ func TestParseGentooMountinfo(t *testing.T) { func TestParseFedoraMountinfoFields(t *testing.T) { r := bytes.NewBuffer([]byte(fedoraMountinfo)) - infos, err := parseInfoFile(r) + infos, err := parseInfoFile(r, nil) if err != nil { t.Fatal(err) } @@ -474,3 +476,27 @@ func TestParseFedoraMountinfoFields(t *testing.T) { t.Fatalf("expected %#v, got %#v", mi, infos[0]) } } + +func TestParseMountinfoFilters(t *testing.T) { + r := bytes.NewReader([]byte(fedoraMountinfo)) + + infos, err := parseInfoFile(r, SingleEntryFilter("/sys/fs/cgroup")) + assert.NilError(t, err) + assert.Equal(t, 1, len(infos)) + + r.Reset([]byte(fedoraMountinfo)) + infos, err = parseInfoFile(r, SingleEntryFilter("nonexistent")) + assert.NilError(t, err) + assert.Equal(t, 0, len(infos)) + + r.Reset([]byte(fedoraMountinfo)) + infos, err = parseInfoFile(r, PrefixFilter("/sys")) + assert.NilError(t, err) + // there are 18 entries starting with /sys in fedoraMountinfo + assert.Equal(t, 18, len(infos)) + + r.Reset([]byte(fedoraMountinfo)) + infos, err = parseInfoFile(r, PrefixFilter("nonexistent")) + assert.NilError(t, err) + assert.Equal(t, 0, len(infos)) +} diff --git a/components/engine/pkg/mount/mountinfo_unsupported.go b/components/engine/pkg/mount/mountinfo_unsupported.go index 2ecc8baed8..fd16d3ed69 100644 --- a/components/engine/pkg/mount/mountinfo_unsupported.go +++ b/components/engine/pkg/mount/mountinfo_unsupported.go @@ -7,6 +7,6 @@ import ( "runtime" ) -func parseMountTable() ([]*Info, error) { +func parseMountTable(f FilterFunc) ([]*Info, error) { return nil, fmt.Errorf("mount.parseMountTable is not implemented on %s/%s", runtime.GOOS, runtime.GOARCH) } diff --git a/components/engine/pkg/mount/mountinfo_windows.go b/components/engine/pkg/mount/mountinfo_windows.go index 7ecba7c13a..27e0f6976e 100644 --- a/components/engine/pkg/mount/mountinfo_windows.go +++ b/components/engine/pkg/mount/mountinfo_windows.go @@ -1,6 +1,6 @@ package mount // import "github.com/docker/docker/pkg/mount" -func parseMountTable() ([]*Info, error) { +func parseMountTable(f FilterFunc) ([]*Info, error) { // Do NOT return an error! return nil, nil } diff --git a/components/engine/volume/local/local.go b/components/engine/volume/local/local.go index df1299d668..3eee5a92f9 100644 --- a/components/engine/volume/local/local.go +++ b/components/engine/volume/local/local.go @@ -66,7 +66,7 @@ func New(scope string, rootIDs idtools.IDPair) (*Root, error) { return nil, err } - mountInfos, err := mount.GetMounts() + mountInfos, err := mount.GetMounts(nil) if err != nil { logrus.Debugf("error looking up mounts for local volume cleanup: %v", err) } diff --git a/components/engine/volume/local/local_test.go b/components/engine/volume/local/local_test.go index e26fdd6386..163314fab7 100644 --- a/components/engine/volume/local/local_test.go +++ b/components/engine/volume/local/local_test.go @@ -215,7 +215,7 @@ func TestCreateWithOpts(t *testing.T) { } }() - mountInfos, err := mount.GetMounts() + mountInfos, err := mount.GetMounts(nil) if err != nil { t.Fatal(err) } From 7e50bdbeb95cbda1f76fd0ffba1c0d7933b7e2e4 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 28 Feb 2018 21:55:57 -0800 Subject: [PATCH 2/7] daemon.cleanupMounts(): use mount.SingleEntryFilter Use mount.SingleEntryFilter as we're only interested in a single entry. Test case data of TestShouldUnmountRoot is modified accordingly, as from now on: 1. `info` can't be nil; 2. the mountpoint check is not performed (as SingleEntryFilter guarantees it to be equal to daemon.root). Signed-off-by: Kir Kolyshkin Upstream-commit: d3ebcde82aae79be8cbecab365367b17adac4b3e Component: engine --- components/engine/daemon/daemon_linux.go | 16 +++++++--------- components/engine/daemon/daemon_linux_test.go | 12 ------------ 2 files changed, 7 insertions(+), 21 deletions(-) diff --git a/components/engine/daemon/daemon_linux.go b/components/engine/daemon/daemon_linux.go index bbac8a4fbc..ccb0974096 100644 --- a/components/engine/daemon/daemon_linux.go +++ b/components/engine/daemon/daemon_linux.go @@ -74,18 +74,22 @@ func (daemon *Daemon) cleanupMounts() error { return err } - infos, err := mount.GetMounts(nil) + info, err := mount.GetMounts(mount.SingleEntryFilter(daemon.root)) if err != nil { return errors.Wrap(err, "error reading mount table for cleanup") } - info := getMountInfo(infos, daemon.root) + if len(info) < 1 { + // no mount found, we're done here + return nil + } + // `info.Root` here is the root mountpoint of the passed in path (`daemon.root`). // The ony cases that need to be cleaned up is when the daemon has performed a // `mount --bind /daemon/root /daemon/root && mount --make-shared /daemon/root` // This is only done when the daemon is started up and `/daemon/root` is not // already on a shared mountpoint. - if !shouldUnmountRoot(daemon.root, info) { + if !shouldUnmountRoot(daemon.root, info[0]) { return nil } @@ -114,12 +118,6 @@ func getRealPath(path string) (string, error) { } func shouldUnmountRoot(root string, info *mount.Info) bool { - if info == nil { - return false - } - if info.Mountpoint != root { - return false - } if !strings.HasSuffix(root, info.Root) { return false } diff --git a/components/engine/daemon/daemon_linux_test.go b/components/engine/daemon/daemon_linux_test.go index 195afb1e0f..671e6f461f 100644 --- a/components/engine/daemon/daemon_linux_test.go +++ b/components/engine/daemon/daemon_linux_test.go @@ -179,12 +179,6 @@ func TestShouldUnmountRoot(t *testing.T) { info: &mount.Info{Root: "/docker", Mountpoint: "/docker"}, expect: true, }, - { - desc: "not a mountpoint", - root: "/docker", - info: nil, - expect: false, - }, { desc: "root is at in a submount from `/`", root: "/foo/docker", @@ -197,12 +191,6 @@ func TestShouldUnmountRoot(t *testing.T) { info: &mount.Info{Root: "/docker/volumes/1234657/_data", Mountpoint: "/docker"}, expect: false, }, - { - desc: "root is mounted in from a parent mount namespace different root dir", - root: "/foo/bar", - info: &mount.Info{Root: "/docker/volumes/1234657/_data", Mountpoint: "/foo/bar"}, - expect: false, - }, } { t.Run(test.desc, func(t *testing.T) { for _, options := range []struct { From cd146e5f0da9199a5eb05f289a74f56a98b04727 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 25 Jan 2018 20:13:46 -0800 Subject: [PATCH 3/7] getSourceMount(): simplify The flow of getSourceMount was: 1 get all entries from /proc/self/mountinfo 2 do a linear search for the `source` directory 3 if found, return its data 4 get the parent directory of `source`, goto 2 The repeated linear search through the whole mountinfo (which can have thousands of records) is inefficient. Instead, let's just 1 collect all the relevant records (only those mount points that can be a parent of `source`) 2 find the record with the longest mountpath, return its data This was tested manually with something like ```go func TestGetSourceMount(t *testing.T) { mnt, flags, err := getSourceMount("/sys/devices/msr/") assert.NoError(t, err) t.Logf("mnt: %v, flags: %v", mnt, flags) } ``` ...but it relies on having a specific mount points on the system being used for testing. [v2: add unit tests for ParentsFilter] Signed-off-by: Kir Kolyshkin Upstream-commit: 871c957242df9f8c74faf751a2f14eb5178d4140 Component: engine --- components/engine/daemon/oci_linux.go | 38 +++++++------------ components/engine/pkg/mount/mount.go | 11 ++++++ .../engine/pkg/mount/mountinfo_linux_test.go | 6 +++ 3 files changed, 30 insertions(+), 25 deletions(-) diff --git a/components/engine/daemon/oci_linux.go b/components/engine/daemon/oci_linux.go index 5f8b9e5aea..dbd9410254 100644 --- a/components/engine/daemon/oci_linux.go +++ b/components/engine/daemon/oci_linux.go @@ -380,15 +380,6 @@ func specMapping(s []idtools.IDMap) []specs.LinuxIDMapping { return ids } -func getMountInfo(mountinfo []*mount.Info, dir string) *mount.Info { - for _, m := range mountinfo { - if m.Mountpoint == dir { - return m - } - } - return nil -} - // Get the source mount point of directory passed in as argument. Also return // optional fields. func getSourceMount(source string) (string, string, error) { @@ -398,29 +389,26 @@ func getSourceMount(source string) (string, string, error) { return "", "", err } - mountinfos, err := mount.GetMounts(nil) + mi, err := mount.GetMounts(mount.ParentsFilter(sourcePath)) if err != nil { return "", "", err } - - mountinfo := getMountInfo(mountinfos, sourcePath) - if mountinfo != nil { - return sourcePath, mountinfo.Optional, nil + if len(mi) < 1 { + return "", "", fmt.Errorf("Can't find mount point of %s", source) } - path := sourcePath - for { - path = filepath.Dir(path) - - mountinfo = getMountInfo(mountinfos, path) - if mountinfo != nil { - return path, mountinfo.Optional, nil - } - - if path == "/" { - break + // find the longest mount point + var idx, maxlen int + for i := range mi { + if len(mi[i].Mountpoint) > maxlen { + maxlen = len(mi[i].Mountpoint) + idx = i } } + // and return it unless it's "/" + if mi[idx].Mountpoint != "/" { + return mi[idx].Mountpoint, mi[idx].Optional, nil + } // If we are here, we did not find parent mount. Something is wrong. return "", "", fmt.Errorf("Could not find source mount of %s", source) diff --git a/components/engine/pkg/mount/mount.go b/components/engine/pkg/mount/mount.go index 68000c38b1..c440b81ce9 100644 --- a/components/engine/pkg/mount/mount.go +++ b/components/engine/pkg/mount/mount.go @@ -36,6 +36,17 @@ func SingleEntryFilter(mp string) FilterFunc { } } +// ParentsFilter returns all entries whose mount points +// can be parents of a path specified, discarding others. +// For example, given `/var/lib/docker/something`, entries +// like `/var/lib/docker`, `/var` and `/` are returned. +func ParentsFilter(path string) FilterFunc { + return func(m *Info) (bool, bool) { + skip := !strings.HasPrefix(path, m.Mountpoint) + return skip, false + } +} + // GetMounts retrieves a list of mounts for the current running process, // with an optional filter applied (use nil for no filter). func GetMounts(f FilterFunc) ([]*Info, error) { diff --git a/components/engine/pkg/mount/mountinfo_linux_test.go b/components/engine/pkg/mount/mountinfo_linux_test.go index 2010ccb719..dea5573e63 100644 --- a/components/engine/pkg/mount/mountinfo_linux_test.go +++ b/components/engine/pkg/mount/mountinfo_linux_test.go @@ -499,4 +499,10 @@ func TestParseMountinfoFilters(t *testing.T) { infos, err = parseInfoFile(r, PrefixFilter("nonexistent")) assert.NilError(t, err) assert.Equal(t, 0, len(infos)) + + r.Reset([]byte(fedoraMountinfo)) + infos, err = parseInfoFile(r, ParentsFilter("/sys/fs/cgroup/cpu,cpuacct")) + assert.NilError(t, err) + // there should be 4 results returned: /sys/fs/cgroup/cpu,cpuacct /sys/fs/cgroup /sys / + assert.Equal(t, 4, len(infos)) } From 96e23c415253cd3f5dd77181debec9d02e27ff2a Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 23 Jan 2018 00:41:22 -0800 Subject: [PATCH 4/7] pkg/mount/mountinfo_linux: parser speed up The mountinfo parser implemented via `fmt.Sscanf()` is slower than the one using `strings.Split()` and `strconv.Atoi()`. This rewrite helps to speed it up to a factor of 8x, here is a result from go bench: > BenchmarkParsingScanf-4 300 22294112 ns/op > BenchmarkParsingSplit-4 3000 2780703 ns/op I tried other approaches, such as using `fmt.Sscanf()` for the first three (integer) fields and `strings.Split()` for the rest, but it slows things down considerably: > BenchmarkParsingMixed-4 1000 8827058 ns/op Note the old code uses `fmt.Sscanf`, when a linear search for '-' field, when a split for the last 3 fields. The new code relies on a single split. I have also added more comments to aid in future development. Finally, the test data is fixed to now have white space before the first field. Signed-off-by: Kir Kolyshkin Upstream-commit: c611f18a7f16d8aa878a5a5c7537d23a0937c40a Component: engine --- .../engine/pkg/mount/mountinfo_linux.go | 164 ++++++++++-------- .../engine/pkg/mount/mountinfo_linux_test.go | 114 ++++++------ 2 files changed, 153 insertions(+), 125 deletions(-) diff --git a/components/engine/pkg/mount/mountinfo_linux.go b/components/engine/pkg/mount/mountinfo_linux.go index 8842c0040e..c1dba01fc3 100644 --- a/components/engine/pkg/mount/mountinfo_linux.go +++ b/components/engine/pkg/mount/mountinfo_linux.go @@ -5,26 +5,106 @@ import ( "fmt" "io" "os" + "strconv" "strings" ) -const ( - /* 36 35 98:0 /mnt1 /mnt2 rw,noatime master:1 - ext3 /dev/root rw,errors=continue - (1)(2)(3) (4) (5) (6) (7) (8) (9) (10) (11) +func parseInfoFile(r io.Reader, filter FilterFunc) ([]*Info, error) { + s := bufio.NewScanner(r) + out := []*Info{} + for s.Scan() { + if err := s.Err(); err != nil { + return nil, err + } + /* + 36 35 98:0 /mnt1 /mnt2 rw,noatime master:1 - ext3 /dev/root rw,errors=continue + (1)(2)(3) (4) (5) (6) (7) (8) (9) (10) (11) - (1) mount ID: unique identifier of the mount (may be reused after umount) - (2) parent ID: ID of parent (or of self for the top of the mount tree) - (3) major:minor: value of st_dev for files on filesystem - (4) root: root of the mount within the filesystem - (5) mount point: mount point relative to the process's root - (6) mount options: per mount options - (7) optional fields: zero or more fields of the form "tag[:value]" - (8) separator: marks the end of the optional fields - (9) filesystem type: name of filesystem of the form "type[.subtype]" - (10) mount source: filesystem specific information or "none" - (11) super options: per super block options*/ - mountinfoFormat = "%d %d %d:%d %s %s %s %s" -) + (1) mount ID: unique identifier of the mount (may be reused after umount) + (2) parent ID: ID of parent (or of self for the top of the mount tree) + (3) major:minor: value of st_dev for files on filesystem + (4) root: root of the mount within the filesystem + (5) mount point: mount point relative to the process's root + (6) mount options: per mount options + (7) optional fields: zero or more fields of the form "tag[:value]" + (8) separator: marks the end of the optional fields + (9) filesystem type: name of filesystem of the form "type[.subtype]" + (10) mount source: filesystem specific information or "none" + (11) super options: per super block options + */ + + text := s.Text() + fields := strings.Split(text, " ") + numFields := len(fields) + if numFields < 10 { + // should be at least 10 fields + return nil, fmt.Errorf("Parsing '%s' failed: not enough fields (%d)", text, numFields) + } + + p := &Info{} + // ignore any numbers parsing errors, as there should not be any + p.ID, _ = strconv.Atoi(fields[0]) + p.Parent, _ = strconv.Atoi(fields[1]) + mm := strings.Split(fields[2], ":") + if len(mm) != 2 { + return nil, fmt.Errorf("Parsing '%s' failed: unexpected minor:major pair %s", text, mm) + } + p.Major, _ = strconv.Atoi(mm[0]) + p.Minor, _ = strconv.Atoi(mm[1]) + + p.Root = fields[3] + p.Mountpoint = fields[4] + p.Opts = fields[5] + + var skip, stop bool + if filter != nil { + // filter out entries we're not interested in + skip, stop = filter(p) + if skip { + continue + } + } + + // one or more optional fields, when a separator (-) + i := 6 + for ; i < numFields && fields[i] != "-"; i++ { + switch i { + case 6: + p.Optional = fields[6] + default: + /* NOTE there might be more optional fields before the such as + fields[7]...fields[N] (where N < sepIndex), although + as of Linux kernel 4.15 the only known ones are + mount propagation flags in fields[6]. The correct + behavior is to ignore any unknown optional fields. + */ + break + } + } + if i == numFields { + return nil, fmt.Errorf("Parsing '%s' failed: missing separator ('-')", text) + } + + // There should be 3 fields after the separator... + if i+4 > numFields { + return nil, fmt.Errorf("Parsing '%s' failed: not enough fields after a separator", text) + } + // ... but in Linux <= 3.9 mounting a cifs with spaces in a share name + // (like "//serv/My Documents") _may_ end up having a space in the last field + // of mountinfo (like "unc=//serv/My Documents"). Since kernel 3.10-rc1, cifs + // option unc= is ignored, so a space should not appear. In here we ignore + // those "extra" fields caused by extra spaces. + p.Fstype = fields[i+1] + p.Source = fields[i+2] + p.VfsOpts = fields[i+3] + + out = append(out, p) + if stop { + break + } + } + return out, nil +} // Parse /proc/self/mountinfo because comparing Dev and ino does not work from // bind mounts @@ -38,58 +118,6 @@ func parseMountTable(filter FilterFunc) ([]*Info, error) { return parseInfoFile(f, filter) } -func parseInfoFile(r io.Reader, filter FilterFunc) ([]*Info, error) { - var ( - s = bufio.NewScanner(r) - out = []*Info{} - ) - - for s.Scan() { - if err := s.Err(); err != nil { - return nil, err - } - - var ( - p = &Info{} - text = s.Text() - optionalFields string - skip, stop bool - ) - - if _, err := fmt.Sscanf(text, mountinfoFormat, - &p.ID, &p.Parent, &p.Major, &p.Minor, - &p.Root, &p.Mountpoint, &p.Opts, &optionalFields); err != nil { - return nil, fmt.Errorf("Scanning '%s' failed: %s", text, err) - } - if filter != nil { - // filter out entries we're not interested in - skip, stop = filter(p) - if skip { - continue - } - } - // Safe as mountinfo encodes mountpoints with spaces as \040. - index := strings.Index(text, " - ") - postSeparatorFields := strings.Fields(text[index+3:]) - if len(postSeparatorFields) < 3 { - return nil, fmt.Errorf("Error found less than 3 fields post '-' in %q", text) - } - - if optionalFields != "-" { - p.Optional = optionalFields - } - - p.Fstype = postSeparatorFields[0] - p.Source = postSeparatorFields[1] - p.VfsOpts = strings.Join(postSeparatorFields[2:], " ") - out = append(out, p) - if stop { - break - } - } - return out, nil -} - // PidMountInfo collects the mounts for a specific process ID. If the process // ID is unknown, it is better to use `GetMounts` which will inspect // "/proc/self/mountinfo" instead. diff --git a/components/engine/pkg/mount/mountinfo_linux_test.go b/components/engine/pkg/mount/mountinfo_linux_test.go index dea5573e63..f8acb9eeae 100644 --- a/components/engine/pkg/mount/mountinfo_linux_test.go +++ b/components/engine/pkg/mount/mountinfo_linux_test.go @@ -11,63 +11,63 @@ import ( const ( fedoraMountinfo = `15 35 0:3 / /proc rw,nosuid,nodev,noexec,relatime shared:5 - proc proc rw - 16 35 0:14 / /sys rw,nosuid,nodev,noexec,relatime shared:6 - sysfs sysfs rw,seclabel - 17 35 0:5 / /dev rw,nosuid shared:2 - devtmpfs devtmpfs rw,seclabel,size=8056484k,nr_inodes=2014121,mode=755 - 18 16 0:15 / /sys/kernel/security rw,nosuid,nodev,noexec,relatime shared:7 - securityfs securityfs rw - 19 16 0:13 / /sys/fs/selinux rw,relatime shared:8 - selinuxfs selinuxfs rw - 20 17 0:16 / /dev/shm rw,nosuid,nodev shared:3 - tmpfs tmpfs rw,seclabel - 21 17 0:10 / /dev/pts rw,nosuid,noexec,relatime shared:4 - devpts devpts rw,seclabel,gid=5,mode=620,ptmxmode=000 - 22 35 0:17 / /run rw,nosuid,nodev shared:21 - tmpfs tmpfs rw,seclabel,mode=755 - 23 16 0:18 / /sys/fs/cgroup rw,nosuid,nodev,noexec shared:9 - tmpfs tmpfs rw,seclabel,mode=755 - 24 23 0:19 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime shared:10 - cgroup cgroup rw,xattr,release_agent=/usr/lib/systemd/systemd-cgroups-agent,name=systemd - 25 16 0:20 / /sys/fs/pstore rw,nosuid,nodev,noexec,relatime shared:20 - pstore pstore rw - 26 23 0:21 / /sys/fs/cgroup/cpuset rw,nosuid,nodev,noexec,relatime shared:11 - cgroup cgroup rw,cpuset,clone_children - 27 23 0:22 / /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime shared:12 - cgroup cgroup rw,cpuacct,cpu,clone_children - 28 23 0:23 / /sys/fs/cgroup/memory rw,nosuid,nodev,noexec,relatime shared:13 - cgroup cgroup rw,memory,clone_children - 29 23 0:24 / /sys/fs/cgroup/devices rw,nosuid,nodev,noexec,relatime shared:14 - cgroup cgroup rw,devices,clone_children - 30 23 0:25 / /sys/fs/cgroup/freezer rw,nosuid,nodev,noexec,relatime shared:15 - cgroup cgroup rw,freezer,clone_children - 31 23 0:26 / /sys/fs/cgroup/net_cls rw,nosuid,nodev,noexec,relatime shared:16 - cgroup cgroup rw,net_cls,clone_children - 32 23 0:27 / /sys/fs/cgroup/blkio rw,nosuid,nodev,noexec,relatime shared:17 - cgroup cgroup rw,blkio,clone_children - 33 23 0:28 / /sys/fs/cgroup/perf_event rw,nosuid,nodev,noexec,relatime shared:18 - cgroup cgroup rw,perf_event,clone_children - 34 23 0:29 / /sys/fs/cgroup/hugetlb rw,nosuid,nodev,noexec,relatime shared:19 - cgroup cgroup rw,hugetlb,clone_children - 35 1 253:2 / / rw,relatime shared:1 - ext4 /dev/mapper/ssd-root--f20 rw,seclabel,data=ordered - 36 15 0:30 / /proc/sys/fs/binfmt_misc rw,relatime shared:22 - autofs systemd-1 rw,fd=38,pgrp=1,timeout=300,minproto=5,maxproto=5,direct - 37 17 0:12 / /dev/mqueue rw,relatime shared:23 - mqueue mqueue rw,seclabel - 38 35 0:31 / /tmp rw shared:24 - tmpfs tmpfs rw,seclabel - 39 17 0:32 / /dev/hugepages rw,relatime shared:25 - hugetlbfs hugetlbfs rw,seclabel - 40 16 0:7 / /sys/kernel/debug rw,relatime shared:26 - debugfs debugfs rw - 41 16 0:33 / /sys/kernel/config rw,relatime shared:27 - configfs configfs rw - 42 35 0:34 / /var/lib/nfs/rpc_pipefs rw,relatime shared:28 - rpc_pipefs sunrpc rw - 43 15 0:35 / /proc/fs/nfsd rw,relatime shared:29 - nfsd sunrpc rw - 45 35 8:17 / /boot rw,relatime shared:30 - ext4 /dev/sdb1 rw,seclabel,data=ordered - 46 35 253:4 / /home rw,relatime shared:31 - ext4 /dev/mapper/ssd-home rw,seclabel,data=ordered - 47 35 253:5 / /var/lib/libvirt/images rw,noatime,nodiratime shared:32 - ext4 /dev/mapper/ssd-virt rw,seclabel,discard,data=ordered - 48 35 253:12 / /mnt/old rw,relatime shared:33 - ext4 /dev/mapper/HelpDeskRHEL6-FedoraRoot rw,seclabel,data=ordered - 121 22 0:36 / /run/user/1000/gvfs rw,nosuid,nodev,relatime shared:104 - fuse.gvfsd-fuse gvfsd-fuse rw,user_id=1000,group_id=1000 - 124 16 0:37 / /sys/fs/fuse/connections rw,relatime shared:107 - fusectl fusectl rw - 165 38 253:3 / /tmp/mnt rw,relatime shared:147 - ext4 /dev/mapper/ssd-root rw,seclabel,data=ordered - 167 35 253:15 / /var/lib/docker/devicemapper/mnt/aae4076022f0e2b80a2afbf8fc6df450c52080191fcef7fb679a73e6f073e5c2 rw,relatime shared:149 - ext4 /dev/mapper/docker-253:2-425882-aae4076022f0e2b80a2afbf8fc6df450c52080191fcef7fb679a73e6f073e5c2 rw,seclabel,discard,stripe=16,data=ordered - 171 35 253:16 / /var/lib/docker/devicemapper/mnt/c71be651f114db95180e472f7871b74fa597ee70a58ccc35cb87139ddea15373 rw,relatime shared:153 - ext4 /dev/mapper/docker-253:2-425882-c71be651f114db95180e472f7871b74fa597ee70a58ccc35cb87139ddea15373 rw,seclabel,discard,stripe=16,data=ordered - 175 35 253:17 / /var/lib/docker/devicemapper/mnt/1bac6ab72862d2d5626560df6197cf12036b82e258c53d981fa29adce6f06c3c rw,relatime shared:157 - ext4 /dev/mapper/docker-253:2-425882-1bac6ab72862d2d5626560df6197cf12036b82e258c53d981fa29adce6f06c3c rw,seclabel,discard,stripe=16,data=ordered - 179 35 253:18 / /var/lib/docker/devicemapper/mnt/d710a357d77158e80d5b2c55710ae07c94e76d34d21ee7bae65ce5418f739b09 rw,relatime shared:161 - ext4 /dev/mapper/docker-253:2-425882-d710a357d77158e80d5b2c55710ae07c94e76d34d21ee7bae65ce5418f739b09 rw,seclabel,discard,stripe=16,data=ordered - 183 35 253:19 / /var/lib/docker/devicemapper/mnt/6479f52366114d5f518db6837254baab48fab39f2ac38d5099250e9a6ceae6c7 rw,relatime shared:165 - ext4 /dev/mapper/docker-253:2-425882-6479f52366114d5f518db6837254baab48fab39f2ac38d5099250e9a6ceae6c7 rw,seclabel,discard,stripe=16,data=ordered - 187 35 253:20 / /var/lib/docker/devicemapper/mnt/8d9df91c4cca5aef49eeb2725292aab324646f723a7feab56be34c2ad08268e1 rw,relatime shared:169 - ext4 /dev/mapper/docker-253:2-425882-8d9df91c4cca5aef49eeb2725292aab324646f723a7feab56be34c2ad08268e1 rw,seclabel,discard,stripe=16,data=ordered - 191 35 253:21 / /var/lib/docker/devicemapper/mnt/c8240b768603d32e920d365dc9d1dc2a6af46cd23e7ae819947f969e1b4ec661 rw,relatime shared:173 - ext4 /dev/mapper/docker-253:2-425882-c8240b768603d32e920d365dc9d1dc2a6af46cd23e7ae819947f969e1b4ec661 rw,seclabel,discard,stripe=16,data=ordered - 195 35 253:22 / /var/lib/docker/devicemapper/mnt/2eb3a01278380bbf3ed12d86ac629eaa70a4351301ee307a5cabe7b5f3b1615f rw,relatime shared:177 - ext4 /dev/mapper/docker-253:2-425882-2eb3a01278380bbf3ed12d86ac629eaa70a4351301ee307a5cabe7b5f3b1615f rw,seclabel,discard,stripe=16,data=ordered - 199 35 253:23 / /var/lib/docker/devicemapper/mnt/37a17fb7c9d9b80821235d5f2662879bd3483915f245f9b49cdaa0e38779b70b rw,relatime shared:181 - ext4 /dev/mapper/docker-253:2-425882-37a17fb7c9d9b80821235d5f2662879bd3483915f245f9b49cdaa0e38779b70b rw,seclabel,discard,stripe=16,data=ordered - 203 35 253:24 / /var/lib/docker/devicemapper/mnt/aea459ae930bf1de913e2f29428fd80ee678a1e962d4080019d9f9774331ee2b rw,relatime shared:185 - ext4 /dev/mapper/docker-253:2-425882-aea459ae930bf1de913e2f29428fd80ee678a1e962d4080019d9f9774331ee2b rw,seclabel,discard,stripe=16,data=ordered - 207 35 253:25 / /var/lib/docker/devicemapper/mnt/928ead0bc06c454bd9f269e8585aeae0a6bd697f46dc8754c2a91309bc810882 rw,relatime shared:189 - ext4 /dev/mapper/docker-253:2-425882-928ead0bc06c454bd9f269e8585aeae0a6bd697f46dc8754c2a91309bc810882 rw,seclabel,discard,stripe=16,data=ordered - 211 35 253:26 / /var/lib/docker/devicemapper/mnt/0f284d18481d671644706e7a7244cbcf63d590d634cc882cb8721821929d0420 rw,relatime shared:193 - ext4 /dev/mapper/docker-253:2-425882-0f284d18481d671644706e7a7244cbcf63d590d634cc882cb8721821929d0420 rw,seclabel,discard,stripe=16,data=ordered - 215 35 253:27 / /var/lib/docker/devicemapper/mnt/d9dd16722ab34c38db2733e23f69e8f4803ce59658250dd63e98adff95d04919 rw,relatime shared:197 - ext4 /dev/mapper/docker-253:2-425882-d9dd16722ab34c38db2733e23f69e8f4803ce59658250dd63e98adff95d04919 rw,seclabel,discard,stripe=16,data=ordered - 219 35 253:28 / /var/lib/docker/devicemapper/mnt/bc4500479f18c2c08c21ad5282e5f826a016a386177d9874c2764751c031d634 rw,relatime shared:201 - ext4 /dev/mapper/docker-253:2-425882-bc4500479f18c2c08c21ad5282e5f826a016a386177d9874c2764751c031d634 rw,seclabel,discard,stripe=16,data=ordered - 223 35 253:29 / /var/lib/docker/devicemapper/mnt/7770c8b24eb3d5cc159a065910076938910d307ab2f5d94e1dc3b24c06ee2c8a rw,relatime shared:205 - ext4 /dev/mapper/docker-253:2-425882-7770c8b24eb3d5cc159a065910076938910d307ab2f5d94e1dc3b24c06ee2c8a rw,seclabel,discard,stripe=16,data=ordered - 227 35 253:30 / /var/lib/docker/devicemapper/mnt/c280cd3d0bf0aa36b478b292279671624cceafc1a67eaa920fa1082601297adf rw,relatime shared:209 - ext4 /dev/mapper/docker-253:2-425882-c280cd3d0bf0aa36b478b292279671624cceafc1a67eaa920fa1082601297adf rw,seclabel,discard,stripe=16,data=ordered - 231 35 253:31 / /var/lib/docker/devicemapper/mnt/8b59a7d9340279f09fea67fd6ad89ddef711e9e7050eb647984f8b5ef006335f rw,relatime shared:213 - ext4 /dev/mapper/docker-253:2-425882-8b59a7d9340279f09fea67fd6ad89ddef711e9e7050eb647984f8b5ef006335f rw,seclabel,discard,stripe=16,data=ordered - 235 35 253:32 / /var/lib/docker/devicemapper/mnt/1a28059f29eda821578b1bb27a60cc71f76f846a551abefabce6efd0146dce9f rw,relatime shared:217 - ext4 /dev/mapper/docker-253:2-425882-1a28059f29eda821578b1bb27a60cc71f76f846a551abefabce6efd0146dce9f rw,seclabel,discard,stripe=16,data=ordered - 239 35 253:33 / /var/lib/docker/devicemapper/mnt/e9aa60c60128cad1 rw,relatime shared:221 - ext4 /dev/mapper/docker-253:2-425882-e9aa60c60128cad1 rw,seclabel,discard,stripe=16,data=ordered - 243 35 253:34 / /var/lib/docker/devicemapper/mnt/5fec11304b6f4713fea7b6ccdcc1adc0a1966187f590fe25a8227428a8df275d-init rw,relatime shared:225 - ext4 /dev/mapper/docker-253:2-425882-5fec11304b6f4713fea7b6ccdcc1adc0a1966187f590fe25a8227428a8df275d-init rw,seclabel,discard,stripe=16,data=ordered - 247 35 253:35 / /var/lib/docker/devicemapper/mnt/5fec11304b6f4713fea7b6ccdcc1adc0a1966187f590fe25a8227428a8df275d rw,relatime shared:229 - ext4 /dev/mapper/docker-253:2-425882-5fec11304b6f4713fea7b6ccdcc1adc0a1966187f590fe25a8227428a8df275d rw,seclabel,discard,stripe=16,data=ordered - 31 21 0:23 / /DATA/foo_bla_bla rw,relatime - cifs //foo/BLA\040BLA\040BLA/ rw,sec=ntlm,cache=loose,unc=\\foo\BLA BLA BLA,username=my_login,domain=mydomain.com,uid=12345678,forceuid,gid=12345678,forcegid,addr=10.1.30.10,file_mode=0755,dir_mode=0755,nounix,rsize=61440,wsize=65536,actimeo=1` +16 35 0:14 / /sys rw,nosuid,nodev,noexec,relatime shared:6 - sysfs sysfs rw,seclabel +17 35 0:5 / /dev rw,nosuid shared:2 - devtmpfs devtmpfs rw,seclabel,size=8056484k,nr_inodes=2014121,mode=755 +18 16 0:15 / /sys/kernel/security rw,nosuid,nodev,noexec,relatime shared:7 - securityfs securityfs rw +19 16 0:13 / /sys/fs/selinux rw,relatime shared:8 - selinuxfs selinuxfs rw +20 17 0:16 / /dev/shm rw,nosuid,nodev shared:3 - tmpfs tmpfs rw,seclabel +21 17 0:10 / /dev/pts rw,nosuid,noexec,relatime shared:4 - devpts devpts rw,seclabel,gid=5,mode=620,ptmxmode=000 +22 35 0:17 / /run rw,nosuid,nodev shared:21 - tmpfs tmpfs rw,seclabel,mode=755 +23 16 0:18 / /sys/fs/cgroup rw,nosuid,nodev,noexec shared:9 - tmpfs tmpfs rw,seclabel,mode=755 +24 23 0:19 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime shared:10 - cgroup cgroup rw,xattr,release_agent=/usr/lib/systemd/systemd-cgroups-agent,name=systemd +25 16 0:20 / /sys/fs/pstore rw,nosuid,nodev,noexec,relatime shared:20 - pstore pstore rw +26 23 0:21 / /sys/fs/cgroup/cpuset rw,nosuid,nodev,noexec,relatime shared:11 - cgroup cgroup rw,cpuset,clone_children +27 23 0:22 / /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime shared:12 - cgroup cgroup rw,cpuacct,cpu,clone_children +28 23 0:23 / /sys/fs/cgroup/memory rw,nosuid,nodev,noexec,relatime shared:13 - cgroup cgroup rw,memory,clone_children +29 23 0:24 / /sys/fs/cgroup/devices rw,nosuid,nodev,noexec,relatime shared:14 - cgroup cgroup rw,devices,clone_children +30 23 0:25 / /sys/fs/cgroup/freezer rw,nosuid,nodev,noexec,relatime shared:15 - cgroup cgroup rw,freezer,clone_children +31 23 0:26 / /sys/fs/cgroup/net_cls rw,nosuid,nodev,noexec,relatime shared:16 - cgroup cgroup rw,net_cls,clone_children +32 23 0:27 / /sys/fs/cgroup/blkio rw,nosuid,nodev,noexec,relatime shared:17 - cgroup cgroup rw,blkio,clone_children +33 23 0:28 / /sys/fs/cgroup/perf_event rw,nosuid,nodev,noexec,relatime shared:18 - cgroup cgroup rw,perf_event,clone_children +34 23 0:29 / /sys/fs/cgroup/hugetlb rw,nosuid,nodev,noexec,relatime shared:19 - cgroup cgroup rw,hugetlb,clone_children +35 1 253:2 / / rw,relatime shared:1 - ext4 /dev/mapper/ssd-root--f20 rw,seclabel,data=ordered +36 15 0:30 / /proc/sys/fs/binfmt_misc rw,relatime shared:22 - autofs systemd-1 rw,fd=38,pgrp=1,timeout=300,minproto=5,maxproto=5,direct +37 17 0:12 / /dev/mqueue rw,relatime shared:23 - mqueue mqueue rw,seclabel +38 35 0:31 / /tmp rw shared:24 - tmpfs tmpfs rw,seclabel +39 17 0:32 / /dev/hugepages rw,relatime shared:25 - hugetlbfs hugetlbfs rw,seclabel +40 16 0:7 / /sys/kernel/debug rw,relatime shared:26 - debugfs debugfs rw +41 16 0:33 / /sys/kernel/config rw,relatime shared:27 - configfs configfs rw +42 35 0:34 / /var/lib/nfs/rpc_pipefs rw,relatime shared:28 - rpc_pipefs sunrpc rw +43 15 0:35 / /proc/fs/nfsd rw,relatime shared:29 - nfsd sunrpc rw +45 35 8:17 / /boot rw,relatime shared:30 - ext4 /dev/sdb1 rw,seclabel,data=ordered +46 35 253:4 / /home rw,relatime shared:31 - ext4 /dev/mapper/ssd-home rw,seclabel,data=ordered +47 35 253:5 / /var/lib/libvirt/images rw,noatime,nodiratime shared:32 - ext4 /dev/mapper/ssd-virt rw,seclabel,discard,data=ordered +48 35 253:12 / /mnt/old rw,relatime shared:33 - ext4 /dev/mapper/HelpDeskRHEL6-FedoraRoot rw,seclabel,data=ordered +121 22 0:36 / /run/user/1000/gvfs rw,nosuid,nodev,relatime shared:104 - fuse.gvfsd-fuse gvfsd-fuse rw,user_id=1000,group_id=1000 +124 16 0:37 / /sys/fs/fuse/connections rw,relatime shared:107 - fusectl fusectl rw +165 38 253:3 / /tmp/mnt rw,relatime shared:147 - ext4 /dev/mapper/ssd-root rw,seclabel,data=ordered +167 35 253:15 / /var/lib/docker/devicemapper/mnt/aae4076022f0e2b80a2afbf8fc6df450c52080191fcef7fb679a73e6f073e5c2 rw,relatime shared:149 - ext4 /dev/mapper/docker-253:2-425882-aae4076022f0e2b80a2afbf8fc6df450c52080191fcef7fb679a73e6f073e5c2 rw,seclabel,discard,stripe=16,data=ordered +171 35 253:16 / /var/lib/docker/devicemapper/mnt/c71be651f114db95180e472f7871b74fa597ee70a58ccc35cb87139ddea15373 rw,relatime shared:153 - ext4 /dev/mapper/docker-253:2-425882-c71be651f114db95180e472f7871b74fa597ee70a58ccc35cb87139ddea15373 rw,seclabel,discard,stripe=16,data=ordered +175 35 253:17 / /var/lib/docker/devicemapper/mnt/1bac6ab72862d2d5626560df6197cf12036b82e258c53d981fa29adce6f06c3c rw,relatime shared:157 - ext4 /dev/mapper/docker-253:2-425882-1bac6ab72862d2d5626560df6197cf12036b82e258c53d981fa29adce6f06c3c rw,seclabel,discard,stripe=16,data=ordered +179 35 253:18 / /var/lib/docker/devicemapper/mnt/d710a357d77158e80d5b2c55710ae07c94e76d34d21ee7bae65ce5418f739b09 rw,relatime shared:161 - ext4 /dev/mapper/docker-253:2-425882-d710a357d77158e80d5b2c55710ae07c94e76d34d21ee7bae65ce5418f739b09 rw,seclabel,discard,stripe=16,data=ordered +183 35 253:19 / /var/lib/docker/devicemapper/mnt/6479f52366114d5f518db6837254baab48fab39f2ac38d5099250e9a6ceae6c7 rw,relatime shared:165 - ext4 /dev/mapper/docker-253:2-425882-6479f52366114d5f518db6837254baab48fab39f2ac38d5099250e9a6ceae6c7 rw,seclabel,discard,stripe=16,data=ordered +187 35 253:20 / /var/lib/docker/devicemapper/mnt/8d9df91c4cca5aef49eeb2725292aab324646f723a7feab56be34c2ad08268e1 rw,relatime shared:169 - ext4 /dev/mapper/docker-253:2-425882-8d9df91c4cca5aef49eeb2725292aab324646f723a7feab56be34c2ad08268e1 rw,seclabel,discard,stripe=16,data=ordered +191 35 253:21 / /var/lib/docker/devicemapper/mnt/c8240b768603d32e920d365dc9d1dc2a6af46cd23e7ae819947f969e1b4ec661 rw,relatime shared:173 - ext4 /dev/mapper/docker-253:2-425882-c8240b768603d32e920d365dc9d1dc2a6af46cd23e7ae819947f969e1b4ec661 rw,seclabel,discard,stripe=16,data=ordered +195 35 253:22 / /var/lib/docker/devicemapper/mnt/2eb3a01278380bbf3ed12d86ac629eaa70a4351301ee307a5cabe7b5f3b1615f rw,relatime shared:177 - ext4 /dev/mapper/docker-253:2-425882-2eb3a01278380bbf3ed12d86ac629eaa70a4351301ee307a5cabe7b5f3b1615f rw,seclabel,discard,stripe=16,data=ordered +199 35 253:23 / /var/lib/docker/devicemapper/mnt/37a17fb7c9d9b80821235d5f2662879bd3483915f245f9b49cdaa0e38779b70b rw,relatime shared:181 - ext4 /dev/mapper/docker-253:2-425882-37a17fb7c9d9b80821235d5f2662879bd3483915f245f9b49cdaa0e38779b70b rw,seclabel,discard,stripe=16,data=ordered +203 35 253:24 / /var/lib/docker/devicemapper/mnt/aea459ae930bf1de913e2f29428fd80ee678a1e962d4080019d9f9774331ee2b rw,relatime shared:185 - ext4 /dev/mapper/docker-253:2-425882-aea459ae930bf1de913e2f29428fd80ee678a1e962d4080019d9f9774331ee2b rw,seclabel,discard,stripe=16,data=ordered +207 35 253:25 / /var/lib/docker/devicemapper/mnt/928ead0bc06c454bd9f269e8585aeae0a6bd697f46dc8754c2a91309bc810882 rw,relatime shared:189 - ext4 /dev/mapper/docker-253:2-425882-928ead0bc06c454bd9f269e8585aeae0a6bd697f46dc8754c2a91309bc810882 rw,seclabel,discard,stripe=16,data=ordered +211 35 253:26 / /var/lib/docker/devicemapper/mnt/0f284d18481d671644706e7a7244cbcf63d590d634cc882cb8721821929d0420 rw,relatime shared:193 - ext4 /dev/mapper/docker-253:2-425882-0f284d18481d671644706e7a7244cbcf63d590d634cc882cb8721821929d0420 rw,seclabel,discard,stripe=16,data=ordered +215 35 253:27 / /var/lib/docker/devicemapper/mnt/d9dd16722ab34c38db2733e23f69e8f4803ce59658250dd63e98adff95d04919 rw,relatime shared:197 - ext4 /dev/mapper/docker-253:2-425882-d9dd16722ab34c38db2733e23f69e8f4803ce59658250dd63e98adff95d04919 rw,seclabel,discard,stripe=16,data=ordered +219 35 253:28 / /var/lib/docker/devicemapper/mnt/bc4500479f18c2c08c21ad5282e5f826a016a386177d9874c2764751c031d634 rw,relatime shared:201 - ext4 /dev/mapper/docker-253:2-425882-bc4500479f18c2c08c21ad5282e5f826a016a386177d9874c2764751c031d634 rw,seclabel,discard,stripe=16,data=ordered +223 35 253:29 / /var/lib/docker/devicemapper/mnt/7770c8b24eb3d5cc159a065910076938910d307ab2f5d94e1dc3b24c06ee2c8a rw,relatime shared:205 - ext4 /dev/mapper/docker-253:2-425882-7770c8b24eb3d5cc159a065910076938910d307ab2f5d94e1dc3b24c06ee2c8a rw,seclabel,discard,stripe=16,data=ordered +227 35 253:30 / /var/lib/docker/devicemapper/mnt/c280cd3d0bf0aa36b478b292279671624cceafc1a67eaa920fa1082601297adf rw,relatime shared:209 - ext4 /dev/mapper/docker-253:2-425882-c280cd3d0bf0aa36b478b292279671624cceafc1a67eaa920fa1082601297adf rw,seclabel,discard,stripe=16,data=ordered +231 35 253:31 / /var/lib/docker/devicemapper/mnt/8b59a7d9340279f09fea67fd6ad89ddef711e9e7050eb647984f8b5ef006335f rw,relatime shared:213 - ext4 /dev/mapper/docker-253:2-425882-8b59a7d9340279f09fea67fd6ad89ddef711e9e7050eb647984f8b5ef006335f rw,seclabel,discard,stripe=16,data=ordered +235 35 253:32 / /var/lib/docker/devicemapper/mnt/1a28059f29eda821578b1bb27a60cc71f76f846a551abefabce6efd0146dce9f rw,relatime shared:217 - ext4 /dev/mapper/docker-253:2-425882-1a28059f29eda821578b1bb27a60cc71f76f846a551abefabce6efd0146dce9f rw,seclabel,discard,stripe=16,data=ordered +239 35 253:33 / /var/lib/docker/devicemapper/mnt/e9aa60c60128cad1 rw,relatime shared:221 - ext4 /dev/mapper/docker-253:2-425882-e9aa60c60128cad1 rw,seclabel,discard,stripe=16,data=ordered +243 35 253:34 / /var/lib/docker/devicemapper/mnt/5fec11304b6f4713fea7b6ccdcc1adc0a1966187f590fe25a8227428a8df275d-init rw,relatime shared:225 - ext4 /dev/mapper/docker-253:2-425882-5fec11304b6f4713fea7b6ccdcc1adc0a1966187f590fe25a8227428a8df275d-init rw,seclabel,discard,stripe=16,data=ordered +247 35 253:35 / /var/lib/docker/devicemapper/mnt/5fec11304b6f4713fea7b6ccdcc1adc0a1966187f590fe25a8227428a8df275d rw,relatime shared:229 - ext4 /dev/mapper/docker-253:2-425882-5fec11304b6f4713fea7b6ccdcc1adc0a1966187f590fe25a8227428a8df275d rw,seclabel,discard,stripe=16,data=ordered +31 21 0:23 / /DATA/foo_bla_bla rw,relatime - cifs //foo/BLA\040BLA\040BLA/ rw,sec=ntlm,cache=loose,unc=\\foo\BLA BLA BLA,username=my_login,domain=mydomain.com,uid=12345678,forceuid,gid=12345678,forcegid,addr=10.1.30.10,file_mode=0755,dir_mode=0755,nounix,rsize=61440,wsize=65536,actimeo=1` ubuntuMountInfo = `15 20 0:14 / /sys rw,nosuid,nodev,noexec,relatime - sysfs sysfs rw 16 20 0:3 / /proc rw,nosuid,nodev,noexec,relatime - proc proc rw From f00e9a9036d08661fba2d49edd99d2f5c90d72ad Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 19 Jan 2018 13:09:11 -0800 Subject: [PATCH 5/7] mount.Unmount(): don't look into /proc/self/mountinfo Now, every Unmount() call takes a burden to parse the whole nine yards of /proc/self/mountinfo to figure out whether the given mount point is mounted or not (and returns an error in case parsing fails somehow). Instead, let's just call umount() and ignore EINVAL, which results in the same behavior, but much better performance. Note that EINVAL is returned from umount(2) not only in the case when `target` is not mounted, but also for invalid flags. As the flags are hardcoded here, it can't be the case. Signed-off-by: Kir Kolyshkin Upstream-commit: a1d095199ddb9b4811e1417b6adcdfadad7d73f4 Component: engine --- components/engine/pkg/mount/mount.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/components/engine/pkg/mount/mount.go b/components/engine/pkg/mount/mount.go index c440b81ce9..874aff6545 100644 --- a/components/engine/pkg/mount/mount.go +++ b/components/engine/pkg/mount/mount.go @@ -3,7 +3,6 @@ package mount // import "github.com/docker/docker/pkg/mount" import ( "sort" "strings" - "syscall" "github.com/sirupsen/logrus" @@ -90,10 +89,12 @@ func ForceMount(device, target, mType, options string) error { // Unmount lazily unmounts a filesystem on supported platforms, otherwise // does a normal unmount. func Unmount(target string) error { - if mounted, err := Mounted(target); err != nil || !mounted { - return err + err := unmount(target, mntDetach) + if err == syscall.EINVAL { + // ignore "not mounted" error + err = nil } - return unmount(target, mntDetach) + return err } // RecursiveUnmount unmounts the target and all mounts underneath, starting with From c2010a1d291c06deea0b3dd3c326e49c818d764a Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 19 Jan 2018 13:27:05 -0800 Subject: [PATCH 6/7] volume/local: call umount unconditionally There is no need to parse mount table and iterate through the list of mounts, and then call Unmount() which again parses the mount table and iterates through the list of mounts. It is totally OK to call Unmount() unconditionally. Signed-off-by: Kir Kolyshkin Upstream-commit: ac39a95ea618601f78662972c35838d928858904 Component: engine --- components/engine/volume/local/local.go | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/components/engine/volume/local/local.go b/components/engine/volume/local/local.go index 3eee5a92f9..226a5dd4fb 100644 --- a/components/engine/volume/local/local.go +++ b/components/engine/volume/local/local.go @@ -19,7 +19,6 @@ import ( "github.com/docker/docker/pkg/mount" "github.com/docker/docker/volume" "github.com/pkg/errors" - "github.com/sirupsen/logrus" ) // VolumeDataPathName is the name of the directory where the volume data is stored. @@ -66,11 +65,6 @@ func New(scope string, rootIDs idtools.IDPair) (*Root, error) { return nil, err } - mountInfos, err := mount.GetMounts(nil) - if err != nil { - logrus.Debugf("error looking up mounts for local volume cleanup: %v", err) - } - for _, d := range dirs { if !d.IsDir() { continue @@ -96,12 +90,7 @@ func New(scope string, rootIDs idtools.IDPair) (*Root, error) { } // unmount anything that may still be mounted (for example, from an unclean shutdown) - for _, info := range mountInfos { - if info.Mountpoint == v.path { - mount.Unmount(v.path) - break - } - } + mount.Unmount(v.path) } } From a1f451a2851793d562335cf0b212067183df08bf Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 8 Mar 2018 15:27:09 -0800 Subject: [PATCH 7/7] volume/local/TestCreateWithOpts(): use mount filter This is not for the sake of test to run faster of course; this is to simplify the code as well as have some more testing for mount.SingleEntryFilter(). Signed-off-by: Kir Kolyshkin Upstream-commit: ce468f0ad0d075c5d0c44c78bd61c489e6d7d70c Component: engine --- components/engine/volume/local/local_test.go | 42 ++++++++++---------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/components/engine/volume/local/local_test.go b/components/engine/volume/local/local_test.go index 163314fab7..d6f6e29171 100644 --- a/components/engine/volume/local/local_test.go +++ b/components/engine/volume/local/local_test.go @@ -184,6 +184,10 @@ func TestCreateWithOpts(t *testing.T) { if runtime.GOOS == "windows" { t.Skip() } + if os.Getuid() != 0 { + t.Skip("root required") + } + rootDir, err := ioutil.TempDir("", "local-volume-test") if err != nil { t.Fatal(err) @@ -215,33 +219,27 @@ func TestCreateWithOpts(t *testing.T) { } }() - mountInfos, err := mount.GetMounts(nil) + mountInfos, err := mount.GetMounts(mount.SingleEntryFilter(dir)) if err != nil { t.Fatal(err) } - - var found bool - for _, info := range mountInfos { - if info.Mountpoint == dir { - found = true - if info.Fstype != "tmpfs" { - t.Fatalf("expected tmpfs mount, got %q", info.Fstype) - } - if info.Source != "tmpfs" { - t.Fatalf("expected tmpfs mount, got %q", info.Source) - } - if !strings.Contains(info.VfsOpts, "uid=1000") { - t.Fatalf("expected mount info to have uid=1000: %q", info.VfsOpts) - } - if !strings.Contains(info.VfsOpts, "size=1024k") { - t.Fatalf("expected mount info to have size=1024k: %q", info.VfsOpts) - } - break - } + if len(mountInfos) != 1 { + t.Fatalf("expected 1 mount, found %d: %+v", len(mountInfos), mountInfos) } - if !found { - t.Fatal("mount not found") + info := mountInfos[0] + t.Logf("%+v", info) + if info.Fstype != "tmpfs" { + t.Fatalf("expected tmpfs mount, got %q", info.Fstype) + } + if info.Source != "tmpfs" { + t.Fatalf("expected tmpfs mount, got %q", info.Source) + } + if !strings.Contains(info.VfsOpts, "uid=1000") { + t.Fatalf("expected mount info to have uid=1000: %q", info.VfsOpts) + } + if !strings.Contains(info.VfsOpts, "size=1024k") { + t.Fatalf("expected mount info to have size=1024k: %q", info.VfsOpts) } if v.active.count != 1 {