| Message ID | 20110827004736.GB19967@fieldses.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/26/2011 05:47 PM, J. Bruce Fields wrote: > From: Boaz Harrosh <bharrosh@panasas.com> > > Even if we fail to write a recovery record, we should still mark the > client as having acquired its first state. Otherwise we leave 4.1 > clients with indefinite ERR_GRACE returns. > > However, an inability to write stable storage records may cause failures > of reboot recovery, and the problem should still be brought to the > server administrator's attention. > > So, make sure the error is logged. > > These errors shouldn't normally be triggered on a corectly functioning > server--this isn't a case where a misconfigured client could spam the > logs. > > Signed-off-by: Boaz Harrosh <bharrosh@panasas.com> > Signed-off-by: J. Bruce Fields <bfields@redhat.com> > --- > fs/nfsd/nfs4recover.c | 15 ++++++++------- > 1 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c > index 3e6ebcf..ed083b9 100644 > --- a/fs/nfsd/nfs4recover.c > +++ b/fs/nfsd/nfs4recover.c > @@ -130,6 +130,7 @@ nfsd4_create_clid_dir(struct nfs4_client *clp) > if (!rec_file || clp->cl_firststate) > return 0; > > + clp->cl_firststate = 1; > status = nfs4_save_creds(&original_cred); > if (status < 0) > return status; > @@ -144,10 +145,8 @@ nfsd4_create_clid_dir(struct nfs4_client *clp) > goto out_unlock; > } > status = -EEXIST; > - if (dentry->d_inode) { > - dprintk("NFSD: nfsd4_create_clid_dir: DIRECTORY EXISTS\n"); > + if (dentry->d_inode) > goto out_put; > - } > status = mnt_want_write(rec_file->f_path.mnt); > if (status) > goto out_put; > @@ -157,12 +156,14 @@ out_put: > dput(dentry); > out_unlock: > mutex_unlock(&dir->d_inode->i_mutex); > - if (status == 0) { > - clp->cl_firststate = 1; > + if (status == 0) > vfs_fsync(rec_file, 0); > - } > + else > + printk(KERN_ERR "NFSD: failed to write recovery record" > + " (err %d); please check that %s exists" > + " and is writeable", status, > + user_recovery_dirname); Whooff, hey thank you bruce, alot. I'm so sorry I assumed the name was readily available only I have hard time finding it in 30 seconds of trying. (So it was me, that's lazy). I did not mean for this to be so much work. But the up side is that I'm sure some future admin's lives will be relieved and that's Karma points ;-) Thanks very much and ACK on both patches. Boaz > nfs4_reset_creds(original_cred); > - dprintk("NFSD: nfsd4_create_clid_dir returns %d\n", status); > return status; > } > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 26, 2011 at 07:07:31PM -0700, Boaz Harrosh wrote: > On 08/26/2011 05:47 PM, J. Bruce Fields wrote: > > From: Boaz Harrosh <bharrosh@panasas.com> > > > > Even if we fail to write a recovery record, we should still mark the > > client as having acquired its first state. Otherwise we leave 4.1 > > clients with indefinite ERR_GRACE returns. > > > > However, an inability to write stable storage records may cause failures > > of reboot recovery, and the problem should still be brought to the > > server administrator's attention. > > > > So, make sure the error is logged. > > > > These errors shouldn't normally be triggered on a corectly functioning > > server--this isn't a case where a misconfigured client could spam the > > logs. > > > > Signed-off-by: Boaz Harrosh <bharrosh@panasas.com> > > Signed-off-by: J. Bruce Fields <bfields@redhat.com> > > --- > > fs/nfsd/nfs4recover.c | 15 ++++++++------- > > 1 files changed, 8 insertions(+), 7 deletions(-) > > > > diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c > > index 3e6ebcf..ed083b9 100644 > > --- a/fs/nfsd/nfs4recover.c > > +++ b/fs/nfsd/nfs4recover.c > > @@ -130,6 +130,7 @@ nfsd4_create_clid_dir(struct nfs4_client *clp) > > if (!rec_file || clp->cl_firststate) > > return 0; > > > > + clp->cl_firststate = 1; > > status = nfs4_save_creds(&original_cred); > > if (status < 0) > > return status; > > @@ -144,10 +145,8 @@ nfsd4_create_clid_dir(struct nfs4_client *clp) > > goto out_unlock; > > } > > status = -EEXIST; > > - if (dentry->d_inode) { > > - dprintk("NFSD: nfsd4_create_clid_dir: DIRECTORY EXISTS\n"); > > + if (dentry->d_inode) > > goto out_put; > > - } > > status = mnt_want_write(rec_file->f_path.mnt); > > if (status) > > goto out_put; > > @@ -157,12 +156,14 @@ out_put: > > dput(dentry); > > out_unlock: > > mutex_unlock(&dir->d_inode->i_mutex); > > - if (status == 0) { > > - clp->cl_firststate = 1; > > + if (status == 0) > > vfs_fsync(rec_file, 0); > > - } > > + else > > + printk(KERN_ERR "NFSD: failed to write recovery record" > > + " (err %d); please check that %s exists" > > + " and is writeable", status, > > + user_recovery_dirname); > > Whooff, hey thank you bruce, alot. I'm so sorry I assumed the name was > readily available only I have hard time finding it in 30 seconds of trying. > (So it was me, that's lazy). I did not mean for this to be so much work. Well, I was exaggerating. > But the up side is that I'm sure some future admin's lives will be relieved > and that's Karma points ;-) > > Thanks very much and ACK on both patches. OK thanks. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c index 3e6ebcf..ed083b9 100644 --- a/fs/nfsd/nfs4recover.c +++ b/fs/nfsd/nfs4recover.c @@ -130,6 +130,7 @@ nfsd4_create_clid_dir(struct nfs4_client *clp) if (!rec_file || clp->cl_firststate) return 0; + clp->cl_firststate = 1; status = nfs4_save_creds(&original_cred); if (status < 0) return status; @@ -144,10 +145,8 @@ nfsd4_create_clid_dir(struct nfs4_client *clp) goto out_unlock; } status = -EEXIST; - if (dentry->d_inode) { - dprintk("NFSD: nfsd4_create_clid_dir: DIRECTORY EXISTS\n"); + if (dentry->d_inode) goto out_put; - } status = mnt_want_write(rec_file->f_path.mnt); if (status) goto out_put; @@ -157,12 +156,14 @@ out_put: dput(dentry); out_unlock: mutex_unlock(&dir->d_inode->i_mutex); - if (status == 0) { - clp->cl_firststate = 1; + if (status == 0) vfs_fsync(rec_file, 0); - } + else + printk(KERN_ERR "NFSD: failed to write recovery record" + " (err %d); please check that %s exists" + " and is writeable", status, + user_recovery_dirname); nfs4_reset_creds(original_cred); - dprintk("NFSD: nfsd4_create_clid_dir returns %d\n", status); return status; }