| Message ID | 87oc20ew5b.fsf@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Eric Van Hensbergen |
| Headers | show |
On Tue, Jun 14, 2011 at 1:59 AM, Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote: > On Mon, 13 Jun 2011 16:12:52 -0500, Eric Van Hensbergen <ericvh@gmail.com> wrote: >> Could be wrong, but a quick regression check of your patches fails >> against a legacy server. Are you making sure the new operations are >> properly protected by a .L flag? > > How about below diff > > [3.0-pending@v9fs]$ git diff > diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c > index f40fdc3..436699e 100644 > --- a/fs/9p/vfs_inode.c > +++ b/fs/9p/vfs_inode.c > @@ -499,13 +499,15 @@ v9fs_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid, > > static int v9fs_remove(struct inode *dir, struct dentry *dentry, int flags) > { > - int retval; > struct inode *inode; > + int retval = -EOPNOTSUPP; > struct p9_fid *v9fid, *dfid; > + struct v9fs_session_info *v9ses; > > P9_DPRINTK(P9_DEBUG_VFS, "inode: %p dentry: %p rmdir: %x\n", > dir, dentry, flags); > > + v9ses = v9fs_inode2v9ses(dir); > inode = dentry->d_inode; > dfid = v9fs_fid_lookup(dentry->d_parent); > if (IS_ERR(dfid)) { > @@ -513,7 +515,8 @@ static int v9fs_remove(struct inode *dir, struct dentry *dentry, int flags) > P9_DPRINTK(P9_DEBUG_VFS, "fid lookup failed %d\n", retval); > return retval; > } > - retval = p9_client_unlinkat(dfid, dentry->d_name.name, flags); > + if (v9fs_proto_dotl(v9ses)) > + retval = p9_client_unlinkat(dfid, dentry->d_name.name, flags); > if (retval == -EOPNOTSUPP) { > /* Try the one based on path */ > v9fid = v9fs_fid_clone(dentry); > Looks right. > > related to renameat i have updated to check for -EOPNOTSUPP instead of > -ENOSYS. I am not sure any other dotl server out there is returning > -ENOSYS. We haven't documented what the server should return in case it > doesn't support any specific operation. > Yeah, the real tricky thing is I'm not sure what the other 9p servers will send back since there are so many of them. I guess for dotl all we need to do is look at diod and see what they are returning (however, I'm sure they'll change if we want to make EOPNOTSUPP standard). -eric ------------------------------------------------------------------------------ EditLive Enterprise is the world's most technically advanced content authoring tool. Experience the power of Track Changes, Inline Image Editing and ensure content is compliant with Accessibility Checking. http://p.sf.net/sfu/ephox-dev2dev
On 06/14/2011 08:14 AM, Eric Van Hensbergen wrote: Another thing we may have to consider is open(2) flags. I recently learned that flags like O_DIRECT map to different bits on different architectures. So our assumption of no mapping needed for dotl is not fully accurate. So I will put out a patch introducing this mapping. [jvrao v9fs-next-upstream]$ find arch -name "fcntl.h" | xargs grep "O_DIRECT"|grep -v O_DIRECTORY arch/alpha/include/asm/fcntl.h:#define O_DIRECT 02000000 /* direct disk access - should check with OSF/1 */ arch/mips/include/asm/fcntl.h:#define O_DIRECT 0x8000 /* direct disk access hint */ arch/arm/include/asm/fcntl.h:#define O_DIRECT 0200000 /* direct disk access hint - currently ignored */ arch/m68k/include/asm/fcntl.h:#define O_DIRECT 0200000 /* direct disk access hint - currently ignored */ arch/h8300/include/asm/fcntl.h:#define O_DIRECT 0200000 /* direct disk access hint - currently ignored */ arch/powerpc/include/asm/fcntl.h:#define O_DIRECT 0400000 /* direct disk access hint */ arch/blackfin/include/asm/fcntl.h:#define O_DIRECT 0200000 /* direct disk access hint - currently ignored */ arch/sparc/include/asm/fcntl.h:#define O_DIRECT 0x100000 /* direct disk access hint */ Thanks, JV > On Tue, Jun 14, 2011 at 1:59 AM, Aneesh Kumar K.V > <aneesh.kumar@linux.vnet.ibm.com> wrote: >> On Mon, 13 Jun 2011 16:12:52 -0500, Eric Van Hensbergen<ericvh@gmail.com> wrote: >>> Could be wrong, but a quick regression check of your patches fails >>> against a legacy server. Are you making sure the new operations are >>> properly protected by a .L flag? >> How about below diff >> >> [3.0-pending@v9fs]$ git diff >> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c >> index f40fdc3..436699e 100644 >> --- a/fs/9p/vfs_inode.c >> +++ b/fs/9p/vfs_inode.c >> @@ -499,13 +499,15 @@ v9fs_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid, >> >> static int v9fs_remove(struct inode *dir, struct dentry *dentry, int flags) >> { >> - int retval; >> struct inode *inode; >> + int retval = -EOPNOTSUPP; >> struct p9_fid *v9fid, *dfid; >> + struct v9fs_session_info *v9ses; >> >> P9_DPRINTK(P9_DEBUG_VFS, "inode: %p dentry: %p rmdir: %x\n", >> dir, dentry, flags); >> >> + v9ses = v9fs_inode2v9ses(dir); >> inode = dentry->d_inode; >> dfid = v9fs_fid_lookup(dentry->d_parent); >> if (IS_ERR(dfid)) { >> @@ -513,7 +515,8 @@ static int v9fs_remove(struct inode *dir, struct dentry *dentry, int flags) >> P9_DPRINTK(P9_DEBUG_VFS, "fid lookup failed %d\n", retval); >> return retval; >> } >> - retval = p9_client_unlinkat(dfid, dentry->d_name.name, flags); >> + if (v9fs_proto_dotl(v9ses)) >> + retval = p9_client_unlinkat(dfid, dentry->d_name.name, flags); >> if (retval == -EOPNOTSUPP) { >> /* Try the one based on path */ >> v9fid = v9fs_fid_clone(dentry); >> > Looks right. >> related to renameat i have updated to check for -EOPNOTSUPP instead of >> -ENOSYS. I am not sure any other dotl server out there is returning >> -ENOSYS. We haven't documented what the server should return in case it >> doesn't support any specific operation. >> > Yeah, the real tricky thing is I'm not sure what the other 9p servers > will send back since there are so many of them. I guess for dotl all > we need to do is look at diod and see what they are returning > (however, I'm sure they'll change if we want to make EOPNOTSUPP > standard). > -eric > > ------------------------------------------------------------------------------ > EditLive Enterprise is the world's most technically advanced content > authoring tool. Experience the power of Track Changes, Inline Image > Editing and ensure content is compliant with Accessibility Checking. > http://p.sf.net/sfu/ephox-dev2dev > _______________________________________________ > V9fs-developer mailing list > V9fs-developer@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/v9fs-developer ------------------------------------------------------------------------------ EditLive Enterprise is the world's most technically advanced content authoring tool. Experience the power of Track Changes, Inline Image Editing and ensure content is compliant with Accessibility Checking. http://p.sf.net/sfu/ephox-dev2dev
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c index f40fdc3..436699e 100644 --- a/fs/9p/vfs_inode.c +++ b/fs/9p/vfs_inode.c @@ -499,13 +499,15 @@ v9fs_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid, static int v9fs_remove(struct inode *dir, struct dentry *dentry, int flags) { - int retval; struct inode *inode; + int retval = -EOPNOTSUPP; struct p9_fid *v9fid, *dfid; + struct v9fs_session_info *v9ses; P9_DPRINTK(P9_DEBUG_VFS, "inode: %p dentry: %p rmdir: %x\n", dir, dentry, flags); + v9ses = v9fs_inode2v9ses(dir); inode = dentry->d_inode; dfid = v9fs_fid_lookup(dentry->d_parent); if (IS_ERR(dfid)) { @@ -513,7 +515,8 @@ static int v9fs_remove(struct inode *dir, struct dentry *dentry, int flags) P9_DPRINTK(P9_DEBUG_VFS, "fid lookup failed %d\n", retval); return retval; } - retval = p9_client_unlinkat(dfid, dentry->d_name.name, flags); + if (v9fs_proto_dotl(v9ses)) + retval = p9_client_unlinkat(dfid, dentry->d_name.name, flags); if (retval == -EOPNOTSUPP) { /* Try the one based on path */ v9fid = v9fs_fid_clone(dentry);