* [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution
@ 2023-04-17 9:48 Qu Wenruo
2023-04-17 21:17 ` Torstein Eide
2023-04-18 23:55 ` David Sterba
0 siblings, 2 replies; 9+ messages in thread
From: Qu Wenruo @ 2023-04-17 9:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: Torstein Eide
[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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution
2023-04-17 9:48 [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution Qu Wenruo
@ 2023-04-17 21:17 ` Torstein Eide
2023-04-17 22:51 ` Qu Wenruo
2023-04-18 23:55 ` David Sterba
1 sibling, 1 reply; 9+ messages in thread
From: Torstein Eide @ 2023-04-17 21:17 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution
2023-04-17 21:17 ` Torstein Eide
@ 2023-04-17 22:51 ` Qu Wenruo
0 siblings, 0 replies; 9+ messages in thread
From: Qu Wenruo @ 2023-04-17 22:51 UTC (permalink / raw)
To: Torstein Eide, Qu Wenruo; +Cc: linux-btrfs
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
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution
2023-04-17 9:48 [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution Qu Wenruo
2023-04-17 21:17 ` Torstein Eide
@ 2023-04-18 23:55 ` David Sterba
2023-04-19 8:04 ` Qu Wenruo
2023-04-19 8:55 ` Andrei Borzenkov
1 sibling, 2 replies; 9+ messages in thread
From: David Sterba @ 2023-04-18 23:55 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, Torstein Eide
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution
2023-04-18 23:55 ` David Sterba
@ 2023-04-19 8:04 ` Qu Wenruo
2023-04-19 8:55 ` Andrei Borzenkov
1 sibling, 0 replies; 9+ messages in thread
From: Qu Wenruo @ 2023-04-19 8:04 UTC (permalink / raw)
To: dsterba; +Cc: linux-btrfs, Torstein Eide
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution
2023-04-18 23:55 ` David Sterba
2023-04-19 8:04 ` Qu Wenruo
@ 2023-04-19 8:55 ` Andrei Borzenkov
2023-04-19 9:50 ` Torstein Eide
2023-04-19 9:50 ` Qu Wenruo
1 sibling, 2 replies; 9+ messages in thread
From: Andrei Borzenkov @ 2023-04-19 8:55 UTC (permalink / raw)
To: dsterba; +Cc: Qu Wenruo, linux-btrfs, Torstein Eide
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution
2023-04-19 8:55 ` Andrei Borzenkov
@ 2023-04-19 9:50 ` Torstein Eide
2023-04-19 9:50 ` Qu Wenruo
1 sibling, 0 replies; 9+ messages in thread
From: Torstein Eide @ 2023-04-19 9:50 UTC (permalink / raw)
To: Andrei Borzenkov; +Cc: dsterba, Qu Wenruo, linux-btrfs
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution
2023-04-19 8:55 ` Andrei Borzenkov
2023-04-19 9:50 ` Torstein Eide
@ 2023-04-19 9:50 ` Qu Wenruo
2023-04-19 10:37 ` Andrei Borzenkov
1 sibling, 1 reply; 9+ messages in thread
From: Qu Wenruo @ 2023-04-19 9:50 UTC (permalink / raw)
To: Andrei Borzenkov, dsterba; +Cc: Qu Wenruo, linux-btrfs, Torstein Eide
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution
2023-04-19 9:50 ` Qu Wenruo
@ 2023-04-19 10:37 ` Andrei Borzenkov
0 siblings, 0 replies; 9+ messages in thread
From: Andrei Borzenkov @ 2023-04-19 10:37 UTC (permalink / raw)
To: Qu Wenruo; +Cc: dsterba, Qu Wenruo, linux-btrfs, Torstein Eide
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".
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-04-19 10:38 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-17 9:48 [PATCH] btrfs-progs: logical-resolve: fix the subvolume path resolution Qu Wenruo
2023-04-17 21:17 ` Torstein Eide
2023-04-17 22:51 ` Qu Wenruo
2023-04-18 23:55 ` David Sterba
2023-04-19 8:04 ` Qu Wenruo
2023-04-19 8:55 ` Andrei Borzenkov
2023-04-19 9:50 ` Torstein Eide
2023-04-19 9:50 ` Qu Wenruo
2023-04-19 10:37 ` Andrei Borzenkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox