vfs: drop Stat#get/setFileId methods

Motivation: A typical (POSIX) filesystem has only one unique identifier for a file, so called inode number. However, dCache as inumber and a corresponding pnfsid. As being originally developed as a part of dcache, nfs4j have inherited that and exposes as Stat#ino and Stat#fileid. After some iterations, only `fileid` is used, and `ino` kept as 32 bit version of it. Modification: drop Stat#get/setFileId methods in favor of Stat#get/setIno (for better compliance with POSIX). Update `ino` to 64 bit long. Update codebase to use `ino`. Result: less confusions with a price of small incompatibility with earlier versions. Acked-by: Lea Morschel Acked-by: Paul Millar Target: master
dCache · Jan 13, 2022 · 67da932 · 67da932
1 parent b12dbeb
commit 67da932
Show file tree

Hide file tree

Showing 15 changed files with 30 additions and 53 deletions.
diff --git a/API-changes.md b/API-changes.md
@@ -1,5 +1,13 @@
 # Changes to NFS4J public API
 
+## 0.23
+
+- dropped Stat#get/setFileId methods
+  - use `Stat#get/setIno` instead
+- Update the signature of `Stat#getIno` to return _long_
+- Update the signature of `Stat#setIno` to accept _long_
+- Update the signature of VirtualFileSystem#access to accept additional `Subject`
+
 ## 0.22
 
 - removed deprecated CompoundContextBuilder#withExportFile

diff --git a/core/src/main/java/org/dcache/nfs/v3/NfsServerV3.java b/core/src/main/java/org/dcache/nfs/v3/NfsServerV3.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009 - 2021 Deutsches Elektronen-Synchroton,
+ * Copyright (c) 2009 - 2022 Deutsches Elektronen-Synchroton,
  * Member of the Helmholtz Association, (DESY), HAMBURG, GERMANY
  *
  * This library is free software; you can redistribute it and/or modify
@@ -751,7 +751,7 @@ public READDIRPLUS3res NFSPROC3_READDIRPLUS_3(RpcCall call$, READDIRPLUS3args ar
                 Inode ef = le.getInode();
 
                 entryplus3 currentEntry = new entryplus3();
-                currentEntry.fileid = new fileid3(new uint64(le.getStat().getFileId()));
+                currentEntry.fileid = new fileid3(new uint64(le.getStat().getIno()));
                 currentEntry.name = new filename3(name);
                 currentEntry.cookie = new cookie3(new uint64(le.getCookie()));
                 currentEntry.name_handle = new post_op_fh3();
@@ -856,7 +856,7 @@ public READDIR3res NFSPROC3_READDIR_3(RpcCall call$, READDIR3args arg1) {
                 String name = le.getName();
 
                 entry3 currentEntry = new entry3();
-                currentEntry.fileid = new fileid3(new uint64(le.getStat().getFileId()));
+                currentEntry.fileid = new fileid3(new uint64(le.getStat().getIno()));
                 currentEntry.name = new filename3(name);
                 currentEntry.cookie = new cookie3(new uint64(le.getCookie()));
 

diff --git a/core/src/main/java/org/dcache/nfs/v3/Utils.java b/core/src/main/java/org/dcache/nfs/v3/Utils.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009 - 2021 Deutsches Elektronen-Synchroton,
+ * Copyright (c) 2009 - 2022 Deutsches Elektronen-Synchroton,
  * Member of the Helmholtz Association, (DESY), HAMBURG, GERMANY
  *
  * This library is free software; you can redistribute it and/or modify
@@ -84,7 +84,7 @@ public static void fill_attributes(Stat stat,  fattr3 at) {
 
         //public int fileid;
         // Get some value for this file/dir
-        at.fileid = new fileid3(new uint64( stat.getFileId() ) );
+        at.fileid = new fileid3(new uint64(stat.getIno()));
 
         at.size = new size3(stat.getSize());
         at.used = new size3(stat.getSize());

diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationGETATTR.java b/core/src/main/java/org/dcache/nfs/v4/OperationGETATTR.java
@@ -257,7 +257,7 @@ static Optional<? extends XdrAble> fattr2xdr(int fattr, VirtualFileSystem fs, In
             case nfs4_prot.FATTR4_CHOWN_RESTRICTED:
                 return Optional.empty();
             case nfs4_prot.FATTR4_FILEID:
-                return Optional.of(new fattr4_fileid(stat.getFileId()));
+                return Optional.of(new fattr4_fileid(stat.getIno()));
             case nfs4_prot.FATTR4_FILES_AVAIL:
                 fsStat = getFsStat(fsStat, fs);
                 fattr4_files_avail files_avail = new fattr4_files_avail(fsStat.getTotalFiles() - fsStat.getUsedFiles());
@@ -352,7 +352,7 @@ static Optional<? extends XdrAble> fattr2xdr(int fattr, VirtualFileSystem fs, In
                  * TODO!!!:
                  */
 
-                long mofi = stat.getFileId();
+                long mofi = stat.getIno();
 
                 if (mofi == 0x00b0a23a /* it's a root*/) {
                     mofi = 0x12345678;

diff --git a/core/src/main/java/org/dcache/nfs/vfs/Stat.java b/core/src/main/java/org/dcache/nfs/vfs/Stat.java
@@ -38,7 +38,6 @@ public enum StatAttribute {
         GROUP,
         RDEV,
         SIZE,
-        FILEID,
         GENERATION,
         ATIME,
         MTIME,
@@ -140,14 +139,13 @@ public static Type fromMode(int mode) {
     }
 
     private int _dev;
-    private int _ino;
+    private long _ino;
     private int _mode;
     private int _nlink;
     private int _owner;
     private int _group;
     private int _rdev;
     private long _size;
-    private long _fileid;
     private long _generation;
 
     /*
@@ -177,15 +175,15 @@ public void setDev(int dev) {
     /**
      * Returns file inode number.
      */
-    public int getIno() {
+    public long getIno() {
         guard(StatAttribute.INO);
         return _ino;
     }
 
     /**
      * Set files inode number.
      */
-    public void setIno(int ino) {
+    public void setIno(long ino) {
         define(StatAttribute.INO);
         _ino = ino;
     }
@@ -350,22 +348,6 @@ public void setBTime(long btime) {
         _btime = btime;
     }
 
-    /**
-     * Returns file inode number.
-     */
-    public long getFileId() {
-        guard(StatAttribute.FILEID);
-        return _fileid;
-    }
-
-    /**
-     * Set file inode number.
-     */
-    public void setFileid(long fileid) {
-        define(StatAttribute.FILEID);
-        _fileid = fileid;
-    }
-
     /**
      * Returns files change counter.
      */

diff --git a/core/src/test/java/org/dcache/nfs/v3/NfsServerV3READDIRPLUS_3Test.java b/core/src/test/java/org/dcache/nfs/v3/NfsServerV3READDIRPLUS_3Test.java
@@ -54,7 +54,7 @@ public void setup() throws Exception {
         dirStat.setUid(1);
         dirStat.setGid(2);
         dirStat.setDev(1);
-        dirStat.setFileid(1);
+        dirStat.setIno(1);
         dirStat.setSize(512);
         vfs = mock(VirtualFileSystem.class); // the vfs serving it
         when(vfs.getattr(eq(dirInode))).thenReturn(dirStat);

diff --git a/core/src/test/java/org/dcache/nfs/v3/NfsServerV3READDIR_3Test.java b/core/src/test/java/org/dcache/nfs/v3/NfsServerV3READDIR_3Test.java
@@ -55,7 +55,7 @@ public void setup() throws Exception {
         dirStat.setUid(1);
         dirStat.setGid(2);
         dirStat.setDev(1);
-        dirStat.setFileid(1);
+        dirStat.setIno(1);
         dirStat.setSize(512);
         vfs = mock(VirtualFileSystem.class); // the vfs serving it
         when(vfs.getattr(eq(dirInode))).thenReturn(dirStat);

diff --git a/core/src/test/java/org/dcache/nfs/v4/OperationREADDIRTest.java b/core/src/test/java/org/dcache/nfs/v4/OperationREADDIRTest.java
@@ -71,7 +71,7 @@ public void setup() throws Exception {
         dirStat.setUid(1);
         dirStat.setGid(2);
         dirStat.setDev(1);
-        dirStat.setFileid(1);
+        dirStat.setIno(1);
         dirStat.setSize(512);
         vfs = mock(VirtualFileSystem.class); // the vfs serving it
         when(vfs.getattr(eq(dirInode))).thenReturn(dirStat);
@@ -183,7 +183,7 @@ private DirectoryEntry dir(String name) {
         stat.setUid(1);
         stat.setGid(2);
         stat.setDev(1);
-        stat.setFileid(cookie);
+        stat.setIno(cookie);
         stat.setSize(512);
 
         try {
@@ -211,7 +211,7 @@ private DirectoryEntry file(String name) {
         stat.setUid(1);
         stat.setGid(2);
         stat.setDev(1);
-        stat.setFileid(cookie);
+        stat.setIno(cookie);
         stat.setSize(1024);
 
         try {

diff --git a/core/src/test/java/org/dcache/nfs/v4/OperationREADTest.java b/core/src/test/java/org/dcache/nfs/v4/OperationREADTest.java
@@ -37,7 +37,7 @@ public void setUp() {
         fileStat.setUid(1);
         fileStat.setGid(2);
         fileStat.setDev(1);
-        fileStat.setFileid(1);
+        fileStat.setIno(1);
         fileStat.setSize(512);
 
         vfs = mock(VirtualFileSystem.class);

diff --git a/core/src/test/java/org/dcache/nfs/v4/OperationREMOVEXATTRTest.java b/core/src/test/java/org/dcache/nfs/v4/OperationREMOVEXATTRTest.java
@@ -37,7 +37,7 @@ public void setUp() throws IOException {
         fileStat.setUid(1);
         fileStat.setGid(2);
         fileStat.setDev(1);
-        fileStat.setFileid(1);
+        fileStat.setIno(1);
         fileStat.setSize(512);
 
         when(vfs.getattr(any())).thenReturn(fileStat);

diff --git a/core/src/test/java/org/dcache/nfs/v4/OperationSETXATTRTest.java b/core/src/test/java/org/dcache/nfs/v4/OperationSETXATTRTest.java
@@ -39,7 +39,7 @@ public void setUp() throws IOException {
         fileStat.setUid(1);
         fileStat.setGid(2);
         fileStat.setDev(1);
-        fileStat.setFileid(1);
+        fileStat.setIno(1);
         fileStat.setSize(512);
 
         when(vfs.getattr(any())).thenReturn(fileStat);

diff --git a/core/src/test/java/org/dcache/nfs/v4/OperationWRITETest.java b/core/src/test/java/org/dcache/nfs/v4/OperationWRITETest.java
@@ -40,7 +40,7 @@ public void setUp() {
         fileStat.setUid(1);
         fileStat.setGid(2);
         fileStat.setDev(1);
-        fileStat.setFileid(1);
+        fileStat.setIno(1);
         fileStat.setSize(512);
 
         vfs = mock(VirtualFileSystem.class);

diff --git a/core/src/test/java/org/dcache/nfs/vfs/DirectoryStreamTest.java b/core/src/test/java/org/dcache/nfs/vfs/DirectoryStreamTest.java
@@ -34,7 +34,7 @@ public void setUp() {
             stat.setUid(1);
             stat.setGid(2);
             stat.setDev(1);
-            stat.setFileid(i);
+            stat.setIno(i);
             stat.setSize(512);
 
             entries.add(new DirectoryEntry(String.format("file-%d", i), inode, stat, i));

diff --git a/core/src/test/java/org/dcache/nfs/vfs/DummyVFS.java b/core/src/test/java/org/dcache/nfs/vfs/DummyVFS.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019 Deutsches Elektronen-Synchroton,
+ * Copyright (c) 2019 - 2022 Deutsches Elektronen-Synchroton,
  * Member of the Helmholtz Association, (DESY), HAMBURG, GERMANY
  *
  * This library is free software; you can redistribute it and/or modify
@@ -473,10 +473,9 @@ private Stat statPath(Path p, long inodeNumber) throws IOException {
         stat.setMode(permissionsToMode(permissions, attrs));
 //        stat.setNlink((Integer) Files.getAttribute(p, "nlink", NOFOLLOW_LINKS));
         stat.setDev(17);
-        stat.setIno((int) inodeNumber);
+        stat.setIno(inodeNumber);
         stat.setRdev(17);
         stat.setSize(attrs.size());
-        stat.setFileid((int) inodeNumber);
         stat.setGeneration(attrs.lastModifiedTime().toMillis());
 
         return stat;

diff --git a/core/src/test/java/org/dcache/nfs/vfs/StatTest.java b/core/src/test/java/org/dcache/nfs/vfs/StatTest.java
@@ -140,11 +140,6 @@ public void testNotDefeinedGetCTime() {
         new Stat().getCTime();
     }
 
-    @Test(expected = IllegalStateException.class)
-    public void testNotDefeinedGetFileId() {
-        new Stat().getFileId();
-    }
-
     @Test(expected = IllegalStateException.class)
     public void testNotDefeinedGetGeneration() {
         new Stat().getGeneration();
@@ -240,13 +235,6 @@ public void testGetCTime() {
         assertEquals(1, stat.getCTime());
     }
 
-    @Test
-    public void testGetFileId() {
-        Stat stat = new Stat();
-        stat.setFileid(1);
-        assertEquals(1, stat.getFileId());
-    }
-
     @Test
     public void testGetGeneration() {
         Stat stat = new Stat();