[PATCH 1/5] am-utils-6.2 - add debug log trace to NFSv3 readdirplus

[Ian Kent]

Add log trace print to NFSv3 readdirplus for debuging purposes.

Signed-off-by: Ian Kent <***@themaw.net>
---
amd/nfs_subr.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/amd/nfs_subr.c b/amd/nfs_subr.c
index 30effba..ef07a4a 100644
--- a/amd/nfs_subr.c
+++ b/amd/nfs_subr.c
@@ -1642,6 +1642,9 @@ am_nfs3_readdirplus_3_svc(am_READDIRPLUS3args *argp, struct svc_req *rqstp)
am_node *mp;
int retry;

+ if (amuDebug(D_TRACE))
+ plog(XLOG_DEBUG, "readdirplus_3:");
+
mp = fh3_to_mp3(dir, &retry, VLOOK_CREATE);
if (mp == NULL) {
if (retry < 0) {

[Ian Kent]

When the NFS v3 access method is called and there is no mount point
corresponding to the path ESTALE needs to be returned the kernel NFS
client so an NFS lookup will be done and the mount point re-mounted.

If there is no map entry to mount the NFS v3 lookup method can then
return the failure.

Signed-off-by: Ian Kent <***@themaw.net>
---
amd/nfs_subr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/amd/nfs_subr.c b/amd/nfs_subr.c
index ef07a4a..85cf98c 100644
--- a/amd/nfs_subr.c
+++ b/amd/nfs_subr.c
@@ -1239,9 +1239,9 @@ am_nfs3_access_3_svc(am_ACCESS3args *argp, struct svc_req *rqstp)
if (!mp) {
post_op_obj = &result.res_u.fail.obj_attributes;
post_op_obj->attributes_follow = 0;
- result.status = nfs_error(ENOENT);
+ result.status = nfs_error(ESTALE);
if (amuDebug(D_TRACE))
- plog(XLOG_DEBUG, "access_3: ENOENT");
+ plog(XLOG_DEBUG, "access_3: ESTALE");
} else {
nfsfattr *fattr = &mp->am_fattr;
am_fattr3 *fattr3;

[Ian Kent]

The NFS v3 lookup method, which returns attributes for the directory
containing the object to be looked up, used incorrect mount point
attributes which was causing unusual file system object visibility
problems.

Signed-off-by: Ian Kent <***@themaw.net>
---
amd/nfs_subr.c | 29 +++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/amd/nfs_subr.c b/amd/nfs_subr.c
index 85cf98c..6a1b717 100644
--- a/amd/nfs_subr.c
+++ b/amd/nfs_subr.c
@@ -978,6 +978,29 @@ static void fattr_to_wcc_attr(nfsfattr *fattr, am_wcc_attr *wcc_attr)
nfstime_to_am_nfstime3(&fattr->na_ctime, &wcc_attr->ctime);
}

+static nfsfattr *get_parent_fattr(am_node *mp)
+{
+ nfsfattr *fattr;
+
+ /* Set attributes to those of the parent only if this
+ * isn't topvol otherwise just use the mp attributes.
+ */
+ fattr = &mp->am_fattr;
+ if (mp->am_parent && mp->am_parent->am_parent &&
+ !(mp->am_parent->am_parent->am_flags & AMF_ROOT))
+ fattr = &mp->am_parent->am_fattr;
+
+ return fattr;
+}
+
+static void parent_fattr_to_fattr3(am_node *mp, am_fattr3 *fattr3)
+{
+ nfsfattr *fattr;
+
+ fattr = get_parent_fattr(mp);
+ fattr_to_fattr3(fattr, fattr3);
+}
+
static am_nfsstat3 return_estale_or_rofs(am_nfs_fh3 *fh,
am_pre_op_attr *pre_op,
am_post_op_attr *post_op)
@@ -1177,9 +1200,7 @@ am_nfs3_lookup_3_svc(am_LOOKUP3args *argp, struct svc_req *rqstp)

/* dir attributes */
post_op_dir->attributes_follow = 1;
- fattr = &mp->am_fattr;
fattr3 = &post_op_dir->am_post_op_attr_u.attributes;
- fattr_to_fattr3(fattr, fattr3);

post_op_obj->attributes_follow = 0;

@@ -1196,6 +1217,7 @@ am_nfs3_lookup_3_svc(am_LOOKUP3args *argp, struct svc_req *rqstp)
amd_stats.d_drops++;
return 0;
}
+ parent_fattr_to_fattr3(mp, fattr3);
result.status = nfs_error(error);
} else {
/*
@@ -1206,6 +1228,9 @@ am_nfs3_lookup_3_svc(am_LOOKUP3args *argp, struct svc_req *rqstp)
if (ap->am_ttl < mp->am_ttl)
ap->am_ttl = mp->am_ttl;

+ /* dir attrs, update after mount */
+ parent_fattr_to_fattr3(mp, fattr3);
+
mp_to_fh3(ap, &result.res_u.ok.object);

/* mount attributes */

[Ian Kent]

The NFS v3 readdir method is expected to return the attributes of
the directory being read for both success and fail cases.

Signed-off-by: Ian Kent <***@themaw.net>
---
amd/nfs_subr.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/amd/nfs_subr.c b/amd/nfs_subr.c
index 6a1b717..00a1dc0 100644
--- a/amd/nfs_subr.c
+++ b/amd/nfs_subr.c
@@ -1627,28 +1627,29 @@ am_nfs3_readdir_3_svc(am_READDIR3args *argp, struct svc_req *rqstp)
result.status = nfs_error(retry);
} else {
am_dirlist3 *list = &result.res_u.ok.reply;
+ nfsfattr *fattr;
+ am_fattr3 *fattr3;
am_nfsstat3 status;

if (amuDebug(D_TRACE))
plog(XLOG_DEBUG, "\treaddir_3(%s)", mp->am_path);

+ fattr = &mp->am_fattr;
+
status = mp->am_al->al_mnt->mf_ops->readdir(mp,
(voidp)&cookie, list, entries, count);
if (status == 0) {
post_op_dir = &result.res_u.ok.dir_attributes;
- nfsfattr *fattr;
- am_fattr3 *fattr3;
-
- fattr = &mp->am_fattr;
- fattr3 = &post_op_dir->am_post_op_attr_u.attributes;
post_op_dir->attributes_follow = 1;
- fattr_to_fattr3(fattr, fattr3);
+ fattr3 = &post_op_dir->am_post_op_attr_u.attributes;
result.status = AM_NFS3_OK;
} else {
post_op_dir = &result.res_u.fail.dir_attributes;
- post_op_dir->attributes_follow = 0;
+ post_op_dir->attributes_follow = 1;
+ fattr3 = &post_op_dir->am_post_op_attr_u.attributes;
result.status = nfs_error(status);
}
+ fattr_to_fattr3(fattr, fattr3);

mp->am_stats.s_readdir++;
}

[Ian Kent]

The function unlink3_or_rmdir3() is called by both NFS v3 remove and
rmdir methods. Both of these methods require post op wcc attributes
to be returned but unlink3_or_rmdir3() was returning only the pre op
wcc attributes.

Signed-off-by: Ian Kent <***@themaw.net>
---
amd/nfs_subr.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/amd/nfs_subr.c b/amd/nfs_subr.c
index 00a1dc0..0a43b98 100644
--- a/amd/nfs_subr.c
+++ b/amd/nfs_subr.c
@@ -1033,7 +1033,7 @@ static am_nfsstat3 unlink3_or_rmdir3(am_diropargs3 *argp,
am_pre_op_attr *pre_op_dir = &wcc_data->before;
am_post_op_attr *post_op_dir = &wcc_data->after;
nfsfattr *fattr;
- am_wcc_attr *wcc_attr;
+ am_wcc_attr *pre_op_wcc_attr, *post_op_wcc_attr;
am_node *mp, *ap;
int retry;

@@ -1050,12 +1050,17 @@ static am_nfsstat3 unlink3_or_rmdir3(am_diropargs3 *argp,
goto out;
}

+ post_op_dir->attributes_follow = 1;
+ post_op_wcc_attr = &post_op_dir->am_post_op_attr_u.attributes;
+
pre_op_dir->attributes_follow = 1;
+ pre_op_wcc_attr = &pre_op_dir->am_pre_op_attr_u.attributes;
+
fattr = &mp->am_fattr;
- wcc_attr = &pre_op_dir->am_pre_op_attr_u.attributes;
- fattr_to_wcc_attr(fattr, wcc_attr);
+ fattr_to_wcc_attr(fattr, pre_op_wcc_attr);

if (mp->am_fattr.na_type != NFDIR) {
+ fattr_to_wcc_attr(fattr, post_op_wcc_attr);
res = nfs_error(ENOTDIR);
goto out;
}
@@ -1075,9 +1080,14 @@ static am_nfsstat3 unlink3_or_rmdir3(am_diropargs3 *argp,
*/
else if (retry == ENOENT)
retry = 0;
+ fattr_to_wcc_attr(fattr, post_op_wcc_attr);
res = nfs_error(retry);
} else {
forcibly_timeout_mp(mp);
+ /* we can't wait for the expire so use the attributes as
+ * they are now for the post op wcc attributes.
+ */
+ fattr_to_wcc_attr(fattr, post_op_wcc_attr);
res = AM_NFS3_OK;
}

[Erez Zadok]

Ian,

Sorry for the belated reply. First, happy new year. Second, thanks for the patches. A cursory look suggests they’re good. And it’s good to know that they appear to work.

Christos: could you look over the patches too, and if they’re good, commit them?

Thanks,
Erez.

The following series fixes several problems with the internal NFSv3 server
implementation that were found when investigating a recent bug report.
---
am-utils-6.2 - add debug log trace to NFSv3 readdirplus
am-utils-6.2 - fix NFSv3 access method return on non-existent mount point
am-utils-6.2 - fix NFSv3 lookup dir attribute return value
am-utils-6.2 - fix NFSv3 readdir post_op_dir attributes return
am-utils-6.2 - fix NFSv3 unlink3_or_rmdir3() post_op attributes return
amd/nfs_subr.c | 67 ++++++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 53 insertions(+), 14 deletions(-)
--
Ian
_______________________________________________
am-utils mailing list
http://www.fsl.cs.sunysb.edu/mailman/listinfo/am-utils

[Christos Zoulas]

On Jan 22, 9:23am, ***@fsl.cs.sunysb.edu (Erez Zadok) wrote:
-- Subject: Re: [PATCH 0/5] Fix several NFSv3 internal server problems

Happy New Year everyone. I've been slacking (and expecting someone else
to commit them). Anyway I've reviewed them and committed them now.

Enjoy,

christos

[Ian Kent]

Post by Erez Zadok
Ian,
Sorry for the belated reply. First, happy new year. Second, thanks
for the patches. A cursory look suggests they’re good. And it’s
good to know that they appear to work.

Thanks Erez, and best wishes to you too.

I expect there are still more subtle difficulties in the NFSv3 code,
time will tell.

After going over the NFSv3 method implementation again, due to the
observed bug, this was all I could find that didn't look quite right.

Post by Erez Zadok
Christos: could you look over the patches too, and if they’re good,
commit them?
Thanks,
Erez.

The following series fixes several problems with the internal NFSv3
server
implementation that were found when investigating a recent bug
report.
---
am-utils-6.2 - add debug log trace to NFSv3 readdirplus
am-utils-6.2 - fix NFSv3 access method return on non-existent
mount point
am-utils-6.2 - fix NFSv3 lookup dir attribute return value
am-utils-6.2 - fix NFSv3 readdir post_op_dir attributes return
am-utils-6.2 - fix NFSv3 unlink3_or_rmdir3() post_op
attributes return
amd/nfs_subr.c | 67 ++++++++++++++++++++++++++++++++++++++++++++-
-----------
1 file changed, 53 insertions(+), 14 deletions(-)
--
Ian
_______________________________________________
am-utils mailing list
http://www.fsl.cs.sunysb.edu/mailman/listinfo/am-utils

[Ian Kent]

Post by Christos Zoulas
-- Subject: Re: [PATCH 0/5] Fix several NFSv3 internal server
problems
Happy New Year everyone. I've been slacking (and expecting someone
else
to commit them). Anyway I've reviewed them and committed them now.

Ha, and best wishes to you also.

That's great, thanks heaps.

Post by Christos Zoulas
Enjoy,
christos

[Erez Zadok]

Yeah, I’m certain there’ll be more patches. This is a big a deal, changing amd’s internal NFS server.

Cheers,
Erez.

Post by Ian Kent
I expect there are still more subtle difficulties in the NFSv3 code,
time will tell.
After going over the NFSv3 method implementation again, due to the
observed bug, this was all I could find that didn't look quite right.