Permalink
Browse files

interfaces/mount: WIP add mount.{Resolve,AliasesOf}

This patch adds a pair of functions that aim to assist in implementation
of Change.Needed.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
  • Loading branch information...
1 parent 3b1ae58 commit fa7fb5206ba87609729c494bf9b6ccaaed93ea9b @zyga committed Apr 12, 2017
Showing with 240 additions and 0 deletions.
  1. +112 −0 interfaces/mount/mountinfo.go
  2. +128 −0 interfaces/mount/mountinfo_test.go
@@ -24,6 +24,8 @@ import (
"fmt"
"io"
"os"
+ "path"
+ "sort"
"strconv"
"strings"
)
@@ -170,3 +172,113 @@ func parseMountOpts(opts string) map[string]string {
}
return result
}
+
+func formatMountOpts(opts map[string]string) string {
+ flat := make([]string, 0, len(opts))
+ for key, value := range opts {
+ if value != "" {
+ flat = append(flat, fmt.Sprintf("%s=%s", key, value))
+ } else {
+ flat = append(flat, key)
+ }
+ }
+ sort.Strings(flat)
+ return strings.Join(flat, ",")
+}
+
+// resolve finds the mountinfo entry and a subdirectory therein to which a given absolute directory belongs to.
+func resolve(dir string, mountinfo []*InfoEntry) (subdir string, entry *InfoEntry, err error) {
+ dir = path.Clean(dir)
+ // Precondition: dir is an absolute path.
+ if !path.IsAbs(dir) {
+ return "", nil, fmt.Errorf("cannot resolve relative directory %q", dir)
+ }
+ //
+ // Precondition: there is something mounted on the root directory.
+ var hasRoot bool
+ for _, mie := range mountinfo {
+ if mie.MountDir == "/" {
+ hasRoot = true
+ break
+ }
+ }
+ if !hasRoot {
+ return "", nil, fmt.Errorf("cannot resolve directory %q: root directory is not mounted", dir)
+ }
+
+ // This loop is guaranteed to terminate because we eventually look for "/"
+ // which is guaranteed to be in mountinfo by the precondition above.
+ var foundEntry *InfoEntry
+ var rfrag, frag []string
+ search := dir
+ for foundEntry == nil {
+ // We have to iterate in reverse here because if more than one element
+ // is mounted at the same place then the kernel will list them in the
+ // order the operations were performed.
+ for i := len(mountinfo) - 1; i >= 0; i-- {
+ mie := mountinfo[i]
+ if search == mie.MountDir {
+ foundEntry = mie
+ break
+ }
+ }
+ if foundEntry == nil {
+ rfrag = append(rfrag, path.Base(search))
+ search = path.Dir(search)
+ }
@jdstrand

jdstrand May 1, 2017

A comment for this section with an example would be nice telling how when searching for /foo/bar/baz if that isn't found, /foo/bar will be checked, onwards until the terminating condition of '/'.

+ }
+
+ // rfrag is reversed, let's correct that and re-construct the path.
+ frag = make([]string, 0, len(rfrag)+1)
+ frag = append(frag, foundEntry.Root)
+ for i := len(rfrag) - 1; i >= 0; i-- {
+ frag = append(frag, rfrag[i])
+ }
+ foundDir := path.Join(frag...)
+ return foundDir, foundEntry, nil
+}
+
+// AliasesOf finds aliases of a given directory in a given mountinfo table.
+//
+// When the kernel bind mounts something it produces another, indistinguishable
+// place where a given entry may be found. The algorithm works by looking at
+// MountSource (what is mounted) and MountOptions (how is it mounted).
+func AliasesOf(dir string, mountinfo []*InfoEntry) (map[string]bool, error) {
+ // Find what the directory represents so that we know what to look for.
+ dir = path.Clean(dir)
+ foundDir, foundEntry, err := resolve(dir, mountinfo)
+ if err != nil {
+ return nil, err
+ }
+
+ // Build a map of mountinfo entries indexed by DevMajor:DevMinor taking
+ // only entries where the whole filesystem is mounted (Root == "/"). Those
+ // aid in lookup of the canonical mount locations of the given one.
+ byMajMin := make(map[[2]int][]*InfoEntry, len(mountinfo))
+ // NOTE: we don't consider mount options relevant here.
+ for _, mie := range mountinfo {
+ if mie.Root == "/" {
+ key := [2]int{mie.DevMajor, mie.DevMinor}
+ byMajMin[key] = append(byMajMin[key], mie)
+ }
+ }
+
+ // Search for equivalent mount points by matching their DevMajor:DevMinor
+ // against what the searched directory resolved to as well as their Root
+ // (fragment mounted) against the fragment (relative to the root of the
+ // mounted filesystem) of the searched directory.
@jdstrand

jdstrand May 1, 2017

The comment here is somewhat convoluted. Perhaps restructure it like:

// Search for equivalent mount points by matching their:
// - DevMajor:DevMinor against what the searched directory resolved to
// - Root (fragment mounted) against the fragment (relative to the root of the
//   mounted filesystem) of the searched directory.

Saying something about how the kernel works here would be really helpful. Why are you looking at major:minor? What are you defining as an 'equivalent mountpoint'? What do you mean by 'fragement mounted'? What are you calling a 'fragment'? etc, etc

It seems you are trying say something about when have something like this (taken from /proc/self/mountinfo within snap on classic):

1950 1784 7:14 / /var/lib/snapd/hostfs/snap/core/1577 rw,relatime master:44 - squashfs /dev/loop14 ro
2001 1781 7:14 / / rw,relatime master:44 - squashfs /dev/loop14 ro
2049 2007 7:14 /etc/alternatives /etc/alternatives rw,relatime master:44 - squashfs /dev/loop14 ro
2064 2050 7:14 / /snap/core/1577 rw,relatime master:44 - squashfs /dev/loop14 ro
1908 1907 7:14 /var/lib/apparmor /var/lib/apparmor rw,relatime master:44 - squashfs /dev/loop14 ro
...

that because all of these have the same major:minor (ie, 7:14), they are all related to the root mount point in an important way (ie, 2001 1781 7:14 / / rw,relatime master:44 - squashfs /dev/loop14 ro). Stating that relationship would be very important here (with references if available).

@zyga

zyga May 2, 2017

Owner

Yeah, you are right. This is still unfinished and I will look carefully at wording and kernel references before proposing this properly.

+ aliases := make(map[string]bool)
+ for i := len(mountinfo) - 1; i >= 0; i-- {
+ mie := mountinfo[i]
+ if mie.DevMajor == foundEntry.DevMajor && mie.DevMinor == foundEntry.DevMinor && mie.Root == foundDir {
+ // TODO: don't add if this was shadowed by something we saw was
+ // mounted here earlier. (This is why we iterate in reverse).
+ aliases[mie.MountDir] = true
+ for _, canonMie := range byMajMin[[2]int{mie.DevMajor, mie.DevMinor}] {
+ canonDir := path.Join(canonMie.MountDir, mie.Root)
+ aliases[canonDir] = true
+ }
+ }
+ }
+ return aliases, nil
+}
@@ -168,3 +168,131 @@ func (s *profileSuite) TestLoadMountInfo2(c *C) {
_, err := mount.LoadMountInfo(fname)
c.Assert(err, ErrorMatches, "*. no such file or directory")
}
+
+// Check that AliesesOf bails out on relative paths.
+func (s *mountinfoSuite) TestAliasesOfPrecondition1(c *C) {
+ mountinfo := mockMountInfo(c,
+ "201 25 8:2 / / rw,reltime shared:1 - ext4 /dev/sda2 rw,errors=remount-ro,data=ordered")
+ _, err := mount.AliasesOf("foo", mountinfo)
+ c.Assert(err, ErrorMatches, `cannot resolve relative directory "foo"`)
+}
+
+// Check that AliesesOf bails out if / is not mounted.
+func (s *mountinfoSuite) TestAliasesOfPrecondition2(c *C) {
+ mountinfo := mockMountInfo(c,
+ "201 25 8:2 /home/zyga/foo /home/zyga/bar rw,relatime shared:1 - ext4 /dev/sda2 rw,errors=remount-ro,data=ordered")
+ _, err := mount.AliasesOf("/foo", mountinfo)
+ c.Assert(err, ErrorMatches, `cannot resolve directory "/foo": root directory is not mounted`)
+}
+
+// Check that AliesesOf finds a simple bind mount.
+func (s *mountinfoSuite) TestAliasesOf1(c *C) {
+ // $ mkdir foo bar; mount --bind foo bar
+ mountinfo := mockMountInfo(c,
+ "201 25 8:2 / / rw,reltime shared:1 - ext4 /dev/sda2 rw,errors=remount-ro,data=ordered",
+ "201 25 8:2 /home/zyga/foo /home/zyga/bar rw,relatime shared:1 - ext4 /dev/sda2 rw,errors=remount-ro,data=ordered")
+ for _, symmetry := range []string{
+ "/home/zyga/foo", "/home/zyga/bar",
+ } {
+ aliases, err := mount.AliasesOf(symmetry, mountinfo)
+ c.Assert(err, IsNil)
+ c.Assert(aliases, DeepEquals, map[string]bool{
+ "/home/zyga/foo": true,
+ "/home/zyga/bar": true,
+ })
@jdstrand

jdstrand May 1, 2017

I'm not sure introducing a new terminology ('mount alias') is wise. Perhaps bounds, err := BoundTogether(...)? That's a little awkward, but at least let's people understand that the function is for bind mounts.

@zyga

zyga May 2, 2017

Owner

Indeed, I was looking for a better word and I knew that alias clashes with other snapd terminology but I just ran with it because I failed to find anything sensible.

+ }
+}
+
+// Check that AliasesOf finds a trio of bind mounts.
+func (s *mountinfoSuite) TestAliasesOf2(c *C) {
+ // $ mkdir foo bar; mount --bind foo bar
+ // $ mkdir froz; mount --bind bar froz
+ mountinfo := mockMountInfo(c,
+ "201 25 8:2 / / rw,reltime shared:1 - ext4 /dev/sda2 rw,errors=remount-ro,data=ordered",
+ "201 25 8:2 /home/zyga/foo /home/zyga/bar rw,relatime shared:1 - ext4 /dev/sda2 rw,errors=remount-ro,data=ordered",
+ "428 25 8:2 /home/zyga/foo /home/zyga/froz rw,relatime shared:1 - ext4 /dev/sda2 rw,errors=remount-ro,data=ordered")
+ for _, symmetry := range []string{
+ "/home/zyga/foo", "/home/zyga/bar", "/home/zyga/froz",
+ } {
+ aliases, err := mount.AliasesOf(symmetry, mountinfo)
+ c.Assert(err, IsNil)
+ c.Assert(aliases, DeepEquals, map[string]bool{
+ "/home/zyga/foo": true,
+ "/home/zyga/bar": true,
+ "/home/zyga/froz": true,
+ }, Commentf("symmetry: %q", symmetry))
+ }
+}
+
+// Check that AliasesOf is not confused by shadowed mounts.
@jdstrand

jdstrand May 1, 2017

Defining a shadowed mount in this comment would help reviewers.

+func (s *mountinfoSuite) TestAliasesOf3(c *C) {
+ // $ mkdir src dst dummy
+ // $ mount --bind dummy dst
+ // $ mount --bind src dst
+ mountinfo := mockMountInfo(c,
+ "25 0 8:2 / / rw,relatime shared:1 - ext4 /dev/sda2 rw,errors=remount-ro,data=ordered",
+ "395 25 8:2 /home/zyga/dummy /home/zyga/dst rw,relatime shared:1 - ext4 /dev/sda2 rw,errors=remount-ro,data=ordered",
+ "404 395 8:2 /home/zyga/src /home/zyga/dst rw,relatime shared:1 - ext4 /dev/sda2 rw,errors=remount-ro,data=ordered",
+ "405 25 8:2 /home/zyga/src /home/zyga/dummy rw,relatime shared:1 - ext4 /dev/sda2 rw,errors=remount-ro,data=ordered")
+ for _, symmetry := range []string{
+ "/home/zyga/src", "/home/zyga/dst",
+ } {
+ aliases, err := mount.AliasesOf(symmetry, mountinfo)
+ c.Assert(err, IsNil)
+ // We don't see "dummy!" here.
+ c.Assert(aliases, DeepEquals, map[string]bool{
+ "/home/zyga/src": true,
+ "/home/zyga/dst": true,
+ }, Commentf("symmetry: %q", symmetry))
+ }
+}
+
+// Check that AliasesOf finds a trio of bind mounts in tmpfs in home.
+func (s *mountinfoSuite) TestAliasesOf4(c *C) {
+ // $ mkdir data; sudo mount -t tmpfs none data; cd data
+ // $ mkdir foo bar; sudo mount --bind foo bar
+ // $ mkdir froz; sudo mount --bind bar froz
+ mountinfo := mockMountInfo(c,
+ "25 0 8:2 / / rw,relatime shared:1 - ext4 /dev/sda2 rw,errors=remount-ro,data=ordered",
+ "392 25 0:47 / /home/zyga/data rw,relatime shared:158 - tmpfs none rw",
+ "400 392 0:47 /foo /home/zyga/data/bar rw,relatime shared:158 - tmpfs none rw",
+ "408 392 0:47 /foo /home/zyga/data/froz rw,relatime shared:158 - tmpfs none rw")
+ for _, symmetry := range []string{
+ "/home/zyga/data/foo", "/home/zyga/data/bar", "/home/zyga/data/froz",
+ } {
+ aliases, err := mount.AliasesOf(symmetry, mountinfo)
+ c.Assert(err, IsNil)
+ c.Assert(aliases, DeepEquals, map[string]bool{
+ "/home/zyga/data/foo": true,
+ "/home/zyga/data/bar": true,
+ "/home/zyga/data/froz": true,
+ }, Commentf("symmetry: %q", symmetry))
+ }
+}
+
+// Check that AliasesOf finds all the canonical mounts.
+// Here the /home/zyga/data directory exists in two places, fully (not via a
+// sub-directory mount where Root != "/") so both of those should be found.
+func (s *mountinfoSuite) TestAliasesOf5(c *C) {
+ // $ mkdir data; sudo mount -t tmpfs none data; cd data
+ // $ mkdir foo bar; sudo mount --bind foo bar
+ // $ mkdir froz; sudo mount --bind bar froz
+ // $ mkdir data-bind; sudo mount --bind data data-bind -o ro
+ // ^ note, not recursive so only data itself is propagated
+ mountinfo := mockMountInfo(c,
+ "25 0 8:2 / / rw,relatime shared:1 - ext4 /dev/sda2 rw,errors=remount-ro,data=ordered",
+ "392 25 0:47 / /home/zyga/data rw,relatime shared:158 - tmpfs none rw",
+ "400 392 0:47 /foo /home/zyga/data/bar rw,relatime - tmpfs none rw",
+ "408 392 0:47 /foo /home/zyga/data/froz rw,relatime shared:158 - tmpfs none rw",
+ "416 25 0:47 / /home/zyga/data-bind ro,relatime shared:158 - tmpfs none rw")
+ for _, symmetry := range []string{
+ "/home/zyga/data", "/home/zyga/data-bind",
+ } {
+ aliases, err := mount.AliasesOf(symmetry, mountinfo)
+ c.Assert(err, IsNil)
+ c.Assert(aliases, DeepEquals, map[string]bool{
+ "/home/zyga/data": true,
+ "/home/zyga/data-bind": true,
+ }, Commentf("symmetry: %q", symmetry))
+ }
+}

0 comments on commit fa7fb52

Please sign in to comment.