shithub: gefs

Download patch

ref: d1a51cc02a74d6f9b19451d1a16b92c7b83b2889
parent: 3b48c49025b592ef59724e864e0c01d03680b070
author: Ori Bernstein <ori@eigenstate.org>
date: Sun May 7 11:56:09 EDT 2023

fs: handle clobbering and renaming files more correctly

this change addresses two issues: first, when creating a file,
we used to simply send a Oinsert down the tree, which meant
that if the message had propagated to a leaf, and we did a
remove for each fid that had a reference, we may remove the
first instance, and then blow up that a second did not exist.
that when removing it via 2 fids, we may remove the first and
then assert when we remove the second.

second, when renaming over an existing dir, we may leak the
old blocks and lose the '..' link that we need to refer to
files relative to the dir.

--- a/dat.h
+++ b/dat.h
@@ -57,6 +57,7 @@
 	Offksz	= 17,			/* type, qid, off */
 	Snapsz	= 9,			/* tag, snapid */
 	Dpfxsz	= 9,			/* directory prefix */
+	Upksz	= 9,			/* directory prefix */
 	Dlksz	= 1+8+8,		/* tag, death, birth */
 	Dlvsz	= Ptrsz+Ptrsz,		/* hd,tl of deadlist */
 	Dlkvpsz	= Dlksz+Dlvsz,		/* full size of dlist kvp */
@@ -96,7 +97,7 @@
 	/* fs keys */
 	Kdat,	/* qid[8] off[8] => ptr:		pointer to data page */
 	Kent,	/* pqid[8] name[n] => dir[n]:		serialized Dir */
-	Ksuper,	/* qid[8] => Kent:			parent dir */
+	Kup,	/* qid[8] => Kent:			parent dir */
 
 	/* snapshot keys */
 	Klabel,	/* name[] => snapid[]:			snapshot label */
@@ -258,6 +259,7 @@
 	Oinsert,	/* new kvp */
 	Odelete,	/* delete kvp */
 	Oclearb,	/* free block ptr if exists */
+	Oclobber,	/* remove file if it exists */
 	Owstat,		/* update kvp dirent */
 	Orefsnap,	/* increment snap refcount by delta */
 	Nmsgtype,	/* maximum message type */
--- a/dump.c
+++ b/dump.c
@@ -10,6 +10,31 @@
 char	spc[128];
 
 static int
+walk1(Tree *t, vlong up, char *name, Qid *qid, vlong *len)
+{
+	char *p, kbuf[Keymax], rbuf[Kvmax];
+	int err;
+	Xdir d;
+	Kvp kv;
+	Key k;
+
+	err = 0;
+	if((p = packdkey(kbuf, sizeof(kbuf), up, name)) == nil)
+		return -1;
+	k.k = kbuf;
+	k.nk = p - kbuf;
+	if(err)
+		return -1;
+	if(btlookup(t, &k, &kv, rbuf, sizeof(rbuf)) != nil)
+		return -1;
+	if(kv2dir(&kv, &d) == -1)
+		return -1;
+	*qid = d.qid;
+	*len = d.length;
+	return 0;
+}
+
+static int
 showkey(Fmt *fmt, Key *k)
 {
 	int n;
@@ -34,7 +59,7 @@
 	case Ksnap:	/* name[n] => tree[24]:	snapshot root */
 		n = fmtprint(fmt, "snap id:%lld", UNPACK64(k->k+1));
 		break;
-	case Ksuper:	/* qid[8] => pqid[8]:		parent dir */
+	case Kup:	/* qid[8] => pqid[8]:		parent dir */
 		n = fmtprint(fmt, "up dir:%llx", UNPACK64(k->k+1));
 		break;
 	case Kslink:
@@ -142,7 +167,7 @@
 	case Klabel:
 		n = fmtprint(fmt, "snap id:\"%lld\"", UNPACK64(v->v+1));
 		break;
-	case Ksuper:	/* qid[8] => pqid[8]:		parent dir */
+	case Kup:	/* qid[8] => pqid[8]:		parent dir */
 		n = fmtprint(fmt, "super dir:%llx, name:\"%.*s\")", UNPACK64(v->v+1), v->nv-11, v->v+11);
 		break;
 	case Kslink:
--- a/fns.h
+++ b/fns.h
@@ -11,6 +11,7 @@
 extern int	debug;
 extern int	permissive;
 extern char*	reamuser;
+extern Blk*	blkbuf;
 
 #define	UNPACK8(p)	(((uchar*)(p))[0])
 #define	UNPACK16(p)	((((uchar*)(p))[0]<<8)|(((uchar*)(p))[1]))
@@ -113,6 +114,7 @@
 void	showblk(int, Blk*, char*, int);
 void	showbp(int, Bptr, int);
 void	showtreeroot(int, Tree*);
+void	showballoc(int, char**, int);
 void	showdlist(int, char**, int);
 void	showtree(int, char**, int);
 void	showsnap(int, char**, int);
--- a/fs.c
+++ b/fs.c
@@ -9,8 +9,31 @@
 #include "fns.h"
 #include "atomic.h"
 
-static char*	clearb(Fid*, vlong, vlong);
+static int
+walk1(Tree *t, vlong up, char *name, Qid *qid, vlong *len)
+{
+	char *p, kbuf[Keymax], rbuf[Kvmax];
+	int err;
+	Xdir d;
+	Kvp kv;
+	Key k;
 
+	err = 0;
+	if((p = packdkey(kbuf, sizeof(kbuf), up, name)) == nil)
+		return -1;
+	k.k = kbuf;
+	k.nk = p - kbuf;
+	if(err)
+		return -1;
+	if(btlookup(t, &k, &kv, rbuf, sizeof(rbuf)) != nil)
+		return -1;
+	if(kv2dir(&kv, &d) == -1)
+		return -1;
+	*qid = d.qid;
+	*len = d.length;
+	return 0;
+}
+
 static void
 snapfs(int fd, char *old, char *new)
 {
@@ -250,22 +273,22 @@
  * the range listed.
  */
 static char*
-clearb(Fid *f, vlong o, vlong sz)
+clearb(Mount *mnt, vlong qid, vlong off, vlong len)
 {
 	char *e, buf[Offksz];
 	Msg m;
 
-	o &= ~(Blksz - 1);
-	for(; o < sz; o += Blksz){
+	off &= ~(Blksz - 1);
+	for(; off < len; off += Blksz){
 		m.k = buf;
 		m.nk = sizeof(buf);
 		m.op = Oclearb;
 		m.k[0] = Kdat;
-		PACK64(m.k+1, f->qpath);
-		PACK64(m.k+9, o);
+		PACK64(m.k+1, qid);
+		PACK64(m.k+9, off);
 		m.v = nil;
 		m.nv = 0;
-		if((e = upsert(f->mnt, &m, 1)) != nil)
+		if((e = upsert(mnt, &m, 1)) != nil)
 			return e;
 	}
 	return nil;
@@ -1117,11 +1140,12 @@
 static void
 fswstat(Fmsg *m)
 {
-	char *p, *e, strs[65535], rnbuf[Kvmax], opbuf[Kvmax];
+	char rnbuf[Kvmax], opbuf[Kvmax], upbuf[Upksz];
+	char *p, *e, strs[65535];
 	int op, nm, rename;
 	Fcall r;
 	Dent *de;
-	Msg mb[2];
+	Msg mb[3];
 	Xdir n;
 	Dir d;
 	Fid *f;
@@ -1285,12 +1309,12 @@
 	/* update directory entry */
 	nm = 0;
 	if(rename && !de->gone){
-		mb[nm].op = Odelete;
+		mb[nm].op = Oclobber;
 		mb[nm].Key = de->Key;
 		mb[nm].v = nil;
 		mb[nm].nv = 0;
 		nm++;
-
+	
 		mb[nm].op = Oinsert;
 		if(dir2kv(f->pqpath, &n, &mb[nm], rnbuf, sizeof(rnbuf)) == -1){
 			rerror(m, Efs);
@@ -1298,6 +1322,16 @@
 		}
 		k = mb[nm].Key;
 		nm++;
+
+		if(de->qid.type & QTDIR){
+			packsuper(upbuf, sizeof(upbuf), f->qpath);
+			mb[nm].op = Oinsert;
+			mb[nm].k = upbuf;
+			mb[nm].nk = Upksz;
+			mb[nm].v = mb[nm-1].k;
+			mb[nm].nv = mb[nm-1].nk;
+			nm++;
+		}
 	}else{
 		opbuf[0] = op;
 		mb[nm].op = Owstat;
@@ -1306,6 +1340,7 @@
 		mb[nm].nv = p - opbuf;
 		nm++;
 	}
+	assert(nm <= nelem(mb));
 	if((e = upsert(f->mnt, mb, nm)) != nil){
 		rerror(m, e);
 		goto Out;
@@ -1352,11 +1387,13 @@
 {
 	char *p, *e, buf[Kvmax], upkbuf[Keymax], upvbuf[Inlmax];
 	Dent *de;
+	vlong oldlen;
+	Qid old;
 	Fcall r;
 	Msg mb[2];
 	Fid *f;
 	Xdir d;
-	int nm;
+	int nm, clobber;
 
 	if(okname(m->name) == -1){
 		rerror(m, Ename);
@@ -1372,6 +1409,11 @@
 		return;
 	}
 	de = f->dent;
+
+	clobber = 0;
+	if(walk1(f->mnt->root, f->qpath, m->name, &old, &oldlen) == 0)
+		clobber = 1;
+
 	rlock(de);
 	if(fsaccess(f, de->mode, de->uid, de->gid, DMWRITE) == -1){
 		runlock(de);
@@ -1438,11 +1480,17 @@
 		rerror(m, Enomem);
 		putfid(f);
 		return;
-	}else{
-		wlock(de);
-		de->length = 0;
+	}
+
+	wlock(de);
+	if(clobber && (e = clearb(f->mnt, old.path, 0, oldlen)) != nil){
+		rerror(m, e);
 		wunlock(de);
+		putfid(f);
+		return;
 	}
+	de->length = 0;
+	wunlock(de);
 
 	lock(f);
 	if(f->mode != -1){
@@ -1456,17 +1504,6 @@
 	f->pqpath = f->qpath;
 	f->qpath = d.qid.path;
 	f->dent = de;
-	wlock(de);
-	de->gone = 0;
-	if((e = clearb(f, 0, de->length)) != nil){
-		unlock(f);
-		clunkdent(de);
-		rerror(m, e);
-		putfid(f);
-		return;
-	}
-	de->length = 0;
-	wunlock(de);
 	unlock(f);
 
 	r.type = Rcreate;
@@ -1500,8 +1537,9 @@
 static void
 fsremove(Fmsg *m)
 {
+	char upbuf[Upksz];
 	Fcall r;
-	Msg mb;
+	Msg mb[2];
 	Fid *f;
 	char *e;
 
@@ -1522,15 +1560,21 @@
 		e = Eperm;
 		goto Error;
 	}
-	mb.op = Odelete;
-	mb.k = f->dent->k;
-	mb.nk = f->dent->nk;
-	mb.nv = 0;
+	mb[0].op = Odelete;
+	mb[0].k = f->dent->k;
+	mb[0].nk = f->dent->nk;
+	mb[0].nv = 0;
 
-	if((e = upsert(f->mnt, &mb, 1)) != nil)
+	packsuper(upbuf, sizeof(upbuf), f->qpath);
+	mb[1].op = Oclobber;
+	mb[1].k = upbuf;
+	mb[1].nk = Upksz;
+	mb[1].nv = 0;
+
+	if((e = upsert(f->mnt, mb, 1)) != nil)
 		goto Error;
 	if(f->dent->qid.type == QTFILE){
-		e = clearb(f, 0, f->dent->length);
+		e = clearb(f->mnt, f->qpath, 0, f->dent->length);
 		if(e != nil)
 			goto Error;
 	}
@@ -1626,7 +1670,7 @@
 		mb.nk = f->dent->nk;
 		mb.v = buf;
 		mb.nv = p - buf;
-		clearb(f, 0, f->dent->length);
+		clearb(f->mnt, f->qpath, 0, f->dent->length);
 		if((e = upsert(f->mnt, &mb, 1)) != nil){
 			wunlock(f->dent);
 			rerror(m, e);
--- a/pack.c
+++ b/pack.c
@@ -188,7 +188,7 @@
 
 	err = 0;
 	ep = p + sz;
-	p = pack8(&err, p, ep, Ksuper);
+	p = pack8(&err, p, ep, Kup);
 	p = pack64(&err, p, ep, up);
 	if(err)
 		return nil;
--- a/tree.c
+++ b/tree.c
@@ -403,6 +403,7 @@
 	switch(m->op){
 	case Oclearb:
 	case Odelete:
+	case Oclobber:
 		assert(keycmp(kv, m) == 0);
 		return 0;
 	case Oinsert:
@@ -480,7 +481,7 @@
 			 * new values must always start as
 			 * an insertion, mutations come after.
 			 */
-			if(m.op == Oclearb)
+			if(m.op == Oclearb || m.op == Oclobber)
 				ok = 0;
 			else if(m.op != Oinsert){
 				fprint(2, "%d(/%d), %d: %M not insert\n", i, b->nval, j, &m);
@@ -516,7 +517,7 @@
 		ok = 1;
 		cpkvp(&v, &m, buf, sizeof(buf));
 		p->pullsz += msgsz(&m);
-		if(m.op == Oclearb)
+		if(m.op == Oclearb || m.op == Oclobber)
 			continue;
 		if(m.op != Oinsert){
 			fprint(2, "%d(/%d), %d: %M not insert\n", i, b->nval, j, &m);
@@ -677,10 +678,10 @@
 			 * new values must always start as
 			 * an insertion, mutations come after.
 			 */
-			if(m.op == Oclearb)
+			if(m.op == Oclearb || m.op == Oclobber)
 				ok = 0;
 			else if(m.op != Oinsert){
-				fprint(2, "%d(/%d), %d: %M not insert\n", i, b->nval, j, &m);
+				fprint(2, "%d(/%d), %d: expected insert, got %M\n", i, b->nval, j, &m);
 				abort();
 			}
 			cpkvp(&v, &m, buf, sizeof(buf));