Skip to content

Conversation

@kohlschuetter
Copy link
Contributor

Commit c8d57f2 introduced support for stateless stateids.

This may not be desirable, as this breaks the inherent sequence order of stateids, adding extra requirements for logging/journaling and advanced VirtualFileSystems. Moreover, this may leave the macOS NFSv4.0 client in a state where all stateids remain "stateless".

Make support for this feature configurable by adding a flag to NFSv4StateHandler and move the support logic into NFSv4StateHandler.

Also fix OperationREAD and OperationWRITE to get the NFSv4StateHandler only once per call from the context.

Commit c8d57f2 introduced support for stateless stateids.

This may not be desirable, as this breaks the inherent sequence order of
stateids, adding extra requirements for logging/journaling and advanced
VirtualFileSystems. Moreover, this may leave the macOS NFSv4.0 client in
a state where all stateids remain "stateless".

Make support for this feature configurable by adding a flag to
NFSv4StateHandler and move the support logic into NFSv4StateHandler.

Also fix OperationREAD and OperationWRITE to get the NFSv4StateHandler
only once per call from the context.

Signed-off-by: Christian Kohlschütter <[email protected]>
@kofemann
Copy link
Member

Instead of changing the state manager to match your needs, you should customise the server code:

  var customOps = new MDSOperationExecutor() {
      @Override
      protected AbstractNFSv4Operation getOperation(nfs_argop4 op) {

          switch (op.argop) {
              case nfs_opnum4.OP_READ:
                  return new OperationREAD(op) {
                      @Override
                      public void process(CompoundContext context, nfs_resop4 result)
                            throws IOException {
                          if (Stateids.isStateLess(op.opread.stateid)) {
                              throw new BadStateidException();
                          }
                          super.process(context, result);
                      }
                  };
              case nfs_opnum4.OP_WRITE:
                  return new OperationWRITE(op) {
                      @Override
                      public void process(CompoundContext context, nfs_resop4 result)
                            throws IOException {
                          if (Stateids.isStateLess(op.opwrite.stateid)) {
                              throw new BadStateidException();
                          }
                          super.process(context, result);
                      }
                  };
              default:
                  return super.getOperation(op);
          }
      }
  };

  nfs4 = new NFSServerV41.Builder()
        .withVfs(vfs)
        .withOperationExecutor(customOps)
        .withExportTable(exportFile)
        .build();

@kohlschuetter
Copy link
Contributor Author

Or we could remove support for stateless stateids again. I'm not sure what we gain from adding, this is turning into a lot of busywork, tbh.

@kofemann
Copy link
Member

it's ok for reference implementation and spec compliance. A real server will probably have a custom implementation. Some examples from me:

https://github.com/kofemann/vfs4j/blob/master/src/main/java/org/dcache/vfs4j/EDSNFSv4OperationFactory.java

https://github.com/kofemann/vfs4j/blob/master/src/main/java/org/dcache/vfs4j/EDSNFSv4OperationFactory.java

https://github.com/dCache/dcache/blob/master/modules/dcache-nfs/src/main/java/org/dcache/chimera/nfsv41/mover/NfsTransferService.java

https://github.com/dCache/dcache/blob/master/modules/dcache-nfs/src/main/java/org/dcache/chimera/nfsv41/door/DoorOperationFactory.java

But, yes, clients should avoid stateless reads. As I was involved in the spec discussion starting in v4.1, I have no idea what the rationale behind its addition to 4.0 was.

Revert is cheap (ATM). So let's see if it makes more harm that good prior reerting.

@kohlschuetter
Copy link
Contributor Author

Given that the change has not actually been tested thoroughly against the affected macOS client I'm hoping the support gets reverted soon. I couldn't get the device back in state, but I definitely do not want it to get stuck in that state forever...

Obviously I have a different focus than you have. As nfs4j appears to be quite stabilized at the moment, I'll probably just keep exploring solutions for my specific scenario in a separate repo and check back from time to time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants