Skip to content

Commit 45df846

Browse files
committed
afs: Fix server list handling
Fix server list handling in the following ways: (1) In afs_alloc_volume(), remove duplicate server list build code. This was already done by afs_alloc_server_list() which afs_alloc_volume() previously called. This just results in twice as many VL RPCs. (2) In afs_deliver_vl_get_entry_by_name_u(), use the number of server records indicated by ->nServers in the UVLDB record returned by the VL.GetEntryByNameU RPC call rather than scanning all NMAXNSERVERS slots. Unused slots may contain garbage. (3) In afs_alloc_server_list(), don't stop converting a UVLDB record into a server list just because we can't look up one of the servers. Just skip that server and go on to the next. If we can't look up any of the servers then we'll fail at the end. Without this patch, an attempt to view the umich.edu root cell using something like "ls /afs/umich.edu" on a dynamic root (future patch) mount or an autocell mount will result in ENOMEDIUM. The failure is due to kafs not stopping after nServers'worth of records have been read, but then trying to access a server with a garbage UUID and getting an error, which aborts the server list build. Fixes: d2ddc77 ("afs: Overhaul volume and server record caching and fileserver rotation") Reported-by: Jonathan Billings <jsbillings@jsbillings.org> Signed-off-by: David Howells <dhowells@redhat.com> cc: stable@vger.kernel.org
1 parent 8305e57 commit 45df846

File tree

3 files changed

+11
-48
lines changed

3 files changed

+11
-48
lines changed

fs/afs/server_list.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ struct afs_server_list *afs_alloc_server_list(struct afs_cell *cell,
5858
server = afs_lookup_server(cell, key, &vldb->fs_server[i]);
5959
if (IS_ERR(server)) {
6060
ret = PTR_ERR(server);
61-
if (ret == -ENOENT)
61+
if (ret == -ENOENT ||
62+
ret == -ENOMEDIUM)
6263
continue;
6364
goto error_2;
6465
}

fs/afs/vlclient.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ static int afs_deliver_vl_get_entry_by_name_u(struct afs_call *call)
2323
struct afs_uvldbentry__xdr *uvldb;
2424
struct afs_vldb_entry *entry;
2525
bool new_only = false;
26-
u32 tmp;
26+
u32 tmp, nr_servers;
2727
int i, ret;
2828

2929
_enter("");
@@ -36,6 +36,10 @@ static int afs_deliver_vl_get_entry_by_name_u(struct afs_call *call)
3636
uvldb = call->buffer;
3737
entry = call->reply[0];
3838

39+
nr_servers = ntohl(uvldb->nServers);
40+
if (nr_servers > AFS_NMAXNSERVERS)
41+
nr_servers = AFS_NMAXNSERVERS;
42+
3943
for (i = 0; i < ARRAY_SIZE(uvldb->name) - 1; i++)
4044
entry->name[i] = (u8)ntohl(uvldb->name[i]);
4145
entry->name[i] = 0;
@@ -44,14 +48,14 @@ static int afs_deliver_vl_get_entry_by_name_u(struct afs_call *call)
4448
/* If there is a new replication site that we can use, ignore all the
4549
* sites that aren't marked as new.
4650
*/
47-
for (i = 0; i < AFS_NMAXNSERVERS; i++) {
51+
for (i = 0; i < nr_servers; i++) {
4852
tmp = ntohl(uvldb->serverFlags[i]);
4953
if (!(tmp & AFS_VLSF_DONTUSE) &&
5054
(tmp & AFS_VLSF_NEWREPSITE))
5155
new_only = true;
5256
}
5357

54-
for (i = 0; i < AFS_NMAXNSERVERS; i++) {
58+
for (i = 0; i < nr_servers; i++) {
5559
struct afs_uuid__xdr *xdr;
5660
struct afs_uuid *uuid;
5761
int j;

fs/afs/volume.c

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,8 @@ static struct afs_volume *afs_alloc_volume(struct afs_mount_params *params,
2626
unsigned long type_mask)
2727
{
2828
struct afs_server_list *slist;
29-
struct afs_server *server;
3029
struct afs_volume *volume;
31-
int ret = -ENOMEM, nr_servers = 0, i, j;
30+
int ret = -ENOMEM, nr_servers = 0, i;
3231

3332
for (i = 0; i < vldb->nr_servers; i++)
3433
if (vldb->fs_mask[i] & type_mask)
@@ -58,49 +57,8 @@ static struct afs_volume *afs_alloc_volume(struct afs_mount_params *params,
5857

5958
refcount_set(&slist->usage, 1);
6059
volume->servers = slist;
61-
62-
/* Make sure a records exists for each server this volume occupies. */
63-
for (i = 0; i < nr_servers; i++) {
64-
if (!(vldb->fs_mask[i] & type_mask))
65-
continue;
66-
67-
server = afs_lookup_server(params->cell, params->key,
68-
&vldb->fs_server[i]);
69-
if (IS_ERR(server)) {
70-
ret = PTR_ERR(server);
71-
if (ret == -ENOENT)
72-
continue;
73-
goto error_2;
74-
}
75-
76-
/* Insertion-sort by server pointer */
77-
for (j = 0; j < slist->nr_servers; j++)
78-
if (slist->servers[j].server >= server)
79-
break;
80-
if (j < slist->nr_servers) {
81-
if (slist->servers[j].server == server) {
82-
afs_put_server(params->net, server);
83-
continue;
84-
}
85-
86-
memmove(slist->servers + j + 1,
87-
slist->servers + j,
88-
(slist->nr_servers - j) * sizeof(struct afs_server_entry));
89-
}
90-
91-
slist->servers[j].server = server;
92-
slist->nr_servers++;
93-
}
94-
95-
if (slist->nr_servers == 0) {
96-
ret = -EDESTADDRREQ;
97-
goto error_2;
98-
}
99-
10060
return volume;
10161

102-
error_2:
103-
afs_put_serverlist(params->net, slist);
10462
error_1:
10563
afs_put_cell(params->net, volume->cell);
10664
kfree(volume);
@@ -328,7 +286,7 @@ static int afs_update_volume_status(struct afs_volume *volume, struct key *key)
328286

329287
/* See if the volume's server list got updated. */
330288
new = afs_alloc_server_list(volume->cell, key,
331-
vldb, (1 << volume->type));
289+
vldb, (1 << volume->type));
332290
if (IS_ERR(new)) {
333291
ret = PTR_ERR(new);
334292
goto error_vldb;

0 commit comments

Comments
 (0)