Skip to content

Initial Implementation of NFS FileChannel - #28

Open
amarcionek wants to merge 1 commit into
EMCECS:masterfrom
Seven10Storage:feature-channel
Open

amarcionek wants to merge 1 commit into
EMCECS:masterfrom
Seven10Storage:feature-channel

Conversation

@amarcionek

Copy link
Copy Markdown
Contributor

Thought people might like this. Model follows the java.nio.Filechannel class. Most methods are supported except for the MappedByteBuffer and locking because its only advisory in NFSv3. Locking could be implemented in a class instance or process scope, or could wrap NLM as a sideband protocol, assuming the server supported it. But we aren't using locking, so the impetus to do it isn't there.

I also plan to add a method where NfsSetAttributes can be used to pass in via the constructor in some way as any newly created files are given default permissions.

If we're to do it it right and made the API 100% compatible with the original FileChannel, the constructor would take a Path instead of an NfsFile and FileAttributes... for the attributes.

Model follows the java.nio.Filechannel class. Most methods are supported
except for the MappedByteBuffer and locking because its only advisory in
NFSv3.  Locking could be implemented in a class instance or process
scope way in the future.
@DavidASeibert

Copy link
Copy Markdown
Contributor

That looks like a very good idea, with a few tweaks. Here are my first 3 thoughts.

  • NfsFileChannel shouldn't extend FileChannel, as the static open methods would then produce a FileChannel object instead of an NfsFileChannel object. Instead, the hierarchy should probably be in parallel to FileChannel: public abstract class NfsFileChannel extends AbstractInterruptibleChannel implements SeekableByteChannel, GatheringByteChannel, ScatteringByteChannel.

  • The package should probably be com.emc.ecs.nfsclient.nfs.nio.channels, so that the relationship to NfsFile is the same as FileChannel to File.

  • It duplicates a lot of the NfsFileXStream functionality, so it should probably use them in the underlying implementation.

This branch has not been deployed

No deployments
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