Commit e7f680f4 authored by David Howells's avatar David Howells

afs: Improve FS server rotation error handling

Improve the error handling in FS server rotation by:

 (1) Cache the latest useful error value for the fs operation as a whole in
     struct afs_fs_cursor separately from the error cached in the
     afs_addr_cursor struct.  The one in the address cursor gets clobbered
     occasionally.  Copy over the error to the fs operation only when it's
     something we'd be interested in passing to userspace.

 (2) Make it so that EDESTADDRREQ is the default that is seen only if no
     addresses are available to be accessed.

 (3) When calling utility functions, such as checking a volume status or
     probing a fileserver, don't let a successful result clobber the cached
     error in the cursor; instead, stash the result in a temporary variable
     until it has been assessed.

 (4) Don't return ETIMEDOUT or ETIME if a better error, such as
     ENETUNREACH, is already cached.

 (5) On leaving the rotation loop, turn any remote abort code into a more
     useful error than ECONNABORTED.

Fixes: d2ddc776 ("afs: Overhaul volume and server record caching and fileserver rotation")
Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
parent 12bdcf33
...@@ -318,10 +318,8 @@ bool afs_iterate_addresses(struct afs_addr_cursor *ac) ...@@ -318,10 +318,8 @@ bool afs_iterate_addresses(struct afs_addr_cursor *ac)
if (ac->index == ac->alist->nr_addrs) if (ac->index == ac->alist->nr_addrs)
ac->index = 0; ac->index = 0;
if (ac->index == ac->start) { if (ac->index == ac->start)
ac->error = -EDESTADDRREQ;
return false; return false;
}
} }
ac->begun = true; ac->begun = true;
......
...@@ -629,6 +629,7 @@ struct afs_fs_cursor { ...@@ -629,6 +629,7 @@ struct afs_fs_cursor {
unsigned int cb_break_2; /* cb_break + cb_s_break (2nd vnode) */ unsigned int cb_break_2; /* cb_break + cb_s_break (2nd vnode) */
unsigned char start; /* Initial index in server list */ unsigned char start; /* Initial index in server list */
unsigned char index; /* Number of servers tried beyond start */ unsigned char index; /* Number of servers tried beyond start */
short error;
unsigned short flags; unsigned short flags;
#define AFS_FS_CURSOR_STOP 0x0001 /* Set to cease iteration */ #define AFS_FS_CURSOR_STOP 0x0001 /* Set to cease iteration */
#define AFS_FS_CURSOR_VBUSY 0x0002 /* Set if seen VBUSY */ #define AFS_FS_CURSOR_VBUSY 0x0002 /* Set if seen VBUSY */
......
...@@ -39,9 +39,10 @@ bool afs_begin_vnode_operation(struct afs_fs_cursor *fc, struct afs_vnode *vnode ...@@ -39,9 +39,10 @@ bool afs_begin_vnode_operation(struct afs_fs_cursor *fc, struct afs_vnode *vnode
fc->vnode = vnode; fc->vnode = vnode;
fc->key = key; fc->key = key;
fc->ac.error = SHRT_MAX; fc->ac.error = SHRT_MAX;
fc->error = -EDESTADDRREQ;
if (mutex_lock_interruptible(&vnode->io_lock) < 0) { if (mutex_lock_interruptible(&vnode->io_lock) < 0) {
fc->ac.error = -EINTR; fc->error = -EINTR;
fc->flags |= AFS_FS_CURSOR_STOP; fc->flags |= AFS_FS_CURSOR_STOP;
return false; return false;
} }
...@@ -80,7 +81,7 @@ static bool afs_start_fs_iteration(struct afs_fs_cursor *fc, ...@@ -80,7 +81,7 @@ static bool afs_start_fs_iteration(struct afs_fs_cursor *fc,
* and have to return an error. * and have to return an error.
*/ */
if (fc->flags & AFS_FS_CURSOR_CUR_ONLY) { if (fc->flags & AFS_FS_CURSOR_CUR_ONLY) {
fc->ac.error = -ESTALE; fc->error = -ESTALE;
return false; return false;
} }
...@@ -127,7 +128,7 @@ static bool afs_sleep_and_retry(struct afs_fs_cursor *fc) ...@@ -127,7 +128,7 @@ static bool afs_sleep_and_retry(struct afs_fs_cursor *fc)
{ {
msleep_interruptible(1000); msleep_interruptible(1000);
if (signal_pending(current)) { if (signal_pending(current)) {
fc->ac.error = -ERESTARTSYS; fc->error = -ERESTARTSYS;
return false; return false;
} }
...@@ -143,11 +144,12 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) ...@@ -143,11 +144,12 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc)
struct afs_addr_list *alist; struct afs_addr_list *alist;
struct afs_server *server; struct afs_server *server;
struct afs_vnode *vnode = fc->vnode; struct afs_vnode *vnode = fc->vnode;
int error = fc->ac.error;
_enter("%u/%u,%u/%u,%d,%d", _enter("%u/%u,%u/%u,%d,%d",
fc->index, fc->start, fc->index, fc->start,
fc->ac.index, fc->ac.start, fc->ac.index, fc->ac.start,
fc->ac.error, fc->ac.abort_code); error, fc->ac.abort_code);
if (fc->flags & AFS_FS_CURSOR_STOP) { if (fc->flags & AFS_FS_CURSOR_STOP) {
_leave(" = f [stopped]"); _leave(" = f [stopped]");
...@@ -155,15 +157,16 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) ...@@ -155,15 +157,16 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc)
} }
/* Evaluate the result of the previous operation, if there was one. */ /* Evaluate the result of the previous operation, if there was one. */
switch (fc->ac.error) { switch (error) {
case SHRT_MAX: case SHRT_MAX:
goto start; goto start;
case 0: case 0:
default: default:
/* Success or local failure. Stop. */ /* Success or local failure. Stop. */
fc->error = error;
fc->flags |= AFS_FS_CURSOR_STOP; fc->flags |= AFS_FS_CURSOR_STOP;
_leave(" = f [okay/local %d]", fc->ac.error); _leave(" = f [okay/local %d]", error);
return false; return false;
case -ECONNABORTED: case -ECONNABORTED:
...@@ -178,7 +181,7 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) ...@@ -178,7 +181,7 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc)
* - May indicate that the fileserver couldn't attach to the vol. * - May indicate that the fileserver couldn't attach to the vol.
*/ */
if (fc->flags & AFS_FS_CURSOR_VNOVOL) { if (fc->flags & AFS_FS_CURSOR_VNOVOL) {
fc->ac.error = -EREMOTEIO; fc->error = -EREMOTEIO;
goto next_server; goto next_server;
} }
...@@ -187,12 +190,12 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) ...@@ -187,12 +190,12 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc)
write_unlock(&vnode->volume->servers_lock); write_unlock(&vnode->volume->servers_lock);
set_bit(AFS_VOLUME_NEEDS_UPDATE, &vnode->volume->flags); set_bit(AFS_VOLUME_NEEDS_UPDATE, &vnode->volume->flags);
fc->ac.error = afs_check_volume_status(vnode->volume, fc->key); error = afs_check_volume_status(vnode->volume, fc->key);
if (fc->ac.error < 0) if (error < 0)
goto failed; goto failed_set_error;
if (test_bit(AFS_VOLUME_DELETED, &vnode->volume->flags)) { if (test_bit(AFS_VOLUME_DELETED, &vnode->volume->flags)) {
fc->ac.error = -ENOMEDIUM; fc->error = -ENOMEDIUM;
goto failed; goto failed;
} }
...@@ -200,7 +203,7 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) ...@@ -200,7 +203,7 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc)
* it's the fileserver having trouble. * it's the fileserver having trouble.
*/ */
if (vnode->volume->servers == fc->server_list) { if (vnode->volume->servers == fc->server_list) {
fc->ac.error = -EREMOTEIO; fc->error = -EREMOTEIO;
goto next_server; goto next_server;
} }
...@@ -215,7 +218,7 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) ...@@ -215,7 +218,7 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc)
case VONLINE: case VONLINE:
case VDISKFULL: case VDISKFULL:
case VOVERQUOTA: case VOVERQUOTA:
fc->ac.error = afs_abort_to_error(fc->ac.abort_code); fc->error = afs_abort_to_error(fc->ac.abort_code);
goto next_server; goto next_server;
case VOFFLINE: case VOFFLINE:
...@@ -224,11 +227,11 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) ...@@ -224,11 +227,11 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc)
clear_bit(AFS_VOLUME_BUSY, &vnode->volume->flags); clear_bit(AFS_VOLUME_BUSY, &vnode->volume->flags);
} }
if (fc->flags & AFS_FS_CURSOR_NO_VSLEEP) { if (fc->flags & AFS_FS_CURSOR_NO_VSLEEP) {
fc->ac.error = -EADV; fc->error = -EADV;
goto failed; goto failed;
} }
if (fc->flags & AFS_FS_CURSOR_CUR_ONLY) { if (fc->flags & AFS_FS_CURSOR_CUR_ONLY) {
fc->ac.error = -ESTALE; fc->error = -ESTALE;
goto failed; goto failed;
} }
goto busy; goto busy;
...@@ -240,7 +243,7 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) ...@@ -240,7 +243,7 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc)
* have a file lock we need to maintain. * have a file lock we need to maintain.
*/ */
if (fc->flags & AFS_FS_CURSOR_NO_VSLEEP) { if (fc->flags & AFS_FS_CURSOR_NO_VSLEEP) {
fc->ac.error = -EBUSY; fc->error = -EBUSY;
goto failed; goto failed;
} }
if (!test_and_set_bit(AFS_VOLUME_BUSY, &vnode->volume->flags)) { if (!test_and_set_bit(AFS_VOLUME_BUSY, &vnode->volume->flags)) {
...@@ -269,16 +272,16 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) ...@@ -269,16 +272,16 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc)
* honour, just in case someone sets up a loop. * honour, just in case someone sets up a loop.
*/ */
if (fc->flags & AFS_FS_CURSOR_VMOVED) { if (fc->flags & AFS_FS_CURSOR_VMOVED) {
fc->ac.error = -EREMOTEIO; fc->error = -EREMOTEIO;
goto failed; goto failed;
} }
fc->flags |= AFS_FS_CURSOR_VMOVED; fc->flags |= AFS_FS_CURSOR_VMOVED;
set_bit(AFS_VOLUME_WAIT, &vnode->volume->flags); set_bit(AFS_VOLUME_WAIT, &vnode->volume->flags);
set_bit(AFS_VOLUME_NEEDS_UPDATE, &vnode->volume->flags); set_bit(AFS_VOLUME_NEEDS_UPDATE, &vnode->volume->flags);
fc->ac.error = afs_check_volume_status(vnode->volume, fc->key); error = afs_check_volume_status(vnode->volume, fc->key);
if (fc->ac.error < 0) if (error < 0)
goto failed; goto failed_set_error;
/* If the server list didn't change, then the VLDB is /* If the server list didn't change, then the VLDB is
* out of sync with the fileservers. This is hopefully * out of sync with the fileservers. This is hopefully
...@@ -290,7 +293,7 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) ...@@ -290,7 +293,7 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc)
* TODO: Retry a few times with sleeps. * TODO: Retry a few times with sleeps.
*/ */
if (vnode->volume->servers == fc->server_list) { if (vnode->volume->servers == fc->server_list) {
fc->ac.error = -ENOMEDIUM; fc->error = -ENOMEDIUM;
goto failed; goto failed;
} }
...@@ -299,20 +302,25 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) ...@@ -299,20 +302,25 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc)
default: default:
clear_bit(AFS_VOLUME_OFFLINE, &vnode->volume->flags); clear_bit(AFS_VOLUME_OFFLINE, &vnode->volume->flags);
clear_bit(AFS_VOLUME_BUSY, &vnode->volume->flags); clear_bit(AFS_VOLUME_BUSY, &vnode->volume->flags);
fc->ac.error = afs_abort_to_error(fc->ac.abort_code); fc->error = afs_abort_to_error(fc->ac.abort_code);
goto failed; goto failed;
} }
case -ETIMEDOUT:
case -ETIME:
if (fc->error != -EDESTADDRREQ)
goto iterate_address;
/* Fall through */
case -ENETUNREACH: case -ENETUNREACH:
case -EHOSTUNREACH: case -EHOSTUNREACH:
case -ECONNREFUSED: case -ECONNREFUSED:
case -ETIMEDOUT:
case -ETIME:
_debug("no conn"); _debug("no conn");
fc->error = error;
goto iterate_address; goto iterate_address;
case -ECONNRESET: case -ECONNRESET:
_debug("call reset"); _debug("call reset");
fc->error = error;
goto failed; goto failed;
} }
...@@ -328,9 +336,9 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) ...@@ -328,9 +336,9 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc)
/* See if we need to do an update of the volume record. Note that the /* See if we need to do an update of the volume record. Note that the
* volume may have moved or even have been deleted. * volume may have moved or even have been deleted.
*/ */
fc->ac.error = afs_check_volume_status(vnode->volume, fc->key); error = afs_check_volume_status(vnode->volume, fc->key);
if (fc->ac.error < 0) if (error < 0)
goto failed; goto failed_set_error;
if (!afs_start_fs_iteration(fc, vnode)) if (!afs_start_fs_iteration(fc, vnode))
goto failed; goto failed;
...@@ -354,10 +362,10 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) ...@@ -354,10 +362,10 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc)
* break request before we've finished decoding the reply and * break request before we've finished decoding the reply and
* installing the vnode. * installing the vnode.
*/ */
fc->ac.error = afs_register_server_cb_interest(vnode, fc->server_list, error = afs_register_server_cb_interest(vnode, fc->server_list,
fc->index); fc->index);
if (fc->ac.error < 0) if (error < 0)
goto failed; goto failed_set_error;
fc->cbi = afs_get_cb_interest(vnode->cb_interest); fc->cbi = afs_get_cb_interest(vnode->cb_interest);
...@@ -422,13 +430,14 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) ...@@ -422,13 +430,14 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc)
if (fc->flags & AFS_FS_CURSOR_VBUSY) if (fc->flags & AFS_FS_CURSOR_VBUSY)
goto restart_from_beginning; goto restart_from_beginning;
fc->ac.error = -EDESTADDRREQ;
goto failed; goto failed;
failed_set_error:
fc->error = error;
failed: failed:
fc->flags |= AFS_FS_CURSOR_STOP; fc->flags |= AFS_FS_CURSOR_STOP;
afs_end_cursor(&fc->ac); afs_end_cursor(&fc->ac);
_leave(" = f [failed %d]", fc->ac.error); _leave(" = f [failed %d]", fc->error);
return false; return false;
} }
...@@ -442,13 +451,14 @@ bool afs_select_current_fileserver(struct afs_fs_cursor *fc) ...@@ -442,13 +451,14 @@ bool afs_select_current_fileserver(struct afs_fs_cursor *fc)
struct afs_vnode *vnode = fc->vnode; struct afs_vnode *vnode = fc->vnode;
struct afs_cb_interest *cbi = vnode->cb_interest; struct afs_cb_interest *cbi = vnode->cb_interest;
struct afs_addr_list *alist; struct afs_addr_list *alist;
int error = fc->ac.error;
_enter(""); _enter("");
switch (fc->ac.error) { switch (error) {
case SHRT_MAX: case SHRT_MAX:
if (!cbi) { if (!cbi) {
fc->ac.error = -ESTALE; fc->error = -ESTALE;
fc->flags |= AFS_FS_CURSOR_STOP; fc->flags |= AFS_FS_CURSOR_STOP;
return false; return false;
} }
...@@ -461,7 +471,7 @@ bool afs_select_current_fileserver(struct afs_fs_cursor *fc) ...@@ -461,7 +471,7 @@ bool afs_select_current_fileserver(struct afs_fs_cursor *fc)
afs_get_addrlist(alist); afs_get_addrlist(alist);
read_unlock(&cbi->server->fs_lock); read_unlock(&cbi->server->fs_lock);
if (!alist) { if (!alist) {
fc->ac.error = -ESTALE; fc->error = -ESTALE;
fc->flags |= AFS_FS_CURSOR_STOP; fc->flags |= AFS_FS_CURSOR_STOP;
return false; return false;
} }
...@@ -475,11 +485,13 @@ bool afs_select_current_fileserver(struct afs_fs_cursor *fc) ...@@ -475,11 +485,13 @@ bool afs_select_current_fileserver(struct afs_fs_cursor *fc)
case 0: case 0:
default: default:
/* Success or local failure. Stop. */ /* Success or local failure. Stop. */
fc->error = error;
fc->flags |= AFS_FS_CURSOR_STOP; fc->flags |= AFS_FS_CURSOR_STOP;
_leave(" = f [okay/local %d]", fc->ac.error); _leave(" = f [okay/local %d]", error);
return false; return false;
case -ECONNABORTED: case -ECONNABORTED:
fc->error = afs_abort_to_error(fc->ac.abort_code);
fc->flags |= AFS_FS_CURSOR_STOP; fc->flags |= AFS_FS_CURSOR_STOP;
_leave(" = f [abort]"); _leave(" = f [abort]");
return false; return false;
...@@ -490,6 +502,7 @@ bool afs_select_current_fileserver(struct afs_fs_cursor *fc) ...@@ -490,6 +502,7 @@ bool afs_select_current_fileserver(struct afs_fs_cursor *fc)
case -ETIMEDOUT: case -ETIMEDOUT:
case -ETIME: case -ETIME:
_debug("no conn"); _debug("no conn");
fc->error = error;
goto iterate_address; goto iterate_address;
} }
...@@ -512,7 +525,6 @@ bool afs_select_current_fileserver(struct afs_fs_cursor *fc) ...@@ -512,7 +525,6 @@ bool afs_select_current_fileserver(struct afs_fs_cursor *fc)
int afs_end_vnode_operation(struct afs_fs_cursor *fc) int afs_end_vnode_operation(struct afs_fs_cursor *fc)
{ {
struct afs_net *net = afs_v2net(fc->vnode); struct afs_net *net = afs_v2net(fc->vnode);
int ret;
mutex_unlock(&fc->vnode->io_lock); mutex_unlock(&fc->vnode->io_lock);
...@@ -520,9 +532,8 @@ int afs_end_vnode_operation(struct afs_fs_cursor *fc) ...@@ -520,9 +532,8 @@ int afs_end_vnode_operation(struct afs_fs_cursor *fc)
afs_put_cb_interest(net, fc->cbi); afs_put_cb_interest(net, fc->cbi);
afs_put_serverlist(net, fc->server_list); afs_put_serverlist(net, fc->server_list);
ret = fc->ac.error; if (fc->error == -ECONNABORTED)
if (ret == -ECONNABORTED) fc->error = afs_abort_to_error(fc->ac.abort_code);
afs_abort_to_error(fc->ac.abort_code);
return fc->ac.error; return fc->error;
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment