On Wed, 29 Apr 2020 19:25:57 +0200 Hans Ulrich Niedermann <h...@n-dimensional.de> wrote:
> On Wed, 29 Apr 2020 17:06:36 +0300 > Anatoly Pugachev <mator...@gmail.com> wrote: > > > Don't run f2fs test on systems with PAGE_SIZE > 4096 bytes, since > > f2fs is not supported on these systems and trying to mount a f2fs > > filesystem would fail. > > "Skip the f2fs test on ..." might be better wording, both in this > paragraph and the Subject. > > Exit code 77 is certainly documented with the word "skip", and "exit > 77" will show up in the "make check" output as "SKIP" as well. > > > v2 changes: > > > > - fix compare > > - quotes around variable expansion > > > > Signed-off-by: Anatoly Pugachev <mator...@gmail.com> > > Reviewed-by: Mike Gilbert <flop...@gentoo.org> > > > > --- > > tests/f2fs_test.in | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/tests/f2fs_test.in b/tests/f2fs_test.in > > index 1ea77c826..3da1dad57 100644 > > --- a/tests/f2fs_test.in > > +++ b/tests/f2fs_test.in > > @@ -15,5 +15,11 @@ if ! which mkfs.f2fs >/dev/null 2>&1; then > > exit 77 > > fi > > > > +PAGE_SIZE=$(getconf PAGE_SIZE) > > +F2FS_BLKSIZE=4096 > > +if [ "$PAGE_SIZE" -gt "$F2FS_BLKSIZE" ]; then > > + printf "f2fs is not supported on PAGE_SIZE(%d) != %d\n" $PAGE_SIZE > > $F2FS_BLKSIZE > > + exit 77 > > +fi > > This confusing to me. You are skipping the test when > > PAGE_SIZE > F2FS_BLKSIZE > > but the corresponding message says > > PAGE_SIZE != F2FS_BLKSIZE > > Now... which condition is it supposed to be? ">" or "!="? > > I know from the Linux kernel's ext2 driver that it is very well > possible that PAGE_SIZE != EXT2_BLOCK_SIZE can work > unless EXT2_BLOCK_SIZE > PAGE_SIZE. > > So ">" and "!=" are not necessarily the same thing, and IMHO the check > and the message use the same condition. The commit title and the commit message are also talking about "PAGE_SIZE > 4096" instead of "PAGE_SIZE != 4096". This adds more confusion over whether F2FS_BLKSIZE just happens to be 4096 somewhere where PAGE_SIZE is 4096, whether F2FS_BLKSIZE is defined to be 4096 everywhere, and whether the whole thing is about the number 4096 or about the value of F2FS_BLKSIZE. (Again, in the ext2 example, filesystems can have many block sizes, but the default is the Linux PAGE_SIZE, so all x86 PCs will create filesystems with block size 4096 but a MIPS system like the Western Digital SOHO external HDD NAS will create a filesystem with block size 65536. The kernel driver just needs PAGE_SIZE >= block size to work, so a MIPS created ext2 filesystem will not mount on a x86 PC.) Mentioning the same condition in commit title, commit message, the actual "if" branch, and then the "printf" message would certainly reduce my confusion. Uli _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
