Date: Mon, 14 Jan 2013 20:20:54 -0500 (EST) From: Rick Macklem <rmacklem@uoguelph.ca> To: John Baldwin <jhb@freebsd.org> Cc: Rick Macklem <rmacklem@freebsd.org>, fs@freebsd.org Subject: Re: [PATCH] Better handle NULL utimes() in the NFS client Message-ID: <162405990.1985479.1358212854967.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <201301141445.29260.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?162405990.1985479.1358212854967.JavaMail.root>