kern: skip unix_connectat fdescfs cases when fdescfs is missing
AcceptedPublic
Actions

Authored by nickbanks_netflix.com on Thu, Aug 27, 6:43 PM.

Details

Reviewers

olivier
asomers
ngie

Group Reviewers

tests

Summary

unix_connectat's fdescfs cases call mount_fdescfs(), which skipped on
ENODEV. nmount(2) never returns ENODEV: vfs_donmount() remaps the
ENODEV from a failed fdescfs module load to EINVAL with errmsg
"Invalid fstype", so the skip never fired and the cases failed on
kernels without fdescfs.

Use ATF_REQUIRE_KERNEL_MODULE("fdescfs") instead, and drop the dead
ENODEV branch. modfind(2) sees loaded and static modules, not
unloaded .ko files, so a host with fdescfs.ko on disk but not loaded
will now skip rather than having nmount(2) implicitly kldload it.
Upstream CI keeps fdescfs mounted via tests/ci/tools/ci.conf, so it
still runs them.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 76224
Build 73107: arc lint + arc unit

Event Timeline

nickbanks_netflix.com created this revision.Thu, Aug 27, 6:43 PM

nickbanks_netflix.com held this revision as a draft.

Herald added a subscriber: imp. · View Herald TranscriptThu, Aug 27, 6:43 PM

Harbormaster completed remote builds in B76224: Diff 185232.Thu, Aug 27, 6:43 PM

nickbanks_netflix.com published this revision for review.Thu, Aug 27, 6:45 PM

olivier added a reviewer: tests.Tue, Sep 1, 7:38 PM

asomers accepted this revision.Tue, Sep 1, 7:44 PM

This revision is now accepted and ready to land.Tue, Sep 1, 7:44 PM

Please use require.kmods instead of the macro as I recommend in the differential.

tests/sys/kern/unix_connectat.c
187	I really need to get rid of that macro since the ATF metadata added by @des makes a ton more sense.

This revision now requires changes to proceed.Wed, Sep 2, 1:56 AM

ngie added inline comments.Wed, Sep 2, 1:57 AM

tests/sys/kern/unix_connectat.c
187	I really need to get rid of that macro since the ATF metadata added by @des makes a ton more sense. It looks like it nerfed the trailing newline. Please add it to the suggested change.

Use require.kmods ATF metadata instead of ATF_REQUIRE_KERNEL_MODULE(), per review. Macro include and its -I${SRCTOP}/tests CFLAGS dropped; macro line continuations and trailing newline verified intact.

Harbormaster completed remote builds in B76492: Diff 185816.Fri, Sep 4, 1:36 PM

nickbanks_netflix.com marked 2 inline comments as done.Fri, Sep 4, 1:48 PM

Please doublecheck that the new support also meets your needs before committing.

This revision is now accepted and ready to land.Fri, Sep 4, 6:08 PM

In D59227#1362798, @ngie wrote:

Please doublecheck that the new support also meets your needs before committing.

I confirmed that all our internal tests here at Netflix were passing still with the updated commit.

In D59227#1362854, @nickbanks_netflix.com wrote:

In D59227#1362798, @ngie wrote:

Please doublecheck that the new support also meets your needs before committing.

I confirmed that all our internal tests here at Netflix were passing still with the updated commit.