[PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution

[PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution

[Qu Wenruo]

[BUG]
There is a bug report that "btrfs inspect logical-resolve" can not even
handle any file inside non-top-level subvolumes:

 # mkfs.btrfs $dev
 # mount $dev $mnt
 # btrfs subvol create $mnt/subv1
 # xfs_io -f -c "pwrite 0 16k" $mnt/subv1/file
 # sync
 # btrfs inspect logical-resolve 13631488 $mnt
 inode 257 subvol subv1 could not be accessed: not mounted

This means the command "btrfs inspect logical-resolve" is almost useless
for resolving logical bytenr to files.

[CAUSE]
"btrfs inspect logical-resolve" firstly resolve the logical bytenr to
root/ino pairs, then call btrfs_subvolid_resolve() to resolve the path
to the subvolume.

Then to handle cases where the subvolume is already mounted somewhere
else, we call find_mount_fsroot().

But if that target subvolume is not yet mounted, we just error out, even
if the @path is the top-level subvolume, and we have already know the
path to the subvolume.

[FIX]
Instead of doing unnecessary subvolume mount point check, just require
the @path to be the mount point of the top-level subvolume.

So that we can access all subvolumes without mounting each one.

Now the command works as expected:

 # ./btrfs inspect logical-resolve 13631488 $mnt
 /mnt/btrfs/subv1/file

And since we're changing the behavior of "logical-resolve" (to a more
user-friendly one), we have to update the test case misc/042 to reflect
that.

Reported-by: Torstein Eide <torsteine@gmail.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 Documentation/btrfs-inspect-internal.rst      |  3 ++
 cmds/inspect.c                                | 54 ++++++++-----------
 .../test.sh                                   | 25 ---------
 3 files changed, 24 insertions(+), 58 deletions(-)

diff --git a/Documentation/btrfs-inspect-internal.rst b/Documentation/btrfs-inspect-internal.rst
index 4265fab6..69da468a 100644
--- a/Documentation/btrfs-inspect-internal.rst
+++ b/Documentation/btrfs-inspect-internal.rst
@@ -149,6 +149,9 @@ logical-resolve [-Pvo] [-s <bufsize>] <logical> <path>
 
         resolve paths to all files at given *logical* address in the linear filesystem space
 
+        User should make sure *path* is the mount point of the top-level
+        subvolume (subvolid 5).
+
         ``Options``
 
         -P
diff --git a/cmds/inspect.c b/cmds/inspect.c
index 20f433b9..dc0e587f 100644
--- a/cmds/inspect.c
+++ b/cmds/inspect.c
@@ -158,6 +158,9 @@ static int cmd_inspect_logical_resolve(const struct cmd_struct *cmd,
 	int ret;
 	int fd;
 	int i;
+	const char *top_subvol = "/";
+	const char *top_subvolid = "5";
+	char *mounted = NULL;
 	bool getpath = true;
 	int bytes_left;
 	struct btrfs_ioctl_logical_ino_args loi = { 0 };
@@ -216,6 +219,23 @@ static int cmd_inspect_logical_resolve(const struct cmd_struct *cmd,
 		goto out;
 	}
 
+	/*
+	 * For logical-resolve, we want the mount point to be top level
+	 * subvolume (5), so that we can access all subvolumes.
+	 */
+	ret = find_mount_fsroot(top_subvol, top_subvolid, &mounted);
+	if (ret) {
+		error("failed to parse mountinfo");
+		goto out;
+	}
+	if (!mounted) {
+		ret = -ENOENT;
+		error("mount point \"%s\" is not the top-level subvolume",
+		      argv[optind + 1]);
+		goto out;
+	}
+	free(mounted);
+
 	ret = ioctl(fd, request, &loi);
 	if (ret < 0) {
 		error("logical ino ioctl: %m");
@@ -258,39 +278,7 @@ static int cmd_inspect_logical_resolve(const struct cmd_struct *cmd,
 				path_fd = fd;
 				strncpy(mount_path, full_path, PATH_MAX);
 			} else {
-				char *mounted = NULL;
-				char subvol[PATH_MAX];
-				char subvolid[PATH_MAX];
-
-				/*
-				 * btrfs_subvolid_resolve returns the full
-				 * path to the subvolume pointed by root, but the
-				 * subvolume can be mounted in a directory name
-				 * different from the subvolume name. In this
-				 * case we need to find the correct mount point
-				 * using same subvolume path and subvol id found
-				 * before.
-				 */
-
-				snprintf(subvol, PATH_MAX, "/%s", name);
-				snprintf(subvolid, PATH_MAX, "%llu", root);
-
-				ret = find_mount_fsroot(subvol, subvolid, &mounted);
-
-				if (ret) {
-					error("failed to parse mountinfo");
-					goto out;
-				}
-
-				if (!mounted) {
-					printf(
-			"inode %llu subvol %s could not be accessed: not mounted\n",
-						inum, name);
-					continue;
-				}
-
-				strncpy(mount_path, mounted, PATH_MAX);
-				free(mounted);
+				snprintf(mount_path, PATH_MAX, "%s%s", full_path, name);
 
 				path_fd = btrfs_open_dir(mount_path, &dirs, 1);
 				if (path_fd < 0) {
diff --git a/tests/misc-tests/042-inspect-internal-logical-resolve/test.sh b/tests/misc-tests/042-inspect-internal-logical-resolve/test.sh
index 2ba7331e..d20d5f74 100755
--- a/tests/misc-tests/042-inspect-internal-logical-resolve/test.sh
+++ b/tests/misc-tests/042-inspect-internal-logical-resolve/test.sh
@@ -51,34 +51,9 @@ run_check $SUDO_HELPER "$TOP/btrfs" subvolume snapshot "$TEST_MNT/@/vol1/subvol1
 
 run_check "$TOP/btrfs" filesystem sync "$TEST_MNT"
 
-run_check_umount_test_dev
-
-run_check $SUDO_HELPER mount -o subvol=/@/vol1 "$TEST_DEV" "$TEST_MNT"
-# Create a bind mount to vol1. logical-resolve should avoid bind mounts,
-# otherwise the test will fail
-run_check $SUDO_HELPER mkdir -p "$TEST_MNT/dir"
-run_check mkdir -p mnt2
-run_check $SUDO_HELPER mount --bind "$TEST_MNT/dir" mnt2
-# Create another bind mount to confuse logical-resolve even more.
-# logical-resolve can return the original mount or mnt3, both are valid
-run_check mkdir -p mnt3
-run_check $SUDO_HELPER mount --bind "$TEST_MNT" mnt3
-
 for offset in $("$TOP/btrfs" inspect-internal dump-tree -t "$vol1id" "$TEST_DEV" |
 		awk '/disk byte/ { print $5 }'); do
 	check_logical_offset_filename "$offset"
 done
 
-run_check_umount_test_dev mnt3
-run_check rmdir -- mnt3
-run_check_umount_test_dev mnt2
-run_check rmdir -- mnt2
-run_check_umount_test_dev
-
-run_check $SUDO_HELPER mount -o subvol="/@/vol1/subvol1" "$TEST_DEV" "$TEST_MNT"
-for offset in $("$TOP/btrfs" inspect-internal dump-tree -t "$subvol1id" "$TEST_DEV" |
-		awk '/disk byte/ { print $5 }'); do
-	check_logical_offset_filename "$offset"
-done
-
 run_check_umount_test_dev
-- 
2.39.0

Re: [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution

[Torstein Eide]

I made btrfs-progs, with `devel` branch and the patch.

Running the `btrfs inspect logical-resolve` against the top level
subvol works, like the example you showed.
# ${btrfs_prog_path}/btrfs inspect logical-resolve 13631488 $mnt
````
$mnt/subv1/file
````

But if running against the a mount subvol it gives useless error:
# mount:
````
/dev/loop20 on /mnt/subtest type btrfs (...,subvol=/subv1)
````
# ${btrfs_prog_path}/btrfs inspect logical-resolve 13631488 subtest
````
ERROR: cannot access 'subtest/subv1': No such file or directory
````

Note to self:
#How to build btrfs-progs
````
apt install make autoconf automake autotools-dev autoconf automake
build-essential e2fslibs-dev libblkid-dev zlib1g-dev libzstd-dev
libudev-dev python3.8 python3.8-dev liblzo2-dev libbtrfsutil-dev
make install_python
./autogen.sh && ./configure --disable-documentation
--prefix=/home/torstein/btrfs-progs/BIN
````


man. 17. apr. 2023 kl. 11:48 skrev Qu Wenruo <wqu@suse.com>:
>
> [BUG]
> There is a bug report that "btrfs inspect logical-resolve" can not even
> handle any file inside non-top-level subvolumes:
>
>  # mkfs.btrfs $dev
>  # mount $dev $mnt
>  # btrfs subvol create $mnt/subv1
>  # xfs_io -f -c "pwrite 0 16k" $mnt/subv1/file
>  # sync
>  # btrfs inspect logical-resolve 13631488 $mnt
>  inode 257 subvol subv1 could not be accessed: not mounted
>
> This means the command "btrfs inspect logical-resolve" is almost useless
> for resolving logical bytenr to files.
>
> [CAUSE]
> "btrfs inspect logical-resolve" firstly resolve the logical bytenr to
> root/ino pairs, then call btrfs_subvolid_resolve() to resolve the path
> to the subvolume.
>
> Then to handle cases where the subvolume is already mounted somewhere
> else, we call find_mount_fsroot().
>
> But if that target subvolume is not yet mounted, we just error out, even
> if the @path is the top-level subvolume, and we have already know the
> path to the subvolume.
>
> [FIX]
> Instead of doing unnecessary subvolume mount point check, just require
> the @path to be the mount point of the top-level subvolume.
>
> So that we can access all subvolumes without mounting each one.
>
> Now the command works as expected:
>
>  # ./btrfs inspect logical-resolve 13631488 $mnt
>  /mnt/btrfs/subv1/file
>
> And since we're changing the behavior of "logical-resolve" (to a more
> user-friendly one), we have to update the test case misc/042 to reflect
> that.
>
> Reported-by: Torstein Eide <torsteine@gmail.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  Documentation/btrfs-inspect-internal.rst      |  3 ++
>  cmds/inspect.c                                | 54 ++++++++-----------
>  .../test.sh                                   | 25 ---------
>  3 files changed, 24 insertions(+), 58 deletions(-)
>
> diff --git a/Documentation/btrfs-inspect-internal.rst b/Documentation/btrfs-inspect-internal.rst
> index 4265fab6..69da468a 100644
> --- a/Documentation/btrfs-inspect-internal.rst
> +++ b/Documentation/btrfs-inspect-internal.rst
> @@ -149,6 +149,9 @@ logical-resolve [-Pvo] [-s <bufsize>] <logical> <path>
>
>          resolve paths to all files at given *logical* address in the linear filesystem space
>
> +        User should make sure *path* is the mount point of the top-level
> +        subvolume (subvolid 5).
> +
>          ``Options``
>
>          -P
> diff --git a/cmds/inspect.c b/cmds/inspect.c
> index 20f433b9..dc0e587f 100644
> --- a/cmds/inspect.c
> +++ b/cmds/inspect.c
> @@ -158,6 +158,9 @@ static int cmd_inspect_logical_resolve(const struct cmd_struct *cmd,
>         int ret;
>         int fd;
>         int i;
> +       const char *top_subvol = "/";
> +       const char *top_subvolid = "5";
> +       char *mounted = NULL;
>         bool getpath = true;
>         int bytes_left;
>         struct btrfs_ioctl_logical_ino_args loi = { 0 };
> @@ -216,6 +219,23 @@ static int cmd_inspect_logical_resolve(const struct cmd_struct *cmd,
>                 goto out;
>         }
>
> +       /*
> +        * For logical-resolve, we want the mount point to be top level
> +        * subvolume (5), so that we can access all subvolumes.
> +        */
> +       ret = find_mount_fsroot(top_subvol, top_subvolid, &mounted);
> +       if (ret) {
> +               error("failed to parse mountinfo");
> +               goto out;
> +       }
> +       if (!mounted) {
> +               ret = -ENOENT;
> +               error("mount point \"%s\" is not the top-level subvolume",
> +                     argv[optind + 1]);
> +               goto out;
> +       }
> +       free(mounted);
> +
>         ret = ioctl(fd, request, &loi);
>         if (ret < 0) {
>                 error("logical ino ioctl: %m");
> @@ -258,39 +278,7 @@ static int cmd_inspect_logical_resolve(const struct cmd_struct *cmd,
>                                 path_fd = fd;
>                                 strncpy(mount_path, full_path, PATH_MAX);
>                         } else {
> -                               char *mounted = NULL;
> -                               char subvol[PATH_MAX];
> -                               char subvolid[PATH_MAX];
> -
> -                               /*
> -                                * btrfs_subvolid_resolve returns the full
> -                                * path to the subvolume pointed by root, but the
> -                                * subvolume can be mounted in a directory name
> -                                * different from the subvolume name. In this
> -                                * case we need to find the correct mount point
> -                                * using same subvolume path and subvol id found
> -                                * before.
> -                                */
> -
> -                               snprintf(subvol, PATH_MAX, "/%s", name);
> -                               snprintf(subvolid, PATH_MAX, "%llu", root);
> -
> -                               ret = find_mount_fsroot(subvol, subvolid, &mounted);
> -
> -                               if (ret) {
> -                                       error("failed to parse mountinfo");
> -                                       goto out;
> -                               }
> -
> -                               if (!mounted) {
> -                                       printf(
> -                       "inode %llu subvol %s could not be accessed: not mounted\n",
> -                                               inum, name);
> -                                       continue;
> -                               }
> -
> -                               strncpy(mount_path, mounted, PATH_MAX);
> -                               free(mounted);
> +                               snprintf(mount_path, PATH_MAX, "%s%s", full_path, name);
>
>                                 path_fd = btrfs_open_dir(mount_path, &dirs, 1);
>                                 if (path_fd < 0) {
> diff --git a/tests/misc-tests/042-inspect-internal-logical-resolve/test.sh b/tests/misc-tests/042-inspect-internal-logical-resolve/test.sh
> index 2ba7331e..d20d5f74 100755
> --- a/tests/misc-tests/042-inspect-internal-logical-resolve/test.sh
> +++ b/tests/misc-tests/042-inspect-internal-logical-resolve/test.sh
> @@ -51,34 +51,9 @@ run_check $SUDO_HELPER "$TOP/btrfs" subvolume snapshot "$TEST_MNT/@/vol1/subvol1
>
>  run_check "$TOP/btrfs" filesystem sync "$TEST_MNT"
>
> -run_check_umount_test_dev
> -
> -run_check $SUDO_HELPER mount -o subvol=/@/vol1 "$TEST_DEV" "$TEST_MNT"
> -# Create a bind mount to vol1. logical-resolve should avoid bind mounts,
> -# otherwise the test will fail
> -run_check $SUDO_HELPER mkdir -p "$TEST_MNT/dir"
> -run_check mkdir -p mnt2
> -run_check $SUDO_HELPER mount --bind "$TEST_MNT/dir" mnt2
> -# Create another bind mount to confuse logical-resolve even more.
> -# logical-resolve can return the original mount or mnt3, both are valid
> -run_check mkdir -p mnt3
> -run_check $SUDO_HELPER mount --bind "$TEST_MNT" mnt3
> -
>  for offset in $("$TOP/btrfs" inspect-internal dump-tree -t "$vol1id" "$TEST_DEV" |
>                 awk '/disk byte/ { print $5 }'); do
>         check_logical_offset_filename "$offset"
>  done
>
> -run_check_umount_test_dev mnt3
> -run_check rmdir -- mnt3
> -run_check_umount_test_dev mnt2
> -run_check rmdir -- mnt2
> -run_check_umount_test_dev
> -
> -run_check $SUDO_HELPER mount -o subvol="/@/vol1/subvol1" "$TEST_DEV" "$TEST_MNT"
> -for offset in $("$TOP/btrfs" inspect-internal dump-tree -t "$subvol1id" "$TEST_DEV" |
> -               awk '/disk byte/ { print $5 }'); do
> -       check_logical_offset_filename "$offset"
> -done
> -
>  run_check_umount_test_dev
> --
> 2.39.0
>

Re: [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution

[Qu Wenruo]

On 2023/4/18 05:17, Torstein Eide wrote:
> I made btrfs-progs, with `devel` branch and the patch.
> 
> Running the `btrfs inspect logical-resolve` against the top level
> subvol works, like the example you showed.
> # ${btrfs_prog_path}/btrfs inspect logical-resolve 13631488 $mnt
> ````
> $mnt/subv1/file
> ````
> 
> But if running against the a mount subvol it gives useless error:
> # mount:
> ````
> /dev/loop20 on /mnt/subtest type btrfs (...,subvol=/subv1)
> ````
> # ${btrfs_prog_path}/btrfs inspect logical-resolve 13631488 subtest
> ````
> ERROR: cannot access 'subtest/subv1': No such file or directory
> ````

Weird, in my test env, it failed with a clear error message:

# mount /dev/test/scratch1  -o subvolid=256 /mnt/btrfs/
# ./btrfs ins log 13631488 /mnt/btrfs/
ERROR: mount point "/mnt/btrfs/" is not the top-level subvolume


> 
> Note to self:
> #How to build btrfs-progs
> ````
> apt install make autoconf automake autotools-dev autoconf automake
> build-essential e2fslibs-dev libblkid-dev zlib1g-dev libzstd-dev
> libudev-dev python3.8 python3.8-dev liblzo2-dev libbtrfsutil-dev
> make install_python
> ./autogen.sh && ./configure --disable-documentation
> --prefix=/home/torstein/btrfs-progs/BIN
> ````
> 
> 
> man. 17. apr. 2023 kl. 11:48 skrev Qu Wenruo <wqu@suse.com>:
>>
>> [BUG]
>> There is a bug report that "btrfs inspect logical-resolve" can not even
>> handle any file inside non-top-level subvolumes:
>>
>>   # mkfs.btrfs $dev
>>   # mount $dev $mnt
>>   # btrfs subvol create $mnt/subv1
>>   # xfs_io -f -c "pwrite 0 16k" $mnt/subv1/file
>>   # sync
>>   # btrfs inspect logical-resolve 13631488 $mnt
>>   inode 257 subvol subv1 could not be accessed: not mounted
>>
>> This means the command "btrfs inspect logical-resolve" is almost useless
>> for resolving logical bytenr to files.
>>
>> [CAUSE]
>> "btrfs inspect logical-resolve" firstly resolve the logical bytenr to
>> root/ino pairs, then call btrfs_subvolid_resolve() to resolve the path
>> to the subvolume.
>>
>> Then to handle cases where the subvolume is already mounted somewhere
>> else, we call find_mount_fsroot().
>>
>> But if that target subvolume is not yet mounted, we just error out, even
>> if the @path is the top-level subvolume, and we have already know the
>> path to the subvolume.
>>
>> [FIX]
>> Instead of doing unnecessary subvolume mount point check, just require
>> the @path to be the mount point of the top-level subvolume.
>>
>> So that we can access all subvolumes without mounting each one.
>>
>> Now the command works as expected:
>>
>>   # ./btrfs inspect logical-resolve 13631488 $mnt
>>   /mnt/btrfs/subv1/file
>>
>> And since we're changing the behavior of "logical-resolve" (to a more
>> user-friendly one), we have to update the test case misc/042 to reflect
>> that.
>>
>> Reported-by: Torstein Eide <torsteine@gmail.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   Documentation/btrfs-inspect-internal.rst      |  3 ++
>>   cmds/inspect.c                                | 54 ++++++++-----------
>>   .../test.sh                                   | 25 ---------
>>   3 files changed, 24 insertions(+), 58 deletions(-)
>>
>> diff --git a/Documentation/btrfs-inspect-internal.rst b/Documentation/btrfs-inspect-internal.rst
>> index 4265fab6..69da468a 100644
>> --- a/Documentation/btrfs-inspect-internal.rst
>> +++ b/Documentation/btrfs-inspect-internal.rst
>> @@ -149,6 +149,9 @@ logical-resolve [-Pvo] [-s <bufsize>] <logical> <path>
>>
>>           resolve paths to all files at given *logical* address in the linear filesystem space
>>
>> +        User should make sure *path* is the mount point of the top-level
>> +        subvolume (subvolid 5).
>> +
>>           ``Options``
>>
>>           -P
>> diff --git a/cmds/inspect.c b/cmds/inspect.c
>> index 20f433b9..dc0e587f 100644
>> --- a/cmds/inspect.c
>> +++ b/cmds/inspect.c
>> @@ -158,6 +158,9 @@ static int cmd_inspect_logical_resolve(const struct cmd_struct *cmd,
>>          int ret;
>>          int fd;
>>          int i;
>> +       const char *top_subvol = "/";
>> +       const char *top_subvolid = "5";
>> +       char *mounted = NULL;
>>          bool getpath = true;
>>          int bytes_left;
>>          struct btrfs_ioctl_logical_ino_args loi = { 0 };
>> @@ -216,6 +219,23 @@ static int cmd_inspect_logical_resolve(const struct cmd_struct *cmd,
>>                  goto out;
>>          }
>>
>> +       /*
>> +        * For logical-resolve, we want the mount point to be top level
>> +        * subvolume (5), so that we can access all subvolumes.
>> +        */
>> +       ret = find_mount_fsroot(top_subvol, top_subvolid, &mounted);
>> +       if (ret) {
>> +               error("failed to parse mountinfo");
>> +               goto out;
>> +       }
>> +       if (!mounted) {
>> +               ret = -ENOENT;
>> +               error("mount point \"%s\" is not the top-level subvolume",
>> +                     argv[optind + 1]);
>> +               goto out;
>> +       }
>> +       free(mounted);
>> +
>>          ret = ioctl(fd, request, &loi);
>>          if (ret < 0) {
>>                  error("logical ino ioctl: %m");
>> @@ -258,39 +278,7 @@ static int cmd_inspect_logical_resolve(const struct cmd_struct *cmd,
>>                                  path_fd = fd;
>>                                  strncpy(mount_path, full_path, PATH_MAX);
>>                          } else {
>> -                               char *mounted = NULL;
>> -                               char subvol[PATH_MAX];
>> -                               char subvolid[PATH_MAX];
>> -
>> -                               /*
>> -                                * btrfs_subvolid_resolve returns the full
>> -                                * path to the subvolume pointed by root, but the
>> -                                * subvolume can be mounted in a directory name
>> -                                * different from the subvolume name. In this
>> -                                * case we need to find the correct mount point
>> -                                * using same subvolume path and subvol id found
>> -                                * before.
>> -                                */
>> -
>> -                               snprintf(subvol, PATH_MAX, "/%s", name);
>> -                               snprintf(subvolid, PATH_MAX, "%llu", root);
>> -
>> -                               ret = find_mount_fsroot(subvol, subvolid, &mounted);
>> -
>> -                               if (ret) {
>> -                                       error("failed to parse mountinfo");
>> -                                       goto out;
>> -                               }
>> -
>> -                               if (!mounted) {
>> -                                       printf(
>> -                       "inode %llu subvol %s could not be accessed: not mounted\n",
>> -                                               inum, name);
>> -                                       continue;
>> -                               }
>> -
>> -                               strncpy(mount_path, mounted, PATH_MAX);
>> -                               free(mounted);
>> +                               snprintf(mount_path, PATH_MAX, "%s%s", full_path, name);
>>
>>                                  path_fd = btrfs_open_dir(mount_path, &dirs, 1);
>>                                  if (path_fd < 0) {
>> diff --git a/tests/misc-tests/042-inspect-internal-logical-resolve/test.sh b/tests/misc-tests/042-inspect-internal-logical-resolve/test.sh
>> index 2ba7331e..d20d5f74 100755
>> --- a/tests/misc-tests/042-inspect-internal-logical-resolve/test.sh
>> +++ b/tests/misc-tests/042-inspect-internal-logical-resolve/test.sh
>> @@ -51,34 +51,9 @@ run_check $SUDO_HELPER "$TOP/btrfs" subvolume snapshot "$TEST_MNT/@/vol1/subvol1
>>
>>   run_check "$TOP/btrfs" filesystem sync "$TEST_MNT"
>>
>> -run_check_umount_test_dev
>> -
>> -run_check $SUDO_HELPER mount -o subvol=/@/vol1 "$TEST_DEV" "$TEST_MNT"
>> -# Create a bind mount to vol1. logical-resolve should avoid bind mounts,
>> -# otherwise the test will fail
>> -run_check $SUDO_HELPER mkdir -p "$TEST_MNT/dir"
>> -run_check mkdir -p mnt2
>> -run_check $SUDO_HELPER mount --bind "$TEST_MNT/dir" mnt2
>> -# Create another bind mount to confuse logical-resolve even more.
>> -# logical-resolve can return the original mount or mnt3, both are valid
>> -run_check mkdir -p mnt3
>> -run_check $SUDO_HELPER mount --bind "$TEST_MNT" mnt3
>> -
>>   for offset in $("$TOP/btrfs" inspect-internal dump-tree -t "$vol1id" "$TEST_DEV" |
>>                  awk '/disk byte/ { print $5 }'); do
>>          check_logical_offset_filename "$offset"
>>   done
>>
>> -run_check_umount_test_dev mnt3
>> -run_check rmdir -- mnt3
>> -run_check_umount_test_dev mnt2
>> -run_check rmdir -- mnt2
>> -run_check_umount_test_dev
>> -
>> -run_check $SUDO_HELPER mount -o subvol="/@/vol1/subvol1" "$TEST_DEV" "$TEST_MNT"
>> -for offset in $("$TOP/btrfs" inspect-internal dump-tree -t "$subvol1id" "$TEST_DEV" |
>> -               awk '/disk byte/ { print $5 }'); do
>> -       check_logical_offset_filename "$offset"
>> -done
>> -
>>   run_check_umount_test_dev
>> --
>> 2.39.0
>>

Re: [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution

[David Sterba]

On Mon, Apr 17, 2023 at 05:48:10PM +0800, Qu Wenruo wrote:
> [BUG]
> There is a bug report that "btrfs inspect logical-resolve" can not even
> handle any file inside non-top-level subvolumes:
> 
>  # mkfs.btrfs $dev
>  # mount $dev $mnt
>  # btrfs subvol create $mnt/subv1
>  # xfs_io -f -c "pwrite 0 16k" $mnt/subv1/file
>  # sync
>  # btrfs inspect logical-resolve 13631488 $mnt
>  inode 257 subvol subv1 could not be accessed: not mounted
> 
> This means the command "btrfs inspect logical-resolve" is almost useless
> for resolving logical bytenr to files.
> 
> [CAUSE]
> "btrfs inspect logical-resolve" firstly resolve the logical bytenr to
> root/ino pairs, then call btrfs_subvolid_resolve() to resolve the path
> to the subvolume.
> 
> Then to handle cases where the subvolume is already mounted somewhere
> else, we call find_mount_fsroot().
> 
> But if that target subvolume is not yet mounted, we just error out, even
> if the @path is the top-level subvolume, and we have already know the
> path to the subvolume.
> 
> [FIX]
> Instead of doing unnecessary subvolume mount point check, just require
> the @path to be the mount point of the top-level subvolume.

This is a change in the semantics of the command, can't we make it work
on non-toplevel subvolumes instead? Access to the mounted toplevel
subvolume is not always provided, e.g. on openSUSE the subvolume layout
does not mount 5 and there are likely other distros following that
scheme.

Re: [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution

[Qu Wenruo]

On 2023/4/19 07:55, David Sterba wrote:
> On Mon, Apr 17, 2023 at 05:48:10PM +0800, Qu Wenruo wrote:
>> [BUG]
>> There is a bug report that "btrfs inspect logical-resolve" can not even
>> handle any file inside non-top-level subvolumes:
>>
>>   # mkfs.btrfs $dev
>>   # mount $dev $mnt
>>   # btrfs subvol create $mnt/subv1
>>   # xfs_io -f -c "pwrite 0 16k" $mnt/subv1/file
>>   # sync
>>   # btrfs inspect logical-resolve 13631488 $mnt
>>   inode 257 subvol subv1 could not be accessed: not mounted
>>
>> This means the command "btrfs inspect logical-resolve" is almost useless
>> for resolving logical bytenr to files.
>>
>> [CAUSE]
>> "btrfs inspect logical-resolve" firstly resolve the logical bytenr to
>> root/ino pairs, then call btrfs_subvolid_resolve() to resolve the path
>> to the subvolume.
>>
>> Then to handle cases where the subvolume is already mounted somewhere
>> else, we call find_mount_fsroot().
>>
>> But if that target subvolume is not yet mounted, we just error out, even
>> if the @path is the top-level subvolume, and we have already know the
>> path to the subvolume.
>>
>> [FIX]
>> Instead of doing unnecessary subvolume mount point check, just require
>> the @path to be the mount point of the top-level subvolume.
> 
> This is a change in the semantics of the command, can't we make it work
> on non-toplevel subvolumes instead? Access to the mounted toplevel
> subvolume is not always provided, e.g. on openSUSE the subvolume layout
> does not mount 5 and there are likely other distros following that
> scheme.

But mounting 5 is not that hard if one is trying to locate the offending 
file.

The original semantics is almost impossible to make real usage, just 
check this mail:

https://lore.kernel.org/linux-btrfs/CAL5DHTFAUCKBmW_j737j8dzRvaBnKWa9Wo5VtvoAgW8f93oR9A@mail.gmail.com/

Doing a mount point search for the subvolume is fine, but requiring each 
subvolume to be mounted while top level subvolume is already mounted is 
not user friendly at all.

Thanks,
Qu

Re: [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution

[Andrei Borzenkov]

On Wed, Apr 19, 2023 at 3:24 AM David Sterba <dsterba@suse.cz> wrote:
>
> On Mon, Apr 17, 2023 at 05:48:10PM +0800, Qu Wenruo wrote:
> > [BUG]
> > There is a bug report that "btrfs inspect logical-resolve" can not even
> > handle any file inside non-top-level subvolumes:
> >
> >  # mkfs.btrfs $dev
> >  # mount $dev $mnt
> >  # btrfs subvol create $mnt/subv1
> >  # xfs_io -f -c "pwrite 0 16k" $mnt/subv1/file
> >  # sync
> >  # btrfs inspect logical-resolve 13631488 $mnt
> >  inode 257 subvol subv1 could not be accessed: not mounted
> >
> > This means the command "btrfs inspect logical-resolve" is almost useless
> > for resolving logical bytenr to files.
> >
> > [CAUSE]
> > "btrfs inspect logical-resolve" firstly resolve the logical bytenr to
> > root/ino pairs, then call btrfs_subvolid_resolve() to resolve the path
> > to the subvolume.
> >
> > Then to handle cases where the subvolume is already mounted somewhere
> > else, we call find_mount_fsroot().
> >
> > But if that target subvolume is not yet mounted, we just error out, even
> > if the @path is the top-level subvolume, and we have already know the
> > path to the subvolume.
> >
> > [FIX]
> > Instead of doing unnecessary subvolume mount point check, just require
> > the @path to be the mount point of the top-level subvolume.
>
> This is a change in the semantics of the command, can't we make it work
> on non-toplevel subvolumes instead? Access to the mounted toplevel
> subvolume is not always provided, e.g. on openSUSE the subvolume layout
> does not mount 5 and there are likely other distros following that
> scheme.

The BTRFS_IOC_INO_PATHS ioctl used to map inode number to path expects
opened file on subvolume as argument and it is not possible unless
subvolume is accessible. So either different ioctl is needed that
takes subvolume is/name as argument or command has to mount subvolume
temporarily if it is not available.

Re: [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution

[Torstein Eide]

As a user i think this result will be good:

(use -q for script use, print lists for 1, 2 ,and 4 )

1.  File is resolved in current mount path:
````
# ./btrfs inspect logical-resolve 13631488 $mnt
Files is accessible:
- /mnt/btrfs/subv1/file
- /mnt/btrfs/subv1/.snapshot/1/file


````

2.  File is not  in current mount path, but part of the volume:
````
# ./btrfs inspect logical-resolve 13631488 $mnt
Files is part subvolumes, but not accessible under current path:
-  subv1/file
-  snapshot1/file
````

3. Hybrid
````
# ./btrfs inspect logical-resolve 13631488 $mnt_snapshot
Files is accessible:
- /mnt/btrfs/subv1/.snapshot/1/file

Files is part subvolumes, but not accessible under current path:
-  subv1/file
````

4. List subvolumes a file is part of:
````
# ./btrfs inspect logical-resolve 13631488 $mnt --list-subvolumes
Filename:

Files is part subvolumes
- subv1  (subvolid 256)
- snapshot1 (subvolid 1025)
````

5.  failed
````
# ./btrfs inspect logical-resolve 1 $mnt --list-subvolumes
Error: logical address not in use.
````

ons. 19. apr. 2023 kl. 10:55 skrev Andrei Borzenkov <arvidjaar@gmail.com>:
>
> On Wed, Apr 19, 2023 at 3:24 AM David Sterba <dsterba@suse.cz> wrote:
> >
> > On Mon, Apr 17, 2023 at 05:48:10PM +0800, Qu Wenruo wrote:
> > > [BUG]
> > > There is a bug report that "btrfs inspect logical-resolve" can not even
> > > handle any file inside non-top-level subvolumes:
> > >
> > >  # mkfs.btrfs $dev
> > >  # mount $dev $mnt
> > >  # btrfs subvol create $mnt/subv1
> > >  # xfs_io -f -c "pwrite 0 16k" $mnt/subv1/file
> > >  # sync
> > >  # btrfs inspect logical-resolve 13631488 $mnt
> > >  inode 257 subvol subv1 could not be accessed: not mounted
> > >
> > > This means the command "btrfs inspect logical-resolve" is almost useless
> > > for resolving logical bytenr to files.
> > >
> > > [CAUSE]
> > > "btrfs inspect logical-resolve" firstly resolve the logical bytenr to
> > > root/ino pairs, then call btrfs_subvolid_resolve() to resolve the path
> > > to the subvolume.
> > >
> > > Then to handle cases where the subvolume is already mounted somewhere
> > > else, we call find_mount_fsroot().
> > >
> > > But if that target subvolume is not yet mounted, we just error out, even
> > > if the @path is the top-level subvolume, and we have already know the
> > > path to the subvolume.
> > >
> > > [FIX]
> > > Instead of doing unnecessary subvolume mount point check, just require
> > > the @path to be the mount point of the top-level subvolume.
> >
> > This is a change in the semantics of the command, can't we make it work
> > on non-toplevel subvolumes instead? Access to the mounted toplevel
> > subvolume is not always provided, e.g. on openSUSE the subvolume layout
> > does not mount 5 and there are likely other distros following that
> > scheme.
>
> The BTRFS_IOC_INO_PATHS ioctl used to map inode number to path expects
> opened file on subvolume as argument and it is not possible unless
> subvolume is accessible. So either different ioctl is needed that
> takes subvolume is/name as argument or command has to mount subvolume
> temporarily if it is not available.

Re: [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution

[Qu Wenruo]

On 2023/4/19 16:55, Andrei Borzenkov wrote:
> On Wed, Apr 19, 2023 at 3:24 AM David Sterba <dsterba@suse.cz> wrote:
>>
>> On Mon, Apr 17, 2023 at 05:48:10PM +0800, Qu Wenruo wrote:
>>> [BUG]
>>> There is a bug report that "btrfs inspect logical-resolve" can not even
>>> handle any file inside non-top-level subvolumes:
>>>
>>>   # mkfs.btrfs $dev
>>>   # mount $dev $mnt
>>>   # btrfs subvol create $mnt/subv1
>>>   # xfs_io -f -c "pwrite 0 16k" $mnt/subv1/file
>>>   # sync
>>>   # btrfs inspect logical-resolve 13631488 $mnt
>>>   inode 257 subvol subv1 could not be accessed: not mounted
>>>
>>> This means the command "btrfs inspect logical-resolve" is almost useless
>>> for resolving logical bytenr to files.
>>>
>>> [CAUSE]
>>> "btrfs inspect logical-resolve" firstly resolve the logical bytenr to
>>> root/ino pairs, then call btrfs_subvolid_resolve() to resolve the path
>>> to the subvolume.
>>>
>>> Then to handle cases where the subvolume is already mounted somewhere
>>> else, we call find_mount_fsroot().
>>>
>>> But if that target subvolume is not yet mounted, we just error out, even
>>> if the @path is the top-level subvolume, and we have already know the
>>> path to the subvolume.
>>>
>>> [FIX]
>>> Instead of doing unnecessary subvolume mount point check, just require
>>> the @path to be the mount point of the top-level subvolume.
>>
>> This is a change in the semantics of the command, can't we make it work
>> on non-toplevel subvolumes instead? Access to the mounted toplevel
>> subvolume is not always provided, e.g. on openSUSE the subvolume layout
>> does not mount 5 and there are likely other distros following that
>> scheme.
> 
> The BTRFS_IOC_INO_PATHS ioctl used to map inode number to path expects
> opened file on subvolume as argument and it is not possible unless
> subvolume is accessible. So either different ioctl is needed that
> takes subvolume is/name as argument or command has to mount subvolume
> temporarily if it is not available.

Nope, if you're at the toplevel subvolume, all subvolumes can be 
accessed, and btrfs_subvolid_resolve() gives us the path from TOP-LEVEL 
subvolume to the target one.

Re: [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution

[Andrei Borzenkov]

On Wed, Apr 19, 2023 at 12:50 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> On 2023/4/19 16:55, Andrei Borzenkov wrote:
> > On Wed, Apr 19, 2023 at 3:24 AM David Sterba <dsterba@suse.cz> wrote:
> >>
> >> On Mon, Apr 17, 2023 at 05:48:10PM +0800, Qu Wenruo wrote:
> >>> [BUG]
> >>> There is a bug report that "btrfs inspect logical-resolve" can not even
> >>> handle any file inside non-top-level subvolumes:
> >>>
> >>>   # mkfs.btrfs $dev
> >>>   # mount $dev $mnt
> >>>   # btrfs subvol create $mnt/subv1
> >>>   # xfs_io -f -c "pwrite 0 16k" $mnt/subv1/file
> >>>   # sync
> >>>   # btrfs inspect logical-resolve 13631488 $mnt
> >>>   inode 257 subvol subv1 could not be accessed: not mounted
> >>>
> >>> This means the command "btrfs inspect logical-resolve" is almost useless
> >>> for resolving logical bytenr to files.
> >>>
> >>> [CAUSE]
> >>> "btrfs inspect logical-resolve" firstly resolve the logical bytenr to
> >>> root/ino pairs, then call btrfs_subvolid_resolve() to resolve the path
> >>> to the subvolume.
> >>>
> >>> Then to handle cases where the subvolume is already mounted somewhere
> >>> else, we call find_mount_fsroot().
> >>>
> >>> But if that target subvolume is not yet mounted, we just error out, even
> >>> if the @path is the top-level subvolume, and we have already know the
> >>> path to the subvolume.
> >>>
> >>> [FIX]
> >>> Instead of doing unnecessary subvolume mount point check, just require
> >>> the @path to be the mount point of the top-level subvolume.
> >>
> >> This is a change in the semantics of the command, can't we make it work
> >> on non-toplevel subvolumes instead? Access to the mounted toplevel
> >> subvolume is not always provided, e.g. on openSUSE the subvolume layout
> >> does not mount 5 and there are likely other distros following that
> >> scheme.
> >
> > The BTRFS_IOC_INO_PATHS ioctl used to map inode number to path expects
> > opened file on subvolume as argument and it is not possible unless
> > subvolume is accessible. So either different ioctl is needed that
> > takes subvolume is/name as argument or command has to mount subvolume
> > temporarily if it is not available.
>
> Nope, if you're at the toplevel subvolume, all subvolumes can be
> accessed, and btrfs_subvolid_resolve() gives us the path from TOP-LEVEL
> subvolume to the target one.
>
>

Sure. I was replying to "can we make it work on non-toplevel subvolume".