CI: add draft xfstests workflow - #18845
Conversation
Add a workflow that runs xfstests, as discussed in openzfs#15565. This reuses a good bit of the existing qemu steps and scripts, with some copypaste. It should be enough to get some rough results, once that works well we can integrate it properly. Signed-off-by: Bartosz Stebel <bartoszstebel@gmail.com>
github-actions
Bot
added
the
Status: Work in Progress
behlendorf
left a comment
behlendorf
left a comment
There was a problem hiding this comment.
IMO it'd be easier to merge an imperfect version of this PR, then iterate on it when it's runnable.
Agreed, particularly since the workflow is manually triggered and passing the (cut-down) quick tests I'm fine with pulling it in. That should make it easier to iterate on.
Ideally these changes could be upstreamed, but I'm not very hopeful considering the kernel's unwelcoming attitude towards zfs in the past.
I suspect you're right, making another run at upstreaming (some or all) of the changes is something we can decide after it's all working well.
The pool config used for testing is somewhat arbitrary. It'd be good to expand this to a matrix where various exciting features (dedup, encryption, etc) are enabled.
Absolutely.
There's a lot of excluded tests. Some may be issues with the xfstests port, some are intended semantic differences, some are real ZFS bugs.
And some are for Linux interfaces which we haven't yet been implemented. It's nice to have these clearly identified and tests ready to go.
| # hook prompts "Password:" when root su's to fsgqa (to derive the key for the | ||
| # nonexistent rpool/home/fsgqa), and that prompt lands in test output and fails | ||
| # every _user_do-based test (generic/123, 128, 314, ...). We don't use encrypted | ||
| # homes here, so strip pam_zfs_key from the PAM config. |
There was a problem hiding this comment.
Alternately, OpenZFS could be built with ./configure --disable-pam which disables building the optional pam package. That said, it is nice to have this potential snag documented here for developers testing locally. Either way if fine with me, but I wanted to mention it.
| OS="$1" | ||
|
|
||
| # TODO: move the sources under openzfs | ||
| XFSTESTS_REPO="${XFSTESTS_REPO:-https://github.com/implr/xfstests}" |
There was a problem hiding this comment.
Eventually it should at least move under the openzfs org
I'm happy to create the repository under the openzfs organization so we have an official home for it. It looks like things are working well enough that it wouldn't be inconvenient. Just let me know when you're ready for this.
| # (acltype=posix + xattr=sa are required for several generic/ tests). | ||
| sudo zpool create -f \ | ||
| -O mountpoint=legacy -O acltype=posix -O xattr=sa \ | ||
| -O compression=off -O relatime=off \ |
There was a problem hiding this comment.
I presume compression=off and relatime=off are required for some tests as well?
There was a problem hiding this comment.
In general for now I was aiming for the config that passes the maximum amount of tests with minimal changes.
I had to do some archaeology to remember why I originally made these choices, but:
- compression is off because there's a bunch of tests which explicitly don't want it (
_require_no_compress), only implemented in btrfs until now. There might be a few more that aren't tagged as such by upstream that would still fail. The typical failure mode is trying to fill up the fs (either to ENOSPC,or to certain size), then count physical blocks or expect an error. Writing from urandom instead of a constant pattern would help, but that was a bigger change. - Good catch on relatime=off, it's not actually needed. There's a couple that rely on atime, but relatime is enough there.
|
I have found some tests make assumptions that zfs does not follow, cause it is COW based, and 64k works for most tests, where 128k fails many of them (cause the tests assumes 64k would be large enough to hit a full block), while some other tests it tests at 16k and it needs to be smaller than 64k to test correctly. I have started running a complete round of these tests to find things, cause they also fail on different vdev configurations. What I'm testing currently as I tested more configs but seems these hit the cases that matter: |
3 participants
Motivation and Context
More tests is more better :)
xfstests is linux's main FS test suite for pretty much every FS implemented in the kernel (don't be misled by the name).
ZFS used to be tested by xfstests in the past, thanks to a patch by @behlendorf. This hasn't worked for a long while due to changes in pretty much everything and general bitrot. I rebased that patch on top of current (as of a few months ago) xfstests and fixed several new categories of tests, though some still need to be excluded.
See #15565 for some more context.
Description
Add a CI workflow that runs xfstests. This reuses a good bit of the existing qemu steps and scripts, with some copypaste. It should be enough to get some rough results, once that works well we can integrate it properly.
Most useful GH action triggers require the workflow to exist on master. IMO it'd be easier to merge an imperfect version of this PR, then iterate on it when it's runnable.
Open points:
How Has This Been Tested?
Manually triggered workflow run in my fork:
Types of Changes
Checklist
Signed-off-by.