Merge pull request #3119 from restic/keep-mountpoints

Keep mountpoints as empty directories for --one-file-system

changelog/unreleased/issue-909

@@ -0,0 +1,12 @@
+Enhancement: Keep mountpoints as empty directories
+
+When the `--one-file-system` option is specified to `restic backup`, it
+ignores all file systems mounted below one of the target directories. This
+means that when a snapshot is restored, users needed to manually recreate the
+mountpoint directories.
+
+Restic now keeps mountpoints as empty directories and therefore implements
+the same approach as `tar`.
+
+https://github.com/restic/restic/issues/909
+https://github.com/restic/restic/pull/3119

cmd/restic/exclude.go

@@ -199,12 +199,17 @@ func isDirExcludedByFile(dir, tagFilename, header string) bool {
 	return true
 }
 
-// gatherDevices returns the set of unique device ids of the files and/or
+// DeviceMap is used to track allowed source devices for backup. This is used to
-// directory paths listed in "items".
+// check for crossing mount points during backup (for --one-file-system). It
-func gatherDevices(items []string) (deviceMap map[string]uint64, err error) {
+// maps the name of a source path to its device ID.
-	deviceMap = make(map[string]uint64)
+type DeviceMap map[string]uint64
-	for _, item := range items {
+
-		item, err = filepath.Abs(filepath.Clean(item))
+// NewDeviceMap creates a new device map from the list of source paths.
+func NewDeviceMap(allowedSourcePaths []string) (DeviceMap, error) {
+	deviceMap := make(map[string]uint64)
+
+	for _, item := range allowedSourcePaths {
+		item, err := filepath.Abs(filepath.Clean(item))
 		if err != nil {
 			return nil, err
 		}
@@ -213,30 +218,63 @@ func gatherDevices(items []string) (deviceMap map[string]uint64, err error) {
 		if err != nil {
 			return nil, err
 		}
+
 		id, err := fs.DeviceID(fi)
 		if err != nil {
 			return nil, err
 		}
+
 		deviceMap[item] = id
 	}
+
 	if len(deviceMap) == 0 {
 		return nil, errors.New("zero allowed devices")
 	}
+
 	return deviceMap, nil
 }
 
+// IsAllowed returns true if the path is located on an allowed device.
+func (m DeviceMap) IsAllowed(item string, deviceID uint64) (bool, error) {
+	for dir := item; ; dir = filepath.Dir(dir) {
+		debug.Log("item %v, test dir %v", item, dir)
+
+		// find a parent directory that is on an allowed device (otherwise
+		// we would not traverse the directory at all)
+		allowedID, ok := m[dir]
+		if !ok {
+			if dir == filepath.Dir(dir) {
+				// arrived at root, no allowed device found. this should not happen.
+				break
+			}
+			continue
+		}
+
+		// if the item has a different device ID than the parent directory,
+		// we crossed a file system boundary
+		if allowedID != deviceID {
+			debug.Log("item %v (dir %v) on disallowed device %d", item, dir, deviceID)
+			return false, nil
+		}
+
+		// item is on allowed device, accept it
+		debug.Log("item %v allowed", item)
+		return true, nil
+	}
+
+	return false, fmt.Errorf("item %v (device ID %v) not found, deviceMap: %v", item, deviceID, m)
+}
+
 // rejectByDevice returns a RejectFunc that rejects files which are on a
 // different file systems than the files/dirs in samples.
 func rejectByDevice(samples []string) (RejectFunc, error) {
-	allowed, err := gatherDevices(samples)
+	deviceMap, err := NewDeviceMap(samples)
 	if err != nil {
 		return nil, err
 	}
-	debug.Log("allowed devices: %v\n", allowed)
+	debug.Log("allowed devices: %v\n", deviceMap)
 
 	return func(item string, fi os.FileInfo) bool {
-		item = filepath.Clean(item)
-
 		id, err := fs.DeviceID(fi)
 		if err != nil {
 			// This should never happen because gatherDevices() would have
@@ -244,26 +282,55 @@ func rejectByDevice(samples []string) (RejectFunc, error) {
 			panic(err)
 		}
 
-		for dir := item; ; dir = filepath.Dir(dir) {
+		allowed, err := deviceMap.IsAllowed(filepath.Clean(item), id)
-			debug.Log("item %v, test dir %v", item, dir)
+		if err != nil {
-
+			// this should not happen
-			allowedID, ok := allowed[dir]
+			panic(fmt.Sprintf("error checking device ID of %v: %v", item, err))
-			if !ok {
+		}
-				if dir == filepath.Dir(dir) {
-					break
-				}
-				continue
-			}
-
-			if allowedID != id {
-				debug.Log("path %q on disallowed device %d", item, id)
-				return true
-			}
 
+		if allowed {
+			// accept item
 			return false
 		}
 
-		panic(fmt.Sprintf("item %v, device id %v not found, allowedDevs: %v", item, id, allowed))
+		// reject everything except directories
+		if !fi.IsDir() {
+			return true
+		}
+
+		// special case: make sure we keep mountpoints (directories which
+		// contain a mounted file system). Test this by checking if the parent
+		// directory would be included.
+		parentDir := filepath.Dir(filepath.Clean(item))
+
+		parentFI, err := fs.Lstat(parentDir)
+		if err != nil {
+			debug.Log("item %v: error running lstat() on parent directory: %v", item, err)
+			// if in doubt, reject
+			return true
+		}
+
+		parentDeviceID, err := fs.DeviceID(parentFI)
+		if err != nil {
+			debug.Log("item %v: getting device ID of parent directory: %v", item, err)
+			// if in doubt, reject
+			return true
+		}
+
+		parentAllowed, err := deviceMap.IsAllowed(parentDir, parentDeviceID)
+		if err != nil {
+			debug.Log("item %v: error checking parent directory: %v", item, err)
+			// if in doubt, reject
+			return true
+		}
+
+		if parentAllowed {
+			// we found a mount point, so accept the directory
+			return false
+		}
+
+		// reject everything else
+		return true
 	}, nil
 }
 

cmd/restic/exclude_test.go

@@ -318,3 +318,47 @@ func TestIsExcludedByFileSize(t *testing.T) {
 		}
 	}
 }
+
+func TestDeviceMap(t *testing.T) {
+	deviceMap := DeviceMap{
+		filepath.FromSlash("/"):          1,
+		filepath.FromSlash("/usr/local"): 5,
+	}
+
+	var tests = []struct {
+		item     string
+		deviceID uint64
+		allowed  bool
+	}{
+		{"/root", 1, true},
+		{"/usr", 1, true},
+
+		{"/proc", 2, false},
+		{"/proc/1234", 2, false},
+
+		{"/usr", 3, false},
+		{"/usr/share", 3, false},
+
+		{"/usr/local", 5, true},
+		{"/usr/local/foobar", 5, true},
+
+		{"/usr/local/foobar/submount", 23, false},
+		{"/usr/local/foobar/submount/file", 23, false},
+
+		{"/usr/local/foobar/outhersubmount", 1, false},
+		{"/usr/local/foobar/outhersubmount/otherfile", 1, false},
+	}
+
+	for _, test := range tests {
+		t.Run("", func(t *testing.T) {
+			res, err := deviceMap.IsAllowed(filepath.FromSlash(test.item), test.deviceID)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if res != test.allowed {
+				t.Fatalf("wrong result returned by IsAllowed(%v): want %v, got %v", test.item, test.allowed, res)
+			}
+		})
+	}
+}