| Message ID | 6e8980b7306716ed8a71dc50868169ae96424e79.1687854248.git.anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: dump-super: fix dump-super on aarch64 | expand |
On 2023/6/27 16:53, Anand Jain wrote: > On aarch64 systems with glibc 2.28, several btrfs-progs test cases are > failing because the command 'btrfs inspect dump-super -a <dev>' reports > an error when it attempts to read beyond the disk/file-image size. > > $ btrfs inspect dump-super -a /dev/vdb12 > <snap> > ERROR: Failed to read the superblock on /dev/vdb12 at 274877906944 > ERROR: Error = 'No such file or directory', errno = 2 > > And btrfs/184 also fails, as it uses -s 2 option to dump the last super > block. > > $ ./check btrfs/184 > FSTYP -- btrfs > PLATFORM -- Linux/aarch64 a4k 6.4.0-rc7+ #7 SMP PREEMPT Sat Jun 24 02:47:24 EDT 2023 > MKFS_OPTIONS -- /dev/vdb2 > MOUNT_OPTIONS -- /dev/vdb2 /mnt/scratch > > btrfs/184 1s ... [failed, exit status 1]- output mismatch (see /Volumes/ws/xfstests-dev/results//btrfs/184.out.bad) > --- tests/btrfs/184.out 2020-03-03 00:26:40.172081468 -0500 > +++ /Volumes/ws/xfstests-dev/results//btrfs/184.out.bad 2023-06-24 05:54:40.868210737 -0400 > @@ -1,2 +1,3 @@ > QA output created by 184 > -Silence is golden > +Deleted dev superblocks not scratched > +(see /Volumes/ws/xfstests-dev/results//btrfs/184.full for details) > ... > (Run 'diff -u /Volumes/ws/xfstests-dev/tests/btrfs/184.out /Volumes/ws/xfstests-dev/results//btrfs/184.out.bad' to see the entire diff) > Ran: btrfs/184 > Failures: btrfs/184 > Failed 1 of 1 tests > > This is because `pread()` behaves differently on aarch64 and sets > `errno = 2` instead of the usual `errno = 0`. > > To fix check if the sb offset is beyond the device size or regular file > size and skip the corresponding sbread(). > > Also, move putchar('\n') after a successful call to load_and_dump_sb() to > the load_and_dump_sb() itself. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > cmds/inspect-dump-super.c | 24 +++++++++++++++++++++--- > 1 file changed, 21 insertions(+), 3 deletions(-) > > diff --git a/cmds/inspect-dump-super.c b/cmds/inspect-dump-super.c > index f32c67fd5c4d..a1c3dcd9d90b 100644 > --- a/cmds/inspect-dump-super.c > +++ b/cmds/inspect-dump-super.c > @@ -20,6 +20,7 @@ > #include <fcntl.h> > #include <errno.h> > #include <getopt.h> > +#include <sys/stat.h> > #include "kernel-shared/ctree.h" > #include "kernel-shared/disk-io.h" > #include "kernel-shared/print-tree.h" > @@ -33,8 +34,27 @@ static int load_and_dump_sb(char *filename, int fd, u64 sb_bytenr, int full, > int force) > { > struct btrfs_super_block sb; > + struct stat st; > u64 ret; > > + if (fstat(fd, &st)) { > + error("error = '%m', errno = %d", errno); > + return 1; > + } > + > + if (S_ISBLK(st.st_mode) || S_ISREG(st.st_mode)) { > + off_t last_byte; > + > + last_byte = lseek(fd, 0, SEEK_END); > + if (last_byte == -1) { > + error("error = '%m', errno = %d", errno); > + return 1; > + } > + > + if (sb_bytenr > last_byte) > + return 0; > + } > + > ret = sbread(fd, &sb, sb_bytenr); > if (ret != BTRFS_SUPER_INFO_SIZE) { > /* check if the disk if too short for further superblock */ > @@ -54,6 +74,7 @@ static int load_and_dump_sb(char *filename, int fd, u64 sb_bytenr, int full, > return 1; > } > btrfs_print_superblock(&sb, full); > + putchar('\n'); > return 0; > } > > @@ -168,15 +189,12 @@ static int cmd_inspect_dump_super(const struct cmd_struct *cmd, > close(fd); > return 1; > } > - > - putchar('\n'); > } > } else { > if (load_and_dump_sb(filename, fd, sb_bytenr, full, force)) { > close(fd); > return 1; > } > - putchar('\n'); > } > close(fd); > }
On Tue, Jun 27, 2023 at 04:53:15PM +0800, Anand Jain wrote: > @@ -33,8 +34,27 @@ static int load_and_dump_sb(char *filename, int fd, u64 sb_bytenr, int full, > int force) > { > struct btrfs_super_block sb; > + struct stat st; > u64 ret; > > + if (fstat(fd, &st)) { > + error("error = '%m', errno = %d", errno); > + return 1; > + } > + > + if (S_ISBLK(st.st_mode) || S_ISREG(st.st_mode)) { > + off_t last_byte; > + > + last_byte = lseek(fd, 0, SEEK_END); > + if (last_byte == -1) { > + error("error = '%m', errno = %d", errno); Such error messages are not useful, there should be a description of the problem with %m for the error text. I've updated it. > + return 1; > + } > + > + if (sb_bytenr > last_byte) > + return 0; > + } > + > ret = sbread(fd, &sb, sb_bytenr); > if (ret != BTRFS_SUPER_INFO_SIZE) { > /* check if the disk if too short for further superblock */ > @@ -54,6 +74,7 @@ static int load_and_dump_sb(char *filename, int fd, u64 sb_bytenr, int full, > return 1; > } > btrfs_print_superblock(&sb, full); > + putchar('\n'); > return 0; > }
>> + error("error = '%m', errno = %d", errno); > > Such error messages are not useful, there should be a description of the > problem with %m for the error text. I've updated it. > error("cannot read end of file %s: %m", filename); Yep. Now much better. Thanks, Anand
diff --git a/cmds/inspect-dump-super.c b/cmds/inspect-dump-super.c index f32c67fd5c4d..a1c3dcd9d90b 100644 --- a/cmds/inspect-dump-super.c +++ b/cmds/inspect-dump-super.c @@ -20,6 +20,7 @@ #include <fcntl.h> #include <errno.h> #include <getopt.h> +#include <sys/stat.h> #include "kernel-shared/ctree.h" #include "kernel-shared/disk-io.h" #include "kernel-shared/print-tree.h" @@ -33,8 +34,27 @@ static int load_and_dump_sb(char *filename, int fd, u64 sb_bytenr, int full, int force) { struct btrfs_super_block sb; + struct stat st; u64 ret; + if (fstat(fd, &st)) { + error("error = '%m', errno = %d", errno); + return 1; + } + + if (S_ISBLK(st.st_mode) || S_ISREG(st.st_mode)) { + off_t last_byte; + + last_byte = lseek(fd, 0, SEEK_END); + if (last_byte == -1) { + error("error = '%m', errno = %d", errno); + return 1; + } + + if (sb_bytenr > last_byte) + return 0; + } + ret = sbread(fd, &sb, sb_bytenr); if (ret != BTRFS_SUPER_INFO_SIZE) { /* check if the disk if too short for further superblock */ @@ -54,6 +74,7 @@ static int load_and_dump_sb(char *filename, int fd, u64 sb_bytenr, int full, return 1; } btrfs_print_superblock(&sb, full); + putchar('\n'); return 0; } @@ -168,15 +189,12 @@ static int cmd_inspect_dump_super(const struct cmd_struct *cmd, close(fd); return 1; } - - putchar('\n'); } } else { if (load_and_dump_sb(filename, fd, sb_bytenr, full, force)) { close(fd); return 1; } - putchar('\n'); } close(fd); }
On aarch64 systems with glibc 2.28, several btrfs-progs test cases are failing because the command 'btrfs inspect dump-super -a <dev>' reports an error when it attempts to read beyond the disk/file-image size. $ btrfs inspect dump-super -a /dev/vdb12 <snap> ERROR: Failed to read the superblock on /dev/vdb12 at 274877906944 ERROR: Error = 'No such file or directory', errno = 2 And btrfs/184 also fails, as it uses -s 2 option to dump the last super block. $ ./check btrfs/184 FSTYP -- btrfs PLATFORM -- Linux/aarch64 a4k 6.4.0-rc7+ #7 SMP PREEMPT Sat Jun 24 02:47:24 EDT 2023 MKFS_OPTIONS -- /dev/vdb2 MOUNT_OPTIONS -- /dev/vdb2 /mnt/scratch btrfs/184 1s ... [failed, exit status 1]- output mismatch (see /Volumes/ws/xfstests-dev/results//btrfs/184.out.bad) --- tests/btrfs/184.out 2020-03-03 00:26:40.172081468 -0500 +++ /Volumes/ws/xfstests-dev/results//btrfs/184.out.bad 2023-06-24 05:54:40.868210737 -0400 @@ -1,2 +1,3 @@ QA output created by 184 -Silence is golden +Deleted dev superblocks not scratched +(see /Volumes/ws/xfstests-dev/results//btrfs/184.full for details) ... (Run 'diff -u /Volumes/ws/xfstests-dev/tests/btrfs/184.out /Volumes/ws/xfstests-dev/results//btrfs/184.out.bad' to see the entire diff) Ran: btrfs/184 Failures: btrfs/184 Failed 1 of 1 tests This is because `pread()` behaves differently on aarch64 and sets `errno = 2` instead of the usual `errno = 0`. To fix check if the sb offset is beyond the device size or regular file size and skip the corresponding sbread(). Also, move putchar('\n') after a successful call to load_and_dump_sb() to the load_and_dump_sb() itself. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- cmds/inspect-dump-super.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-)