Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide open and close callbacks to virtual filesystem implementation… #91

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/src/main/java/org/dcache/nfs/v4/OperationCLOSE.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public void process(CompoundContext context, nfs_resop4 result)
Inode inode = context.currentInode();

stateid4 stateid = Stateids.getCurrentStateidIfNeeded(context, _args.opclose.open_stateid);
context.getFs().close(inode, stateid);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can trigger false evens it client tries to close invalid inode or stateid. I think this should be bound to state disposal

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. We have worked around this by tracking the stateid in the virtual filesystem implementation.

NFS4Client client;
if (context.getMinorversion() > 0) {
client = context.getSession().getClient();
Expand Down
15 changes: 11 additions & 4 deletions core/src/main/java/org/dcache/nfs/v4/OperationOPEN.java
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF
}

context.currentInode(inode);
context.getFs().open(inode, this.getAccessMode(_args.opopen.share_access), result.opopen.resok4.stateid);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you will mess all opens by file handle, which is quite common, when client already got hanlde by previous readdir, lookup or open.


break;
case open_claim_type4.CLAIM_PREVIOUS:
Expand Down Expand Up @@ -279,11 +280,10 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF
}


private void checkCanAccess(CompoundContext context, Inode inode, uint32_t share_access) throws IOException {

int accessMode;
private int getAccessMode(final uint32_t share_access) throws IOException {
final int accessMode;

switch (share_access.value & ~nfs4_prot.OPEN4_SHARE_ACCESS_WANT_DELEG_MASK) {
switch(share_access.value & ~nfs4_prot.OPEN4_SHARE_ACCESS_WANT_DELEG_MASK) {
case nfs4_prot.OPEN4_SHARE_ACCESS_READ:
accessMode = nfs4_prot.ACCESS4_READ;
break;
Expand All @@ -296,6 +296,13 @@ private void checkCanAccess(CompoundContext context, Inode inode, uint32_t share
default:
throw new InvalException("Invalid share_access mode: " + share_access.value);
}
return accessMode;
}


private void checkCanAccess(CompoundContext context, Inode inode, uint32_t share_access) throws IOException {

int accessMode = getAccessMode(share_access);

if (context.getFs().access(inode, accessMode) != accessMode) {
throw new AccessException();
Expand Down
11 changes: 11 additions & 0 deletions core/src/main/java/org/dcache/nfs/vfs/PseudoFs.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.dcache.nfs.status.*;
import org.dcache.nfs.v4.acl.Acls;
import org.dcache.nfs.v4.xdr.acemask4;
import org.dcache.nfs.v4.xdr.stateid4;
import org.dcache.oncrpc4j.rpc.RpcCall;

import static org.dcache.nfs.v4.xdr.nfs4_prot.*;
Expand Down Expand Up @@ -183,6 +184,16 @@ public Inode create(Inode parent, Stat.Type type, String path, Subject subject,
return pushExportIndex(parent, _inner.create(parent, type, path, effectiveSubject, mode));
}

@Override
public void open(Inode inode, int mode, stateid4 stateid) throws IOException {

}

@Override
public void close(Inode inode, stateid4 stateid) throws IOException {

}

@Override
public Inode getRootInode() throws IOException {
/*
Expand Down
11 changes: 11 additions & 0 deletions core/src/main/java/org/dcache/nfs/vfs/VfsCache.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import javax.security.auth.Subject;
import org.dcache.nfs.util.GuavaCacheMXBeanImpl;
import org.dcache.nfs.util.Opaque;
import org.dcache.nfs.v4.xdr.stateid4;

import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -172,6 +173,16 @@ public Inode create(Inode parent, Stat.Type type, String path, Subject subject,
return inode;
}

@Override
public void open(Inode inode, int mode, stateid4 stateid) throws IOException {
_inner.open(inode, mode, stateid);
}

@Override
public void close(Inode inode, stateid4 stateid) throws IOException {
_inner.close(inode, stateid);
}

@Override
public Stat getattr(Inode inode) throws IOException {
return statFromCacheOrLoad(inode);
Expand Down
18 changes: 18 additions & 0 deletions core/src/main/java/org/dcache/nfs/vfs/VirtualFileSystem.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,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.v4.xdr.stateid4;

/**
* An interface to file system.
Expand Down Expand Up @@ -69,6 +70,23 @@ public interface VirtualFileSystem {
*/
Inode create(Inode parent, Stat.Type type, String name, Subject subject, int mode) throws IOException;

/**
* Notify about file handle opened
* @param inode inode of the object
* @param mode Access mode bitmask like ACCESS4_READ
* @param stateid Open state id
* @throws IOException
*/
void open(Inode inode, int mode, stateid4 stateid) throws IOException;

/**
* Notify about file handle closed
* @param inode inode of the object
* @param stateid Open state id
* @throws IOException
*/
void close(Inode inode, stateid4 stateid) throws IOException;

/**
* Get file system's usage information.
*
Expand Down
3 changes: 3 additions & 0 deletions core/src/test/java/org/dcache/nfs/v4/NFS4ClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
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.VirtualFileSystem;
import org.junit.Before;
import org.junit.Test;

Expand Down Expand Up @@ -73,6 +74,7 @@ public void testStateCleanOnOpenCloseV41() throws Exception {
OperationCLOSE CLOSE = new OperationCLOSE(close_args);
result = nfs_resop4.resopFor(nfs_opnum4.OP_CLOSE);
context = new CompoundContextBuilder()
.withFs(mock(VirtualFileSystem.class))
.withStateHandler(stateHandler)
.withMinorversion(1)
.withDeviceManager(mock(NFSv41DeviceManager.class))
Expand Down Expand Up @@ -100,6 +102,7 @@ public void testStateCleanOnOpenCloseV40() throws Exception {
OperationCLOSE CLOSE = new OperationCLOSE(close_args);
result = nfs_resop4.resopFor(nfs_opnum4.OP_CLOSE);
context = new CompoundContextBuilder()
.withFs(mock(VirtualFileSystem.class))
.withStateHandler(stateHandler)
.withMinorversion(0)
.withCall(generateRpcCall())
Expand Down
11 changes: 11 additions & 0 deletions core/src/test/java/org/dcache/nfs/vfs/DummyVFS.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.common.jimfs.Configuration;
import com.google.common.jimfs.Jimfs;
import com.google.common.primitives.Longs;
import org.dcache.nfs.v4.xdr.stateid4;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -208,6 +209,16 @@ public Inode create(Inode parent, Type type, String path, Subject subject, int m
return toFileHandle(newInodeNumber);
}

@Override
public void open(Inode inode, int mode, stateid4 stateid) throws IOException {

}

@Override
public void close(Inode inode, stateid4 stateid) throws IOException {

}

@Override
public FsStat getFsStat() throws IOException {
FileStore store = Files.getFileStore(_root);
Expand Down