NFSv4: truncate returns I/O error

NFSv4: truncate returns I/O error

[Miklos Szeredi]

[-- Attachment #1: Type: text/plain, Size: 189 bytes --]

The attached test program reliably fails on an NFSv4 mount.

# mount -tnfs -onfsvers=4 127.0.0.1:/ /mnt/nfs
# ./truncate-test /mnt/nfs/tmp/xyz
truncate: Input/output error

Thanks,
Miklos


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: truncate-test.c --]
[-- Type: text/x-csrc, Size: 317 bytes --]

#include <stdio.h>
#include <unistd.h>
#include <fcntl.h>

int main(int argc, char *argv[])
{
	int res;
	char *name = argv[1];

	unlink(name);
	close(open(name, O_WRONLY | O_CREAT, 0644));
	close(open(name, O_RDONLY));

	res = truncate(name, 1);
	if (res == -1) {
		perror("truncate");
		return 1;
	}
    return 0;
}

Re: NFSv4: truncate returns I/O error

[Myklebust, Trond]

On Tue, 2012-03-06 at 11:10 +0100, Miklos Szeredi wrote:
> The attached test program reliably fails on an NFSv4 mount.
> 
> # mount -tnfs -onfsvers=4 127.0.0.1:/ /mnt/nfs
> # ./truncate-test /mnt/nfs/tmp/xyz
> truncate: Input/output error
> 
> Thanks,
> Miklos

Thanks! I'll see if I can reproduce and figure out what's going wrong.

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

[PATCH 0/2] RE: NFSv4: truncate returns I/O error

[Trond Myklebust]

Hi Miklos,

The base cause of the test failure was that the client was holding
onto a read delegation while trying to truncate the file. Whereas
some servers allow the client to do this (provided that nobody else
holds a read delegation), the Linux server does not, and returns
an error.
The fix is therefore to return the delegation if the server rejects
our attempt (see patch 2/2).

I've included patch 1/2 since it too was already queued for the
stable@vger.kernel.org queue, and so 2/2 has a dependency...

Thanks for the bug report!

Cheers
 Trond

Trond Myklebust (2):
  NFS: Properly handle the case where the delegation is revoked
  NFSv4: Return the delegation if the server returns NFS4ERR_OPENMODE

 fs/nfs/delegation.c |   11 +++++++++++
 fs/nfs/delegation.h |    1 +
 fs/nfs/nfs4_fs.h    |    3 +++
 fs/nfs/nfs4proc.c   |   31 ++++++++++++++++++++++++++++---
 fs/nfs/nfs4state.c  |   29 +++++++++++++++++++++++++++--
 5 files changed, 70 insertions(+), 5 deletions(-)

-- 
1.7.7.6

[PATCH 1/2] NFS: Properly handle the case where the delegation is revoked

[Trond Myklebust]

If we know that the delegation stateid is bad or revoked, we need to
remove that delegation as soon as possible, and then mark all the
stateids that relied on that delegation for recovery. We cannot use
the delegation as part of the recovery process.

Also note that NFSv4.1 uses a different error code (NFS4ERR_DELEG_REVOKED)
to indicate that the delegation was revoked.

Finally, ensure that setlk() and setattr() can both recover safely from
a revoked delegation.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: stable@vger.kernel.org
---
 fs/nfs/delegation.c |   11 +++++++++++
 fs/nfs/delegation.h |    1 +
 fs/nfs/nfs4_fs.h    |    2 ++
 fs/nfs/nfs4proc.c   |   18 ++++++++++++++++--
 fs/nfs/nfs4state.c  |   29 +++++++++++++++++++++++++++--
 5 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 7f26540..ac889af 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -466,6 +466,17 @@ static void nfs_delegation_run_state_manager(struct nfs_client *clp)
 		nfs4_schedule_state_manager(clp);
 }
 
+void nfs_remove_bad_delegation(struct inode *inode)
+{
+	struct nfs_delegation *delegation;
+
+	delegation = nfs_detach_delegation(NFS_I(inode), NFS_SERVER(inode));
+	if (delegation) {
+		nfs_inode_find_state_and_recover(inode, &delegation->stateid);
+		nfs_free_delegation(delegation);
+	}
+}
+
 /**
  * nfs_expire_all_delegation_types
  * @clp: client to process
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index d9322e4..691a796 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -45,6 +45,7 @@ void nfs_expire_unreferenced_delegations(struct nfs_client *clp);
 void nfs_handle_cb_pathdown(struct nfs_client *clp);
 int nfs_client_return_marked_delegations(struct nfs_client *clp);
 int nfs_delegations_present(struct nfs_client *clp);
+void nfs_remove_bad_delegation(struct inode *inode);
 
 void nfs_delegation_mark_reclaim(struct nfs_client *clp);
 void nfs_delegation_reap_unclaimed(struct nfs_client *clp);
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 4d7d0ae..5c9b20b 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -327,6 +327,8 @@ extern void nfs4_put_open_state(struct nfs4_state *);
 extern void nfs4_close_state(struct nfs4_state *, fmode_t);
 extern void nfs4_close_sync(struct nfs4_state *, fmode_t);
 extern void nfs4_state_set_mode_locked(struct nfs4_state *, fmode_t);
+extern void nfs_inode_find_state_and_recover(struct inode *inode,
+		const nfs4_stateid *stateid);
 extern void nfs4_schedule_lease_recovery(struct nfs_client *);
 extern void nfs4_schedule_state_manager(struct nfs_client *);
 extern void nfs4_schedule_path_down_recovery(struct nfs_client *clp);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index ec9f6ef..3d26bab 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -265,8 +265,11 @@ static int nfs4_handle_exception(struct nfs_server *server, int errorcode, struc
 	switch(errorcode) {
 		case 0:
 			return 0;
+		case -NFS4ERR_DELEG_REVOKED:
 		case -NFS4ERR_ADMIN_REVOKED:
 		case -NFS4ERR_BAD_STATEID:
+			if (state != NULL)
+				nfs_remove_bad_delegation(state->inode);
 		case -NFS4ERR_OPENMODE:
 			if (state == NULL)
 				break;
@@ -1319,8 +1322,11 @@ int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state
 				 * The show must go on: exit, but mark the
 				 * stateid as needing recovery.
 				 */
+			case -NFS4ERR_DELEG_REVOKED:
 			case -NFS4ERR_ADMIN_REVOKED:
 			case -NFS4ERR_BAD_STATEID:
+				nfs_inode_find_state_and_recover(state->inode,
+						stateid);
 				nfs4_schedule_stateid_recovery(server, state);
 			case -EKEYEXPIRED:
 				/*
@@ -1900,7 +1906,9 @@ static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred,
 			   struct nfs4_state *state)
 {
 	struct nfs_server *server = NFS_SERVER(inode);
-	struct nfs4_exception exception = { };
+	struct nfs4_exception exception = {
+		.state = state,
+	};
 	int err;
 	do {
 		err = nfs4_handle_exception(server,
@@ -3714,8 +3722,11 @@ nfs4_async_handle_error(struct rpc_task *task, const struct nfs_server *server,
 	if (task->tk_status >= 0)
 		return 0;
 	switch(task->tk_status) {
+		case -NFS4ERR_DELEG_REVOKED:
 		case -NFS4ERR_ADMIN_REVOKED:
 		case -NFS4ERR_BAD_STATEID:
+			if (state != NULL)
+				nfs_remove_bad_delegation(state->inode);
 		case -NFS4ERR_OPENMODE:
 			if (state == NULL)
 				break;
@@ -4533,7 +4544,9 @@ out:
 
 static int nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock *request)
 {
-	struct nfs4_exception exception = { };
+	struct nfs4_exception exception = {
+		.state = state,
+	};
 	int err;
 
 	do {
@@ -4626,6 +4639,7 @@ int nfs4_lock_delegation_recall(struct nfs4_state *state, struct file_lock *fl)
 				 * The show must go on: exit, but mark the
 				 * stateid as needing recovery.
 				 */
+			case -NFS4ERR_DELEG_REVOKED:
 			case -NFS4ERR_ADMIN_REVOKED:
 			case -NFS4ERR_BAD_STATEID:
 			case -NFS4ERR_OPENMODE:
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 4539203..0e60df1 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1132,12 +1132,37 @@ void nfs4_schedule_stateid_recovery(const struct nfs_server *server, struct nfs4
 {
 	struct nfs_client *clp = server->nfs_client;
 
-	if (test_and_clear_bit(NFS_DELEGATED_STATE, &state->flags))
-		nfs_async_inode_return_delegation(state->inode, &state->stateid);
 	nfs4_state_mark_reclaim_nograce(clp, state);
 	nfs4_schedule_state_manager(clp);
 }
 
+void nfs_inode_find_state_and_recover(struct inode *inode,
+		const nfs4_stateid *stateid)
+{
+	struct nfs_client *clp = NFS_SERVER(inode)->nfs_client;
+	struct nfs_inode *nfsi = NFS_I(inode);
+	struct nfs_open_context *ctx;
+	struct nfs4_state *state;
+	bool found = false;
+
+	spin_lock(&inode->i_lock);
+	list_for_each_entry(ctx, &nfsi->open_files, list) {
+		state = ctx->state;
+		if (state == NULL)
+			continue;
+		if (!test_bit(NFS_DELEGATED_STATE, &state->flags))
+			continue;
+		if (memcmp(state->stateid.data, stateid->data, sizeof(state->stateid.data)) != 0)
+			continue;
+		nfs4_state_mark_reclaim_nograce(clp, state);
+		found = true;
+	}
+	spin_unlock(&inode->i_lock);
+	if (found)
+		nfs4_schedule_state_manager(clp);
+}
+
+
 static int nfs4_reclaim_locks(struct nfs4_state *state, const struct nfs4_state_recovery_ops *ops)
 {
 	struct inode *inode = state->inode;
-- 
1.7.7.6

[PATCH 2/2] NFSv4: Return the delegation if the server returns NFS4ERR_OPENMODE

[Trond Myklebust]

If a setattr() fails because of an NFS4ERR_OPENMODE error, it is
probably due to us holding a read delegation. Ensure that the
recovery routines return that delegation in this case.

Reported-by: Miklos Szeredi <miklos@szeredi.hu>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: stable@vger.kernel.org
---
 fs/nfs/nfs4_fs.h  |    1 +
 fs/nfs/nfs4proc.c |   13 ++++++++++++-
 2 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 5c9b20b..8ccaf24 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -193,6 +193,7 @@ struct nfs4_exception {
 	long timeout;
 	int retry;
 	struct nfs4_state *state;
+	struct inode *inode;
 };
 
 struct nfs4_state_recovery_ops {
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 3d26bab..bfcaa03 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -259,18 +259,28 @@ static int nfs4_handle_exception(struct nfs_server *server, int errorcode, struc
 {
 	struct nfs_client *clp = server->nfs_client;
 	struct nfs4_state *state = exception->state;
+	struct inode *inode = exception->inode;
 	int ret = errorcode;
 
 	exception->retry = 0;
 	switch(errorcode) {
 		case 0:
 			return 0;
+		case -NFS4ERR_OPENMODE:
+			if (nfs_have_delegation(inode, FMODE_READ)) {
+				nfs_inode_return_delegation(inode);
+				exception->retry = 1;
+				return 0;
+			}
+			if (state == NULL)
+				break;
+			nfs4_schedule_stateid_recovery(server, state);
+			goto wait_on_recovery;
 		case -NFS4ERR_DELEG_REVOKED:
 		case -NFS4ERR_ADMIN_REVOKED:
 		case -NFS4ERR_BAD_STATEID:
 			if (state != NULL)
 				nfs_remove_bad_delegation(state->inode);
-		case -NFS4ERR_OPENMODE:
 			if (state == NULL)
 				break;
 			nfs4_schedule_stateid_recovery(server, state);
@@ -1908,6 +1918,7 @@ static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred,
 	struct nfs_server *server = NFS_SERVER(inode);
 	struct nfs4_exception exception = {
 		.state = state,
+		.inode = inode,
 	};
 	int err;
 	do {
-- 
1.7.7.6

Re: [PATCH 2/2] NFSv4: Return the delegation if the server returns NFS4ERR_OPENMODE

[Olga Kornievskaia]

wouldn't it be better for you to proactively return a read delegation
then unnecessarily erroring?

i also don't understand how this error occurs. doing a setattr in this
case you must have used a non-special stateid. the server would only
return an err_openmode if you sent the setattr with a read delegation
stateid. i guess my question is what stateid would you use that from
client's perspective represent a write-type state id but yet a server
would flag as having wrong access type?

also i'm curious why would a server, instead of returning
err_openmode, would not first recall your read delegation?

On Wed, Mar 7, 2012 at 5:40 PM, Trond Myklebust
<Trond.Myklebust@netapp.com> wrote:
> If a setattr() fails because of an NFS4ERR_OPENMODE error, it is
> probably due to us holding a read delegation. Ensure that the
> recovery routines return that delegation in this case.
>
> Reported-by: Miklos Szeredi <miklos@szeredi.hu>
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> Cc: stable@vger.kernel.org
> ---
>  fs/nfs/nfs4_fs.h  |    1 +
>  fs/nfs/nfs4proc.c |   13 ++++++++++++-
>  2 files changed, 13 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> index 5c9b20b..8ccaf24 100644
> --- a/fs/nfs/nfs4_fs.h
> +++ b/fs/nfs/nfs4_fs.h
> @@ -193,6 +193,7 @@ struct nfs4_exception {
>        long timeout;
>        int retry;
>        struct nfs4_state *state;
> +       struct inode *inode;
>  };
>
>  struct nfs4_state_recovery_ops {
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 3d26bab..bfcaa03 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -259,18 +259,28 @@ static int nfs4_handle_exception(struct nfs_server *server, int errorcode, struc
>  {
>        struct nfs_client *clp = server->nfs_client;
>        struct nfs4_state *state = exception->state;
> +       struct inode *inode = exception->inode;
>        int ret = errorcode;
>
>        exception->retry = 0;
>        switch(errorcode) {
>                case 0:
>                        return 0;
> +               case -NFS4ERR_OPENMODE:
> +                       if (nfs_have_delegation(inode, FMODE_READ)) {
> +                               nfs_inode_return_delegation(inode);
> +                               exception->retry = 1;
> +                               return 0;
> +                       }
> +                       if (state == NULL)
> +                               break;
> +                       nfs4_schedule_stateid_recovery(server, state);
> +                       goto wait_on_recovery;
>                case -NFS4ERR_DELEG_REVOKED:
>                case -NFS4ERR_ADMIN_REVOKED:
>                case -NFS4ERR_BAD_STATEID:
>                        if (state != NULL)
>                                nfs_remove_bad_delegation(state->inode);
> -               case -NFS4ERR_OPENMODE:
>                        if (state == NULL)
>                                break;
>                        nfs4_schedule_stateid_recovery(server, state);
> @@ -1908,6 +1918,7 @@ static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred,
>        struct nfs_server *server = NFS_SERVER(inode);
>        struct nfs4_exception exception = {
>                .state = state,
> +               .inode = inode,
>        };
>        int err;
>        do {
> --
> 1.7.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] NFSv4: Return the delegation if the server returns NFS4ERR_OPENMODE

[Myklebust, Trond]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1746 bytes --]

On Thu, 2012-03-08 at 12:52 -0500, Olga Kornievskaia wrote:
> wouldn't it be better for you to proactively return a read delegation
> then unnecessarily erroring?

If nobody else holds a delegation, then the NFS client is actually
allowed to keep its read delegation while writing to the file. It does
admittedly need to request an OPEN stateid for write in that case...
(See section 10.4 of RFC3530bis draft 16)

That said, in either case we do need to deal with the fact that a new
delegation may be granted after we return the old one. There is no
locking around the setattr call to prevent this.

> i also don't understand how this error occurs. doing a setattr in this
> case you must have used a non-special stateid. the server would only
> return an err_openmode if you sent the setattr with a read delegation
> stateid. i guess my question is what stateid would you use that from
> client's perspective represent a write-type state id but yet a server
> would flag as having wrong access type?

The read delegation stateid is being sent as per the prescription in
section 9.1.3.6 of RFC3530bis.

> also i'm curious why would a server, instead of returning
> err_openmode, would not first recall your read delegation?

It could, but why do so when it can just return an error value? The
presence of the delegation stateid in the SETATTR request allows it to
communicate directly to the client what the problem is without the need
for any callbacks.

Cheers
  Trond

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±û"žØ^n‡r¡ö¦zË\x1aëh™¨è­Ú&¢îý»\x05ËÛÔØï¦v¬Îf\x1dp)¹¹br	šê+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹\x1e®w¥¢¸?™¨è­Ú&¢)ߢ^[f

Re: [PATCH 2/2] NFSv4: Return the delegation if the server returns NFS4ERR_OPENMODE

[Olga Kornievskaia]

On Thu, Mar 8, 2012 at 1:15 PM, Myklebust, Trond
<Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org> wrote:
> On Thu, 2012-03-08 at 12:52 -0500, Olga Kornievskaia wrote:
>> wouldn't it be better for you to proactively return a read delegation
>> then unnecessarily erroring?
>
> If nobody else holds a delegation, then the NFS client is actually
> allowed to keep its read delegation while writing to the file. It does
> admittedly need to request an OPEN stateid for write in that case...
> (See section 10.4 of RFC3530bis draft 16)

If we both agree that there has to be a request for an open stateid for
a write, then instead of returning the read delegation if the client receives
err_openmode (when it send the request with read delegation stateid
as you said per 3560bis), can't the client resend the setattr with the open
stateid? The ordering of the stateid usage is a "should" and not a "must".

In rfc5661, it really doesn't make sense to ever send a setattr with
a read delegation stateid. According to 9.1.2, the server "MUST" return
err_open_mode" error in that case.

I gather you are in this case dealing with 4.0 delegations. But I wonder
if you'll do something else for 4.1 delegation then?

Another comment is that I was suggesting a return of delegation only
because (from what i recall) the 4.1 servers out there will recall the
previously
given read delegation in this case.

> That said, in either case we do need to deal with the fact that a new
> delegation may be granted after we return the old one. There is no
> locking around the setattr call to prevent this.

Can that really happen since we agreed that the client also has an open
stateid for write in this case?

>> i also don't understand how this error occurs. doing a setattr in this
>> case you must have used a non-special stateid. the server would only
>> return an err_openmode if you sent the setattr with a read delegation
>> stateid. i guess my question is what stateid would you use that from
>> client's perspective represent a write-type state id but yet a server
>> would flag as having wrong access type?
>
> The read delegation stateid is being sent as per the prescription in
> section 9.1.3.6 of RFC3530bis.

>> also i'm curious why would a server, instead of returning
>> err_openmode, would not first recall your read delegation?
>
> It could, but why do so when it can just return an error value? The
> presence of the delegation stateid in the SETATTR request allows it to
> communicate directly to the client what the problem is without the need
> for any callbacks.

> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org
> www.netapp.com
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] NFSv4: Return the delegation if the server returns NFS4ERR_OPENMODE

[J. Bruce Fields]

On Thu, Mar 08, 2012 at 03:23:34PM -0500, Olga Kornievskaia wrote:
> On Thu, Mar 8, 2012 at 1:15 PM, Myklebust, Trond
> <Trond.Myklebust@netapp.com> wrote:
> > On Thu, 2012-03-08 at 12:52 -0500, Olga Kornievskaia wrote:
> >> wouldn't it be better for you to proactively return a read delegation
> >> then unnecessarily erroring?
> >
> > If nobody else holds a delegation, then the NFS client is actually
> > allowed to keep its read delegation while writing to the file. It does
> > admittedly need to request an OPEN stateid for write in that case...
> > (See section 10.4 of RFC3530bis draft 16)
> 
> If we both agree that there has to be a request for an open stateid for
> a write, then instead of returning the read delegation if the client receives
> err_openmode (when it send the request with read delegation stateid
> as you said per 3560bis), can't the client resend the setattr with the open
> stateid? The ordering of the stateid usage is a "should" and not a "must".
> 
> In rfc5661, it really doesn't make sense to ever send a setattr with
> a read delegation stateid. According to 9.1.2, the server "MUST" return
> err_open_mode" error in that case.
> 
> I gather you are in this case dealing with 4.0 delegations. But I wonder
> if you'll do something else for 4.1 delegation then?

3530bis has the same language ("...must verify that the access mode
allows writing and return an NFS4ERR_OPENMODE error if it does not").

--b.

> 
> Another comment is that I was suggesting a return of delegation only
> because (from what i recall) the 4.1 servers out there will recall the
> previously
> given read delegation in this case.
> 
> > That said, in either case we do need to deal with the fact that a new
> > delegation may be granted after we return the old one. There is no
> > locking around the setattr call to prevent this.
> 
> Can that really happen since we agreed that the client also has an open
> stateid for write in this case?
> 
> >> i also don't understand how this error occurs. doing a setattr in this
> >> case you must have used a non-special stateid. the server would only
> >> return an err_openmode if you sent the setattr with a read delegation
> >> stateid. i guess my question is what stateid would you use that from
> >> client's perspective represent a write-type state id but yet a server
> >> would flag as having wrong access type?
> >
> > The read delegation stateid is being sent as per the prescription in
> > section 9.1.3.6 of RFC3530bis.
> 
> >> also i'm curious why would a server, instead of returning
> >> err_openmode, would not first recall your read delegation?
> >
> > It could, but why do so when it can just return an error value? The
> > presence of the delegation stateid in the SETATTR request allows it to
> > communicate directly to the client what the problem is without the need
> > for any callbacks.
> 
> > Trond Myklebust
> > Linux NFS client maintainer
> >
> > NetApp
> > Trond.Myklebust@netapp.com
> > www.netapp.com
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] NFSv4: Return the delegation if the server returns NFS4ERR_OPENMODE

[Myklebust, Trond]

On Thu, 2012-03-08 at 15:42 -0500, J. Bruce Fields wrote:
> On Thu, Mar 08, 2012 at 03:23:34PM -0500, Olga Kornievskaia wrote:
> > On Thu, Mar 8, 2012 at 1:15 PM, Myklebust, Trond
> > <Trond.Myklebust@netapp.com> wrote:
> > > On Thu, 2012-03-08 at 12:52 -0500, Olga Kornievskaia wrote:
> > >> wouldn't it be better for you to proactively return a read delegation
> > >> then unnecessarily erroring?
> > >
> > > If nobody else holds a delegation, then the NFS client is actually
> > > allowed to keep its read delegation while writing to the file. It does
> > > admittedly need to request an OPEN stateid for write in that case...
> > > (See section 10.4 of RFC3530bis draft 16)
> > 
> > If we both agree that there has to be a request for an open stateid for
> > a write, then instead of returning the read delegation if the client receives
> > err_openmode (when it send the request with read delegation stateid
> > as you said per 3560bis), can't the client resend the setattr with the open
> > stateid? The ordering of the stateid usage is a "should" and not a "must".
> > 
> > In rfc5661, it really doesn't make sense to ever send a setattr with
> > a read delegation stateid. According to 9.1.2, the server "MUST" return
> > err_open_mode" error in that case.
> > 
> > I gather you are in this case dealing with 4.0 delegations. But I wonder
> > if you'll do something else for 4.1 delegation then?
> 
> 3530bis has the same language ("...must verify that the access mode
> allows writing and return an NFS4ERR_OPENMODE error if it does not").

OK, so we shouldn't send the delegation stateid either for v4 or v4.1.
However should we pre-emptively return the delegation? I've been
assuming not.

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [PATCH 2/2] NFSv4: Return the delegation if the server returns NFS4ERR_OPENMODE

[J. Bruce Fields]

On Thu, Mar 08, 2012 at 08:50:14PM +0000, Myklebust, Trond wrote:
> On Thu, 2012-03-08 at 15:42 -0500, J. Bruce Fields wrote:
> > On Thu, Mar 08, 2012 at 03:23:34PM -0500, Olga Kornievskaia wrote:
> > > On Thu, Mar 8, 2012 at 1:15 PM, Myklebust, Trond
> > > <Trond.Myklebust@netapp.com> wrote:
> > > > On Thu, 2012-03-08 at 12:52 -0500, Olga Kornievskaia wrote:
> > > >> wouldn't it be better for you to proactively return a read delegation
> > > >> then unnecessarily erroring?
> > > >
> > > > If nobody else holds a delegation, then the NFS client is actually
> > > > allowed to keep its read delegation while writing to the file. It does
> > > > admittedly need to request an OPEN stateid for write in that case...
> > > > (See section 10.4 of RFC3530bis draft 16)
> > > 
> > > If we both agree that there has to be a request for an open stateid for
> > > a write, then instead of returning the read delegation if the client receives
> > > err_openmode (when it send the request with read delegation stateid
> > > as you said per 3560bis), can't the client resend the setattr with the open
> > > stateid? The ordering of the stateid usage is a "should" and not a "must".
> > > 
> > > In rfc5661, it really doesn't make sense to ever send a setattr with
> > > a read delegation stateid. According to 9.1.2, the server "MUST" return
> > > err_open_mode" error in that case.
> > > 
> > > I gather you are in this case dealing with 4.0 delegations. But I wonder
> > > if you'll do something else for 4.1 delegation then?
> > 
> > 3530bis has the same language ("...must verify that the access mode
> > allows writing and return an NFS4ERR_OPENMODE error if it does not").
> 
> OK, so we shouldn't send the delegation stateid either for v4 or v4.1.
> However should we pre-emptively return the delegation? I've been
> assuming not.

The server's only legal option is to recall it, so it seems odd not to
pre-emptively return--but as you say there's nothing to prevent the
server from then handing one right back, possibly before you get a
chance to send the setattr.

(And the linux server might well do that--maybe it should have some
heuristic not to hand out a delegation that was just returned--not so
much for this case as just because a return is a sign that the
delegation isn't useful to that client.)

--b.

Re: [PATCH 2/2] NFSv4: Return the delegation if the server returns NFS4ERR_OPENMODE

[Myklebust, Trond]

On Thu, 2012-03-08 at 15:57 -0500, J. Bruce Fields wrote:
> On Thu, Mar 08, 2012 at 08:50:14PM +0000, Myklebust, Trond wrote:
> > On Thu, 2012-03-08 at 15:42 -0500, J. Bruce Fields wrote:
> > > On Thu, Mar 08, 2012 at 03:23:34PM -0500, Olga Kornievskaia wrote:
> > > > On Thu, Mar 8, 2012 at 1:15 PM, Myklebust, Trond
> > > > <Trond.Myklebust@netapp.com> wrote:
> > > > > On Thu, 2012-03-08 at 12:52 -0500, Olga Kornievskaia wrote:
> > > > >> wouldn't it be better for you to proactively return a read delegation
> > > > >> then unnecessarily erroring?
> > > > >
> > > > > If nobody else holds a delegation, then the NFS client is actually
> > > > > allowed to keep its read delegation while writing to the file. It does
> > > > > admittedly need to request an OPEN stateid for write in that case...
> > > > > (See section 10.4 of RFC3530bis draft 16)
> > > > 
> > > > If we both agree that there has to be a request for an open stateid for
> > > > a write, then instead of returning the read delegation if the client receives
> > > > err_openmode (when it send the request with read delegation stateid
> > > > as you said per 3560bis), can't the client resend the setattr with the open
> > > > stateid? The ordering of the stateid usage is a "should" and not a "must".
> > > > 
> > > > In rfc5661, it really doesn't make sense to ever send a setattr with
> > > > a read delegation stateid. According to 9.1.2, the server "MUST" return
> > > > err_open_mode" error in that case.
> > > > 
> > > > I gather you are in this case dealing with 4.0 delegations. But I wonder
> > > > if you'll do something else for 4.1 delegation then?
> > > 
> > > 3530bis has the same language ("...must verify that the access mode
> > > allows writing and return an NFS4ERR_OPENMODE error if it does not").
> > 
> > OK, so we shouldn't send the delegation stateid either for v4 or v4.1.
> > However should we pre-emptively return the delegation? I've been
> > assuming not.
> 
> The server's only legal option is to recall it, so it seems odd not to
> pre-emptively return--but as you say there's nothing to prevent the
> server from then handing one right back, possibly before you get a
> chance to send the setattr.

Why would it need to recall it if this is the only client holding a
delegation? I agree that for the case of NFSv4 once we don't send the
stateid, the server has no way to know that this is the same client, but
the NFSv4.1 server doesn't have that limitation.

> (And the linux server might well do that--maybe it should have some
> heuristic not to hand out a delegation that was just returned--not so
> much for this case as just because a return is a sign that the
> delegation isn't useful to that client.)

Umm... An NFSv4.1 client could simply request a delegation. As I said
earlier, we don't keep tabs on what other processes are doing.

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [PATCH 2/2] NFSv4: Return the delegation if the server returns NFS4ERR_OPENMODE

[J. Bruce Fields]

On Thu, Mar 08, 2012 at 09:02:43PM +0000, Myklebust, Trond wrote:
> On Thu, 2012-03-08 at 15:57 -0500, J. Bruce Fields wrote:
> > On Thu, Mar 08, 2012 at 08:50:14PM +0000, Myklebust, Trond wrote:
> > > On Thu, 2012-03-08 at 15:42 -0500, J. Bruce Fields wrote:
> > > > On Thu, Mar 08, 2012 at 03:23:34PM -0500, Olga Kornievskaia wrote:
> > > > > On Thu, Mar 8, 2012 at 1:15 PM, Myklebust, Trond
> > > > > <Trond.Myklebust@netapp.com> wrote:
> > > > > > On Thu, 2012-03-08 at 12:52 -0500, Olga Kornievskaia wrote:
> > > > > >> wouldn't it be better for you to proactively return a read delegation
> > > > > >> then unnecessarily erroring?
> > > > > >
> > > > > > If nobody else holds a delegation, then the NFS client is actually
> > > > > > allowed to keep its read delegation while writing to the file. It does
> > > > > > admittedly need to request an OPEN stateid for write in that case...
> > > > > > (See section 10.4 of RFC3530bis draft 16)
> > > > > 
> > > > > If we both agree that there has to be a request for an open stateid for
> > > > > a write, then instead of returning the read delegation if the client receives
> > > > > err_openmode (when it send the request with read delegation stateid
> > > > > as you said per 3560bis), can't the client resend the setattr with the open
> > > > > stateid? The ordering of the stateid usage is a "should" and not a "must".
> > > > > 
> > > > > In rfc5661, it really doesn't make sense to ever send a setattr with
> > > > > a read delegation stateid. According to 9.1.2, the server "MUST" return
> > > > > err_open_mode" error in that case.
> > > > > 
> > > > > I gather you are in this case dealing with 4.0 delegations. But I wonder
> > > > > if you'll do something else for 4.1 delegation then?
> > > > 
> > > > 3530bis has the same language ("...must verify that the access mode
> > > > allows writing and return an NFS4ERR_OPENMODE error if it does not").
> > > 
> > > OK, so we shouldn't send the delegation stateid either for v4 or v4.1.
> > > However should we pre-emptively return the delegation? I've been
> > > assuming not.
> > 
> > The server's only legal option is to recall it, so it seems odd not to
> > pre-emptively return--but as you say there's nothing to prevent the
> > server from then handing one right back, possibly before you get a
> > chance to send the setattr.
> 
> Why would it need to recall it if this is the only client holding a
> delegation? I agree that for the case of NFSv4 once we don't send the
> stateid, the server has no way to know that this is the same client, but
> the NFSv4.1 server doesn't have that limitation.
> 
> > (And the linux server might well do that--maybe it should have some
> > heuristic not to hand out a delegation that was just returned--not so
> > much for this case as just because a return is a sign that the
> > delegation isn't useful to that client.)
> 
> Umm... An NFSv4.1 client could simply request a delegation. As I said
> earlier, we don't keep tabs on what other processes are doing.

You're right, I wasn't thinking about the 4.1 case: so in the 4.1 case,
the server does know which client the setattr is coming from, and has
the option not to recall.  And in 4.1 you have the option of asking for
no delegation.

So your question was whether to preemptively return in the 4.0 case?

--b.

Re: [PATCH 2/2] NFSv4: Return the delegation if the server returns NFS4ERR_OPENMODE

[Boaz Harrosh]

On 03/08/2012 12:57 PM, J. Bruce Fields wrote:
> On Thu, Mar 08, 2012 at 08:50:14PM +0000, Myklebust, Trond wrote:
>> On Thu, 2012-03-08 at 15:42 -0500, J. Bruce Fields wrote:
>>> On Thu, Mar 08, 2012 at 03:23:34PM -0500, Olga Kornievskaia wrote:
>>>> On Thu, Mar 8, 2012 at 1:15 PM, Myklebust, Trond
>>>> <Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org> wrote:
>>>>> On Thu, 2012-03-08 at 12:52 -0500, Olga Kornievskaia wrote:
>>>>>> wouldn't it be better for you to proactively return a read delegation
>>>>>> then unnecessarily erroring?
>>>>>
>>>>> If nobody else holds a delegation, then the NFS client is actually
>>>>> allowed to keep its read delegation while writing to the file. It does
>>>>> admittedly need to request an OPEN stateid for write in that case...
>>>>> (See section 10.4 of RFC3530bis draft 16)
>>>>
>>>> If we both agree that there has to be a request for an open stateid for
>>>> a write, then instead of returning the read delegation if the client receives
>>>> err_openmode (when it send the request with read delegation stateid
>>>> as you said per 3560bis), can't the client resend the setattr with the open
>>>> stateid? The ordering of the stateid usage is a "should" and not a "must".
>>>>
>>>> In rfc5661, it really doesn't make sense to ever send a setattr with
>>>> a read delegation stateid. According to 9.1.2, the server "MUST" return
>>>> err_open_mode" error in that case.
>>>>
>>>> I gather you are in this case dealing with 4.0 delegations. But I wonder
>>>> if you'll do something else for 4.1 delegation then?
>>>
>>> 3530bis has the same language ("...must verify that the access mode
>>> allows writing and return an NFS4ERR_OPENMODE error if it does not").
>>
>> OK, so we shouldn't send the delegation stateid either for v4 or v4.1.
>> However should we pre-emptively return the delegation? I've been
>> assuming not.
> 
> The server's only legal option is to recall it, so it seems odd not to
> pre-emptively return--

Also from the client that sent the setattr? I, as Trond, understood that
the read delegation must be recalled from all clients but the one
doing the setattr/write.

other wise what does it mean:
 "allowed to keep its read delegation while writing to the file"

I think the server should filter out it's global recall to dis-include
the caller.

Though I agree that the client must get a writable stateid for
setattr, and should not use it's  read-delegation-stateid

> but as you say there's nothing to prevent the
> server from then handing one right back, possibly before you get a
> chance to send the setattr.
> 

The above recall filtering will solve that. I know that in layout recalls
we have such facility, and we actually use it in RAID5 exofs.

> (And the linux server might well do that--maybe it should have some
> heuristic not to hand out a delegation that was just returned--not so
> much for this case as just because a return is a sign that the
> delegation isn't useful to that client.)
> 

Thanks
Boaz

> --b.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] NFSv4: Return the delegation if the server returns NFS4ERR_OPENMODE

[Olga Kornievskaia]

On Thu, Mar 8, 2012 at 3:50 PM, Myklebust, Trond
<Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org> wrote:
> On Thu, 2012-03-08 at 15:42 -0500, J. Bruce Fields wrote:
>> On Thu, Mar 08, 2012 at 03:23:34PM -0500, Olga Kornievskaia wrote:
>> > On Thu, Mar 8, 2012 at 1:15 PM, Myklebust, Trond
>> > <Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org> wrote:
>> > > On Thu, 2012-03-08 at 12:52 -0500, Olga Kornievskaia wrote:
>> > >> wouldn't it be better for you to proactively return a read delegation
>> > >> then unnecessarily erroring?
>> > >
>> > > If nobody else holds a delegation, then the NFS client is actually
>> > > allowed to keep its read delegation while writing to the file. It does
>> > > admittedly need to request an OPEN stateid for write in that case...
>> > > (See section 10.4 of RFC3530bis draft 16)
>> >
>> > If we both agree that there has to be a request for an open stateid for
>> > a write, then instead of returning the read delegation if the client receives
>> > err_openmode (when it send the request with read delegation stateid
>> > as you said per 3560bis), can't the client resend the setattr with the open
>> > stateid? The ordering of the stateid usage is a "should" and not a "must".
>> >
>> > In rfc5661, it really doesn't make sense to ever send a setattr with
>> > a read delegation stateid. According to 9.1.2, the server "MUST" return
>> > err_open_mode" error in that case.
>> >
>> > I gather you are in this case dealing with 4.0 delegations. But I wonder
>> > if you'll do something else for 4.1 delegation then?
>>
>> 3530bis has the same language ("...must verify that the access mode
>> allows writing and return an NFS4ERR_OPENMODE error if it does not").
>
> OK, so we shouldn't send the delegation stateid either for v4 or v4.1.
> However should we pre-emptively return the delegation? I've been
> assuming not.

It would be nice not to pre-emptively return delegations but for that
we need server implementors to get on board with the idea.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html