Fix bug causing invalid parents to be used

Issues have come up where backup failed with different error messages such as: > ERROR: rename o258-8-0 -> bin failed. Directory not empty Investigation has shown that these are the result of unrelated existing snapshots of different subvolumes being used as clone sources. The scenario when such unrelated snapshots are taken into consideration is when the path of the subvolume to backup is a prefix of existing other subvolumes of which snapshots are located in the same repository. In particular, when backing up a btrfs subvolume that is located in the root ('/') of the file system hierarchy, all other existing snapshots were used as parents. This change fixes the problem by taking a trailing separator between the path encoded in a snapshot's name and its time into consideration when comparing snapshot names. This trailing separator, which cannot be contained in a snapshot's name, acts as a suffix making comparisons based on the base name safe.

Author: d-e-s-o

btrfs-backup/src/deso/btrfs/repository.py

@@ -182,7 +182,7 @@ def _snapshotFiles(directory, extension, repository):
  # Mind the asterisk between the time and the extension. It is required
  # because we can have snapshots with equal time stamps which are then
  # numbered in an increasing fashion.
- expression = regex(r"^%s-.*-%s.*%s$" % (base, time, extension))
+ expression = regex(r"^%s.*-%s.*%s$" % (base, time, extension))
 
  # We have to rely on the '-v' parameter here to get a properly sorted
  # list and '-1' to display one file per line. We should not have to
@@ -292,9 +292,9 @@ def _snapshotBaseName(subvolume):
  # the ones in the middle with underscores to make names look less
  # confusing.
  path = subvolume.strip(sep).replace(sep, "_")
- return "%s-%s" % (name, path)
+ return "%s-%s-" % (name, path)
  else:
- return name
+ return "%s-" % name
 
 
 def _ensureUniqueName(snapshot, snapshots):
@@ -334,7 +334,7 @@ def _snapshotName(subvolume, snapshots):
  """Retrieve a fully qualified, unique snapshot name."""
  name = _snapshotBaseName(subvolume)
  time = datetime.strftime(datetime.now(), _TIME_FORMAT)
- snapshot = "%s-%s" % (name, time)
+ snapshot = "%s%s" % (name, time)
 
  return _ensureUniqueName(snapshot, snapshots)
 

btrfs-backup/src/deso/btrfs/test/testRepository.py

@@ -604,6 +604,37 @@ def testRepositoryMultipleSubvolumeSync(self):
  self.assertTrue(isfile(dst.path(root["path"], ".ssh", "key.pub")))
 
 
+ def testRepositorySubsumingSubvolumesAreTreatedProperly(self):
+ """Verify that subvolumes with names subsuming that of another one are backed up correctly."""
+ with alias(self._mount) as m:
+ snaps = make(m, "snapshots")
+ backup = make(m, "backup")
+
+ src = Repository(snaps)
+ dst = Repository(backup)
+
+ make(m, "root", subvol=True)
+ make(m, "root", "file", data=b"a")
+
+ subvols = [m.path("root")]
+ for i in range(2):
+ # Create subvolumes with names that are a prefix of the
+ # previously created one.
+ make(m, "root%d" % i, subvol=True)
+ make(m, "root%d" % i, "file", data=bytes("%d" % i, "utf-8"))
+
+ subvols += [m.path("root%d" % i)]
+
+ # Create backups of all the subvolumes. After this operation we
+ # also have snapshots if them.
+ syncRepos(subvols, src, dst)
+
+ src_snaps = src.snapshots()
+ dst_snaps = dst.snapshots()
+ parents = dst.parents(None, m.path("root"), src_snaps, dst_snaps)
+ self.assertEqual(len(parents), 1)
+
+
  def testRepositoryUniqueSnapshotName(self):
  """Verify that subvolume paths are correctly canonicalized for snapshot names."""
  with alias(self._mount) as m: