2011-06-06 18:01:58

by Aneesh Kumar K.V

[permalink] [raw]
Subject: [PATCH 1/3] 9p: add 9P2000.L renameat operation

renameat - change name of file or directory

size[4] Trenameat tag[2] olddirfid[4] oldname[s] newdirfid[4] newname[s]
size[4] Rrenameat tag[2]

older Trename have the below request format

size[4] Trename tag[2] fid[4] newdirfid[4] name[s]

The rename message is used to change the name of a file, possibly moving it
to a new directory. The rename opreation is actually a directory opertation
and should ideally have olddirfid, if not we cannot represent the fid on server
with anything other than name. We will have to derive the old directory name
from fid in the Trename request.

Signed-off-by: Aneesh Kumar K.V <[email protected]>
---
fs/9p/vfs_inode.c | 9 ++++++---
include/net/9p/9p.h | 2 ++
include/net/9p/client.h | 5 ++++-
net/9p/client.c | 33 ++++++++++++++++++++++++++++++++-
4 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 7f6c677..651f577 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -862,9 +862,12 @@ v9fs_vfs_rename(struct inode *old_dir, struct dentry *old_dentry,

down_write(&v9ses->rename_sem);
if (v9fs_proto_dotl(v9ses)) {
- retval = p9_client_rename(oldfid, newdirfid,
- (char *) new_dentry->d_name.name);
- if (retval != -ENOSYS)
+ retval = p9_client_renameat(olddirfid, old_dentry->d_name.name,
+ newdirfid, new_dentry->d_name.name);
+ if (retval == -EOPNOTSUPP)
+ retval = p9_client_rename(oldfid, newdirfid,
+ new_dentry->d_name.name);
+ if (retval != -EOPNOTSUPP)
goto clunk_newdir;
}
if (old_dentry->d_parent != new_dentry->d_parent) {
diff --git a/include/net/9p/9p.h b/include/net/9p/9p.h
index 008711e..094c3dd 100644
--- a/include/net/9p/9p.h
+++ b/include/net/9p/9p.h
@@ -175,6 +175,8 @@ enum p9_msg_t {
P9_RLINK,
P9_TMKDIR = 72,
P9_RMKDIR,
+ P9_TRENAMEAT = 74,
+ P9_RRENAMEAT,
P9_TVERSION = 100,
P9_RVERSION,
P9_TAUTH = 102,
diff --git a/include/net/9p/client.h b/include/net/9p/client.h
index d26d5e9..5353cbc 100644
--- a/include/net/9p/client.h
+++ b/include/net/9p/client.h
@@ -211,7 +211,10 @@ struct p9_dirent {
};

int p9_client_statfs(struct p9_fid *fid, struct p9_rstatfs *sb);
-int p9_client_rename(struct p9_fid *fid, struct p9_fid *newdirfid, char *name);
+int p9_client_rename(struct p9_fid *fid, struct p9_fid *newdirfid,
+ const char *name);
+int p9_client_renameat(struct p9_fid *olddirfid, const char *old_name,
+ struct p9_fid *newdirfid, const char *new_name);
struct p9_client *p9_client_create(const char *dev_name, char *options);
void p9_client_destroy(struct p9_client *clnt);
void p9_client_disconnect(struct p9_client *clnt);
diff --git a/net/9p/client.c b/net/9p/client.c
index 9e3b0e6..677ae8d 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -1643,7 +1643,8 @@ error:
}
EXPORT_SYMBOL(p9_client_statfs);

-int p9_client_rename(struct p9_fid *fid, struct p9_fid *newdirfid, char *name)
+int p9_client_rename(struct p9_fid *fid,
+ struct p9_fid *newdirfid, const char *name)
{
int err;
struct p9_req_t *req;
@@ -1670,6 +1671,36 @@ error:
}
EXPORT_SYMBOL(p9_client_rename);

+int p9_client_renameat(struct p9_fid *olddirfid, const char *old_name,
+ struct p9_fid *newdirfid, const char *new_name)
+{
+ int err;
+ struct p9_req_t *req;
+ struct p9_client *clnt;
+
+ err = 0;
+ clnt = olddirfid->clnt;
+
+ P9_DPRINTK(P9_DEBUG_9P, ">>> TRENAMEAT olddirfid %d old name %s"
+ " newdirfid %d new name %s\n", olddirfid->fid, old_name,
+ newdirfid->fid, new_name);
+
+ req = p9_client_rpc(clnt, P9_TRENAMEAT, "dsds", olddirfid->fid,
+ old_name, newdirfid->fid, new_name);
+ if (IS_ERR(req)) {
+ err = PTR_ERR(req);
+ goto error;
+ }
+
+ P9_DPRINTK(P9_DEBUG_9P, "<<< RRENAMEAT newdirfid %d new name %s\n",
+ newdirfid->fid, new_name);
+
+ p9_free_req(clnt, req);
+error:
+ return err;
+}
+EXPORT_SYMBOL(p9_client_renameat);
+
/*
* An xattrwalk without @attr_name gives the fid for the lisxattr namespace
*/
--
1.7.4.1


2011-06-06 18:02:02

by Aneesh Kumar K.V

[permalink] [raw]
Subject: [PATCH 2/3] fs/9p: remove rename work around in 9p

Now that VFS does the right thing remove the work around.

Signed-off-by: Aneesh Kumar K.V <[email protected]>
---
fs/9p/vfs_inode.c | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 651f577..ded6309 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -892,11 +892,6 @@ clunk_newdir:
clear_nlink(new_inode);
else
drop_nlink(new_inode);
- /*
- * Work around vfs rename rehash bug with
- * FS_RENAME_DOES_D_MOVE
- */
- v9fs_invalidate_inode_attr(new_inode);
}
if (S_ISDIR(old_inode->i_mode)) {
if (!new_inode)
--
1.7.4.1

2011-06-06 18:02:19

by Aneesh Kumar K.V

[permalink] [raw]
Subject: [PATCH 3/3] 9p: add 9P2000.L unlinkat operation

unlinkat - Remove a directory entry

size[4] Tunlinkat tag[2] dirfid[4] name[s] flag[4]
size[4] Runlinkat tag[2]

older Tremove have the below request format

size[4] Tremove tag[2] fid[4]

The remove message is used to remove a directory entry either file or directory
The remove opreation is actually a directory opertation and should ideally have
dirfid, if not we cannot represent the fid on server with anything other than
name. We will have to derive the directory name from fid in the Tremove request.

NOTE: The operation doesn't clunk the unlink fid.

Signed-off-by: Aneesh Kumar K.V <[email protected]>
---
fs/9p/vfs_inode.c | 43 ++++++++++++++++++++++++++-----------------
include/net/9p/9p.h | 2 ++
include/net/9p/client.h | 1 +
net/9p/client.c | 23 +++++++++++++++++++++++
4 files changed, 52 insertions(+), 17 deletions(-)

diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index ded6309..f40fdc3 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -492,38 +492,47 @@ v9fs_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
/**
* v9fs_remove - helper function to remove files and directories
* @dir: directory inode that is being deleted
- * @file: dentry that is being deleted
+ * @dentry: dentry that is being deleted
* @rmdir: removing a directory
*
*/

-static int v9fs_remove(struct inode *dir, struct dentry *file, int rmdir)
+static int v9fs_remove(struct inode *dir, struct dentry *dentry, int flags)
{
int retval;
- struct p9_fid *v9fid;
- struct inode *file_inode;
-
- P9_DPRINTK(P9_DEBUG_VFS, "inode: %p dentry: %p rmdir: %d\n", dir, file,
- rmdir);
+ struct inode *inode;
+ struct p9_fid *v9fid, *dfid;

- file_inode = file->d_inode;
- v9fid = v9fs_fid_clone(file);
- if (IS_ERR(v9fid))
- return PTR_ERR(v9fid);
+ P9_DPRINTK(P9_DEBUG_VFS, "inode: %p dentry: %p rmdir: %x\n",
+ dir, dentry, flags);

- retval = p9_client_remove(v9fid);
+ inode = dentry->d_inode;
+ dfid = v9fs_fid_lookup(dentry->d_parent);
+ if (IS_ERR(dfid)) {
+ retval = PTR_ERR(dfid);
+ P9_DPRINTK(P9_DEBUG_VFS, "fid lookup failed %d\n", retval);
+ return retval;
+ }
+ retval = p9_client_unlinkat(dfid, dentry->d_name.name, flags);
+ if (retval == -EOPNOTSUPP) {
+ /* Try the one based on path */
+ v9fid = v9fs_fid_clone(dentry);
+ if (IS_ERR(v9fid))
+ return PTR_ERR(v9fid);
+ retval = p9_client_remove(v9fid);
+ }
if (!retval) {
/*
* directories on unlink should have zero
* link count
*/
- if (rmdir) {
- clear_nlink(file_inode);
+ if (flags & AT_REMOVEDIR) {
+ clear_nlink(inode);
drop_nlink(dir);
} else
- drop_nlink(file_inode);
+ drop_nlink(inode);

- v9fs_invalidate_inode_attr(file_inode);
+ v9fs_invalidate_inode_attr(inode);
v9fs_invalidate_inode_attr(dir);
}
return retval;
@@ -814,7 +823,7 @@ int v9fs_vfs_unlink(struct inode *i, struct dentry *d)

int v9fs_vfs_rmdir(struct inode *i, struct dentry *d)
{
- return v9fs_remove(i, d, 1);
+ return v9fs_remove(i, d, AT_REMOVEDIR);
}

/**
diff --git a/include/net/9p/9p.h b/include/net/9p/9p.h
index 094c3dd..fe1efa6 100644
--- a/include/net/9p/9p.h
+++ b/include/net/9p/9p.h
@@ -177,6 +177,8 @@ enum p9_msg_t {
P9_RMKDIR,
P9_TRENAMEAT = 74,
P9_RRENAMEAT,
+ P9_TUNLINKAT = 76,
+ P9_RUNLINKAT,
P9_TVERSION = 100,
P9_RVERSION,
P9_TAUTH = 102,
diff --git a/include/net/9p/client.h b/include/net/9p/client.h
index 5353cbc..976d12a 100644
--- a/include/net/9p/client.h
+++ b/include/net/9p/client.h
@@ -234,6 +234,7 @@ int p9_client_create_dotl(struct p9_fid *ofid, char *name, u32 flags, u32 mode,
int p9_client_clunk(struct p9_fid *fid);
int p9_client_fsync(struct p9_fid *fid, int datasync);
int p9_client_remove(struct p9_fid *fid);
+int p9_client_unlinkat(struct p9_fid *dfid, const char *name, int flags);
int p9_client_read(struct p9_fid *fid, char *data, char __user *udata,
u64 offset, u32 count);
int p9_client_write(struct p9_fid *fid, char *data, const char __user *udata,
diff --git a/net/9p/client.c b/net/9p/client.c
index 677ae8d..d225158 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -1281,6 +1281,29 @@ error:
}
EXPORT_SYMBOL(p9_client_remove);

+int p9_client_unlinkat(struct p9_fid *dfid, const char *name, int flags)
+{
+ int err = 0;
+ struct p9_req_t *req;
+ struct p9_client *clnt;
+
+ P9_DPRINTK(P9_DEBUG_9P, ">>> TUNLINKAT fid %d %s %d\n",
+ dfid->fid, name, flags);
+
+ clnt = dfid->clnt;
+ req = p9_client_rpc(clnt, P9_TUNLINKAT, "dsd", dfid->fid, name, flags);
+ if (IS_ERR(req)) {
+ err = PTR_ERR(req);
+ goto error;
+ }
+ P9_DPRINTK(P9_DEBUG_9P, "<<< RUNLINKAT fid %d %s\n", dfid->fid, name);
+
+ p9_free_req(clnt, req);
+error:
+ return err;
+}
+EXPORT_SYMBOL(p9_client_unlinkat);
+
int
p9_client_read(struct p9_fid *fid, char *data, char __user *udata, u64 offset,
u32 count)
--
1.7.4.1

2011-06-13 21:19:47

by Eric Van Hensbergen

[permalink] [raw]
Subject: Re: [V9fs-developer] [PATCH 3/3] 9p: add 9P2000.L unlinkat operation

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?

-eric


On Mon, Jun 6, 2011 at 1:01 PM, Aneesh Kumar K.V
<[email protected]> wrote:
> unlinkat - Remove a directory entry
>
> size[4] Tunlinkat tag[2] dirfid[4] name[s] flag[4]
> size[4] Runlinkat tag[2]
>
> older Tremove have the below request format
>
> size[4] Tremove tag[2] fid[4]
>
> The remove message is used to remove a directory entry either file or directory
> The remove opreation is actually a directory opertation and should ideally have
> dirfid, if not we cannot represent the fid on server with anything other than
> name. We will have to derive the directory name from fid in the Tremove request.
>
> NOTE: The operation doesn't clunk the unlink fid.
>
> Signed-off-by: Aneesh Kumar K.V <[email protected]>
> ---
> ?fs/9p/vfs_inode.c ? ? ? | ? 43 ++++++++++++++++++++++++++-----------------
> ?include/net/9p/9p.h ? ? | ? ?2 ++
> ?include/net/9p/client.h | ? ?1 +
> ?net/9p/client.c ? ? ? ? | ? 23 +++++++++++++++++++++++
> ?4 files changed, 52 insertions(+), 17 deletions(-)
>
> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> index ded6309..f40fdc3 100644
> --- a/fs/9p/vfs_inode.c
> +++ b/fs/9p/vfs_inode.c
> @@ -492,38 +492,47 @@ v9fs_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
> ?/**
> ?* v9fs_remove - helper function to remove files and directories
> ?* @dir: directory inode that is being deleted
> - * @file: ?dentry that is being deleted
> + * @dentry: ?dentry that is being deleted
> ?* @rmdir: removing a directory
> ?*
> ?*/
>
> -static int v9fs_remove(struct inode *dir, struct dentry *file, int rmdir)
> +static int v9fs_remove(struct inode *dir, struct dentry *dentry, int flags)
> ?{
> ? ? ? ?int retval;
> - ? ? ? struct p9_fid *v9fid;
> - ? ? ? struct inode *file_inode;
> -
> - ? ? ? P9_DPRINTK(P9_DEBUG_VFS, "inode: %p dentry: %p rmdir: %d\n", dir, file,
> - ? ? ? ? ? ? ? rmdir);
> + ? ? ? struct inode *inode;
> + ? ? ? struct p9_fid *v9fid, *dfid;
>
> - ? ? ? file_inode = file->d_inode;
> - ? ? ? v9fid = v9fs_fid_clone(file);
> - ? ? ? if (IS_ERR(v9fid))
> - ? ? ? ? ? ? ? return PTR_ERR(v9fid);
> + ? ? ? P9_DPRINTK(P9_DEBUG_VFS, "inode: %p dentry: %p rmdir: %x\n",
> + ? ? ? ? ? ? ? ? ?dir, dentry, flags);
>
> - ? ? ? retval = p9_client_remove(v9fid);
> + ? ? ? inode = dentry->d_inode;
> + ? ? ? dfid = v9fs_fid_lookup(dentry->d_parent);
> + ? ? ? if (IS_ERR(dfid)) {
> + ? ? ? ? ? ? ? retval = PTR_ERR(dfid);
> + ? ? ? ? ? ? ? P9_DPRINTK(P9_DEBUG_VFS, "fid lookup failed %d\n", retval);
> + ? ? ? ? ? ? ? return retval;
> + ? ? ? }
> + ? ? ? retval = p9_client_unlinkat(dfid, dentry->d_name.name, flags);
> + ? ? ? if (retval == -EOPNOTSUPP) {
> + ? ? ? ? ? ? ? /* Try the one based on path */
> + ? ? ? ? ? ? ? v9fid = v9fs_fid_clone(dentry);
> + ? ? ? ? ? ? ? if (IS_ERR(v9fid))
> + ? ? ? ? ? ? ? ? ? ? ? return PTR_ERR(v9fid);
> + ? ? ? ? ? ? ? retval = p9_client_remove(v9fid);
> + ? ? ? }
> ? ? ? ?if (!retval) {
> ? ? ? ? ? ? ? ?/*
> ? ? ? ? ? ? ? ? * directories on unlink should have zero
> ? ? ? ? ? ? ? ? * link count
> ? ? ? ? ? ? ? ? */
> - ? ? ? ? ? ? ? if (rmdir) {
> - ? ? ? ? ? ? ? ? ? ? ? clear_nlink(file_inode);
> + ? ? ? ? ? ? ? if (flags & AT_REMOVEDIR) {
> + ? ? ? ? ? ? ? ? ? ? ? clear_nlink(inode);
> ? ? ? ? ? ? ? ? ? ? ? ?drop_nlink(dir);
> ? ? ? ? ? ? ? ?} else
> - ? ? ? ? ? ? ? ? ? ? ? drop_nlink(file_inode);
> + ? ? ? ? ? ? ? ? ? ? ? drop_nlink(inode);
>
> - ? ? ? ? ? ? ? v9fs_invalidate_inode_attr(file_inode);
> + ? ? ? ? ? ? ? v9fs_invalidate_inode_attr(inode);
> ? ? ? ? ? ? ? ?v9fs_invalidate_inode_attr(dir);
> ? ? ? ?}
> ? ? ? ?return retval;
> @@ -814,7 +823,7 @@ int v9fs_vfs_unlink(struct inode *i, struct dentry *d)
>
> ?int v9fs_vfs_rmdir(struct inode *i, struct dentry *d)
> ?{
> - ? ? ? return v9fs_remove(i, d, 1);
> + ? ? ? return v9fs_remove(i, d, AT_REMOVEDIR);
> ?}
>
> ?/**
> diff --git a/include/net/9p/9p.h b/include/net/9p/9p.h
> index 094c3dd..fe1efa6 100644
> --- a/include/net/9p/9p.h
> +++ b/include/net/9p/9p.h
> @@ -177,6 +177,8 @@ enum p9_msg_t {
> ? ? ? ?P9_RMKDIR,
> ? ? ? ?P9_TRENAMEAT = 74,
> ? ? ? ?P9_RRENAMEAT,
> + ? ? ? P9_TUNLINKAT = 76,
> + ? ? ? P9_RUNLINKAT,
> ? ? ? ?P9_TVERSION = 100,
> ? ? ? ?P9_RVERSION,
> ? ? ? ?P9_TAUTH = 102,
> diff --git a/include/net/9p/client.h b/include/net/9p/client.h
> index 5353cbc..976d12a 100644
> --- a/include/net/9p/client.h
> +++ b/include/net/9p/client.h
> @@ -234,6 +234,7 @@ int p9_client_create_dotl(struct p9_fid *ofid, char *name, u32 flags, u32 mode,
> ?int p9_client_clunk(struct p9_fid *fid);
> ?int p9_client_fsync(struct p9_fid *fid, int datasync);
> ?int p9_client_remove(struct p9_fid *fid);
> +int p9_client_unlinkat(struct p9_fid *dfid, const char *name, int flags);
> ?int p9_client_read(struct p9_fid *fid, char *data, char __user *udata,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?u64 offset, u32 count);
> ?int p9_client_write(struct p9_fid *fid, char *data, const char __user *udata,
> diff --git a/net/9p/client.c b/net/9p/client.c
> index 677ae8d..d225158 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -1281,6 +1281,29 @@ error:
> ?}
> ?EXPORT_SYMBOL(p9_client_remove);
>
> +int p9_client_unlinkat(struct p9_fid *dfid, const char *name, int flags)
> +{
> + ? ? ? int err = 0;
> + ? ? ? struct p9_req_t *req;
> + ? ? ? struct p9_client *clnt;
> +
> + ? ? ? P9_DPRINTK(P9_DEBUG_9P, ">>> TUNLINKAT fid %d %s %d\n",
> + ? ? ? ? ? ? ? ? ?dfid->fid, name, flags);
> +
> + ? ? ? clnt = dfid->clnt;
> + ? ? ? req = p9_client_rpc(clnt, P9_TUNLINKAT, "dsd", dfid->fid, name, flags);
> + ? ? ? if (IS_ERR(req)) {
> + ? ? ? ? ? ? ? err = PTR_ERR(req);
> + ? ? ? ? ? ? ? goto error;
> + ? ? ? }
> + ? ? ? P9_DPRINTK(P9_DEBUG_9P, "<<< RUNLINKAT fid %d %s\n", dfid->fid, name);
> +
> + ? ? ? p9_free_req(clnt, req);
> +error:
> + ? ? ? return err;
> +}
> +EXPORT_SYMBOL(p9_client_unlinkat);
> +
> ?int
> ?p9_client_read(struct p9_fid *fid, char *data, char __user *udata, u64 offset,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?u32 count)
> --
> 1.7.4.1
>
>
> ------------------------------------------------------------------------------
> Simplify data backup and recovery for your virtual environment with vRanger.
> Installation's a snap, and flexible recovery options mean your data is safe,
> secure and there when you need it. Discover what all the cheering's about.
> Get your free trial download today.
> http://p.sf.net/sfu/quest-dev2dev2
> _______________________________________________
> V9fs-developer mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/v9fs-developer
>

2011-06-14 06:59:28

by Aneesh Kumar K.V

[permalink] [raw]
Subject: Re: [V9fs-developer] [PATCH 3/3] 9p: add 9P2000.L unlinkat operation

On Mon, 13 Jun 2011 16:12:52 -0500, Eric Van Hensbergen <[email protected]> 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);


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.

-aneesh

2011-06-14 15:16:26

by Eric Van Hensbergen

[permalink] [raw]
Subject: Re: [V9fs-developer] [PATCH 3/3] 9p: add 9P2000.L unlinkat operation

On Tue, Jun 14, 2011 at 1:59 AM, Aneesh Kumar K.V
<[email protected]> wrote:
> On Mon, 13 Jun 2011 16:12:52 -0500, Eric Van Hensbergen <[email protected]> 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