Should this now be closed since it was resolved in D19811?

This seems like a very useful improvement.

I would preface the actual condition "The nbp parameter is non-NULL when the mapping is for a block that contains data, one of an external data block, a direct block, or the final block in a chain of indirect blocks." before the correct clarification "If mapping an extended attribute block, nbp must point to a buffer for that block."

Looks good, thanks for clarifying -f.

Your change looks good. I'll approve once you update the suggested mandoc refinement.

I recently added the new error EINTEGRITY which is intended for use when a cylinder group or other filesystem structure has an integrity error. My initial plan is to use it for check-hash failures, but this is another good place to use it. It means that the callers of the functions that return this error need to be prepared to handle it. Specifically when allocating blocks or files trying to allocate from a different cylinder group. When deleting blocks or files, not immediately giving up, but trying to release all the remaining blocks of the file. A similar analysis would need to be used for this error (notably trying to allocate from a different cylinder group).

The addition to fstab.5 is helpful, but slightly incomplete in that it is not clear in what context the update keyword would be used.I would add text at the end of your new block that says something like:
The
.Dq update
option is typically used in conjuction with two
.Nm
files.
The first
.Nm
file is used to set up the initial set of file systems.
The second
.Nm
file is then run to update the initial set of file systems and
to add additional file systems.

Note that update of manual page date fell out on corrective update.

Looks good.

As noted, date at top of manual pages needs to be updated (which I will admit, I failed to do when I added underscores).

Update the intro(2) manual page to better reflect the meaning as suggested by imp, cem, etal.

In D18765#401317, @imp wrote:

In D18765#401272, @mckusick wrote:

At a minimum EINTEGRITY is useful for mount since it allows the mount command to notify the user that the mount failed because of an integrity error which very likely can be fixed with a run of fsck versus an I/O error which probably cannot.

We should make mount return a different exit value for this perhaps... We could use that to automate a running of fsck in some cases.
EINTEGRITY means the end-to-end notion of what should be there isn't. This is similar to EIO (can't read the sector). The integrity error, though, means that some can't fail sanity check failed, which is a larger class of errors.

At a minimum EINTEGRITY is useful for mount since it allows the mount command to notify the user that the mount failed because of an integrity error which very likely can be fixed with a run of fsck versus an I/O error which probably cannot.

Based on feedback from Ed Maste and Ed Schouten do not add EINTEGRITY to CloudABI. Rather map it to EINVAL as is done in Linux compatibility.

It is useful to the application or user to distinguish EINTEGRITY from EIO. With EIO they know that the underlying media has failed and there is really nothing they can do. With EINTEGRITY the individual file has become corrupted, but if they still have its contents, they can remove it and rewrite it as the filesystem media itself is still working. Alternatively they can request the system administrator to run a check of the filesystem to attempt to clean up the file.

Mount is just one of the places that can return EINTEGRITY. It can also come back when accessing a file or directory whose meta information fails a check-hash.

Update diffs based on reviewer comments:
gnn - corrections to File05 cddl/lib/libdtrace/errno.d
dim - drop unneeded changes to sanitizer, previously File14 and File15
Drop dteske as reviewer as gnn has reviewed the DTrace change

When a mount fails because of a hash-check failure it is helpful to get "Integrity check failed" rather than "Invalid Argument" back. There are numerous reasons that you can get "Invalid argument" including miss-typed disk or mount-point names and other administrative errors. Knowing that it was due to an integrity check gives a strong indication that a run of fsck is needed. Actually in the case of mount we have the ability to print a more complete message, but the low-level functions in the filesystems need to be able to return something other than EINVAL so that the mount-layer function can know how to formulate its error message.

In D18765#400620, @dim wrote:

The libc++ changes look OK to me, and I'll take care of sending them upstream.

In D18765#400403, @cem wrote:

This proposal is to create a new EINTEGRITY error number. EINTEGRITY's purpose is to respond to errors where an integrity check such as a check-hash or a cross-correlation has failed. For example, an uncorrectable error has been found in a filesystem or a disk subsystem such as gmirror(8) or graid3(8).

How do you envision making use of this error code? Does it meaningfully differ from EIO? In the case of gmirror in particular, AFAIK it cannot determine bitrot and relies on the underlying volume to signal a bad block with EIO (which it then restarts on any remaining component, I think).
(I believe we could use a tighter integration between UFS and underlying geoms like gmirror/graid, I'm just not sure how this error number fits in to that.)
Thanks!
I'll take a look at the lua bits soon.

This change now looks good to me.

Given that this plausibly works only with two different /etc/fstab files, I think your change should also include an update to the manual page to explain how the update keyword is intended to be used.

This change does not seem wrong, though the only way I can see it being usable is to have the read-only mount's in /etc/fstab, then a special fstab that has the desired update mount lines in it. Because if the update mount's are in /etc/fstab, then for 'mount -a' the read-only mount will happen immediately followed by the read-write mount. Though I suppose that the update mount line could appear first (and fail) followed by the read-only mount which would presumably succeed. Then on the second 'mount -a' the update would succeed and the read-only would be skipped because it was already mounted. But this seems like a really obscure way of doing things. Plus you would have to change mount so that would not exit when the initial "ro,update" failed.

This now looks reasonable to me.

This now looks good to me. I concur with kib's followup comments.

Generally this looks reasonable.

OK, keeping it as currently specified is fine. I was concerned that using it as an lseek parameter might be an issue for ZFS,and OneFS, but given that it appears that these filesystems are fine with that tells me that the above implementation works fine.

My intent when adding the d_off field was that it be used for this purpose, so I am glad to see someone actually making it happen.

I prefer not to do things halfway. Either we use lseek/read or we use pread. Having it half & half is just crappy coding style. It is possible to write some tests that will exercise the error cases, so if you want to make this change, then I would like you to do it completely.

In answer to kib:

This change looks fine to me.

This seems like the correct approach to me.