[PVFS-developers] chown bug, part 2
Porter Don
PorterDE@mercury.hendrix.edu
Mon, 27 Oct 2003 10:48:13 -0600
This message is in MIME format. Since your mail reader does not understand
this format, some or all of this message may not be legible.
------_=_NextPart_000_01C39CAA.1C7CA510
Content-Type: text/plain
The more I looked at this, the problem is caused by the dmeta->sd_path entry
getting corrupted, most notably when one is calling get_dmeta on the root of
the filesystem.
It seems, however, that the sd_path is used _nowhere_ in the code except
when reading/writing dmeta and at one point in do_mkdir as a buffer to
append a '/' onto the rd_path.
So, unless I am missing something, there is really no reason to store the
subdirectory path (or filename for that matter) in all of the .pvfsdir
files. I can see why the rd_path (root directory) is useful, but a simple
stat would be the best way to get the actual sd_path.
So, as an experiment, I stripped out all of the places in the pvfs code
where dmeta->sd_path and dmeta->fname are used, and everything seems to work
just fine. Also, the problems with chroot go away. Further, because these
are the last two entries in the .pvfsdir file, they can be easily ignored if
they are already there.
Please let me know if I am missing something, but these seem to be vestigial
structures that we can do without. Attached is a patch that basically
prunes these out, works around the buffer thing in do_mkdir (just in case!),
and fixes the chown problem.
Thanks,
Don Porter
------_=_NextPart_000_01C39CAA.1C7CA510
Content-Type: application/octet-stream;
name="no_sd_path.patch"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
filename="no_sd_path.patch"
diff -ur pvfs-1.6.0/include/meta.h pvfs-chown/include/meta.h=0A=
--- pvfs-1.6.0/include/meta.h Tue Jun 10 19:31:08 2003=0A=
+++ pvfs-chown/include/meta.h Mon Oct 27 08:58:57 2003=0A=
@@ -37,8 +37,6 @@=0A=
u_int16_t port;=0A=
char *host;=0A=
char *rd_path;=0A=
- char *sd_path;=0A=
- char *fname;=0A=
};=0A=
=0A=
typedef struct pvfs_filestat pvfs_filestat, *pvfs_filestat_p;=0A=
diff -ur pvfs-1.6.0/lib/lib.h pvfs-chown/lib/lib.h=0A=
--- pvfs-1.6.0/lib/lib.h Tue Jun 10 19:31:09 2003=0A=
+++ pvfs-chown/lib/lib.h Mon Oct 27 09:04:28 2003=0A=
@@ -71,8 +71,6 @@=0A=
int inc_ref_count(int slot);=0A=
int dec_ref_count(int slot);=0A=
=0A=
-char *dmeta2fname(dmeta_p);=0A=
-=0A=
int do_jobs_handle_error(jlist_p jl_p, sockset_p ss_p, int msec, int* =
badsock);=0A=
int do_jobs(jlist_p jl_p, sockset_p ss_p, int msec);=0A=
int do_job(int sock, jinfo_p j_p, sockset_p ss_p);=0A=
diff -ur pvfs-1.6.0/mgr/meta/dmetaio.c pvfs-chown/mgr/meta/dmetaio.c=0A=
--- pvfs-1.6.0/mgr/meta/dmetaio.c Fri Oct 25 12:33:05 2002=0A=
+++ pvfs-chown/mgr/meta/dmetaio.c Mon Oct 27 09:01:49 2003=0A=
@@ -114,8 +114,6 @@=0A=
/* read strings and save temporary locations in tmpbuf */=0A=
dm_p->host =3D strtok(NULL, "\n");=0A=
dm_p->rd_path =3D strtok(NULL, "\n");=0A=
- dm_p->sd_path =3D strtok(NULL, "\n");=0A=
- dm_p->fname =3D NULL; /* what is this about? */=0A=
=0A=
/* copy strings into the buffer; check to make sure we have the space =
*/=0A=
b_p =3D (char *) buf + sizeof(struct dmeta);=0A=
@@ -138,15 +136,6 @@=0A=
// dm_p->rd_path=3Db_p;=0A=
b_p +=3D tlen;=0A=
=0A=
- tlen =3D strlen(dm_p->sd_path)+1;=0A=
- if (b_p + tlen > (char *) buf + len) {=0A=
- errno =3D EOVERFLOW;=0A=
- return(-1);=0A=
- }=0A=
- bcopy(dm_p->sd_path, b_p, tlen); /* sub dir */=0A=
-// dm_p->sd_path =3D b_p;=0A=
- b_p +=3D tlen;=0A=
-=0A=
ret =3D (int)(b_p - (char *) buf);=0A=
return((ret < 0) ? -1 : ret);=0A=
} /* end of dmeta_read() */=0A=
diff -ur pvfs-1.6.0/mgr/meta/get_dmeta.c =
pvfs-chown/mgr/meta/get_dmeta.c=0A=
--- pvfs-1.6.0/mgr/meta/get_dmeta.c Fri Oct 25 12:33:05 2002=0A=
+++ pvfs-chown/mgr/meta/get_dmeta.c Mon Oct 27 09:04:16 2003=0A=
@@ -83,70 +83,12 @@=0A=
}=0A=
dmeta_close(fd);=0A=
=0A=
- /* fill in the filename part of the dmeta structure */=0A=
- /* this is a big hack to keep someone's idiot code from breaking =
*/=0A=
- d_p =3D (struct dmeta *) dmbuf;=0A=
-=0A=
- /* WORKAROUND FOR SUBDIRECTORY PROBLEM:=0A=
- * Here we need to substitute the subdirectory that was passed in =
for=0A=
- * the one that is in the directory metadata. This is to work =
around=0A=
- * the problem of recursive renames changing the actual =
subdirectory=0A=
- * without updating the metadata.=0A=
- *=0A=
- * In actuality, we're moving more and more away from this root=0A=
- * directory, subdirectory, file name nonsense, but for the moment =
we=0A=
- * need to keep things up to date. The next generation PVFS will =
not=0A=
- * have any of this.=0A=
- *=0A=
- * We assume that the root directory is ok in the dmeta structure =
and=0A=
- * that the sd_path pointer will point past the rd_path area. We=0A=
- * then rewrite the sd_path and the fname and reset the fname=0A=
- * pointer.=0A=
- */=0A=
- rd_len =3D strlen(d_p->rd_path)+1;=0A=
- strcpy(d_p->sd_path, &nbuf[rd_len]);=0A=
- d_p->fname =3D d_p->sd_path + strlen(d_p->sd_path)+1;=0A=
-=0A=
- if (is_dir) {=0A=
- *d_p->fname =3D '\0';=0A=
- }=0A=
- else {=0A=
- strcpy(d_p->fname, &nbuf[len+1]); /* could check len... */=0A=
- }=0A=
-=0A=
/* copy the dmeta structure to the specified location */=0A=
*dir =3D *(struct dmeta *)dmbuf;=0A=
=0A=
return(0);=0A=
}=0A=
=0A=
-/* DMETA2FNAME() - returns pointer to the filename stored in parts =
in=0A=
- * the dmeta structure=0A=
- *=0A=
- * There are numerous spots in the code where this is done in an=0A=
- * inappropriate manner; this function should be used in all those=0A=
- * places <smile>!=0A=
- *=0A=
- * Returns a pointer to a region where the file name is stored; =
this=0A=
- * region is overwritten the next time this call is used.=0A=
- */=0A=
-char *dmeta2fname(struct dmeta *d_p)=0A=
-{=0A=
- static char buf[MAXPATHLEN+2];=0A=
- =0A=
- if (!d_p) {=0A=
- errno =3D EINVAL;=0A=
- return(0);=0A=
- }=0A=
-=0A=
- strcpy(buf, d_p->rd_path);=0A=
- strcat(buf, "/");=0A=
- strcat(buf, d_p->sd_path);=0A=
- strcat(buf, "/");=0A=
- strcat(buf, d_p->fname);=0A=
-=0A=
- return(buf);=0A=
-}=0A=
/*=0A=
* Local variables:=0A=
* c-indent-level: 3=0A=
diff -ur pvfs-1.6.0/mgr/meta/md_mkdir.c =
pvfs-chown/mgr/meta/md_mkdir.c=0A=
--- pvfs-1.6.0/mgr/meta/md_mkdir.c Fri Oct 25 12:33:05 2002=0A=
+++ pvfs-chown/mgr/meta/md_mkdir.c Mon Oct 27 09:05:30 2003=0A=
@@ -59,7 +59,7 @@=0A=
/* write dotfile */=0A=
fprintf(fp, "%Ld\n%d\n%d\n", dir->fs_ino, dir->dr_uid, =
dir->dr_gid);=0A=
fprintf(fp, "%07o\n%d\n%s\n", dir->dr_mode, dir->port, dir->host);=0A=
- fprintf(fp, "%s\n%s/%s\n", dir->rd_path, dir->sd_path, =
dir->fname);=0A=
+ fprintf(fp, "%s\n", dir->rd_path);=0A=
fclose(fp);=0A=
=0A=
/* return success */=0A=
diff -ur pvfs-1.6.0/mgr/meta/put_dmeta.c =
pvfs-chown/mgr/meta/put_dmeta.c=0A=
--- pvfs-1.6.0/mgr/meta/put_dmeta.c Fri Oct 25 12:33:05 2002=0A=
+++ pvfs-chown/mgr/meta/put_dmeta.c Mon Oct 27 10:06:33 2003=0A=
@@ -46,8 +46,6 @@=0A=
strcat(temp, "/.pvfsdir");=0A=
old_umask =3D umask(0033);=0A=
=0A=
- subdir =3D fname + strlen(dir->rd_path) + 1;=0A=
-=0A=
/* should use the dmeta_xxx calls */=0A=
if ((fp =3D fopen(temp, "w+")) =3D=3D 0) {=0A=
perror("Error opening dot file");=0A=
@@ -56,9 +54,9 @@=0A=
}=0A=
=0A=
/* write dotfile */=0A=
- fprintf(fp,"%Ld\n%d\n%d\n%07o\n%d\n%s\n%s\n%s\n\n",dir->fs_ino,=0A=
+ fprintf(fp,"%Ld\n%d\n%d\n%07o\n%d\n%s\n%s\n",dir->fs_ino,=0A=
dir->dr_uid,dir->dr_gid,dir->dr_mode,dir->port,dir->host,=0A=
- dir->rd_path,subdir);=0A=
+ dir->rd_path);=0A=
fclose(fp);=0A=
umask(old_umask);=0A=
=0A=
diff -ur pvfs-1.6.0/mgr/mgr.c pvfs-chown/mgr/mgr.c=0A=
--- pvfs-1.6.0/mgr/mgr.c Mon Jun 16 09:52:41 2003=0A=
+++ pvfs-chown/mgr/mgr.c Mon Oct 27 10:18:17 2003=0A=
@@ -1549,7 +1549,7 @@=0A=
* It's important to use md_mkdir() here instead of put_dmeta;=0A=
* md_mkdir() understands to use the fname portion of the dmeta=0A=
* structure as part of the name of the directory, so it will get =
the=0A=
- * sd_path field correct. put_dmeta() will not.=0A=
+ * sd_path field correct. put_dmeta() will not. -- Not anymore! =
DP=0A=
*/=0A=
if (md_mkdir(data_p, &dir) < 0) {=0A=
errno =3D ENOENT;=0A=
@@ -2022,9 +2022,11 @@=0A=
static mreq fakereq;=0A=
fsinfo_p fs_p;=0A=
dmeta dir;=0A=
+ static char rd_path[MAXPATHLEN];=0A=
=0A=
memset(&dir, 0, sizeof(dir));=0A=
memset(&fakereq, 0, sizeof(fakereq));=0A=
+ memset(rd_path, 0, MAXPATHLEN);=0A=
=0A=
/* fill in fake request for do_mount() */=0A=
if (get_dmeta(fname, &dir) < 0) {=0A=
@@ -2036,10 +2038,13 @@=0A=
if ((fs_p =3D fs_search(active_p, dir.fs_ino)) !=3D 0) =
return(fs_p);=0A=
=0A=
LOG1("quick_mount: mounting %s\n", dir.rd_path);=0A=
- /* need a / on the end of the root path - hack it - WBL */=0A=
- /* this trashes the sd_path, but that shouldn't matter */=0A=
- *(dir.sd_path-1) =3D '/';=0A=
- *(dir.sd_path) =3D '\0';=0A=
+ /* need a / on the end of the root path - use static buffer to=0A=
+ ensure long enough. Because dmeta is only good from call to=0A=
+ call, this shouldn't cause problems. -DP*/=0A=
+ strcpy(rd_path, dir.rd_path);=0A=
+ /* The memset above should have set the next character to \0 -DP =
*/=0A=
+ rd_path[strlen(rd_path)] =3D '/'; =0A=
+ dir.rd_path =3D rd_path;=0A=
fakereq.type =3D MGR_MOUNT;=0A=
fakereq.uid =3D uid;=0A=
fakereq.gid =3D gid;=0A=
diff -ur pvfs-1.6.0/utils/mkdot.c pvfs-chown/utils/mkdot.c=0A=
--- pvfs-1.6.0/utils/mkdot.c Thu Oct 24 10:09:33 2002=0A=
+++ pvfs-chown/utils/mkdot.c Mon Oct 27 10:16:53 2003=0A=
@@ -141,8 +141,8 @@=0A=
perror("Error creating stream");=0A=
exit(0);=0A=
}=0A=
- =
fprintf(fp,"%ld\n%d\n%d\n%o\n%s\n%s\n%s\n%s\n\n",sbuf.st_ino,intuid,=0A=
- intgid,intmode,port,host,rd_path,sd_path);=0A=
+ fprintf(fp,"%ld\n%d\n%d\n%o\n%s\n%s\n%s\n",sbuf.st_ino,intuid,=0A=
+ intgid,intmode,port,host,rd_path);=0A=
fprintf(stderr,"File created.\n");=0A=
=0A=
return 0;=0A=
------_=_NextPart_000_01C39CAA.1C7CA510--