diff mbox series

[v3] btrfs: prefer to use "/dev/mapper/name" symlink instead of "/dev/dm-*"

Message ID cb400b48ce549a16705adfe454fd39fbdb89f85c.1736207407.git.wqu@suse.com (mailing list archive)
State New
Headers show
Series [v3] btrfs: prefer to use "/dev/mapper/name" symlink instead of "/dev/dm-*" | expand

Commit Message

Qu Wenruo Jan. 6, 2025, 11:50 p.m. UTC
[BUG]
There is a gentoo bug report that upstream commit 2e8b6bc0ab41
("btrfs: avoid unnecessary device path update for the same device") breaks
6.6 LTS kernel behavior where previously lsblk can properly show multiple
mount points of the same btrfs:

 With kernel 6.6.62:
 NAME         MAJ:MIN RM   SIZE RO TYPE  MOUNTPOINTS
 sdb            8:16   0   9,1T  0 disk
 `-sdb1         8:17   0   9,1T  0 part
   `-hdd2     254:6    0   9,1T  0 crypt /mnt/hdd2
                                         /var/cache/distfiles
                                         /var/cache/binpkgs

But not with that upstream commit backported:

 With kernel 6.6.67:
 sdb            8:16   0   9,1T  0 disk
 `-sdb1         8:17   0   9,1T  0 part
   `-hdd2     254:6    0   9,1T  0 crypt /mnt/hdd2

[CAUSE]
With that upstream patch backported to 6.6 LTS, the main change is in
the mount info result:

Before:
 /dev/mapper/hdd2 /mnt/hdd2 btrfs rw,relatime,space_cache=v2,subvolid=382,subvol=/hdd2 0 0
 /dev/mapper/hdd2 /mnt/dump btrfs rw,relatime,space_cache=v2,subvolid=393,subvol=/dump 0 0

After:
 /dev/dm-1 /mnt/hdd2 btrfs rw,relatime,space_cache=v2,subvolid=382,subvol=/hdd2 0 0
 /dev/dm-1 /mnt/dump btrfs rw,relatime,space_cache=v2,subvolid=393,subvol=/dump 0 0

I believe that change of mount device is causing problems for lsblk.

And before that patch, even if udev registered "/dev/dm-1" to btrfs,
later mount triggered a rescan and change the name back to
"/dev/mapper/hdd2" thus older kernels will work as expected.

But after that patch, if udev registered "/dev/dm-1", then mount
triggered a rescan, but since btrfs detects both "/dev/dm-1" and
"/dev/mapper/hdd2" are pointing to the same block device, btrfs refuses
to do the rename, causing the less human readable "/dev/dm-1" to be
there forever, and trigger the bug for lsblk.

Fortunately for newer kernels, I guess due to the migration to fsconfig
mount API, even if the end user explicitly mount the fs using
"/dev/dm-1", mount will resolve the path to "/dev/mapper/hdd2" link
anyway.

So for newer kernels it won't be a big deal but one extra safety net.

[FIX]
Add an extra exception for is_same_device(), that if both paths are
pointing to the same device, but the newer path begins with "/dev/mapper"
and the older path is not, then still treat them as different devices,
so that we can rename to use the more human readable device path.

Reported-by: David Duchesne <aether@disroot.org>
Link: https://bugs.gentoo.org/947126
Fixes: 2e8b6bc0ab41 ("btrfs: avoid unnecessary device path update for the same device")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
Changelog:
v3:
- Fix the term "soft link" to "symlink"
- Use the proper upstream commit hash
- Use "DEV_MAPPER_PATH" macro for both the path and strlen() parameter

v2:
- Fix unnecessary rename where the filename is the same
  The problem is in the exception handling that if both old and new path
  matches, we will still do the rename.
  Fix it by revert the exception check condition and setting is_same to true when
  path_equal() is true.

- Fix special chars in the commit message
---
 fs/btrfs/volumes.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index c8b079ad1dfa..80687fcf25fb 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -805,6 +805,8 @@  static int get_canonical_dev_path(const char *dev_path, char *canonical)
 	return ret;
 }
 
+#define	DEV_MAPPER_PATH		("/dev/mapper/")
+
 static bool is_same_device(struct btrfs_device *device, const char *new_path)
 {
 	struct path old = { .mnt = NULL, .dentry = NULL };
@@ -832,8 +834,21 @@  static bool is_same_device(struct btrfs_device *device, const char *new_path)
 	ret = kern_path(new_path, LOOKUP_FOLLOW, &new);
 	if (ret)
 		goto out;
-	if (path_equal(&old, &new))
+	if (path_equal(&old, &new)) {
 		is_same = true;
+		/*
+		 * Special case for device mapper with its symlinks.
+		 *
+		 * We can have the old path as "/dev/dm-3", but udev triggered
+		 * rescan on the symlink "/dev/mapper/test".
+		 * In that case we want to rename to that mapper link, to avoid
+		 * a bug in libblkid where it cannot handle multiple mount
+		 * points if the device is "/dev/dm-3".
+		 */
+		if (strncmp(old_path, DEV_MAPPER_PATH, strlen(DEV_MAPPER_PATH)) &&
+		    strncmp(new_path, DEV_MAPPER_PATH, strlen(DEV_MAPPER_PATH)) == 0)
+			is_same = false;
+	}
 out:
 	kfree(old_path);
 	path_put(&old);