Skip to content

Commit be84af5

Browse files
kofemannkofemann
committed
nfs4: accept delegation stateids for READ/WRITE
Motivation: As delegation delegations are independent from opens, the nfs server should be able to perform IO based on delegation stateids. Modification: - update FileTracker#getShareAccess that accepts open, lock and delegation stateids and extracts the access mode. - update READ/WRITE to use getShareAccess to check access mode - Convert DelegationState into a class as we need to track revocation status Result: better spec compliance Acked-by: Paul Millar Target: master
1 parent d22e64b commit be84af5

File tree

7 files changed

+164
-50
lines changed

7 files changed

+164
-50
lines changed

‎core/src/main/java/org/dcache/nfs/v4/FileTracker.java

Lines changed: 128 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,14 @@
2727
import java.util.Iterator;
2828
import java.util.List;
2929
import java.util.Map;
30+
import java.util.Objects;
3031
import java.util.concurrent.ConcurrentHashMap;
3132
import java.util.concurrent.locks.Lock;
3233
import java.util.stream.Collectors;
3334
import org.dcache.nfs.ChimeraNFSException;
3435
import org.dcache.nfs.status.BadStateidException;
3536
import org.dcache.nfs.status.DelayException;
37+
import org.dcache.nfs.status.DelegRevokedException;
3638
import org.dcache.nfs.status.InvalException;
3739
import org.dcache.nfs.status.ShareDeniedException;
3840
import org.dcache.nfs.status.StaleException;
@@ -134,12 +136,67 @@ public NFS4Client getClient() {
134136
}
135137

136138
/**
137-
* Record associated with open-delegation.
138-
* @param client
139-
* @param delegationStateid
140-
* @param delegationType
139+
* Open-delegation record
141140
*/
142-
record DelegationState(NFS4Client client, NFS4State delegationStateid, int delegationType) {
141+
static final class DelegationState {
142+
private final NFS4Client client;
143+
private final NFS4State delegationStateid;
144+
private final int delegationType;
145+
private boolean revoked;
146+
147+
/**
148+
* @param client
149+
* @param delegationStateid
150+
* @param delegationType
151+
*/
152+
DelegationState(NFS4Client client, NFS4State delegationStateid, int delegationType) {
153+
this.client = client;
154+
this.delegationStateid = delegationStateid;
155+
this.delegationType = delegationType;
156+
this.revoked = false;
157+
}
158+
159+
public NFS4Client client() {
160+
return client;
161+
}
162+
163+
public NFS4State delegationStateid() {
164+
return delegationStateid;
165+
}
166+
167+
public int delegationType() {
168+
return delegationType;
169+
}
170+
171+
public boolean revoked() {
172+
return revoked;
173+
}
174+
175+
@Override
176+
public boolean equals(Object obj) {
177+
if (obj == this) return true;
178+
if (obj == null || obj.getClass() != this.getClass()) return false;
179+
var that = (DelegationState) obj;
180+
return Objects.equals(this.client, that.client) &&
181+
Objects.equals(this.delegationStateid, that.delegationStateid) &&
182+
this.delegationType == that.delegationType &&
183+
Objects.equals(this.revoked, that.revoked);
184+
}
185+
186+
@Override
187+
public int hashCode() {
188+
return Objects.hash(client, delegationStateid, delegationType, revoked);
189+
}
190+
191+
@Override
192+
public String toString() {
193+
return "DelegationState[" +
194+
"client=" + client + ", " +
195+
"delegationStateid=" + delegationStateid + ", " +
196+
"delegationType=" + delegationType + ", " +
197+
"revoked=" + revoked + ']';
198+
}
199+
143200

144201
}
145202

@@ -215,6 +272,7 @@ public OpenRecord addOpen(NFS4Client client, StateOwner owner, Inode inode, int
215272
.reduce(0, (c, d) -> {
216273
try {
217274
d.client().getCB().cbDelegationRecall(fh, d.delegationStateid().stateid(), false);
275+
d.revoked = true;
218276
return c + 1;
219277
} catch (IOException e) {
220278
LOG.warn("Failed to recall delegation from {} : {}", d.client(), e.toString());
@@ -367,6 +425,71 @@ public void delegationReturn(NFS4Client client, stateid4 stateid, Inode inode)
367425
}
368426
}
369427

428+
/**
429+
* Get access mode for a given files, client and stateid. The state is must be either an open,
430+
* lock or delegation stateid.
431+
*
432+
* @param client nfs client who returns the delegation.
433+
* @param inode the inode of the delegated file.
434+
* @param stateid open or delegation stateid
435+
*/
436+
public int getShareAccess(NFS4Client client, Inode inode, stateid4 stateid)
437+
throws ChimeraNFSException {
438+
439+
Opaque fileId = new Opaque(inode.getFileId());
440+
Lock lock = filesLock.get(fileId);
441+
lock.lock();
442+
try {
443+
444+
switch (stateid.other[11]) {
445+
case Stateids.LOCK_STATE_ID:
446+
NFS4State lockState = client.state(stateid);
447+
stateid = lockState.getOpenState().stateid();
448+
// fall through
449+
case Stateids.OPEN_STATE_ID: {
450+
final List<OpenState> opens = files.get(fileId);
451+
452+
if (opens == null) {
453+
throw new BadStateidException("no matching open");
454+
}
455+
456+
final stateid4 openStateid = stateid;
457+
return opens.stream()
458+
.filter(s -> client.getId() == s.client.getId())
459+
.filter(s -> s.stateid.equals(openStateid))
460+
.mapToInt(OpenState::getShareAccess)
461+
.findAny()
462+
.orElseThrow(BadStateidException::new);
463+
}
464+
case Stateids.DELEGATION_STATE_ID: {
465+
466+
var fileDelegations = delegations.get(fileId);
467+
if (fileDelegations == null) {
468+
throw new BadStateidException("no delegation found");
469+
}
470+
471+
stateid4 delegationStateid = stateid;
472+
473+
var delegation = fileDelegations.stream()
474+
.filter(d -> d.client().getId().equals(client.getId()))
475+
.filter(d -> d.delegationStateid().stateid().equals(delegationStateid))
476+
.findAny()
477+
.orElseThrow(BadStateidException::new);
478+
479+
if (delegation.revoked()) {
480+
throw new DelegRevokedException();
481+
}
482+
// NOTE: as delegation types match access modes we don't convert the values.
483+
return delegation.delegationType();
484+
}
485+
486+
default:
487+
throw new BadStateidException();
488+
}
489+
} finally {
490+
lock.unlock();
491+
}
492+
}
370493

371494
/**
372495
* Remove an open from the list.
@@ -404,37 +527,6 @@ void removeOpen(Inode inode, stateid4 stateid) {
404527
}
405528
}
406529

407-
/**
408-
* Get open access type used by opened file.
409-
* @param client nfs client which performs the request.
410-
* @param inode of the opened file
411-
* @param stateid associated with the open.
412-
* @return share access typed used.
413-
* @throws BadStateidException if no open file associated with provided state id.
414-
*/
415-
public int getShareAccess(NFS4Client client, Inode inode, stateid4 stateid) throws BadStateidException {
416-
417-
Opaque fileId = new Opaque(inode.getFileId());
418-
Lock lock = filesLock.get(fileId);
419-
lock.lock();
420-
try {
421-
final List<OpenState> opens = files.get(fileId);
422-
423-
if (opens == null) {
424-
throw new BadStateidException("no matching open");
425-
}
426-
427-
return opens.stream()
428-
.filter(s -> client.getId() == s.client.getId())
429-
.filter(s -> s.stateid.equals(stateid))
430-
.mapToInt(OpenState::getShareAccess)
431-
.findFirst()
432-
.orElseThrow(BadStateidException::new);
433-
} finally {
434-
lock.unlock();
435-
}
436-
}
437-
438530
/**
439531
* Get all currently open files with associated clients. The resulting map contains file's inodes
440532
* as key and collection of nfs clients that have this file opened as a value.

‎core/src/main/java/org/dcache/nfs/v4/OperationREAD.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import java.io.IOException;
2323
import java.nio.ByteBuffer;
2424
import org.dcache.nfs.nfsstat;
25+
import org.dcache.nfs.status.OpenModeException;
26+
import org.dcache.nfs.v4.xdr.nfs4_prot;
2527
import org.dcache.nfs.v4.xdr.nfs_argop4;
2628
import org.dcache.nfs.v4.xdr.nfs_opnum4;
2729
import org.dcache.nfs.v4.xdr.READ4resok;
@@ -30,6 +32,7 @@
3032
import org.dcache.nfs.status.IsDirException;
3133
import org.dcache.nfs.status.NfsIoException;
3234
import org.dcache.nfs.v4.xdr.nfs_resop4;
35+
import org.dcache.nfs.v4.xdr.stateid4;
3336
import org.dcache.nfs.vfs.Stat;
3437
import org.slf4j.Logger;
3538
import org.slf4j.LoggerFactory;
@@ -47,6 +50,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti
4750
final READ4res res = result.opread;
4851

4952
Stat inodeStat = context.getFs().getattr(context.currentInode());
53+
stateid4 stateid = Stateids.getCurrentStateidIfNeeded(context, _args.opread.stateid);
5054

5155
if (inodeStat.type() == Stat.Type.DIRECTORY) {
5256
throw new IsDirException();
@@ -56,6 +60,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti
5660
throw new InvalException();
5761
}
5862

63+
NFS4Client client;
5964
if (context.getMinorversion() == 0) {
6065
/*
6166
* The NFSv4.0 spec requires lease renewal on READ.
@@ -64,18 +69,24 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti
6469
* With introduction of sessions in v4.1 update of the
6570
* lease time done through SEQUENCE operations.
6671
*/
67-
context.getStateHandler().updateClientLeaseTime(_args.opread.stateid);
72+
context.getStateHandler().updateClientLeaseTime(stateid);
73+
client = context.getStateHandler().getClientIdByStateId(stateid);
6874
} else {
69-
var client = context.getSession().getClient();
70-
client.state(_args.opread.stateid); // will throw BAD_STATEID if stateid is not valid
75+
client = context.getSession().getClient();
76+
}
77+
78+
var inode = context.currentInode();
79+
int shareAccess = context.getStateHandler().getFileTracker().getShareAccess(client, inode, stateid);
80+
if ((shareAccess & nfs4_prot.OPEN4_SHARE_ACCESS_READ) == 0) {
81+
throw new OpenModeException("Invalid open mode");
7182
}
7283

7384
long offset = _args.opread.offset.value;
7485
int count = _args.opread.count.value;
7586

7687
ByteBuffer buf = ByteBuffer.allocate(count);
7788

78-
int bytesReaded = context.getFs().read(context.currentInode(), buf, offset);
89+
int bytesReaded = context.getFs().read(inode, buf, offset);
7990
if (bytesReaded < 0) {
8091
throw new NfsIoException("IO not allowed");
8192
}

‎core/src/main/java/org/dcache/nfs/v4/OperationWRITE.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.dcache.nfs.status.IsDirException;
3535
import org.dcache.nfs.status.NfsIoException;
3636
import org.dcache.nfs.v4.xdr.nfs_resop4;
37+
import org.dcache.nfs.v4.xdr.stateid4;
3738
import org.dcache.nfs.vfs.Stat;
3839
import org.dcache.nfs.vfs.VirtualFileSystem;
3940
import org.slf4j.Logger;
@@ -55,6 +56,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF
5556
_args.opwrite.offset.checkOverflow(_args.opwrite.data.remaining(), "offset + length overflow");
5657

5758
Stat stat = context.getFs().getattr(context.currentInode());
59+
stateid4 stateid = Stateids.getCurrentStateidIfNeeded(context, _args.opwrite.stateid);
5860

5961
if (stat.type() == Stat.Type.DIRECTORY) {
6062
throw new IsDirException();
@@ -73,15 +75,15 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF
7375
* With introduction of sessions in v4.1 update of the
7476
* lease time done through SEQUENCE operations.
7577
*/
76-
context.getStateHandler().updateClientLeaseTime(_args.opwrite.stateid);
77-
client = context.getStateHandler().getClientIdByStateId(_args.opwrite.stateid);
78+
context.getStateHandler().updateClientLeaseTime(stateid);
79+
client = context.getStateHandler().getClientIdByStateId(stateid);
7880
} else {
7981
client = context.getSession().getClient();
8082
}
8183

8284
var inode = context.currentInode();
83-
// will throw BAD_STATEID if stateid is not valid
84-
int shareAccess = context.getStateHandler().getFileTracker().getShareAccess(client, inode, _args.opwrite.stateid);
85+
86+
int shareAccess = context.getStateHandler().getFileTracker().getShareAccess(client, inode, stateid);
8587
if ((shareAccess & nfs4_prot.OPEN4_SHARE_ACCESS_WRITE) == 0) {
8688
throw new OpenModeException("Invalid open mode");
8789
}

‎core/src/test/java/org/dcache/nfs/v4/NfsTestUtils.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.dcache.nfs.v4.xdr.nfs_argop4;
3434
import org.dcache.nfs.v4.xdr.nfs_fh4;
3535
import org.dcache.nfs.v4.xdr.nfs_resop4;
36+
import org.dcache.nfs.v4.xdr.stateid4;
3637
import org.dcache.nfs.v4.xdr.verifier4;
3738
import org.dcache.oncrpc4j.util.Bytes;
3839
import org.dcache.oncrpc4j.rpc.RpcAuth;
@@ -129,4 +130,12 @@ public static RpcCall generateRpcCall() {
129130

130131
return call;
131132
}
133+
134+
135+
public static stateid4 generateStateId() {
136+
byte[] b = new byte[12];
137+
RANDOM.nextBytes(b);
138+
b[11] = Stateids.OPEN_STATE_ID;
139+
return new stateid4(b, 1);
140+
}
132141
}

‎core/src/test/java/org/dcache/nfs/v4/OperationREADTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public void setUp() {
4747
@Test
4848
public void testLeaseUpdateForV40Client() throws UnknownHostException, ChimeraNFSException, IOException {
4949

50-
stateid4 stateid = mock(stateid4.class);
50+
stateid4 stateid = generateStateId();
5151
NFSv4StateHandler stateHandler = mock(NFSv4StateHandler.class);
5252
NFS4Client client = mock(NFS4Client.class);
5353
NFSv41Session session = mock(NFSv41Session.class);
@@ -83,7 +83,7 @@ public void testLeaseUpdateForV40Client() throws UnknownHostException, ChimeraNF
8383
@Test
8484
public void testNoLeaseUpdateForV41Client() throws UnknownHostException, ChimeraNFSException, IOException {
8585

86-
stateid4 stateid = mock(stateid4.class);
86+
stateid4 stateid = generateStateId();
8787
NFSv4StateHandler stateHandler = mock(NFSv4StateHandler.class);
8888
NFS4Client client = mock(NFS4Client.class);
8989
NFSv41Session session = mock(NFSv41Session.class);

‎core/src/test/java/org/dcache/nfs/v4/OperationWRITETest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public void setUp() {
5050
@Test
5151
public void testLeaseUpdateForV40Client() throws UnknownHostException, ChimeraNFSException, IOException {
5252

53-
stateid4 stateid = mock(stateid4.class);
53+
stateid4 stateid = generateStateId();
5454
NFSv4StateHandler stateHandler = mock(NFSv4StateHandler.class);
5555
NFS4Client client = mock(NFS4Client.class);
5656
NFSv41Session session = mock(NFSv41Session.class);
@@ -86,7 +86,7 @@ public void testLeaseUpdateForV40Client() throws UnknownHostException, ChimeraNF
8686
@Test
8787
public void testNoLeaseUpdateForV41Client() throws UnknownHostException, ChimeraNFSException, IOException {
8888

89-
stateid4 stateid = mock(stateid4.class);
89+
stateid4 stateid = generateStateId();
9090
NFSv4StateHandler stateHandler = mock(NFSv4StateHandler.class);
9191
NFS4Client client = mock(NFS4Client.class);
9292
NFSv41Session session = mock(NFSv41Session.class);
@@ -122,7 +122,7 @@ public void testNoLeaseUpdateForV41Client() throws UnknownHostException, Chimera
122122
@Test
123123
public void testReturnWriteVerifier() throws UnknownHostException, ChimeraNFSException, IOException {
124124

125-
stateid4 stateid = mock(stateid4.class);
125+
stateid4 stateid = generateStateId();
126126
NFSv4StateHandler stateHandler = mock(NFSv4StateHandler.class);
127127
NFS4Client client = mock(NFS4Client.class);
128128
NFSv41Session session = mock(NFSv41Session.class);

‎full-test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ services:
3838
noSEC7 noWRT1 noWRT11 noWRT13 noWRT14 noWRT15 noWRT18 noWRT19 noWRT1b noWRT2 \
3939
noWRT3 noWRT6a noWRT6b noWRT6c noWRT6d noWRT6f noWRT6s noWRT8 noWRT9
4040
/run-nfs4.1.sh --noinit --minorversion=2 --xml=/report/xunit-report-v41.xml nfs4j:/data all nochar nosocket noblock nofifo deleg xattr \
41-
noCOUR2 noCSESS25 noCSESS26 noCSESS27 noCSESS28 noCSESS29 noCSID3 noCSID4 noCSID9 noEID5f \
41+
noCOUR2 noCSESS25 noCSESS26 noCSESS27 noCSESS28 noCSESS29 noCSID9 noEID5f \
4242
noEID50 noOPEN31 noSEQ6 noRECC3 noSEQ7 noSEQ10b noSEQ2 noXATT11 noXATT10 noALLOC1 noALLOC2 noALLOC3
4343
volumes:
4444
- ./report:/report

0 commit comments

Comments
 (0)