Subject: Re: kern/33861
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: netbsd-bugs
Date: 06/30/2006 09:40:02
The following reply was made to PR kern/33861; it has been noted by GNATS.
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
To: gnats-bugs@NetBSD.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org, jld@panix.com
Subject: Re: kern/33861
Date: Fri, 30 Jun 2006 18:39:20 +0900
--NextPart-20060630183347-0368000
Content-Type: Text/Plain; charset=us-ascii
> It's still worthwhile to double-check the existence of the source file
> before going ahead with the sillyrename, I feel, to avoid unnecessary
> silliness. (It also happens that OpenSolaris does this, too.)
i'm not sure if it's a good idea.
isn't it a matter of VOP interface difference rather than carefulness?
> So, I've included a patch which does those two things; I've done some
> simple tests with it, but it could use more looking-at, as I had to
> outline from nfs_link() the code that does the actual RPC, as opposed to
> vnode glue.
coincidently i've written the similar patch. (attached)
next time, let's take gnats "Responsible:" before fixing. :-)
YAMAMOTO Takashi
--NextPart-20060630183347-0368000
Content-Type: Text/Plain; charset=us-ascii
Content-Disposition: attachment; filename="a.diff"
Index: nfs_var.h
===================================================================
--- nfs_var.h (revision 1657)
+++ nfs_var.h (working copy)
@@ -119,7 +119,8 @@ int nfs_renamerpc(struct vnode *, const
const char *, int, kauth_cred_t, struct lwp *);
int nfs_readdirrpc(struct vnode *, struct uio *, kauth_cred_t);
int nfs_readdirplusrpc(struct vnode *, struct uio *, kauth_cred_t);
-int nfs_sillyrename(struct vnode *, struct vnode *, struct componentname *);
+int nfs_sillyrename(struct vnode *, struct vnode *, struct componentname *,
+ boolean_t);
int nfs_lookitup(struct vnode *, const char *, int, kauth_cred_t,
struct lwp *, struct nfsnode **);
int nfs_commit(struct vnode *, off_t, uint32_t, struct lwp *);
Index: nfs_vnops.c
===================================================================
--- nfs_vnops.c (revision 1702)
+++ nfs_vnops.c (working copy)
@@ -239,6 +239,8 @@ const struct vnodeopv_entry_desc fifo_nf
const struct vnodeopv_desc fifo_nfsv2nodeop_opv_desc =
{ &fifo_nfsv2nodeop_p, fifo_nfsv2nodeop_entries };
+static int nfs_linkrpc(struct vnode *, struct vnode *, const char *,
+ size_t, kauth_cred_t, struct lwp *);
static void nfs_writerpc_extfree(struct mbuf *, caddr_t, size_t, void *);
/*
@@ -1883,7 +1885,7 @@ nfs_remove(v)
printf("nfs_remove: vnode still has %u pages\n",
vp->v_uobj.uo_npages);
} else if (!np->n_sillyrename)
- error = nfs_sillyrename(dvp, vp, cnp);
+ error = nfs_sillyrename(dvp, vp, cnp, FALSE);
PNBUF_PUT(cnp->cn_pnbuf);
if (!error && nfs_getattrcache(vp, &vattr) == 0 &&
vattr.va_nlink == 1) {
@@ -2002,7 +2004,7 @@ nfs_rename(v)
* rename of the new file over it.
*/
if (tvp && tvp->v_usecount > 1 && !VTONFS(tvp)->n_sillyrename &&
- tvp->v_type != VDIR && !nfs_sillyrename(tdvp, tvp, tcnp)) {
+ tvp->v_type != VDIR && !nfs_sillyrename(tdvp, tvp, tcnp, TRUE)) {
VN_KNOTE(tvp, NOTE_DELETE);
vput(tvp);
tvp = NULL;
@@ -2107,6 +2109,54 @@ nfs_renamerpc(fdvp, fnameptr, fnamelen,
return (error);
}
+static int
+nfs_linkrpc(struct vnode *dvp, struct vnode *vp, const char *name,
+ size_t namelen, kauth_cred_t cred, struct lwp *l)
+{
+ u_int32_t *tl;
+ caddr_t cp;
+#ifndef NFS_V2_ONLY
+ int32_t t1;
+ caddr_t cp2;
+#endif
+ int32_t t2;
+ caddr_t bpos, dpos;
+ int error = 0, wccflag = NFSV3_WCCRATTR, attrflag = 0;
+ struct mbuf *mreq, *mrep, *md, *mb;
+ const int v3 = NFS_ISV3(dvp);
+ int rexmit = 0;
+ struct nfsnode *np = VTONFS(vp);
+
+ nfsstats.rpccnt[NFSPROC_LINK]++;
+ nfsm_reqhead(np, NFSPROC_LINK,
+ NFSX_FH(v3)*2 + NFSX_UNSIGNED + nfsm_rndup(namelen));
+ nfsm_fhtom(np, v3);
+ nfsm_fhtom(VTONFS(dvp), v3);
+ nfsm_strtom(name, namelen, NFS_MAXNAMLEN);
+ nfsm_request1(np, NFSPROC_LINK, l, cred, &rexmit);
+#ifndef NFS_V2_ONLY
+ if (v3) {
+ nfsm_postop_attr(vp, attrflag, 0);
+ nfsm_wcc_data(dvp, wccflag, 0, !error);
+ }
+#endif
+ nfsm_reqdone;
+
+ VTONFS(dvp)->n_flag |= NMODIFIED;
+ if (!attrflag)
+ NFS_INVALIDATE_ATTRCACHE(VTONFS(vp));
+ if (!wccflag)
+ NFS_INVALIDATE_ATTRCACHE(VTONFS(dvp));
+
+ /*
+ * Kludge: Map EEXIST => 0 assuming that it is a reply to a retry.
+ */
+ if (rexmit && error == EEXIST)
+ error = 0;
+
+ return error;
+}
+
/*
* nfs hard link create call
*/
@@ -2122,20 +2172,7 @@ nfs_link(v)
struct vnode *vp = ap->a_vp;
struct vnode *dvp = ap->a_dvp;
struct componentname *cnp = ap->a_cnp;
- u_int32_t *tl;
- caddr_t cp;
-#ifndef NFS_V2_ONLY
- int32_t t1;
- caddr_t cp2;
-#endif
- int32_t t2;
- caddr_t bpos, dpos;
- int error = 0, wccflag = NFSV3_WCCRATTR, attrflag = 0;
- struct mbuf *mreq, *mrep, *md, *mb;
- /* XXX Should be const and initialised? */
- int v3;
- int rexmit = 0;
- struct nfsnode *np;
+ int error = 0;
if (dvp->v_mount != vp->v_mount) {
VOP_ABORTOP(dvp, cnp);
@@ -2158,40 +2195,17 @@ nfs_link(v)
*/
VOP_FSYNC(vp, cnp->cn_cred, FSYNC_WAIT, 0, 0, cnp->cn_lwp);
- v3 = NFS_ISV3(vp);
- nfsstats.rpccnt[NFSPROC_LINK]++;
- np = VTONFS(vp);
- nfsm_reqhead(np, NFSPROC_LINK,
- NFSX_FH(v3)*2 + NFSX_UNSIGNED + nfsm_rndup(cnp->cn_namelen));
- nfsm_fhtom(np, v3);
- nfsm_fhtom(VTONFS(dvp), v3);
- nfsm_strtom(cnp->cn_nameptr, cnp->cn_namelen, NFS_MAXNAMLEN);
- nfsm_request1(np, NFSPROC_LINK, cnp->cn_lwp, cnp->cn_cred, &rexmit);
-#ifndef NFS_V2_ONLY
- if (v3) {
- nfsm_postop_attr(vp, attrflag, 0);
- nfsm_wcc_data(dvp, wccflag, 0, !error);
- }
-#endif
- nfsm_reqdone;
- if (error == 0 || error == EEXIST)
+ error = nfs_linkrpc(dvp, vp, cnp->cn_nameptr, cnp->cn_namelen,
+ cnp->cn_cred, cnp->cn_lwp);
+
+ if (error == 0)
cache_purge1(dvp, cnp, 0);
PNBUF_PUT(cnp->cn_pnbuf);
- VTONFS(dvp)->n_flag |= NMODIFIED;
- if (!attrflag)
- NFS_INVALIDATE_ATTRCACHE(VTONFS(vp));
- if (!wccflag)
- NFS_INVALIDATE_ATTRCACHE(VTONFS(dvp));
if (dvp != vp)
VOP_UNLOCK(vp, 0);
VN_KNOTE(vp, NOTE_LINK);
VN_KNOTE(dvp, NOTE_WRITE);
vput(dvp);
- /*
- * Kludge: Map EEXIST => 0 assuming that it is a reply to a retry.
- */
- if (rexmit && error == EEXIST)
- error = 0;
return (error);
}
@@ -3000,9 +3014,10 @@ nfsmout:
* nfs_rename() completes, but...
*/
int
-nfs_sillyrename(dvp, vp, cnp)
+nfs_sillyrename(dvp, vp, cnp, dolink)
struct vnode *dvp, *vp;
struct componentname *cnp;
+ boolean_t dolink;
{
struct sillyrename *sp;
struct nfsnode *np;
@@ -3039,7 +3054,18 @@ nfs_sillyrename(dvp, vp, cnp)
goto bad;
}
}
- error = nfs_renameit(dvp, cnp, sp);
+ if (dolink) {
+ error = nfs_linkrpc(dvp, vp, sp->s_name, sp->s_namlen,
+ sp->s_cred, cnp->cn_lwp);
+ /*
+ * nfs_request maps NFSERR_NOTSUPP to ENOTSUP.
+ */
+ if (error == ENOTSUP) {
+ error = nfs_renameit(dvp, cnp, sp);
+ }
+ } else {
+ error = nfs_renameit(dvp, cnp, sp);
+ }
if (error)
goto bad;
error = nfs_lookitup(dvp, sp->s_name, sp->s_namlen, sp->s_cred,
--NextPart-20060630183347-0368000--