On Tue, Nov 27, 2007 at 08:08:49PM -0500, Thor Lancelot Simon wrote:
> 
> I considered adding an new "docopy" value to union_copyup() to cause it
> to hold the vnode locked and remove the new upper-layer node after
> confirming that it can create it.  Unfortunately this would, I think,
> cause real problems with NFS as an upper layer.

I tried this (see below).  It doesn't work, with a locking-against-myself
panic down in FFS from the VOP_LOOKUP called by relookup().  It's not uvp
that it's trying to lock, nor un_dirvp, so I'm a bit confused what in
fact is doubly-locked here -- but if I remove the relookup, I get a
different panic down in the FFS code, so clearly the relookup is needed.

I don't know if this _should_ work, but certainly it doesn't.  Advice
would be much appreciated.  The docopy == 2 code is lifted from one
of the other union_subr functions, union_vn_create I believe.

Index: union_subr.c
===================================================================
RCS file: /workspaces/cvsroot/nbsrc/sys/fs/union/union_subr.c,v
retrieving revision 1.1.1.2
diff -u -r1.1.1.2 union_subr.c
--- union_subr.c	18 Oct 2007 20:25:20 -0000	1.1.1.2
+++ union_subr.c	28 Nov 2007 02:13:45 -0000
@@ -724,7 +724,7 @@
 
 	lvp = un->un_lowervp;
 
-	if (docopy) {
+	if (docopy == 1) {
 		/*
 		 * XX - should not ignore errors
 		 * from VOP_CLOSE
@@ -753,7 +753,47 @@
 
 	}
 	union_vn_close(uvp, FWRITE, cred, l);
-
+	
+	if (docopy == 2)
+	{
+		struct componentname cn;
+		/*
+		 * Build a new componentname structure (for the same
+		 * reasons outlines in union_mkshadow).
+		 * The difference here is that the file is owned by
+		 * the current user, rather than by the person who
+		 * did the mount, since the current user needs to be
+		 * able to write the file (that's why it is being
+		 * copied in the first place).
+		 */
+		cn.cn_namelen = strlen(un->un_path);
+		if ((cn.cn_namelen + 1) > MAXPATHLEN)
+			return (ENAMETOOLONG);
+		cn.cn_pnbuf = PNBUF_GET();
+		memcpy(cn.cn_pnbuf, un->un_path, cn.cn_namelen+1);
+		cn.cn_nameiop = DELETE;
+		cn.cn_flags = (LOCKPARENT|HASBUF|SAVENAME|ISLASTCN);
+		cn.cn_lwp = l;
+		cn.cn_cred = l->l_cred;
+		cn.cn_nameptr = cn.cn_pnbuf;
+		cn.cn_hash = un->un_hash;
+		cn.cn_consume = 0;
+
+		vn_lock(un->un_dirvp, LK_EXCLUSIVE | LK_RETRY);
+		error = relookup(un->un_dirvp, &uvp, &cn);
+		if (error) {
+			VOP_UNLOCK(un->un_dirvp, 0);
+			panic("relookup failed in copyup called by access");
+		}
+		error = VOP_REMOVE(un->un_dirvp, uvp, &cn);
+		if (error)
+			panic("Remove of upper layer turd failed");
+		else {
+			union_removed_upper(un);
+			return 0;
+		}
+	}
+	
 	/*
 	 * Subsequent IOs will go to the top layer, so
 	 * call close on the lower vnode and open on the
Index: union_vnops.c
===================================================================
RCS file: /workspaces/cvsroot/nbsrc/sys/fs/union/union_vnops.c,v
retrieving revision 1.1.1.2
diff -u -r1.1.1.2 union_vnops.c
--- union_vnops.c	18 Oct 2007 20:25:21 -0000	1.1.1.2
+++ union_vnops.c	28 Nov 2007 01:12:09 -0000
@@ -755,7 +755,8 @@
 		case VDIR:
 		case VLNK:
 		case VREG:
-			if (vp->v_mount->mnt_flag & MNT_RDONLY)
+			if (um->um_uppervp->v_mount->mnt_flag & MNT_RDONLY)
+			/* if (vp->v_mount->mnt_flag & MNT_RDONLY) */
 				return (EROFS);
 			break;
 		case VBAD:
@@ -778,6 +779,14 @@
 
 	if ((vp = un->un_lowervp) != NULLVP) {
 		vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
+
+		if (ap->a_mode & VWRITE) {
+			error = union_copyup(un, 2, ap->a_cred, ap->a_l);
+			VOP_UNLOCK(vp, 0);
+			return (error);
+		}
+					
+				
 		ap->a_vp = vp;
 		error = VCALL(vp, VOFFSET(vop_access), ap);
 		if (error == 0) {