Message ID | 20230417094810.42214-1-wqu@suse.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | btrfs-progs: logical-resolve: fix the subvolume path resolution | expand |
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 >
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 >>
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.
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
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.
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.
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.
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".
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
[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(-)