On Fri, 2017-08-18 at 12:53 -0400, Chuck Lever wrote:
> > On Aug 18, 2017, at 12:39 PM, Trond Myklebust <trondmy@primarydata.
> > com> wrote:
> >
> > On Fri, 2017-06-09 at 14:04 -0400, Chuck Lever wrote:
> > > Hi Trond-
> > >
> > > As we discussed yesterday, I'm concerned about the test in
> > > _nfs4_proc_open that uses the values of the cinfo.before and
> > > cinfo.after fields without checking cinfo.atomic.
> > >
> > > 2288 if (o_arg->open_flags & O_CREAT) {
> > > 2289 if (o_arg->open_flags & O_EXCL)
> > > 2290 data->file_created = 1;
> > > 2291 else if (o_res->cinfo.before != o_res-
> > > > cinfo.after)
> > >
> > > 2292 data->file_created = 1;
> > > 2293 if (data->file_created || dir->i_version !=
> > > o_res->cinfo.after)
> > > 2294 update_changeattr(dir, &o_res-
> > > >cinfo,
> > > 2295 o_res->f_attr-
> > > > time_start);
> > >
> > > 2296 }
> > >
> > > Line 2291 is the issue.
> > >
> > > Suppose the server filesystem substitutes a weak ctime for
> > > the change attribute. Sometimes when a file is successfully
> > > created on the server, the ctime of the parent doesn't
> > > change. This test then fails and leaves file_created set
> > > to zero.
> >
> > A server which uses a ctime with insufficient resolution is in
> > violation of the requirements for a change attribute. The spec
> > explicitly disallows it in https://tools.ietf.org/html/rfc5661#sect
> > ion-
> > 5.8.1.4
> >
> > IOW: the remainder of the spec (including the definition of OPEN)
> > does
> > expect that you can rely on the change attribute for detecting file
> > changes.
>
> Fair enough.
>
> It's been suggested that Linux should prevent NFSv4 access of
> any filesystem that can't support the change attribute
> adequately. That would block existing xfs filesystems with
> older on-disk formats. Any xfs filesystem formatted on a RHEL7
> system has one of these older formats.
>
> This is actually a pretty difficult problem for legacy file
> systems. In this case I wonder if RFC 5661 is impractically
> draconian.
>
> The alternative is that the Linux NFS server will have to
> recognize such filesystems and somehow generate proper change
> attributes for them, even though it can't save them on disk.
>
>
> > > Later, nfs4_opendata_access looks at file_created. If not
> > > set, the access check fails and open(2) returns EACCES as
> > > a result, even though the file was created successfully
> > > and OPEN returned NFS4_OK.
> > >
> > > You mentioned that the server should set cinfo.atomic to
> > > be false when a weak ctime is used as the change attribute.
> > > Will that help in this case?
> >
> > No. The cinfo.atomic flag is there to let you know that the change
> > you
> > are seeing in the directory was caused by the operation that just
> > executed. If it is set to "false" then that does not really tell
> > you
> > much about whether or not you can trust that a change occurred.
>
> A client can't trust the change attribute when cinfo.atomic
> is false. In that case, the client might depend on other means
> to ensure correct operation.
>
I've always read the spec as saying that it is supposed to be able to
trust the actual value of the change attribute. It just cannot trust
that the change came from this operation. It could be due to another
operation that acted on the same directory and raced with this one.
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@xxxxxxxxxxxxxxx
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
