From dc8d38d7855e69f7374b11aa99af1ec3faacd5e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Kohlschu=CC=88tter?= Date: Mon, 7 Jul 2025 21:55:11 +0200 Subject: [PATCH 1/3] core: Reduce amount of calls to getattr/Stat, improve vfs.read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For many operations, we currently call VirtualFileSystem#getattr(Inode), which gets a full set of "stat" metadata for a given Inode. In many cases however, we're only interested in the type of the Inode (e.g., directory, regular file, symlink, etc.). Certain VirtualFileSystem implementations may be able to retrieve that specific information significantly faster than the full metadata set. Replace calls to vfs.getattr(Inode).type() with vfs.getStatType() where applicable, and provide default implementations for backwards compatibility. Also don't retrieve Stat information at all where not required: 1. In PseudoFS.checkAccess with ACE4_READ_ATTRIBUTES, and 2. For OperationREAD, allow VirtualFileSystem to skip the Stat check (currently checking StatType but also file size), which is currently done per every call to "read": Clarify the requirement of signaling "eof reached" as per NFSv4 spec, provide a new read() method to VirtualFileSystem with a corresponding callback function, and provide a default implementation for backwards compatibility. Signed-off-by: Christian Kohlschütter --- .../java/org/dcache/nfs/v3/MountServer.java | 8 +-- .../java/org/dcache/nfs/v4/OperationLINK.java | 12 ++-- .../org/dcache/nfs/v4/OperationLOCKT.java | 2 +- .../org/dcache/nfs/v4/OperationLOOKUP.java | 6 +- .../org/dcache/nfs/v4/OperationLOOKUPP.java | 6 +- .../java/org/dcache/nfs/v4/OperationOPEN.java | 4 +- .../dcache/nfs/v4/OperationOPEN_CONFIRM.java | 7 +-- .../java/org/dcache/nfs/v4/OperationREAD.java | 31 +++------- .../org/dcache/nfs/v4/OperationREADDIR.java | 4 +- .../org/dcache/nfs/v4/OperationREADLINK.java | 4 +- .../org/dcache/nfs/v4/OperationSECINFO.java | 4 +- .../org/dcache/nfs/v4/OperationWRITE.java | 6 +- .../dcache/nfs/v4/ds/DSOperationCOMMIT.java | 6 +- .../dcache/nfs/v4/ds/DSOperationWRITE.java | 6 +- .../org/dcache/nfs/v4/xdr/READ4resok.java | 3 + .../dcache/nfs/vfs/ForwardingFileSystem.java | 10 ++++ .../java/org/dcache/nfs/vfs/PseudoFs.java | 22 ++++++- .../org/dcache/nfs/vfs/VirtualFileSystem.java | 60 +++++++++++++++++++ .../org/dcache/nfs/v3/MountServerTest.java | 1 + .../dcache/nfs/v4/OperationREADDIRTest.java | 3 + 20 files changed, 143 insertions(+), 62 deletions(-) diff --git a/core/src/main/java/org/dcache/nfs/v3/MountServer.java b/core/src/main/java/org/dcache/nfs/v3/MountServer.java index cd875a40..b64443e0 100644 --- a/core/src/main/java/org/dcache/nfs/v3/MountServer.java +++ b/core/src/main/java/org/dcache/nfs/v3/MountServer.java @@ -105,18 +105,18 @@ public mountres3 MOUNTPROC3_MNT_3(RpcCall call$, dirpath arg1) { try { Inode rootInode = path2Inode(_vfs, mountPoint); - Stat stat = _vfs.getattr(rootInode); + Stat.Type type = _vfs.getStatType(rootInode); - if (stat.type() == Stat.Type.SYMLINK) { + if (type == Stat.Type.SYMLINK) { /* * we resolve symlink only once */ String path = _vfs.readlink(rootInode); rootInode = path2Inode(_vfs, path); - stat = _vfs.getattr(rootInode); + type = _vfs.getStatType(rootInode); } - if (stat.type() != Stat.Type.DIRECTORY) { + if (type != Stat.Type.DIRECTORY) { throw new NotDirException("Path is not a directory"); } diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationLINK.java b/core/src/main/java/org/dcache/nfs/v4/OperationLINK.java index 0c53d935..21256dd7 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationLINK.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationLINK.java @@ -54,15 +54,15 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF result.oplink.resok4.cinfo.atomic = true; Inode parent = context.currentInode(); - Stat parentDirStat = context.getFs().getattr(parent); - Stat inodeStat = context.getFs().getattr(context.savedInode()); - if (parentDirStat.type() != Stat.Type.DIRECTORY) { - throw new NotDirException("Can't create a hard-link in non directory object"); + Stat.Type inodeStatType = context.getFs().getStatType(context.savedInode()); + if (inodeStatType == Stat.Type.DIRECTORY) { + throw new IsDirException("Can't hard-link a directory"); } - if (inodeStat.type() == Stat.Type.DIRECTORY) { - throw new IsDirException("Can't hard-link a directory"); + Stat parentDirStat = context.getFs().getattr(parent); + if (parentDirStat.type() != Stat.Type.DIRECTORY) { + throw new NotDirException("Can't create a hard-link in non directory object"); } result.oplink.resok4.cinfo.before = new changeid4(parentDirStat.getGeneration()); diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationLOCKT.java b/core/src/main/java/org/dcache/nfs/v4/OperationLOCKT.java index fc15e2ec..45f79219 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationLOCKT.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationLOCKT.java @@ -57,7 +57,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti throw new InvalException("zero lock len"); } - switch (context.getFs().getattr(inode).type()) { + switch (context.getFs().getStatType(inode)) { case REGULAR: // OK break; diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationLOOKUP.java b/core/src/main/java/org/dcache/nfs/v4/OperationLOOKUP.java index b6b3fdf8..401c4cf3 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationLOOKUP.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationLOOKUP.java @@ -48,12 +48,12 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF String name = NameFilter.convertName(_args.oplookup.objname.value); - Stat stat = context.getFs().getattr(context.currentInode()); - if (stat.type() == Stat.Type.SYMLINK) { + Stat.Type statType = context.getFs().getStatType(context.currentInode()); + if (statType == Stat.Type.SYMLINK) { throw new SymlinkException("parent not a symbolic link"); } - if (stat.type() != Stat.Type.DIRECTORY) { + if (statType != Stat.Type.DIRECTORY) { throw new NotDirException("parent not a directory"); } diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationLOOKUPP.java b/core/src/main/java/org/dcache/nfs/v4/OperationLOOKUPP.java index a93f6345..bd8b60d5 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationLOOKUPP.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationLOOKUPP.java @@ -46,13 +46,13 @@ public OperationLOOKUPP(nfs_argop4 args) { public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNFSException, IOException { final LOOKUPP4res res = result.oplookupp; - Stat stat = context.getFs().getattr(context.currentInode()); + Stat.Type statType = context.getFs().getStatType(context.currentInode()); - if (stat.type() == Stat.Type.SYMLINK) { + if (statType == Stat.Type.SYMLINK) { throw new SymlinkException("get parent on a symlink"); } - if (stat.type() != Stat.Type.DIRECTORY) { + if (statType != Stat.Type.DIRECTORY) { throw new NotDirException("not a directory"); } diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationOPEN.java b/core/src/main/java/org/dcache/nfs/v4/OperationOPEN.java index 1ee33dc0..e741ebd4 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationOPEN.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationOPEN.java @@ -316,8 +316,8 @@ private void checkCanAccess(CompoundContext context, Inode inode, uint32_t share throw new AccessException(); } - Stat stat = context.getFs().getattr(inode); - switch (stat.type()) { + Stat.Type statType = context.getFs().getStatType(inode); + switch (statType) { case REGULAR: // OK break; diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationOPEN_CONFIRM.java b/core/src/main/java/org/dcache/nfs/v4/OperationOPEN_CONFIRM.java index 06306c82..f4038512 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationOPEN_CONFIRM.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationOPEN_CONFIRM.java @@ -53,14 +53,13 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti throw new NotSuppException("operation OPEN_CONFIRM4 is obsolete in 4.x, x > 0"); } - Inode inode = context.currentInode(); - Stat stat = context.getFs().getattr(context.currentInode()); + Stat.Type statType = context.getFs().getStatType(context.currentInode()); - if (stat.type() == Stat.Type.DIRECTORY) { + if (statType == Stat.Type.DIRECTORY) { throw new IsDirException(); } - if (stat.type() == Stat.Type.SYMLINK) { + if (statType == Stat.Type.SYMLINK) { throw new InvalException(); } diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationREAD.java b/core/src/main/java/org/dcache/nfs/v4/OperationREAD.java index d4e118f3..649fe457 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationREAD.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationREAD.java @@ -23,9 +23,6 @@ import java.nio.ByteBuffer; import org.dcache.nfs.nfsstat; -import org.dcache.nfs.status.InvalException; -import org.dcache.nfs.status.IsDirException; -import org.dcache.nfs.status.NfsIoException; import org.dcache.nfs.status.OpenModeException; import org.dcache.nfs.v4.xdr.READ4res; import org.dcache.nfs.v4.xdr.READ4resok; @@ -34,7 +31,6 @@ import org.dcache.nfs.v4.xdr.nfs_opnum4; import org.dcache.nfs.v4.xdr.nfs_resop4; import org.dcache.nfs.v4.xdr.stateid4; -import org.dcache.nfs.vfs.Stat; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -50,17 +46,8 @@ public OperationREAD(nfs_argop4 args) { public void process(CompoundContext context, nfs_resop4 result) throws IOException { final READ4res res = result.opread; - Stat inodeStat = context.getFs().getattr(context.currentInode()); stateid4 stateid = Stateids.getCurrentStateidIfNeeded(context, _args.opread.stateid); - if (inodeStat.type() == Stat.Type.DIRECTORY) { - throw new IsDirException(); - } - - if (inodeStat.type() == Stat.Type.SYMLINK) { - throw new InvalException(); - } - NFS4Client client; if (context.getMinorversion() == 0) { /* @@ -85,19 +72,17 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti ByteBuffer buf = ByteBuffer.allocate(count); - int bytesReaded = context.getFs().read(inode, buf, offset); - if (bytesReaded < 0) { - throw new NfsIoException("IO not allowed"); - } - - buf.flip(); - res.status = nfsstat.NFS_OK; res.resok4 = new READ4resok(); + int bytesRead = context.getFs().read(inode, buf, offset, res.resok4::setEOF); - res.resok4.data = buf; - - if (offset + bytesReaded >= inodeStat.getSize()) { + if (bytesRead < 0) { + buf.clear(); res.resok4.eof = true; + } else { + buf.flip(); } + + res.status = nfsstat.NFS_OK; + res.resok4.data = buf; } } diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationREADDIR.java b/core/src/main/java/org/dcache/nfs/v4/OperationREADDIR.java index c95ecc46..4a4d11ea 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationREADDIR.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationREADDIR.java @@ -94,8 +94,8 @@ public void process(final CompoundContext context, nfs_resop4 result) throws Chi throw new BadCookieException("bad cookie : " + startValue); } - Stat stat = context.getFs().getattr(dir); - if (stat.type() != Stat.Type.DIRECTORY) { + Stat.Type statType = context.getFs().getStatType(dir); + if (statType != Stat.Type.DIRECTORY) { throw new NotDirException(); } diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationREADLINK.java b/core/src/main/java/org/dcache/nfs/v4/OperationREADLINK.java index b243ef95..7b0e9356 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationREADLINK.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationREADLINK.java @@ -47,8 +47,8 @@ public OperationREADLINK(nfs_argop4 args) { public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNFSException, IOException { final READLINK4res res = result.opreadlink; - Stat stat = context.getFs().getattr(context.currentInode()); - if (stat.type() != Stat.Type.SYMLINK) { + Stat.Type statType = context.getFs().getStatType(context.currentInode()); + if (statType != Stat.Type.SYMLINK) { throw new InvalException("not a symlink"); } diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationSECINFO.java b/core/src/main/java/org/dcache/nfs/v4/OperationSECINFO.java index b0fe18c4..03e1a6ca 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationSECINFO.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationSECINFO.java @@ -48,8 +48,8 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti final SECINFO4res res = result.opsecinfo; Inode dir = context.currentInode(); - Stat stat = context.getFs().getattr(dir); - if (stat.type() != Stat.Type.DIRECTORY) { + Stat.Type statType = context.getFs().getStatType(dir); + if (statType != Stat.Type.DIRECTORY) { throw new NotDirException(); } diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationWRITE.java b/core/src/main/java/org/dcache/nfs/v4/OperationWRITE.java index 6d15710f..f1fdb06c 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationWRITE.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationWRITE.java @@ -55,14 +55,14 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF _args.opwrite.offset.checkOverflow(_args.opwrite.data.remaining(), "offset + length overflow"); - Stat stat = context.getFs().getattr(context.currentInode()); + Stat.Type statType = context.getFs().getStatType(context.currentInode()); stateid4 stateid = Stateids.getCurrentStateidIfNeeded(context, _args.opwrite.stateid); - if (stat.type() == Stat.Type.DIRECTORY) { + if (statType == Stat.Type.DIRECTORY) { throw new IsDirException(); } - if (stat.type() == Stat.Type.SYMLINK) { + if (statType == Stat.Type.SYMLINK) { throw new InvalException("path is a symlink"); } diff --git a/core/src/main/java/org/dcache/nfs/v4/ds/DSOperationCOMMIT.java b/core/src/main/java/org/dcache/nfs/v4/ds/DSOperationCOMMIT.java index c55910e7..a4c3db57 100644 --- a/core/src/main/java/org/dcache/nfs/v4/ds/DSOperationCOMMIT.java +++ b/core/src/main/java/org/dcache/nfs/v4/ds/DSOperationCOMMIT.java @@ -56,13 +56,13 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF final COMMIT4res res = result.opcommit; if (context.getFs() != null) { Inode inode = context.currentInode(); - Stat stat = context.getFs().getattr(inode); + Stat.Type statType = context.getFs().getStatType(inode); - if (stat.type() == Stat.Type.DIRECTORY) { + if (statType == Stat.Type.DIRECTORY) { throw new IsDirException("Invalid can't commit a directory"); } - if (stat.type() != Stat.Type.REGULAR) { + if (statType != Stat.Type.REGULAR) { throw new InvalException("Invalid object type"); } diff --git a/core/src/main/java/org/dcache/nfs/v4/ds/DSOperationWRITE.java b/core/src/main/java/org/dcache/nfs/v4/ds/DSOperationWRITE.java index 7f136f8d..b268d7bd 100644 --- a/core/src/main/java/org/dcache/nfs/v4/ds/DSOperationWRITE.java +++ b/core/src/main/java/org/dcache/nfs/v4/ds/DSOperationWRITE.java @@ -62,13 +62,13 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti _args.opwrite.offset.checkOverflow(_args.opwrite.data.remaining(), "offset + length overflow"); Inode inode = context.currentInode(); - Stat stat = context.getFs().getattr(inode); + Stat.Type statType = context.getFs().getStatType(inode); - if (stat.type() == Stat.Type.DIRECTORY) { + if (statType == Stat.Type.DIRECTORY) { throw new IsDirException("Can't WRITE into a directory inode"); } - if (stat.type() != Stat.Type.REGULAR) { + if (statType != Stat.Type.REGULAR) { throw new InvalException("Invalid object type"); } diff --git a/core/src/main/java/org/dcache/nfs/v4/xdr/READ4resok.java b/core/src/main/java/org/dcache/nfs/v4/xdr/READ4resok.java index 9d11bff6..05736c2f 100644 --- a/core/src/main/java/org/dcache/nfs/v4/xdr/READ4resok.java +++ b/core/src/main/java/org/dcache/nfs/v4/xdr/READ4resok.java @@ -51,5 +51,8 @@ public void xdrDecode(XdrDecodingStream xdr) data = xdr.xdrDecodeByteBuffer(); } + public void setEOF() { + this.eof = true; + } } // End of READ4resok.java diff --git a/core/src/main/java/org/dcache/nfs/vfs/ForwardingFileSystem.java b/core/src/main/java/org/dcache/nfs/vfs/ForwardingFileSystem.java index 3ebd9b22..3996769e 100644 --- a/core/src/main/java/org/dcache/nfs/vfs/ForwardingFileSystem.java +++ b/core/src/main/java/org/dcache/nfs/vfs/ForwardingFileSystem.java @@ -104,6 +104,11 @@ public int read(Inode inode, ByteBuffer data, long offset) throws IOException { return delegate().read(inode, data, offset); } + @Override + public int read(Inode inode, ByteBuffer data, long offset, Runnable eofReached) throws IOException { + return delegate().read(inode, data, offset, eofReached); + } + @Override public String readlink(Inode inode) throws IOException { return delegate().readlink(inode); @@ -141,6 +146,11 @@ public Stat getattr(Inode inode) throws IOException { return delegate().getattr(inode); } + @Override + public Stat.Type getStatType(Inode inode) throws IOException { + return delegate().getStatType(inode); + } + @Override public void setattr(Inode inode, Stat stat) throws IOException { delegate().setattr(inode, stat); diff --git a/core/src/main/java/org/dcache/nfs/vfs/PseudoFs.java b/core/src/main/java/org/dcache/nfs/vfs/PseudoFs.java index 5e6f3ff3..a366c80d 100644 --- a/core/src/main/java/org/dcache/nfs/vfs/PseudoFs.java +++ b/core/src/main/java/org/dcache/nfs/vfs/PseudoFs.java @@ -45,6 +45,7 @@ import org.dcache.nfs.v4.acl.Acls; import org.dcache.nfs.v4.xdr.acemask4; import org.dcache.nfs.v4.xdr.nfsace4; +import org.dcache.nfs.vfs.Stat.Type; import org.dcache.oncrpc4j.rpc.RpcAuth; import org.dcache.oncrpc4j.rpc.RpcAuthType; import org.dcache.oncrpc4j.rpc.RpcCall; @@ -297,6 +298,12 @@ public int read(Inode inode, ByteBuffer data, long offset) throws IOException { return _inner.read(innerInode(inode), data, offset); } + @Override + public int read(Inode inode, ByteBuffer data, long offset, Runnable eofReached) throws IOException { + checkAccess(inode, ACE4_READ_DATA); + return _inner.read(innerInode(inode), data, offset, eofReached); + } + @Override public String readlink(Inode inode) throws IOException { checkAccess(inode, ACE4_READ_DATA); @@ -351,6 +358,12 @@ public Stat getattr(Inode inode) throws IOException { return _inner.getattr(innerInode(inode)); } + @Override + public Type getStatType(Inode inode) throws IOException { + checkAccess(inode, ACE4_READ_ATTRIBUTES); + return _inner.getStatType(innerInode(inode)); + } + @Override public void setattr(Inode inode, Stat stat) throws IOException { int mask = ACE4_WRITE_ATTRIBUTES; @@ -420,7 +433,14 @@ private Subject checkAccess(Inode inode, int requestedMask) throws IOException { } private Subject checkAccess(Inode inode, int requestedMask, boolean shouldLog) throws IOException { - return checkAccess(inode, _inner.getattr(innerInode(inode)), requestedMask, shouldLog); + Stat stat; + if (requestedMask == ACE4_READ_ATTRIBUTES) { + stat = null; // not required + } else { + stat = _inner.getattr(innerInode(inode)); + } + + return checkAccess(inode, stat, requestedMask, shouldLog); } private Subject checkAccess(Inode inode, Stat stat, int requestedMask, boolean shouldLog) throws IOException { diff --git a/core/src/main/java/org/dcache/nfs/vfs/VirtualFileSystem.java b/core/src/main/java/org/dcache/nfs/vfs/VirtualFileSystem.java index ec736d79..60f92040 100644 --- a/core/src/main/java/org/dcache/nfs/vfs/VirtualFileSystem.java +++ b/core/src/main/java/org/dcache/nfs/vfs/VirtualFileSystem.java @@ -22,9 +22,12 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.util.concurrent.CompletableFuture; +import java.util.function.Consumer; import javax.security.auth.Subject; +import org.dcache.nfs.status.InvalException; +import org.dcache.nfs.status.IsDirException; import org.dcache.nfs.status.NotSuppException; import org.dcache.nfs.v4.NfsIdMapping; import org.dcache.nfs.v4.xdr.nfsace4; @@ -194,6 +197,7 @@ public interface VirtualFileSystem { * @param offset file's position to read from. * @return number of bytes read from the file, possibly zero. -1 if EOF is reached. * @throws IOException + * @see {@link #read(Inode, ByteBuffer, long, Runnable)} */ default int read(Inode inode, ByteBuffer data, long offset) throws IOException { ByteBuffer buf = ByteBuffer.allocate(data.remaining()); @@ -205,6 +209,47 @@ default int read(Inode inode, ByteBuffer data, long offset) throws IOException { return n; } + /** + * Read data from file with a given inode into {@code data}. + *

+ * Note that the NFSv4 specification requires to detect the "end of file" slightly different from the Java + * semantics. If we have read all bytes of a file, then we should signal "end of file" immediately instead of + * waiting for another call where we return {@code -1}. If we detect such a case (by default, we compare the offset + * and the number of bytes read to the size returned by {@link Stat#getSize()} from {@link #getattr(Inode)}), then + * we can notify the NFS server via the given callback function. + *

+ * In practice, depending on your NFS clients and usage scenario, calling {@code eofReached} may be optional -- in + * the event that exactly the remaining number of bytes have been read, a real-world NFS client will simply attempt + * to read more bytes in a subsequent operation and then encounter the EOF via {@code read} returning {@code -1}. + * That would allow a {@link VirtualFileSystem} implementation to completely skip retrieving {@link Stat} + * information via {@link #getattr(Inode)}), which may incur a higher, recurring cost than the inconvenience of a + * single additional client roundtrip at the end of the file. + * + * @param inode inode of the file to read from. + * @param data byte array for writing. + * @param offset file's position to read from. + * @param eofReached a non-blocking callback to indicate that the end of the file was just reached (particularly, + * when a subsequent call would return {@code -1}). + * @return number of bytes read from the file, possibly zero. -1 if EOF is reached. + * @throws IOException + */ + default int read(Inode inode, ByteBuffer data, long offset, Runnable eofReached) throws IOException { + Stat stat = getattr(inode); + + Stat.Type statType = stat.type(); + if (statType == Stat.Type.DIRECTORY) { + throw new IsDirException(); + } else if (statType == Stat.Type.SYMLINK) { + throw new InvalException(); + } + + int numRead = read(inode, data, offset); + if (numRead >= 0 && (offset + numRead) >= stat.getSize()) { + eofReached.run(); + } + return numRead; + } + /** * Get value of a symbolic link object. * @@ -291,6 +336,21 @@ default WriteResult write(Inode inode, ByteBuffer data, long offset, StabilityLe */ Stat getattr(Inode inode) throws IOException; + /** + * Gets the type of an inode. + *

+ * This is equivalent to calling {@code getattr(inode).type()}), except that some implementations may choose to + * optimize this code path. Note that it is permissible to return a result for a stale inode, i.e., unlike calling + * {@link #getattr(Inode)}, no actual "stat" call needs to be made. + * + * @param inode inode of the file system object. + * @return The type. + * @throws IOException on error. + */ + default Stat.Type getStatType(Inode inode) throws IOException { + return getattr(inode).type(); + } + /** * Set/update file system object's attributes. * diff --git a/core/src/test/java/org/dcache/nfs/v3/MountServerTest.java b/core/src/test/java/org/dcache/nfs/v3/MountServerTest.java index 58550c52..be9b2adf 100644 --- a/core/src/test/java/org/dcache/nfs/v3/MountServerTest.java +++ b/core/src/test/java/org/dcache/nfs/v3/MountServerTest.java @@ -141,6 +141,7 @@ MountServertestHelper withPath(String path) throws IOException { given(_fs.getRootInode()).willReturn(inode); given(_fs.getattr(inode)).willReturn(stat); + given(_fs.getStatType(any())).willCallRealMethod(); for (String pathElement : splitter.split(path)) { Inode objectInode = mock(Inode.class); diff --git a/core/src/test/java/org/dcache/nfs/v4/OperationREADDIRTest.java b/core/src/test/java/org/dcache/nfs/v4/OperationREADDIRTest.java index a0cfc5c2..311540ec 100644 --- a/core/src/test/java/org/dcache/nfs/v4/OperationREADDIRTest.java +++ b/core/src/test/java/org/dcache/nfs/v4/OperationREADDIRTest.java @@ -10,6 +10,8 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.*; import java.io.IOException; @@ -75,6 +77,7 @@ public void setup() throws Exception { dirStat.setSize(512); vfs = mock(VirtualFileSystem.class); // the vfs serving it when(vfs.getattr(eq(dirInode))).thenReturn(dirStat); + given(vfs.getStatType(any())).willCallRealMethod(); // default implementation calls getattr result = nfs_resop4.resopFor(nfs_opnum4.OP_READDIR); context = new CompoundContextBuilder() From aa121219412dae55de526e98d38a864d7f270f0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Kohlschu=CC=88tter?= Date: Mon, 7 Jul 2025 22:24:55 +0200 Subject: [PATCH 2/3] pom: Update mockito-core dependency to 5.18.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christian Kohlschütter --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 6a24c8c8..b3c390c0 100644 --- a/pom.xml +++ b/pom.xml @@ -159,7 +159,7 @@ org.mockito mockito-core - 2.28.2 + 5.18.0 test From 10393e430419baf5cd0bc4110d4e27ddea660dbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Kohlschu=CC=88tter?= Date: Mon, 14 Jul 2025 12:03:53 +0200 Subject: [PATCH 3/3] core: Replace VirtualFileSystem.getStatType with getattr EnumSet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As per request, let's handle getting the Stat type not as a special case but via a generalizable getattr method that takes an EnumSet of StatAttributes. Signed-off-by: Christian Kohlschütter --- .../java/org/dcache/nfs/v3/MountServer.java | 4 +- .../java/org/dcache/nfs/v4/OperationLINK.java | 2 +- .../org/dcache/nfs/v4/OperationLOCKT.java | 3 +- .../org/dcache/nfs/v4/OperationLOOKUP.java | 2 +- .../org/dcache/nfs/v4/OperationLOOKUPP.java | 2 +- .../java/org/dcache/nfs/v4/OperationOPEN.java | 2 +- .../dcache/nfs/v4/OperationOPEN_CONFIRM.java | 3 +- .../org/dcache/nfs/v4/OperationREADDIR.java | 2 +- .../org/dcache/nfs/v4/OperationREADLINK.java | 2 +- .../org/dcache/nfs/v4/OperationSECINFO.java | 2 +- .../org/dcache/nfs/v4/OperationWRITE.java | 2 +- .../dcache/nfs/v4/ds/DSOperationCOMMIT.java | 2 +- .../dcache/nfs/v4/ds/DSOperationWRITE.java | 2 +- .../dcache/nfs/vfs/ForwardingFileSystem.java | 6 ++- .../java/org/dcache/nfs/vfs/PseudoFs.java | 45 ++++++++++++++++--- .../main/java/org/dcache/nfs/vfs/Stat.java | 6 +++ .../org/dcache/nfs/vfs/VirtualFileSystem.java | 17 +++---- .../org/dcache/nfs/v3/MountServerTest.java | 2 +- .../dcache/nfs/v4/OperationREADDIRTest.java | 2 +- .../org/dcache/nfs/v4/OperationWRITETest.java | 3 ++ 20 files changed, 75 insertions(+), 36 deletions(-) diff --git a/core/src/main/java/org/dcache/nfs/v3/MountServer.java b/core/src/main/java/org/dcache/nfs/v3/MountServer.java index b64443e0..ef3b2d7c 100644 --- a/core/src/main/java/org/dcache/nfs/v3/MountServer.java +++ b/core/src/main/java/org/dcache/nfs/v3/MountServer.java @@ -105,7 +105,7 @@ public mountres3 MOUNTPROC3_MNT_3(RpcCall call$, dirpath arg1) { try { Inode rootInode = path2Inode(_vfs, mountPoint); - Stat.Type type = _vfs.getStatType(rootInode); + Stat.Type type = _vfs.getattr(rootInode, Stat.STAT_ATTRIBUTES_TYPE_ONLY).type(); if (type == Stat.Type.SYMLINK) { /* @@ -113,7 +113,7 @@ public mountres3 MOUNTPROC3_MNT_3(RpcCall call$, dirpath arg1) { */ String path = _vfs.readlink(rootInode); rootInode = path2Inode(_vfs, path); - type = _vfs.getStatType(rootInode); + type = _vfs.getattr(rootInode, Stat.STAT_ATTRIBUTES_TYPE_ONLY).type(); } if (type != Stat.Type.DIRECTORY) { diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationLINK.java b/core/src/main/java/org/dcache/nfs/v4/OperationLINK.java index 21256dd7..df1e5ff7 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationLINK.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationLINK.java @@ -55,7 +55,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF Inode parent = context.currentInode(); - Stat.Type inodeStatType = context.getFs().getStatType(context.savedInode()); + Stat.Type inodeStatType = context.getFs().getattr(context.savedInode(), Stat.STAT_ATTRIBUTES_TYPE_ONLY).type(); if (inodeStatType == Stat.Type.DIRECTORY) { throw new IsDirException("Can't hard-link a directory"); } diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationLOCKT.java b/core/src/main/java/org/dcache/nfs/v4/OperationLOCKT.java index 45f79219..63613353 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationLOCKT.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationLOCKT.java @@ -37,6 +37,7 @@ import org.dcache.nfs.v4.xdr.offset4; import org.dcache.nfs.v4.xdr.state_owner4; import org.dcache.nfs.vfs.Inode; +import org.dcache.nfs.vfs.Stat; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -57,7 +58,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti throw new InvalException("zero lock len"); } - switch (context.getFs().getStatType(inode)) { + switch (context.getFs().getattr(inode, Stat.STAT_ATTRIBUTES_TYPE_ONLY).type()) { case REGULAR: // OK break; diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationLOOKUP.java b/core/src/main/java/org/dcache/nfs/v4/OperationLOOKUP.java index 401c4cf3..b8281a7b 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationLOOKUP.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationLOOKUP.java @@ -48,7 +48,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF String name = NameFilter.convertName(_args.oplookup.objname.value); - Stat.Type statType = context.getFs().getStatType(context.currentInode()); + Stat.Type statType = context.getFs().getattr(context.currentInode(), Stat.STAT_ATTRIBUTES_TYPE_ONLY).type(); if (statType == Stat.Type.SYMLINK) { throw new SymlinkException("parent not a symbolic link"); } diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationLOOKUPP.java b/core/src/main/java/org/dcache/nfs/v4/OperationLOOKUPP.java index bd8b60d5..2216bcb2 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationLOOKUPP.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationLOOKUPP.java @@ -46,7 +46,7 @@ public OperationLOOKUPP(nfs_argop4 args) { public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNFSException, IOException { final LOOKUPP4res res = result.oplookupp; - Stat.Type statType = context.getFs().getStatType(context.currentInode()); + Stat.Type statType = context.getFs().getattr(context.currentInode(), Stat.STAT_ATTRIBUTES_TYPE_ONLY).type(); if (statType == Stat.Type.SYMLINK) { throw new SymlinkException("get parent on a symlink"); diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationOPEN.java b/core/src/main/java/org/dcache/nfs/v4/OperationOPEN.java index e741ebd4..790f3b71 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationOPEN.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationOPEN.java @@ -316,7 +316,7 @@ private void checkCanAccess(CompoundContext context, Inode inode, uint32_t share throw new AccessException(); } - Stat.Type statType = context.getFs().getStatType(inode); + Stat.Type statType = context.getFs().getattr(inode, Stat.STAT_ATTRIBUTES_TYPE_ONLY).type(); switch (statType) { case REGULAR: // OK diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationOPEN_CONFIRM.java b/core/src/main/java/org/dcache/nfs/v4/OperationOPEN_CONFIRM.java index f4038512..749840b3 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationOPEN_CONFIRM.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationOPEN_CONFIRM.java @@ -31,7 +31,6 @@ import org.dcache.nfs.v4.xdr.nfs_opnum4; import org.dcache.nfs.v4.xdr.nfs_resop4; import org.dcache.nfs.v4.xdr.stateid4; -import org.dcache.nfs.vfs.Inode; import org.dcache.nfs.vfs.Stat; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -53,7 +52,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti throw new NotSuppException("operation OPEN_CONFIRM4 is obsolete in 4.x, x > 0"); } - Stat.Type statType = context.getFs().getStatType(context.currentInode()); + Stat.Type statType = context.getFs().getattr(context.currentInode(), Stat.STAT_ATTRIBUTES_TYPE_ONLY).type(); if (statType == Stat.Type.DIRECTORY) { throw new IsDirException(); diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationREADDIR.java b/core/src/main/java/org/dcache/nfs/v4/OperationREADDIR.java index 4a4d11ea..588f9ded 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationREADDIR.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationREADDIR.java @@ -94,7 +94,7 @@ public void process(final CompoundContext context, nfs_resop4 result) throws Chi throw new BadCookieException("bad cookie : " + startValue); } - Stat.Type statType = context.getFs().getStatType(dir); + Stat.Type statType = context.getFs().getattr(dir, Stat.STAT_ATTRIBUTES_TYPE_ONLY).type(); if (statType != Stat.Type.DIRECTORY) { throw new NotDirException(); } diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationREADLINK.java b/core/src/main/java/org/dcache/nfs/v4/OperationREADLINK.java index 7b0e9356..16561d0a 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationREADLINK.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationREADLINK.java @@ -47,7 +47,7 @@ public OperationREADLINK(nfs_argop4 args) { public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNFSException, IOException { final READLINK4res res = result.opreadlink; - Stat.Type statType = context.getFs().getStatType(context.currentInode()); + Stat.Type statType = context.getFs().getattr(context.currentInode(), Stat.STAT_ATTRIBUTES_TYPE_ONLY).type(); if (statType != Stat.Type.SYMLINK) { throw new InvalException("not a symlink"); } diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationSECINFO.java b/core/src/main/java/org/dcache/nfs/v4/OperationSECINFO.java index 03e1a6ca..eb76f3f9 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationSECINFO.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationSECINFO.java @@ -48,7 +48,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti final SECINFO4res res = result.opsecinfo; Inode dir = context.currentInode(); - Stat.Type statType = context.getFs().getStatType(dir); + Stat.Type statType = context.getFs().getattr(dir, Stat.STAT_ATTRIBUTES_TYPE_ONLY).type(); if (statType != Stat.Type.DIRECTORY) { throw new NotDirException(); } diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationWRITE.java b/core/src/main/java/org/dcache/nfs/v4/OperationWRITE.java index f1fdb06c..27690b02 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationWRITE.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationWRITE.java @@ -55,7 +55,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF _args.opwrite.offset.checkOverflow(_args.opwrite.data.remaining(), "offset + length overflow"); - Stat.Type statType = context.getFs().getStatType(context.currentInode()); + Stat.Type statType = context.getFs().getattr(context.currentInode(), Stat.STAT_ATTRIBUTES_TYPE_ONLY).type(); stateid4 stateid = Stateids.getCurrentStateidIfNeeded(context, _args.opwrite.stateid); if (statType == Stat.Type.DIRECTORY) { diff --git a/core/src/main/java/org/dcache/nfs/v4/ds/DSOperationCOMMIT.java b/core/src/main/java/org/dcache/nfs/v4/ds/DSOperationCOMMIT.java index a4c3db57..45be3bc6 100644 --- a/core/src/main/java/org/dcache/nfs/v4/ds/DSOperationCOMMIT.java +++ b/core/src/main/java/org/dcache/nfs/v4/ds/DSOperationCOMMIT.java @@ -56,7 +56,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF final COMMIT4res res = result.opcommit; if (context.getFs() != null) { Inode inode = context.currentInode(); - Stat.Type statType = context.getFs().getStatType(inode); + Stat.Type statType = context.getFs().getattr(inode, Stat.STAT_ATTRIBUTES_TYPE_ONLY).type(); if (statType == Stat.Type.DIRECTORY) { throw new IsDirException("Invalid can't commit a directory"); diff --git a/core/src/main/java/org/dcache/nfs/v4/ds/DSOperationWRITE.java b/core/src/main/java/org/dcache/nfs/v4/ds/DSOperationWRITE.java index b268d7bd..c0cc35e8 100644 --- a/core/src/main/java/org/dcache/nfs/v4/ds/DSOperationWRITE.java +++ b/core/src/main/java/org/dcache/nfs/v4/ds/DSOperationWRITE.java @@ -62,7 +62,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti _args.opwrite.offset.checkOverflow(_args.opwrite.data.remaining(), "offset + length overflow"); Inode inode = context.currentInode(); - Stat.Type statType = context.getFs().getStatType(inode); + Stat.Type statType = context.getFs().getattr(inode, Stat.STAT_ATTRIBUTES_TYPE_ONLY).type(); if (statType == Stat.Type.DIRECTORY) { throw new IsDirException("Can't WRITE into a directory inode"); diff --git a/core/src/main/java/org/dcache/nfs/vfs/ForwardingFileSystem.java b/core/src/main/java/org/dcache/nfs/vfs/ForwardingFileSystem.java index 3996769e..c22e1108 100644 --- a/core/src/main/java/org/dcache/nfs/vfs/ForwardingFileSystem.java +++ b/core/src/main/java/org/dcache/nfs/vfs/ForwardingFileSystem.java @@ -21,12 +21,14 @@ import java.io.IOException; import java.nio.ByteBuffer; +import java.util.EnumSet; import java.util.concurrent.CompletableFuture; import javax.security.auth.Subject; import org.dcache.nfs.v4.NfsIdMapping; import org.dcache.nfs.v4.xdr.nfsace4; +import org.dcache.nfs.vfs.Stat.StatAttribute; /** * A file system which forwards all its method calls to another file system. Subclasses should override one or more @@ -147,8 +149,8 @@ public Stat getattr(Inode inode) throws IOException { } @Override - public Stat.Type getStatType(Inode inode) throws IOException { - return delegate().getStatType(inode); + public Stat getattr(Inode inode, EnumSet attributes) throws IOException { + return delegate().getattr(inode, attributes); } @Override diff --git a/core/src/main/java/org/dcache/nfs/vfs/PseudoFs.java b/core/src/main/java/org/dcache/nfs/vfs/PseudoFs.java index a366c80d..e5e26478 100644 --- a/core/src/main/java/org/dcache/nfs/vfs/PseudoFs.java +++ b/core/src/main/java/org/dcache/nfs/vfs/PseudoFs.java @@ -20,15 +20,41 @@ package org.dcache.nfs.vfs; import static com.google.common.collect.Lists.newArrayList; -import static org.dcache.nfs.util.UnixSubjects.*; -import static org.dcache.nfs.v4.xdr.nfs4_prot.*; -import static org.dcache.nfs.vfs.AclCheckable.Access; +import static org.dcache.nfs.util.UnixSubjects.hasGid; +import static org.dcache.nfs.util.UnixSubjects.hasUid; +import static org.dcache.nfs.util.UnixSubjects.isNobodySubject; +import static org.dcache.nfs.util.UnixSubjects.isRootSubject; +import static org.dcache.nfs.util.UnixSubjects.toSubject; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACCESS4_DELETE; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACCESS4_EXECUTE; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACCESS4_EXTEND; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACCESS4_LOOKUP; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACCESS4_MODIFY; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACCESS4_READ; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACCESS4_XALIST; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACCESS4_XAREAD; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACCESS4_XAWRITE; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACE4_ADD_FILE; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACE4_ADD_SUBDIRECTORY; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACE4_APPEND_DATA; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACE4_DELETE; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACE4_DELETE_CHILD; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACE4_EXECUTE; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACE4_LIST_DIRECTORY; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACE4_READ_ACL; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACE4_READ_ATTRIBUTES; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACE4_READ_DATA; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACE4_WRITE_ACL; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACE4_WRITE_ATTRIBUTES; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACE4_WRITE_DATA; +import static org.dcache.nfs.v4.xdr.nfs4_prot.ACE4_WRITE_OWNER; import java.io.IOException; import java.net.InetSocketAddress; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Collection; +import java.util.EnumSet; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -40,12 +66,17 @@ import org.dcache.nfs.ExportTable; import org.dcache.nfs.FsExport; import org.dcache.nfs.nfsstat; -import org.dcache.nfs.status.*; +import org.dcache.nfs.status.AccessException; +import org.dcache.nfs.status.InvalException; +import org.dcache.nfs.status.NoEntException; +import org.dcache.nfs.status.PermException; +import org.dcache.nfs.status.RoFsException; import org.dcache.nfs.util.SubjectHolder; import org.dcache.nfs.v4.acl.Acls; import org.dcache.nfs.v4.xdr.acemask4; import org.dcache.nfs.v4.xdr.nfsace4; -import org.dcache.nfs.vfs.Stat.Type; +import org.dcache.nfs.vfs.AclCheckable.Access; +import org.dcache.nfs.vfs.Stat.StatAttribute; import org.dcache.oncrpc4j.rpc.RpcAuth; import org.dcache.oncrpc4j.rpc.RpcAuthType; import org.dcache.oncrpc4j.rpc.RpcCall; @@ -359,9 +390,9 @@ public Stat getattr(Inode inode) throws IOException { } @Override - public Type getStatType(Inode inode) throws IOException { + public Stat getattr(Inode inode, EnumSet attributes) throws IOException { checkAccess(inode, ACE4_READ_ATTRIBUTES); - return _inner.getStatType(innerInode(inode)); + return _inner.getattr(innerInode(inode), attributes); } @Override diff --git a/core/src/main/java/org/dcache/nfs/vfs/Stat.java b/core/src/main/java/org/dcache/nfs/vfs/Stat.java index 00e03c52..c866383f 100644 --- a/core/src/main/java/org/dcache/nfs/vfs/Stat.java +++ b/core/src/main/java/org/dcache/nfs/vfs/Stat.java @@ -33,6 +33,12 @@ public enum StatAttribute { DEV, INO, MODE, NLINK, OWNER, GROUP, RDEV, SIZE, GENERATION, ATIME, MTIME, CTIME, BTIME }; + /** + * A special {@link EnumSet} indicating that we're really only interested in the file type, which is usually part of + * {@link StatAttribute#MODE}; the remaining bits of {@code mode} may be 0. + */ + public static final EnumSet STAT_ATTRIBUTES_TYPE_ONLY = EnumSet.of(StatAttribute.MODE); + private static final long serialVersionUID = 1L; private static final DateTimeFormatter LS_TIME_FORMAT = DateTimeFormatter.ofPattern("MMM dd HH:mm"); diff --git a/core/src/main/java/org/dcache/nfs/vfs/VirtualFileSystem.java b/core/src/main/java/org/dcache/nfs/vfs/VirtualFileSystem.java index 60f92040..fd1e778d 100644 --- a/core/src/main/java/org/dcache/nfs/vfs/VirtualFileSystem.java +++ b/core/src/main/java/org/dcache/nfs/vfs/VirtualFileSystem.java @@ -21,8 +21,8 @@ import java.io.IOException; import java.nio.ByteBuffer; +import java.util.EnumSet; import java.util.concurrent.CompletableFuture; -import java.util.function.Consumer; import javax.security.auth.Subject; @@ -32,6 +32,7 @@ import org.dcache.nfs.v4.NfsIdMapping; import org.dcache.nfs.v4.xdr.nfsace4; import org.dcache.nfs.v4.xdr.stable_how4; +import org.dcache.nfs.vfs.Stat.StatAttribute; import com.google.common.annotations.Beta; @@ -337,18 +338,14 @@ default WriteResult write(Inode inode, ByteBuffer data, long offset, StabilityLe Stat getattr(Inode inode) throws IOException; /** - * Gets the type of an inode. - *

- * This is equivalent to calling {@code getattr(inode).type()}), except that some implementations may choose to - * optimize this code path. Note that it is permissible to return a result for a stale inode, i.e., unlike calling - * {@link #getattr(Inode)}, no actual "stat" call needs to be made. + * Gets at least a specific subset of attributes for the given file system object. * * @param inode inode of the file system object. - * @return The type. - * @throws IOException on error. + * @return file's attributes. + * @throws IOException */ - default Stat.Type getStatType(Inode inode) throws IOException { - return getattr(inode).type(); + default Stat getattr(Inode inode, EnumSet attributes) throws IOException { + return getattr(inode); } /** diff --git a/core/src/test/java/org/dcache/nfs/v3/MountServerTest.java b/core/src/test/java/org/dcache/nfs/v3/MountServerTest.java index be9b2adf..6f0223b6 100644 --- a/core/src/test/java/org/dcache/nfs/v3/MountServerTest.java +++ b/core/src/test/java/org/dcache/nfs/v3/MountServerTest.java @@ -141,7 +141,7 @@ MountServertestHelper withPath(String path) throws IOException { given(_fs.getRootInode()).willReturn(inode); given(_fs.getattr(inode)).willReturn(stat); - given(_fs.getStatType(any())).willCallRealMethod(); + given(_fs.getattr(any(), any())).willCallRealMethod(); for (String pathElement : splitter.split(path)) { Inode objectInode = mock(Inode.class); diff --git a/core/src/test/java/org/dcache/nfs/v4/OperationREADDIRTest.java b/core/src/test/java/org/dcache/nfs/v4/OperationREADDIRTest.java index 311540ec..3a2bb978 100644 --- a/core/src/test/java/org/dcache/nfs/v4/OperationREADDIRTest.java +++ b/core/src/test/java/org/dcache/nfs/v4/OperationREADDIRTest.java @@ -77,7 +77,7 @@ public void setup() throws Exception { dirStat.setSize(512); vfs = mock(VirtualFileSystem.class); // the vfs serving it when(vfs.getattr(eq(dirInode))).thenReturn(dirStat); - given(vfs.getStatType(any())).willCallRealMethod(); // default implementation calls getattr + given(vfs.getattr(any(), any())).willCallRealMethod(); // default implementation calls getattr result = nfs_resop4.resopFor(nfs_opnum4.OP_READDIR); context = new CompoundContextBuilder() diff --git a/core/src/test/java/org/dcache/nfs/v4/OperationWRITETest.java b/core/src/test/java/org/dcache/nfs/v4/OperationWRITETest.java index 462dc0f4..b02bfa48 100644 --- a/core/src/test/java/org/dcache/nfs/v4/OperationWRITETest.java +++ b/core/src/test/java/org/dcache/nfs/v4/OperationWRITETest.java @@ -63,6 +63,7 @@ public void testLeaseUpdateForV40Client() throws UnknownHostException, ChimeraNF when(stateHandler.getClientIdByStateId(any())).thenReturn(client); when(vfs.getattr(any())).thenReturn(fileStat); + when(vfs.getattr(any(), any())).thenCallRealMethod(); when(vfs.write(any(), any(), anyLong(), any())) .thenReturn(new VirtualFileSystem.WriteResult(VirtualFileSystem.StabilityLevel.UNSTABLE, 1)); @@ -99,6 +100,7 @@ public void testNoLeaseUpdateForV41Client() throws UnknownHostException, Chimera when(stateHandler.getClientIdByStateId(any())).thenReturn(client); when(vfs.getattr(any())).thenReturn(fileStat); + when(vfs.getattr(any(), any())).thenCallRealMethod(); when(vfs.write(any(), any(), anyLong(), any())) .thenReturn(new VirtualFileSystem.WriteResult(VirtualFileSystem.StabilityLevel.UNSTABLE, 1)); @@ -137,6 +139,7 @@ public void testReturnWriteVerifier() throws UnknownHostException, ChimeraNFSExc verifier4 verifier = mock(verifier4.class); when(vfs.getattr(any())).thenReturn(fileStat); + when(vfs.getattr(any(), any())).thenCallRealMethod(); when(vfs.write(any(), any(), anyLong(), any())) .thenReturn(new VirtualFileSystem.WriteResult(VirtualFileSystem.StabilityLevel.UNSTABLE, 1));