Page MenuHomeFreeBSD

add NFSv4 support for va_birthtime
ClosedPublic

Authored by rmacklem on May 7 2021, 1:22 AM.

Details

Summary

There is a NFSv4 file attribute called TimeCreate
that can be used for va_birthtime.
This patch enables support for setting this attribute
in the server.

It also allows the client to
acquire and set the attribute for a NFSv4
server that supports the attribute.

Test Plan

Tested via the stat(1) command and futimens(2)
system call for an NFSv4 mount between a
patched FreeBSD client and server.
A mount to an unpatched FreeBSD server
did not set the attribute, as expected and
the Linux client that does not use TimeCreate
still functioned as expected (no change in
behaviour).

Diff Detail

Repository
rG FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

rmacklem created this revision.

Thanks, this looks good to me.

sys/fs/nfsclient/nfs_clport.c
418

Does this actually need to be conditional? Note that vop_stdstat() also initializes va_birthtime to the same dummy value of {.tv_sec = -1, .tv_nsec = 0}.

This revision is now accepted and ready to land.May 7 2021, 2:15 PM
sys/fs/nfsclient/nfs_clport.c
418

Yea, I'm "on the fence" about this. As you note, it isn't currently needed.
I did it this way in case the dummy value in vop_stdstat() changes
someday. (ie The dummy in the NFS client does not need to be
changed as well, if the dummy value in vop_stdstat() changes.)

sys/fs/nfsclient/nfs_clport.c
418

We should probably make the default value a named constant to avoid this problem. I believe anyone changing the default would have to audit all filesystems anyway (msdosfs poses the same problem for instance), so I'm not sure it's worth coding defensively here, but it's a minor detail.

rmacklem added inline comments.
sys/fs/nfsclient/nfs_clport.c
418

In a sense it is. VNOVAL, although it is just -1 and is
applied to the tv_sec field.

I agree that doing the assignment unconditionally
makes sense, so I'll commit it that way.

This revision was automatically updated to reflect the committed changes.
rmacklem marked an inline comment as done.