Add test cases

The new syscalls mostly work nicely. I've just found a few bugs. In addition to my inline comments, pdwait never returns a pid. It always returns 0. Is that deliberate? Also, it's unclear to me when a child ought to be reaped. I would think that it should be reaped as soon as pdwait or waitpid returns it (though the pid shouldn't be recycled until the process descriptor is closed). However, I'm seeing that the child doesn't get reaped until close(). That is, pdwait() will return the process multiple times, as if WNOWAIT were set. Is that deliberate?

Rather than add a new syscall for pdwait, what would you say to adding another idtype for waitid and wait6? We already have those syscalls, and their existing interface nicely allows for different kinds of ids. We would just need to add a P_PIDFD idtype value. That's also what Linux does.

• Style, suggested by des@
• Fix the encoded width of tcp-state, suggested by @des

LGTM. But you should wrap the long line in your commit message . And add something to the commit message indicating that the problem is with the "show" subcommand.

I can't reproduce this behavior with "sesutil map --libxo=json,pretty". What is the right way to trigger the bug?

Again, 300s -> 400s is not much of an increase. Might want to go more.

This looks fine. But as the default timeout is already 300s, I suggest increasing it even more. If it were me, I would double it.

Is it kosher to have two gitrefs in one entry?

In D53357#1241293, @guest-svmhdvn wrote:

Sorry to bring this back, but I'm noticing intermittency again on latest 16-CURRENT HEAD. I can reproduce it locally on amd64, and I'm seeing it fail on aarch64 in jenkins with the same result: https://ci.freebsd.org/view/Test/job/FreeBSD-main-aarch64-test/1857/testReport/sys.fs.fusefs/bad_server/main/

I needed a large count (at least 500+ iterations on my local system) to reproduce it:

$ bricoler run freebsd-src-regression-suite --param freebsd-src:url=/usr/src --param freebsd-src:branch= --param freebsd-src-regression-suite:hypervisor=bhyve --param freebsd-src-regression-suite:memory=4096 --param freebsd-src-regression-suite:ncpus=2 --param freebsd-src-regression-suite:parallelism=1 --param freebsd-src-regression-suite:count=600 --param freebsd-src-regression-suite:tests='sys/fs/fusefs/bad_server:main'

Standard output:
[==========] Running 3 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 3 tests from BadServer
[ RUN      ] BadServer.UnknownUnique
[       OK ] BadServer.UnknownUnique (1 ms)
[ RUN      ] BadServer.ShortWrite
/home/siva/src/fbsdsrcgit/tests/sys/fs/fusefs/mockfs.cc:987: Failure
Value of: res >= static_cast<ssize_t>(sizeof(in.header)) || m_quit
  Actual: false
Expected: true

[  FAILED  ] BadServer.ShortWrite (0 ms)
[ RUN      ] BadServer.ErrorWithPayload
[       OK ] BadServer.ErrorWithPayload (0 ms)
[----------] 3 tests from BadServer (2 ms total)

[----------] Global test environment tear-down
[==========] 3 tests from 1 test suite ran. (2 ms total)
[  PASSED  ] 2 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] BadServer.ShortWrite

 1 FAILED TEST
===> Failed tests
sys/fs/fusefs/bad_server:main  ->  failed: Returned non-success exit status 1  [0.006s]

@asomers could you try to reproduce?

LGTM, but you should also create a real Phabricator account now.

@jrtc27's suggestion works. Though there are other problems too, that I haven't solved yet.

• Respond to Jessica's feedback.

LGTM.

Nice! It almost LGTM , but I agree with @ziaee that you should remove the man page link to libxo(3).

In D53950#1235311, @guest-jsollvander wrote:

In D53950#1235136, @asomers wrote:

Nice job! I suggest a few changes, though.

• I suggest using the same field names that gpart list does, where possible.
  • logical_starting_block => start
  • size_in_blocks => sectors
  • provider_name => name
  • attribute => attrib (or better yet, change "gpart list" to show "attribute" instead of "attrib")
• When you do "gpart show -l --libxo=json", it will wrongly print something like "type": "swap0". That must be fixed. It should either show "label": "swap0", or just always have separate fields for "label" and "type".
• There's an asymmetry in the way that "gpart show --libxo=json" displays gaps in the middle of the partition table vs at the end. It only prints "free" if they're in the middle, and they're only in the "unallocated" section if they're at the end. I like the first way better. I think you should get rid of the "unallocated" section.
• Also, the json formatting for "attribute" isn't very machine-parseable. Right now it shows " [bootonce,bootme] ". I'm not sure what format would be best.
• Don't forget to bump the .Dd dates in the man pages

Thanks Alan!

Could you explain why "type" is wrong here? According to the manpage for gpart show this field describes the "... the partition type, ...", so I thought "type" would be suitable field name for "element"? I'll still change it, I'm just curious.

Easy enough to fix.

Let me see what I can do about the attribute field.

Nice job! I suggest a few changes, though.

My 13th gen Framework 13 laptop works ok with or without this patch.

Closing in favor of https://reviews.freebsd.org/D52780

You must also update the test cases in tests/sys/fs/fusefs/fallocate.cc . In particular, I think that the PosixFallocate.eopnotsupp will fail now, unless you update it.

LGTM. Thanks for the contribution, Juraj.

Thanks for doing this. I think it will be a good addition. But I'm curious: why did you choose -F? Obviously -u and -U were already taken.

In D53080#1217336, @guest-svmhdvn wrote:

@asomers As of f4f638eb23d770e19ede167908d8145b8851f835, this test is still failing intermittently in CI as seen here on aarch64: https://ci.freebsd.org/view/Test/job/FreeBSD-main-aarch64-test/1801/testReport/junit/sys.fs.fusefs/bad_server/main.

This seems to also be reproducible locally on amd64 with more than 20+ minutes of testing (very intermittent, but becomes clear when running on slower platforms). I'm not sure this fix is comprehensive, although it did certainly help reduce the intermittency.

Thanks for getting this fixed, @arrowd .

@arrowd now that you've committed the main bmap patch, are you ok with this test?

That explains it. I was never testing with SCTP, and I doubt that Damin was, either.

Can you give an example of the incorrect output? Does it matter whether "-q" is in use?

LGTM

Looks like a good start.

You need to wrap that long line to 80 cols, but otherwise it LGTM.

I have two comments in addition to the inline ones:

In D53079#1212648, @glebius wrote:

I trust Alan's expertise here. IMHO, given that this patch is analog to fusefs one, should be fine. But not expert in the area.

I too am confused by the commit message. Won't the column always be shown in json and xml output, if "-s" is used?