Add basic tests to cover some regressions that we had. It's hard to test floppy because some tests require presence or absense of a diskette in a drive. To simulate test conditions and automate the testing I added "run_*.sh" wrapper scripts that run tests in QEMU.
The first patch just improves check for reverted commits in a commit message. The second patch is required to generate a minimal initrd used in next commits. Rest of commits are basic floppy tests.
Please, comment the approach, selftests integration and suggest tests that you would like to add.
I thought about adding a possibility to remove/insert diskettes inside a test. This is possible if we give the guest an access to the QEMU monitor (eject/change cmds). But I didn't find a better way to do it than to map a monitor to an external port: -monitor tcp:<ip>:<port>,server,nowait and access this ip from the guest. Maybe it's also possible to do with virtserialport.
Denis Efremov (5): checkpatch: improve handling of revert commits gen_initramfs.sh: use absolute path for gen_init_cpio selftests: floppy: add basic tests for opening an empty device selftests: floppy: add basic tests for a readonly disk selftests: floppy: add basic rdwr tests
MAINTAINERS | 1 + scripts/checkpatch.pl | 12 +-- tools/testing/selftests/floppy/.gitignore | 8 ++ tools/testing/selftests/floppy/Makefile | 10 ++ tools/testing/selftests/floppy/config | 1 + tools/testing/selftests/floppy/empty.c | 58 ++++++++++++ tools/testing/selftests/floppy/init.c | 43 +++++++++ tools/testing/selftests/floppy/lib.sh | 67 +++++++++++++ tools/testing/selftests/floppy/rdonly.c | 99 ++++++++++++++++++++ tools/testing/selftests/floppy/rdwr.c | 67 +++++++++++++ tools/testing/selftests/floppy/run_empty.sh | 16 ++++ tools/testing/selftests/floppy/run_rdonly.sh | 22 +++++ tools/testing/selftests/floppy/run_rdwr.sh | 22 +++++ usr/gen_initramfs.sh | 2 +- 14 files changed, 421 insertions(+), 7 deletions(-) create mode 100644 tools/testing/selftests/floppy/.gitignore create mode 100644 tools/testing/selftests/floppy/Makefile create mode 100644 tools/testing/selftests/floppy/config create mode 100644 tools/testing/selftests/floppy/empty.c create mode 100644 tools/testing/selftests/floppy/init.c create mode 100644 tools/testing/selftests/floppy/lib.sh create mode 100644 tools/testing/selftests/floppy/rdonly.c create mode 100644 tools/testing/selftests/floppy/rdwr.c create mode 100755 tools/testing/selftests/floppy/run_empty.sh create mode 100755 tools/testing/selftests/floppy/run_rdonly.sh create mode 100755 tools/testing/selftests/floppy/run_rdwr.sh
Properly handle commits like: commit f2791e7eadf4 ("Revert "floppy: refactor open() flags handling"")
Cc: Joe Perches joe@perches.com Signed-off-by: Denis Efremov efremov@linux.com --- scripts/checkpatch.pl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 461d4221e4a4..cf31e8c994d3 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3200,20 +3200,20 @@ sub process { $long = 1 if ($line =~ /\bcommit\s+[0-9a-f]{41,}/i); $space = 0 if ($line =~ /\bcommit [0-9a-f]/i); $case = 0 if ($line =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/); - if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+("([^"]+)")/i) { + if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+("(.+)")/i) { $orig_desc = $1; $hasparens = 1; } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i && defined $rawlines[$linenr] && - $rawlines[$linenr] =~ /^\s*("([^"]+)")/) { + $rawlines[$linenr] =~ /^\s*("(.+)")/) { $orig_desc = $1; $hasparens = 1; - } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+("[^"]+$/i && + } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+(".+$/i && defined $rawlines[$linenr] && - $rawlines[$linenr] =~ /^\s*[^"]+")/) { - $line =~ /\bcommit\s+[0-9a-f]{5,}\s+("([^"]+)$/i; + $rawlines[$linenr] =~ /^\s*.+")/) { + $line =~ /\bcommit\s+[0-9a-f]{5,}\s+("(.+)$/i; $orig_desc = $1; - $rawlines[$linenr] =~ /^\s*([^"]+)")/; + $rawlines[$linenr] =~ /^\s*(.+)")/; $orig_desc .= " " . $1; $hasparens = 1; }
On Wed, 2021-08-18 at 18:46 +0300, Denis Efremov wrote:
Properly handle commits like: commit f2791e7eadf4 ("Revert "floppy: refactor open() flags handling"")
Try this one:
https://lore.kernel.org/lkml/7f55d9d0369f1ea840fab83eeb77f9f3601fee41.camel@...
On 8/18/21 7:00 PM, Joe Perches wrote:
It works but why not to use .+? then? I'm not sure that non-greedy patterns will properly handle commits like: $ git log --oneline | fgrep '")'
e.g. commit ece2619fe8ed ("extcon: arizona: Fix flags parameter to the gpiod_get("wlf,micd-pol") call")
On Wed, 2021-08-18 at 19:21 +0300, Denis Efremov wrote:
The only way to handle that is to use the $balanced_parens test but it wouldn't work on Andrew's perl version 5.8
Andrew? Do you still use perl 5.8? It's almost 20 years old now. Does anyone still use perl versions 5.8 or lower?
From checkpatch:
# Using $balanced_parens, $LvalOrFunc, or $FuncArg # requires at least perl version v5.10.0 # Any use must be runtime checked with $^V
our $balanced_parens = qr/(((?:[^()]++|(?-1))*))/; our $LvalOrFunc = qr{((?:[&*]\s*)?$Lval)\s*($balanced_parens{0,1})\s*}; our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant|$String)};
So maybe:
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
So that could be: if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+($balanced_parens)/i
and
} elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i && defined $rawlines[$linenr] && "$line $rawlines[$linenr]" =~ /\bcommit\s+[0-9a-f]{5,}\s+($balanced_parens)/i
etc...
On Wed, 18 Aug 2021 13:21:16 -0700 Joe Perches joe@perches.com wrote:
Andrew? Do you still use perl 5.8? It's almost 20 years old now.
5.26 is the oldest I appear to be using.
Hey Denis:
Try this one please and let me know what you think...
--- scripts/checkpatch.pl | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 161ce7fe5d1e5..4e2e79eff9b8c 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3196,26 +3196,21 @@ sub process { $orig_commit = lc($1); }
- $short = 0 if ($line =~ /\bcommit\s+[0-9a-f]{12,40}/i); - $long = 1 if ($line =~ /\bcommit\s+[0-9a-f]{41,}/i); - $space = 0 if ($line =~ /\bcommit [0-9a-f]/i); - $case = 0 if ($line =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/); - if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+("([^"]+)")/i) { - $orig_desc = $1; - $hasparens = 1; - } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i && - defined $rawlines[$linenr] && - $rawlines[$linenr] =~ /^\s*("([^"]+)")/) { - $orig_desc = $1; - $hasparens = 1; - } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+("[^"]+$/i && - defined $rawlines[$linenr] && - $rawlines[$linenr] =~ /^\s*[^"]+")/) { - $line =~ /\bcommit\s+[0-9a-f]{5,}\s+("([^"]+)$/i; + my $input = $line; + for (my $n = 0; $n < 2; $n++) { + $input .= " $rawlines[$linenr + $n]" if ($#lines >= $linenr + $n); + } + + $short = 0 if ($input =~ /\bcommit\s+[0-9a-f]{12,40}/i); + $long = 1 if ($input =~ /\bcommit\s+[0-9a-f]{41,}/i); + $space = 0 if ($input =~ /\bcommit [0-9a-f]/i); + $case = 0 if ($input =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/); + if ($input =~ /\bcommit\s+[0-9a-f]{5,}\s+($balanced_parens)/i) { $orig_desc = $1; - $rawlines[$linenr] =~ /^\s*([^"]+)")/; - $orig_desc .= " " . $1; $hasparens = 1; + # Always strip leading/trailing parens then double quotes if existing + $orig_desc = substr($orig_desc, 1, -1); + $orig_desc = substr($orig_desc, 1, -1) if ($orig_desc =~ /^".*"$/); }
($id, $description) = git_commit_info($orig_commit,
Hi,
On 8/19/21 12:22 AM, Joe Perches wrote:
Hey Denis:
Try this one please and let me know what you think...
Looks good to me. Couple of nitpicks below
Why do you want to add "if ($orig_desc =~ /^".*"$/);" here? and not just substr($orig_desc, 2, -2);?
In your previous patch with '.*?' you added a branch to allow also newlines between commit and shas: ``` commit c3f157259438 (Revert "floppy: reintroduce O_NDELAY fix") ```
Maybe something like this will work (adding a last word from a prevline if line doesn't start from commit) + my $input = $line; if ($line =~ /\b(c)ommit\s+([0-9a-f]{5,})\b/i) { $init_char = $1; $orig_commit = lc($2); } elsif ($line =~ /\b([0-9a-f]{12,40})\b/i) { $orig_commit = lc($1); + $prevline =~ /(\w+)$/; + $line = $1 . " " . $prevline; }
- my $input = $line; for (my $n = 0; $n < 2; $n++) { $input .= " $rawlines[$linenr + $n]" if ($#lines >= $linenr + $n); }
Thanks, Denis
On Thu, 2021-08-19 at 22:52 +0300, Denis Efremov wrote:
yeah, thanks.
How about this one: --- scripts/checkpatch.pl | 72 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 29 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 161ce7fe5d1e5..4988515a0dfb3 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1181,7 +1181,8 @@ sub git_commit_info { # git log --format='%H %s' -1 $line | # echo "commit $(cut -c 1-12,41-)" # done - } elsif ($lines[0] =~ /^fatal: ambiguous argument '$commit': unknown revision or path not in the working tree./) { + } elsif ($lines[0] =~ /^fatal: ambiguous argument '$commit': unknown revision or path not in the working tree./ || + $lines[0] =~ /^fatal: bad object $commit/) { $id = undef; } else { $id = substr($lines[0], 0, 12); @@ -2587,6 +2588,8 @@ sub process { my $reported_maintainer_file = 0; my $non_utf8_charset = 0;
+ my $last_git_commit_id_linenr = -1; + my $last_blank_line = 0; my $last_coalesced_string_linenr = -1;
@@ -3173,7 +3176,8 @@ sub process { if ($in_commit_log && !$commit_log_possible_stack_dump && $line !~ /^\s*(?:Link|Patchwork|http|https|BugLink|base-commit):/i && $line !~ /^This reverts commit [0-9a-f]{7,40}/ && - ($line =~ /\bcommit\s+[0-9a-f]{5,}\b/i || + (($line =~ /\bcommit\s+[0-9a-f]{5,}\b/i || + ($line =~ /\bcommit\s*$/i && defined($rawlines[$linenr]) && $rawlines[$linenr] =~ /^\s*[0-9a-f]{5,}\b/i)) || ($line =~ /(?:\s|^)[0-9a-f]{12,40}(?:[\s"'([]|$)/i && $line !~ /[<[][0-9a-f]{12,40}[>]]/i && $line !~ /\bfixes:\s*[0-9a-f]{12,40}/i))) { @@ -3183,49 +3187,59 @@ sub process { my $long = 0; my $case = 1; my $space = 1; - my $hasdesc = 0; my $hasparens = 0; my $id = '0123456789ab'; my $orig_desc = "commit description"; my $description = ""; + my $herectx = $herecurr; + my $has_parens = 0; + + my $input = $line; + if ($line =~ /(?:\bcommit\s+[0-9a-f]{5,}|\bcommit\s*$)/i) { + for (my $n = 0; $n < 2; $n++) { + if ($input =~ /\bcommit\s+[0-9a-f]{5,}\s*$balanced_parens/i) { + $has_parens = 1; + last; + } + last if ($#lines < $linenr + $n); + $input .= " " . trim($rawlines[$linenr + $n]); + $herectx .= "$rawlines[$linenr + $n]\n"; + } + $herectx = $herecurr if (!$has_parens); + }
- if ($line =~ /\b(c)ommit\s+([0-9a-f]{5,})\b/i) { + if ($input =~ /\b(c)ommit\s+([0-9a-f]{5,})\b/i) { $init_char = $1; $orig_commit = lc($2); - } elsif ($line =~ /\b([0-9a-f]{12,40})\b/i) { + $short = 0 if ($input =~ /\bcommit\s+[0-9a-f]{12,40}/i); + $long = 1 if ($input =~ /\bcommit\s+[0-9a-f]{41,}/i); + $space = 0 if ($input =~ /\bcommit [0-9a-f]/i); + $case = 0 if ($input =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/); + + if ($input =~ /\bcommit\s+[0-9a-f]{5,}\s+($balanced_parens)/i) { + $orig_desc = $1; + # Always strip leading/trailing parens then double quotes if existing + $orig_desc = substr($orig_desc, 1, -1); + if ($orig_desc =~ /^".*"$/) { + $orig_desc = substr($orig_desc, 1, -1); + $hasparens = 1; + } + } + } elsif ($input =~ /\b([0-9a-f]{12,40})\b/i) { $orig_commit = lc($1); }
- $short = 0 if ($line =~ /\bcommit\s+[0-9a-f]{12,40}/i); - $long = 1 if ($line =~ /\bcommit\s+[0-9a-f]{41,}/i); - $space = 0 if ($line =~ /\bcommit [0-9a-f]/i); - $case = 0 if ($line =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/); - if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+("([^"]+)")/i) { - $orig_desc = $1; - $hasparens = 1; - } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i && - defined $rawlines[$linenr] && - $rawlines[$linenr] =~ /^\s*("([^"]+)")/) { - $orig_desc = $1; - $hasparens = 1; - } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+("[^"]+$/i && - defined $rawlines[$linenr] && - $rawlines[$linenr] =~ /^\s*[^"]+")/) { - $line =~ /\bcommit\s+[0-9a-f]{5,}\s+("([^"]+)$/i; - $orig_desc = $1; - $rawlines[$linenr] =~ /^\s*([^"]+)")/; - $orig_desc .= " " . $1; - $hasparens = 1; - } - ($id, $description) = git_commit_info($orig_commit, $id, $orig_desc);
if (defined($id) && - ($short || $long || $space || $case || ($orig_desc ne $description) || !$hasparens)) { + ($short || $long || $space || $case || ($orig_desc ne $description) || !$hasparens) && + $last_git_commit_id_linenr != $linenr - 1) { ERROR("GIT_COMMIT_ID", - "Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: '${init_char}ommit $id ("$description")'\n" . $herecurr); + "Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: '${init_char}ommit $id ("$description")'\n" . $herectx); } + #don't report the next line if this line ends in commit and the sha1 hash is the next line + $last_git_commit_id_linenr = $linenr if ($line =~ /\bcommit\s*$/i); }
# Check for added, moved or deleted files
On Thu, 2021-08-19 at 22:52 +0300, Denis Efremov wrote:
Hi,
Why do you want to add "if ($orig_desc =~ /^".*"$/);" here? and not just substr($orig_desc, 2, -2);?
Because commit descriptions sometimes to not have quotes like
commit <deadbeef> (Multiple word description)
btw:
I tested the last proposal with this script:
$ git log --grep="commit [0-9a-f]" -i --format=%h -1000 | \ while read commit ; do \ echo $commit; \ ./scripts/checkpatch.pl --git --no-summary --quiet --types=GIT_COMMIT_ID $commit ; \ done
and there are still a fair number of ERRORs.
And I'm not sure if this particular ERROR is that useful overall.
On 8/20/21 1:17 AM, Joe Perches wrote:
And I'm not sure if this particular ERROR is that useful overall.
I find it useful to check commit-id and that it matches a title. It's easy to make a typo in commit-id and get an invalid one.
Denis
On Sat, 2021-08-21 at 09:47 +0300, Denis Efremov wrote:
That's true, but I meant requiring the sha1 hash to contain both the word "commit" and use ("title").
Looking at checkpatch's errors produced by this GIT_COMMIT_ID test makes the required form seem a bit too inflexible to me.
For instance: a sha1 hash may be repeated in a commit message where the first instance has the correct form but the second use is just the hash and the warning is still produced.
On 8/21/21 10:12 AM, Joe Perches wrote:
I agree with you. There is also another example with list of commits: - commit <id-1> ("Title1") - commit <id-2> ("Title2") ...
I see no reason in writing "commit" on each line.
Use absolute path to call gen_init_cpio. This allows one to use gen_initramfs.sh from any directory.
Cc: Masahiro Yamada masahiroy@kernel.org Signed-off-by: Denis Efremov efremov@linux.com --- usr/gen_initramfs.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/usr/gen_initramfs.sh b/usr/gen_initramfs.sh index 63476bb70b41..2e4a86181c79 100755 --- a/usr/gen_initramfs.sh +++ b/usr/gen_initramfs.sh @@ -244,4 +244,4 @@ if test -n "$KBUILD_BUILD_TIMESTAMP"; then timestamp="-t $timestamp" fi fi -usr/gen_init_cpio $timestamp $cpio_list > $output +"$(dirname "$0")"/gen_init_cpio $timestamp $cpio_list > $output
On Thu, Aug 19, 2021 at 12:47 AM Denis Efremov efremov@linux.com wrote:
Use absolute path to call gen_init_cpio. This allows one to use gen_initramfs.sh from any directory.
I do not mind this, but $(dirname "$0") is not necessarily an absolute path, is it?
I added test code:
echo dirname is $(dirname $0)
in this script, and I saw
dirname is usr
-- Best Regards Masahiro Yamada
On 8/19/21 3:24 AM, Masahiro Yamada wrote:
Oh, sorry, commit message is wrong. Would that be ok for you if I will change it in v2 to something like:
Prepend gen_init_cpio call with the same path as gen_initramfs.sh called. This allows one to use gen_initramfs.sh from any directory, not only from the kernel's topdir.
On Fri, Aug 20, 2021 at 5:51 AM Denis Efremov efremov@linux.com wrote:
I am fine with it.
This patch is prefixed with 2/5, so I assume you expect another person to pick up the entire series.
With the commit message updated,
Reviewed-by: Masahiro Yamada masahiroy@kernel.org
Add basic tests for O_NDELAY and O_ACCMODE open of an empty floppy device. "empty" test works under assumption that there is no disk in "/dev/fd0". It's possible to run it on a real hardware. "run_empty.sh" automates the testing process by creating a minimal initrd image with "init" and "test" binaries and running the kernel in QEMU.
The tests cover these regressions: - commit f2791e7eadf4 ("Revert "floppy: refactor open() flags handling"") - commit ff06db1efb2a ("floppy: fix open(O_ACCMODE) for ioctl-only open")
Signed-off-by: Denis Efremov efremov@linux.com --- MAINTAINERS | 1 + tools/testing/selftests/floppy/.gitignore | 5 ++ tools/testing/selftests/floppy/Makefile | 10 ++++ tools/testing/selftests/floppy/config | 1 + tools/testing/selftests/floppy/empty.c | 58 +++++++++++++++++++++ tools/testing/selftests/floppy/init.c | 43 +++++++++++++++ tools/testing/selftests/floppy/lib.sh | 57 ++++++++++++++++++++ tools/testing/selftests/floppy/run_empty.sh | 16 ++++++ 8 files changed, 191 insertions(+) create mode 100644 tools/testing/selftests/floppy/.gitignore create mode 100644 tools/testing/selftests/floppy/Makefile create mode 100644 tools/testing/selftests/floppy/config create mode 100644 tools/testing/selftests/floppy/empty.c create mode 100644 tools/testing/selftests/floppy/init.c create mode 100644 tools/testing/selftests/floppy/lib.sh create mode 100755 tools/testing/selftests/floppy/run_empty.sh
diff --git a/MAINTAINERS b/MAINTAINERS index fd25e4ecf0b9..f2a08b793b54 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7227,6 +7227,7 @@ M: Denis Efremov efremov@linux.com L: linux-block@vger.kernel.org S: Odd Fixes F: drivers/block/floppy.c +F: tools/testing/selftests/floppy/
FLYSKY FSIA6B RC RECEIVER M: Markus Koch markus@notsyncing.net diff --git a/tools/testing/selftests/floppy/.gitignore b/tools/testing/selftests/floppy/.gitignore new file mode 100644 index 000000000000..a9fdf1dbaa97 --- /dev/null +++ b/tools/testing/selftests/floppy/.gitignore @@ -0,0 +1,5 @@ +init +cpio_list +initrd + +empty diff --git a/tools/testing/selftests/floppy/Makefile b/tools/testing/selftests/floppy/Makefile new file mode 100644 index 000000000000..83e18cd63210 --- /dev/null +++ b/tools/testing/selftests/floppy/Makefile @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: GPL-2.0 +CFLAGS := -static -I../../../../usr/include + +TEST_PROGS := run_empty.sh +TEST_GEN_FILES := init empty +TEST_FILES := lib.sh + +include ../lib.mk + +EXTRA_CLEAN := initrd cpio_list diff --git a/tools/testing/selftests/floppy/config b/tools/testing/selftests/floppy/config new file mode 100644 index 000000000000..d0c2cd8edfb2 --- /dev/null +++ b/tools/testing/selftests/floppy/config @@ -0,0 +1 @@ +CONFIG_BLK_DEV_FD=y diff --git a/tools/testing/selftests/floppy/empty.c b/tools/testing/selftests/floppy/empty.c new file mode 100644 index 000000000000..d7e2e6a74132 --- /dev/null +++ b/tools/testing/selftests/floppy/empty.c @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <fcntl.h> +#include <unistd.h> +#include "../kselftest_harness.h" + +FIXTURE(floppy) { + const char *dev; + int fd; +}; + +FIXTURE_VARIANT(floppy) { + int flags; +}; + +/* + * See ff06db1efb2a ("floppy: fix open(O_ACCMODE) for ioctl-only open") + * fdutils use O_ACCMODE for probing and ioctl-only open + */ +FIXTURE_VARIANT_ADD(floppy, ACCMODE) { + .flags = O_ACCMODE, +}; + +FIXTURE_VARIANT_ADD(floppy, NACCMODE) { + .flags = O_ACCMODE|O_NDELAY, +}; + +FIXTURE_VARIANT_ADD(floppy, NRD) { + .flags = O_RDONLY|O_NDELAY, +}; + +FIXTURE_VARIANT_ADD(floppy, NWR) { + .flags = O_WRONLY|O_NDELAY, +}; + +FIXTURE_VARIANT_ADD(floppy, NRDWR) { + .flags = O_RDWR|O_NDELAY, +}; + +FIXTURE_SETUP(floppy) +{ + self->dev = "/dev/fd0"; + if (access(self->dev, F_OK)) + ksft_exit_skip("No floppy device found\n"); +}; + +FIXTURE_TEARDOWN(floppy) +{ + ASSERT_EQ(close(self->fd), 0); +} + +TEST_F(floppy, open) +{ + self->fd = open(self->dev, variant->flags); + ASSERT_GT(self->fd, 0); +} + +TEST_HARNESS_MAIN diff --git a/tools/testing/selftests/floppy/init.c b/tools/testing/selftests/floppy/init.c new file mode 100644 index 000000000000..d4df4fcfd21f --- /dev/null +++ b/tools/testing/selftests/floppy/init.c @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <unistd.h> +#include <errno.h> +#include <string.h> +#include <stdio.h> +#include <stdlib.h> +#include <sys/wait.h> +#include <sys/reboot.h> + +__attribute__((noreturn)) static void poweroff(void) +{ + fflush(stdout); + fflush(stderr); + reboot(RB_POWER_OFF); + sleep(10); + fprintf(stderr, "\nFailed to power off\n"); + exit(1); +} + +static void panic(const char *what) +{ + fprintf(stderr, "\nPANIC %s: %s\n", what, strerror(errno)); + poweroff(); +} + +int main(int argc, char *argv[]) +{ + pid_t pid; + + pid = fork(); + if (pid == -1) + panic("fork"); + else if (pid == 0) { + execl("/test", "test", NULL); + panic("exec"); + } + if (waitpid(pid, NULL, 0) < 0) + panic("waitpid"); + + poweroff(); + return 1; +} diff --git a/tools/testing/selftests/floppy/lib.sh b/tools/testing/selftests/floppy/lib.sh new file mode 100644 index 000000000000..22b711ec32ee --- /dev/null +++ b/tools/testing/selftests/floppy/lib.sh @@ -0,0 +1,57 @@ +# SPDX-License-Identifier: GPL-2.0 + +TOP_DIR="$(realpath "$(dirname "$0")"/../../../../)" + +QEMU=qemu-system-x86_64 +KERNEL="$TOP_DIR"/arch/x86/boot/bzImage +INITRD=initrd + +gen_cpio_list() { + echo "file /init init 755 0 0" > cpio_list + echo "file /test $1 755 0 0" >> cpio_list + echo "dir /dev 755 0 0" >> cpio_list + echo "nod /dev/console 644 0 0 c 5 1" >> cpio_list + echo "nod /dev/fd0 666 0 0 b 2 0" >> cpio_list + echo "dir /mnt 755 0 0" >> cpio_list +} + +gen_initrd() { + "$TOP_DIR"/usr/gen_initramfs.sh -o initrd ./cpio_list +} + +get_selftest_log() { + perl -ne '$begin = $_ =~ /^TAP version/ unless $begin; + if ($begin && !$end) { + $_ =~ s/^\s+|\s+$|[.+//g; + print($_ . "\n") if $_; + } + $end = $_ =~ /^# Totals:/ unless $end;' +} + +run_qemu() { + $QEMU -enable-kvm -nodefaults -nographic \ + -kernel "$KERNEL" -initrd "$INITRD" \ + -append "console=ttyS0 earlyprintk=serial" \ + -serial stdio -monitor none -no-reboot "$@" +} + +run_qemu_debug() { + run_qemu "$@" +} + +run_qemu_nodebug() { + run_qemu "$@" | get_selftest_log +} + +detect_debug() { + if [ "x$1" = "x" ]; then + run=run_qemu_nodebug + else + run=run_qemu_debug + fi +} + +run_qemu_empty() { + detect_debug "$1" + $run -drive index=0,if=floppy +} diff --git a/tools/testing/selftests/floppy/run_empty.sh b/tools/testing/selftests/floppy/run_empty.sh new file mode 100755 index 000000000000..f018107dc6a5 --- /dev/null +++ b/tools/testing/selftests/floppy/run_empty.sh @@ -0,0 +1,16 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 + +set -e + +source "$(dirname $0)"/lib.sh + +while getopts d flag; do + case "${flag}" in + d) debug=1;; + esac +done + +gen_cpio_list empty +gen_initrd empty +run_qemu_empty $debug
Add basic tests for a floppy with a readonly disk. "rdonly" test works under following assumptions: - there is a readonly disk in "/dev/fd0" - the disk is VFAT formatted - there is "test" file with "TEST" in it on the disk
"run_rdonly.sh" script simulates the conditions and automates the testing process.
The tests cover the regression: - commit f2791e7eadf4 ("Revert "floppy: refactor open() flags handling"")
Signed-off-by: Denis Efremov efremov@linux.com --- tools/testing/selftests/floppy/.gitignore | 2 + tools/testing/selftests/floppy/Makefile | 6 +- tools/testing/selftests/floppy/lib.sh | 6 ++ tools/testing/selftests/floppy/rdonly.c | 99 ++++++++++++++++++++ tools/testing/selftests/floppy/run_rdonly.sh | 22 +++++ 5 files changed, 132 insertions(+), 3 deletions(-) create mode 100644 tools/testing/selftests/floppy/rdonly.c create mode 100755 tools/testing/selftests/floppy/run_rdonly.sh
diff --git a/tools/testing/selftests/floppy/.gitignore b/tools/testing/selftests/floppy/.gitignore index a9fdf1dbaa97..7642dc0ef281 100644 --- a/tools/testing/selftests/floppy/.gitignore +++ b/tools/testing/selftests/floppy/.gitignore @@ -1,5 +1,7 @@ init cpio_list initrd +testdir/
empty +rdonly diff --git a/tools/testing/selftests/floppy/Makefile b/tools/testing/selftests/floppy/Makefile index 83e18cd63210..ed8fdeb79aea 100644 --- a/tools/testing/selftests/floppy/Makefile +++ b/tools/testing/selftests/floppy/Makefile @@ -1,10 +1,10 @@ # SPDX-License-Identifier: GPL-2.0 CFLAGS := -static -I../../../../usr/include
-TEST_PROGS := run_empty.sh -TEST_GEN_FILES := init empty +TEST_PROGS := run_empty.sh run_rdonly.sh +TEST_GEN_FILES := init empty rdonly TEST_FILES := lib.sh
include ../lib.mk
-EXTRA_CLEAN := initrd cpio_list +EXTRA_CLEAN := initrd cpio_list testdir diff --git a/tools/testing/selftests/floppy/lib.sh b/tools/testing/selftests/floppy/lib.sh index 22b711ec32ee..9988be187bc9 100644 --- a/tools/testing/selftests/floppy/lib.sh +++ b/tools/testing/selftests/floppy/lib.sh @@ -55,3 +55,9 @@ run_qemu_empty() { detect_debug "$1" $run -drive index=0,if=floppy } + +run_qemu_rdonly_fat() { + detect_debug "$2" + $run -drive file=fat:floppy:"$1",index=0,if=floppy,readonly +} + diff --git a/tools/testing/selftests/floppy/rdonly.c b/tools/testing/selftests/floppy/rdonly.c new file mode 100644 index 000000000000..fa60d069540b --- /dev/null +++ b/tools/testing/selftests/floppy/rdonly.c @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <errno.h> +#include <fcntl.h> +#include <unistd.h> +#include <sys/mount.h> +#include <sys/ioctl.h> +#include <linux/fd.h> +#include "../kselftest_harness.h" + +FIXTURE(floppy) { + const char *dev; +}; + +FIXTURE_SETUP(floppy) +{ + int fd; + struct floppy_drive_params params; + + self->dev = "/dev/fd0"; + if (access(self->dev, F_OK)) + ksft_exit_skip("No floppy device found\n"); + if (access(self->dev, R_OK)) + ksft_exit_skip("Floppy is not read accessible\n"); + + fd = open(self->dev, O_ACCMODE|O_NDELAY); + EXPECT_EQ(0, ioctl(fd, FDGETDRVPRM, ¶ms)); + params.flags |= FTD_MSG|FD_DEBUG; + EXPECT_EQ(0, ioctl(fd, FDSETDRVPRM, ¶ms)); + close(fd); +} + +FIXTURE_TEARDOWN(floppy) +{ +} + +TEST_F(floppy, read) +{ + int fd, test; + + fd = open(self->dev, O_RDONLY); + ASSERT_GT(fd, 0); + ASSERT_EQ(read(fd, &test, sizeof(test)), sizeof(test)); + ASSERT_EQ(close(fd), 0); +} + +TEST_F(floppy, open_write_fail) +{ + ASSERT_LT(open(self->dev, O_WRONLY), 0); +} + +TEST_F(floppy, open_rdwr_fail) +{ + ASSERT_LT(open(self->dev, O_RDWR), 0); +} + +TEST_F(floppy, ioctl_disk_writable) +{ + int fd; + struct floppy_drive_struct drive; + + fd = open(self->dev, O_RDONLY|O_NDELAY); + ASSERT_GT(fd, 0); + ASSERT_EQ(0, ioctl(fd, FDGETDRVSTAT, &drive)); + ASSERT_FALSE(drive.flags & FD_DISK_WRITABLE); + ASSERT_EQ(close(fd), 0); +} + +TEST_F(floppy, mount) +{ + int fd; + char test[5] = {}; + + mount(self->dev, "/mnt", "vfat", MS_RDONLY, NULL); + ASSERT_EQ(0, errno); + + fd = open("/mnt/test", O_RDONLY); + read(fd, &test, sizeof(test)); + ASSERT_EQ(0, strncmp(test, "TEST", 4)); +} + +TEST_F(floppy, open_ndelay_write_fail) +{ +#define TEST_DATA "TEST_FAIL_WRITE" + int fd; + char test[] = TEST_DATA; + + fd = open(self->dev, O_RDWR|O_NDELAY); + ASSERT_GT(fd, 0); + + write(fd, test, sizeof(test)); + read(fd, test, sizeof(test)); + ASSERT_NE(0, strncmp(TEST_DATA, test, sizeof(TEST_DATA))); + + ASSERT_EQ(close(fd), 0); +#undef TEST_DATA +} + +TEST_HARNESS_MAIN diff --git a/tools/testing/selftests/floppy/run_rdonly.sh b/tools/testing/selftests/floppy/run_rdonly.sh new file mode 100755 index 000000000000..a358a1b6e58d --- /dev/null +++ b/tools/testing/selftests/floppy/run_rdonly.sh @@ -0,0 +1,22 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 + +set -e + +source "$(dirname $0)"/lib.sh + +while getopts d flag; do + case "${flag}" in + d) debug=1;; + esac +done + +if [ -z $debug ]; then + trap "rm -rf testdir" EXIT +fi +mkdir -p testdir +echo -n TEST > testdir/test + +gen_cpio_list rdonly +gen_initrd rdonly +run_qemu_rdonly_fat testdir $debug
Add basic tests for a floppy with writable disk. "rdwr" test works under following assumptions: - writable disk in "/dev/fd0"
To simulate the conditions and automate the testing process there is "run_rdwr.sh".
Signed-off-by: Denis Efremov efremov@linux.com --- tools/testing/selftests/floppy/.gitignore | 1 + tools/testing/selftests/floppy/Makefile | 4 +- tools/testing/selftests/floppy/lib.sh | 4 ++ tools/testing/selftests/floppy/rdwr.c | 67 ++++++++++++++++++++++ tools/testing/selftests/floppy/run_rdwr.sh | 22 +++++++ 5 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 tools/testing/selftests/floppy/rdwr.c create mode 100755 tools/testing/selftests/floppy/run_rdwr.sh
diff --git a/tools/testing/selftests/floppy/.gitignore b/tools/testing/selftests/floppy/.gitignore index 7642dc0ef281..f53e70197edd 100644 --- a/tools/testing/selftests/floppy/.gitignore +++ b/tools/testing/selftests/floppy/.gitignore @@ -5,3 +5,4 @@ testdir/
empty rdonly +rdwr diff --git a/tools/testing/selftests/floppy/Makefile b/tools/testing/selftests/floppy/Makefile index ed8fdeb79aea..c5d010dd4445 100644 --- a/tools/testing/selftests/floppy/Makefile +++ b/tools/testing/selftests/floppy/Makefile @@ -1,8 +1,8 @@ # SPDX-License-Identifier: GPL-2.0 CFLAGS := -static -I../../../../usr/include
-TEST_PROGS := run_empty.sh run_rdonly.sh -TEST_GEN_FILES := init empty rdonly +TEST_PROGS := run_empty.sh run_rdonly.sh run_rdwr.sh +TEST_GEN_FILES := init empty rdonly rdwr TEST_FILES := lib.sh
include ../lib.mk diff --git a/tools/testing/selftests/floppy/lib.sh b/tools/testing/selftests/floppy/lib.sh index 9988be187bc9..0eab702b355a 100644 --- a/tools/testing/selftests/floppy/lib.sh +++ b/tools/testing/selftests/floppy/lib.sh @@ -61,3 +61,7 @@ run_qemu_rdonly_fat() { $run -drive file=fat:floppy:"$1",index=0,if=floppy,readonly }
+run_qemu_rdwr_img() { + detect_debug "$2" + $run -drive file="$1",index=0,if=floppy,format=raw +} diff --git a/tools/testing/selftests/floppy/rdwr.c b/tools/testing/selftests/floppy/rdwr.c new file mode 100644 index 000000000000..44ad18701530 --- /dev/null +++ b/tools/testing/selftests/floppy/rdwr.c @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <fcntl.h> +#include <unistd.h> +#include <sys/mount.h> +#include <errno.h> +#include <linux/fd.h> +#include "../kselftest_harness.h" + +FIXTURE(floppy) { + const char *dev; +}; + +FIXTURE_SETUP(floppy) +{ + int fd; + struct floppy_drive_params params; + + self->dev = "/dev/fd0"; + if (access(self->dev, F_OK)) + ksft_exit_skip("No floppy device found\n"); + if (access(self->dev, R_OK)) + ksft_exit_skip("Floppy is not read accessible\n"); + if (access(self->dev, W_OK)) + ksft_exit_skip("Floppy is not write accessible\n"); + + fd = open("/dev/fd0", O_ACCMODE|O_NDELAY); + EXPECT_EQ(0, ioctl(fd, FDGETDRVPRM, ¶ms)); + params.flags |= FD_DEBUG; + EXPECT_EQ(0, ioctl(fd, FDSETDRVPRM, ¶ms)); + close(fd); +} + +FIXTURE_TEARDOWN(floppy) +{ +} + +TEST_F(floppy, write) +{ +#define TEST_DATA "TEST_WRITE" + int fd; + char test[] = TEST_DATA; + + fd = open(self->dev, O_RDWR); + ASSERT_GT(fd, 0); + + ASSERT_EQ(sizeof(test), write(fd, test, sizeof(test))); + ASSERT_EQ(sizeof(test), read(fd, test, sizeof(test))); + ASSERT_NE(0, strncmp(TEST_DATA, test, sizeof(TEST_DATA))); + + ASSERT_EQ(close(fd), 0); +#undef TEST_DATA +} + +TEST_F(floppy, ioctl_disk_writable) +{ + int fd; + struct floppy_drive_struct drive; + + fd = open(self->dev, O_RDONLY|O_NDELAY); + ASSERT_GT(fd, 0); + ASSERT_EQ(0, ioctl(fd, FDGETDRVSTAT, &drive)); + ASSERT_TRUE(drive.flags & FD_DISK_WRITABLE); + ASSERT_EQ(close(fd), 0); +} + +TEST_HARNESS_MAIN diff --git a/tools/testing/selftests/floppy/run_rdwr.sh b/tools/testing/selftests/floppy/run_rdwr.sh new file mode 100755 index 000000000000..0ebe8bd6bc69 --- /dev/null +++ b/tools/testing/selftests/floppy/run_rdwr.sh @@ -0,0 +1,22 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 + +set -e + +source "$(dirname $0)"/lib.sh + +while getopts d flag; do + case "${flag}" in + d) debug=1;; + esac +done + +if [ -z $debug ]; then + trap "rm -rf testdir" EXIT +fi +mkdir -p testdir +head -c 1474560 /dev/zero > testdir/floppy.raw + +gen_cpio_list rdwr +gen_initrd rdwr +run_qemu_rdwr_img testdir/floppy.raw $debug
linux-kselftest-mirror@lists.linaro.org
-
Andrew Morton -
Denis Efremov -
Joe Perches -
Masahiro Yamada