core: Reduce amount of calls to getattr/Stat, improve vfs.read

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 <[email protected]>

Author: kohlschuetter

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");
             }
 

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());

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;

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");
         }
 

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");
         }
 

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;

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();
         }
 

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;
     }
 }

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();
         }
 

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");
         }