[PATCH] Better handle NULL utimes() in the NFS client

[Rick Macklem]

John Baldwin wrote:
> The NFS client tries to infer when an application has passed NULL to
> utimes()
> so that it can let the server set the timestamp rather than using a
> client-
> supplied timestamp. It does this by checking to see if the desired
> timestamp's second matches the current second. However, this breaks
> applications that are intentionally trying to set a specific timestamp
> within
> the current second. In addition, utimes() sets a flag to indicate if
> NULL was
> passed to utimes(). The patch below changes the NFS client to check
> this flag
> and only use the server-supplied time in that case:
> 
> Index: fs/nfsclient/nfs_clport.c
> ===================================================================
> --- fs/nfsclient/nfs_clport.c (revision 225511)
> +++ fs/nfsclient/nfs_clport.c (working copy)
> @@ -762,7 +762,7 @@
> *tl = newnfs_false;
> }
> if (vap->va_atime.tv_sec != VNOVAL) {
> - if (vap->va_atime.tv_sec != curtime.tv_sec) {
> + if (!(vap->va_vaflags & VA_UTIMES_NULL)) {
> NFSM_BUILD(tl, u_int32_t *, 3 * NFSX_UNSIGNED);
> *tl++ = txdr_unsigned(NFSV3SATTRTIME_TOCLIENT);
> txdr_nfsv3time(&vap->va_atime, tl);
> @@ -775,7 +775,7 @@
> *tl = txdr_unsigned(NFSV3SATTRTIME_DONTCHANGE);
> }
> if (vap->va_mtime.tv_sec != VNOVAL) {
> - if (vap->va_mtime.tv_sec != curtime.tv_sec) {
> + if (!(vap->va_vaflags & VA_UTIMES_NULL)) {
> NFSM_BUILD(tl, u_int32_t *, 3 * NFSX_UNSIGNED);
> *tl++ = txdr_unsigned(NFSV3SATTRTIME_TOCLIENT);
> txdr_nfsv3time(&vap->va_mtime, tl);
> Index: nfsclient/nfs_subs.c
> ===================================================================
> --- nfsclient/nfs_subs.c (revision 225511)
> +++ nfsclient/nfs_subs.c (working copy)
> @@ -1119,7 +1119,7 @@
> *tl = nfs_false;
> }
> if (va->va_atime.tv_sec != VNOVAL) {
> - if (va->va_atime.tv_sec != time_second) {
> + if (!(vattr.va_vaflags & VA_UTIMES_NULL)) {
> tl = nfsm_build_xx(3 * NFSX_UNSIGNED, mb, bpos);
> *tl++ = txdr_unsigned(NFSV3SATTRTIME_TOCLIENT);
> txdr_nfsv3time(&va->va_atime, tl);
> @@ -1132,7 +1132,7 @@
> *tl = txdr_unsigned(NFSV3SATTRTIME_DONTCHANGE);
> }
> if (va->va_mtime.tv_sec != VNOVAL) {
> - if (va->va_mtime.tv_sec != time_second) {
> + if (!(vattr.va_vaflags & VA_UTIMES_NULL)) {
> tl = nfsm_build_xx(3 * NFSX_UNSIGNED, mb, bpos);
> *tl++ = txdr_unsigned(NFSV3SATTRTIME_TOCLIENT);
> txdr_nfsv3time(&va->va_mtime, tl);
> 
> --
> John Baldwin
I think this patch is ok, too.

In the old days, a lot of NFS servers only stored times at a
resolution of 1sec, which I think is why the code had the habit
of comparing "seconds equal". If there is some app. out there
that sets "current time" via utimes(2) with a curent time argument
instead of a NULL argument would seem to be broken to me.
(It is conceivable that some app. did this to avoid clock
 skew between the client and server, but I doubt it.)

Have fun with it, rick
ps: If you were concerned that the change might break something
    that depended on the old behaviour, you could apply the patch
    to the new client only. Then switching to an "oldnfs" mount
    would provide the old "same sec->set time to current time on
    the server" behaviour.