I have sent you my edit of the manual page in a separate email rather than trying to put all my comments into the above draft.

Tried it out and had no problems and got expected errors.

Unless anyone has further feedback, I will plan to commit this early next week.

This is an impressive performance improvement and one well worth adding to the system.

This approach seems like the right way to get the extended errors back from the underlying devices.

Structurally, this looks good to me. I will await Phil's commentary on its JSON/XML usefulness.

Respond to latest comments.

Get rid of unneeded else statement.

Add comment on why it is not necessary to check for NULL return from prependdevtopath().

Responding to Dag-Erling's comments.

Respond to Dag-Erling's comments.

This looks good to me, but I will await a review from Johan before proceeding.

I will put in this change to fix the tests.
Pleas start a new review to address the additional suggestions.

Indeed this functionality is now embodied in ffs_search.

Fix is correct. Clearly a cut & paste blunder when I wrote it.

In D37615#1212734, @guest-jsollvander wrote:

In D37615#1212613, @guest-svmhdvn wrote:

This patch seems to break several testcases in the sys/geom/class/multipath/misc test suite with this output:

===> sys/geom/class/multipath/misc:add
Result:     failed: ACTIVE !=  (ACTIVE != )

I'll take a look.

I note that the sshd daemon is restarted when it is reinstalled. The sshd daemon is restarted since its update may be security related and thus the security fix should take effect as soon as the updated version is available.

First, sorry for letting this languish. Though I have not dealt with libxo, I assume that you have ensured that this generates the output that you need. Structurally it is properly added to the geom utility. If there are no issues raised by the other reviewers by Monday October 13th, I will commit it.

Mentor approval.

Mentor approval.

Looks ready to go.

Interesting history of this bug.

This has been committed to head and MFC'ed to 14.

Mentor approval.

Mentor approval.

Mentor approval.

Mentor approval for commit.

Mentor approval.

Mentor approval - looks good to go.

Mentor approval for commit.

Mentor approval for commit.

I am always in favor of improvement to code clarity.

Mentor approval for commit.

I concur with @imp comment that I like the notion of abstracting them so we don't churn that too much...

This is (still) a sensible thing to do.

With Mark's review and approval this looks good to go.

This change is correct.

Review process looks good and complete.
Adding my accept as your mentor.

Review process looks good and complete.
Adding my accept as your mentor.

Review process looks good and complete.
Adding my accept as your mentor.

Review process is complete with all the right folks signing off.

I suggest that you add 'Sponsored by: The FreeBSD Foundation' to your commit as it helps show that they are supporting your work.

I too suggest that you add 'Sponsored by: The FreeBSD Foundation' to your commit as it helps show that they are supporting your work.

Looks good.

This seems to be a reasonable change.

Agree with change.

One grammer nit.

Sorry for being out of the loop here, I just got back from a trip to the arctic where unsurprisingly there was little to no Internet.

In D48374#1145383, @emaste wrote:

There's another set of questions:

• Is SU+J detrimental under certain conditions?
• Should we be asking the user this question in the installer?

It's the second question that prompted this review. I don't care too much about whether SU+J is always on by default, always off by default, or on by default iff the fs is on spinning rust (or some equivalent). I do want to remove cognitive load from installer questions that aren't really required.

This has been fixed in newfs for some time. Along with numerous other changes like checkhash validations, superblock integrity checks, and other fixes and improvements. In my opinion makefs/ffs should just use use newfs/mkfs.c rather than (poorly) trying to parody it.

Clarify when the superblock is a copy and fix yet another Peter Holm test failure.

Update with kib comments.
Add fixes for bugs found by Peter Holm.

This has been committed to head and MFC'ed to 14 and 13. It was committed to 13 in time to be part of the 13.5 distribution.

I have accepted gleb's changes, so this issue is resolved.

I have gone with Gleb's fix. It is simple and as Warner has pointed out `It's the least bad outcome. We are doing ffs things, even if they are minimal.' As I have pointed out, your change breaks libufs and would require many additional changes after making the needed fixes in libufs to the clients of libufs.

Actually, there is an even easier fix. Just move the declaration of vfs_ffs from ffs_alloc.c to ffs_subr.c. Then everything just works.

This will cause libufs to fail to build. Also, ffs_subr.c is supposed to contain all the kernel functionality needed by filesystem utilities. And ffs_oldfscompat_inode_read() is one of those functions.

Updates to respond to reviewer feedback,

I will follow up these comments with a new set of diffs reflecting suggested changes.

Committed as 661ca921e8cd56b17fc6615bc7e596e56e0e7c31