Page MenuHomeFreeBSD

Add VFCF_FILEREVINC to indicate a file system increments va_filerev by one
AcceptedPublic

Authored by rmacklem on Sun, Jan 12, 9:55 PM.

Details

Summary

Richard Kojedzinszky <richard@kojedz.in> reported a problem via
email, where the Linux NFSv4.2 client did not detect a change in a
directory on a FreeBSD NFSv4.2 server.

Adding support for the NFSv4.2 change_attr_type attribute seems
to have fixed the problem. This requires that the server file system
indicate if it increments va_filerev by one, since that file attribute
is used for the NFSv4.2 change attribute.

This patch adds VFCF_FILEREVINC to indicate this.

Test Plan

Tested by Richard for an uncoditional setting of increment
by one. Tested to see that VFCF_FILEREV indicates that UFS
increments by one.

A separate patch for ZFS will br presented to OpenZFS as a
Pull Request.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

And where is the use of the new flag?
Also, what writable and exportable filesystems do not increment va_filerev on changes? I looked and only seems to find smbfs and tmpfs. It might be simpler to fix tmpfs than adding a flag.

In D48436#1105027, @kib wrote:

And where is the use of the new flag?
Also, what writable and exportable filesystems do not increment va_filerev on changes? I looked and only seems to find smbfs and tmpfs. It might be simpler to fix tmpfs than adding a flag.

Do a

  1. cd /usr/srrc/sys/fs
  2. find . -name "*.c" -exec fgrep -l va_filerev {} \;

and you'll see most of them just set it to 0 or an
initial value based on time.
Only UFS and ZFS seem to increment it by one for each change.
(Admittedly, most of these other file systems don't get exported
or get exported read-only, so va_filerev should never change.)
However, msdosfs never changes the value of va_filerev as
far as I can see and then there is fuse. I can't tell what fuse
does w.r.t. va_filerev?

I left the NFS server patch out of this, since it is just pure NFS code,
but if you want to see it, I can put it up as a separate patch.
(The NFS server patch returns NFS4_CHANGE_TYPE_IS_VERSION_COUNTER_NOPNFS
if it is set and NFS4_CHANGE_TYPE_IS_UNDEFINED if it is not set.
There is also NFS4_CHANGE_TYPE_IS_TIME_METADATA and
NFS4_CHANGE_TYPE_ISI_MONOTIC_INCREASE, but no FreeBSD
file system appears to do thses? fuse might require these flags as well.

When the default of NFS4_CHANGE_TYPE_IS_UNDEFINED is set (as it is
now) for the Linux NFSv4 client, it appears to use the modify time (and
not metadata/ctime?) which works against the Linux server, but not the
FreeBSD one. My guess is that the Linux nfsd uses a very high resolution
time (or makes sure it changes for every change.
Returning NFS4_CHANGE_TYPE_IS_VERSION_COUNTER_NOPNFS fixes
the problem. The patch I gave Richard for testing just does that
unconditionally, but I felt that wasn't appropriate and that the file systems
should say if that is the correct reply?

I see that msdosfs did not update de_modrev, If I fix msdosfs and tmpfs to update vs_filerev, do we still need the flag? Or is it better to keep tmpfs/msdosfs as is and go ahead with the flag?

fusefs currently doesn't set va_filerev at all. I can fix that. But with fuse, ctime updates can come from either the kernel or the server. So the easiest solution would probably be to make va_filerev be a function of ctime. Would that work with the Linux client, assuming we set NFS4_CHANGE_TYPE_IS_TIME_METADATA ?

This revision is now accepted and ready to land.Mon, Jan 13, 12:18 AM

fusefs currently doesn't set va_filerev at all. I can fix that. But with fuse, ctime updates can come from either the kernel or the server. So the easiest solution would probably be to make va_filerev be a function of ctime. Would that work with the Linux client, assuming we set NFS4_CHANGE_TYPE_IS_TIME_METADATA ?

That should probably be an additional VFCF_xxx flag so the NFS server can
reply NFS4_CHANGE_TYPE_IS_TIME_METADATA.

In D48436#1105038, @kib wrote:

I see that msdosfs did not update de_modrev, If I fix msdosfs and tmpfs to update vs_filerev, do we still need the flag? Or is it better to keep tmpfs/msdosfs as is and go ahead with the flag?

I think it is better to go ahead with the flag.
Technically, unless va_filerevv is stored on-disk it cannot be
correctly supported. It is supposed to be preserved across
server reboots.
And since msdosfs (fat) doesn't store ctime either, I think
NFS4_CHANGE_TYPE_UNDEFINED is probably the
best answer?

Are you ok with an additional flag for ctime?