From 1de61da685e6195f90735091a0cc2225146edb8f Mon Sep 17 00:00:00 2001 From: Tigran Mkrtchyan Date: Mon, 6 Jul 2020 14:41:23 +0200 Subject: [PATCH] vfs: add IO read/write methods that accept ByteBuffer Motivation: The use of ByteBuffer for IO operations adds more flexibility as ByteBuffers can be sliced, shared and passed directly to various channels. Moreover, any byte array can be easily wrapped into ByteBuffer, if needed. Modification: added vfs#read and vfs#write methods with default implementation falling back to byte array based methods to maintain backward compatibility. Updated OperationREAD and OperationWRITE to use the new methods. Deprecate byte array based methods. Result: more flexibility with same functionality. Acked-by: Paul Millar Target: master --- .../java/org/dcache/nfs/v4/OperationREAD.java | 10 +--- .../org/dcache/nfs/v4/OperationWRITE.java | 6 +-- .../dcache/nfs/vfs/ForwardingFileSystem.java | 11 +++++ .../java/org/dcache/nfs/vfs/PseudoFs.java | 13 ++++++ .../org/dcache/nfs/vfs/VirtualFileSystem.java | 46 ++++++++++++++++++- .../org/dcache/nfs/v4/OperationWRITETest.java | 6 +-- 6 files changed, 74 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationREAD.java b/core/src/main/java/org/dcache/nfs/v4/OperationREAD.java index ce5566ba7..2909a34b8 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationREAD.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationREAD.java @@ -73,18 +73,12 @@ public void process(CompoundContext context, nfs_resop4 result) throws IOExcepti ByteBuffer buf = ByteBuffer.allocate(count); - int bytesReaded = context.getFs().read(context.currentInode(), - buf.array(), offset, count); + int bytesReaded = context.getFs().read(context.currentInode(), buf, offset); if (bytesReaded < 0) { throw new NfsIoException("IO not allowd"); } - /* - * As we have written directly into back-end byte array tell the byte - * buffer where the limit is. - */ - buf.limit(bytesReaded); - + buf.flip(); res.status = nfsstat.NFS_OK; res.resok4 = new READ4resok(); diff --git a/core/src/main/java/org/dcache/nfs/v4/OperationWRITE.java b/core/src/main/java/org/dcache/nfs/v4/OperationWRITE.java index ff0d42b7c..472c1d64d 100644 --- a/core/src/main/java/org/dcache/nfs/v4/OperationWRITE.java +++ b/core/src/main/java/org/dcache/nfs/v4/OperationWRITE.java @@ -75,12 +75,8 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF } long offset = _args.opwrite.offset.value; - int count = _args.opwrite.data.remaining(); - byte[] data = new byte[count]; - _args.opwrite.data.get(data); - VirtualFileSystem.WriteResult writeResult = context.getFs().write(context.currentInode(), - data, offset, count, VirtualFileSystem.StabilityLevel.fromStableHow(_args.opwrite.stable)); + _args.opwrite.data, offset, VirtualFileSystem.StabilityLevel.fromStableHow(_args.opwrite.stable)); if (writeResult.getBytesWritten() < 0) { throw new NfsIoException("IO not allowed"); diff --git a/core/src/main/java/org/dcache/nfs/vfs/ForwardingFileSystem.java b/core/src/main/java/org/dcache/nfs/vfs/ForwardingFileSystem.java index db9e4cea8..55c91f511 100644 --- a/core/src/main/java/org/dcache/nfs/vfs/ForwardingFileSystem.java +++ b/core/src/main/java/org/dcache/nfs/vfs/ForwardingFileSystem.java @@ -20,6 +20,7 @@ package org.dcache.nfs.vfs; import java.io.IOException; +import java.nio.ByteBuffer; import javax.security.auth.Subject; import org.dcache.nfs.v4.NfsIdMapping; import org.dcache.nfs.v4.xdr.nfsace4; @@ -95,6 +96,11 @@ public int read(Inode inode, byte[] data, long offset, int count) throws IOExcep return delegate().read(inode, data, offset, count); } + @Override + public int read(Inode inode, ByteBuffer data, long offset) throws IOException { + return delegate().read(inode, data, offset); + } + @Override public String readlink(Inode inode) throws IOException { return delegate().readlink(inode); @@ -115,6 +121,11 @@ public WriteResult write(Inode inode, byte[] data, long offset, int count, Stabi return delegate().write(inode, data, offset, count, stabilityLevel); } + @Override + public WriteResult write(Inode inode, ByteBuffer data, long offset, StabilityLevel stabilityLevel) throws IOException { + return delegate().write(inode, data, offset, stabilityLevel); + } + @Override public void commit(Inode inode, long offset, int count) throws IOException { delegate().commit(inode, offset, count); diff --git a/core/src/main/java/org/dcache/nfs/vfs/PseudoFs.java b/core/src/main/java/org/dcache/nfs/vfs/PseudoFs.java index b0182b0d1..670177f04 100644 --- a/core/src/main/java/org/dcache/nfs/vfs/PseudoFs.java +++ b/core/src/main/java/org/dcache/nfs/vfs/PseudoFs.java @@ -22,6 +22,7 @@ import com.google.common.base.Splitter; import java.io.IOException; import java.net.InetAddress; +import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; @@ -281,6 +282,12 @@ public int read(Inode inode, byte[] data, long offset, int count) throws IOExcep return _inner.read(inode, data, offset, count); } + @Override + public int read(Inode inode, ByteBuffer data, long offset) throws IOException { + checkAccess(inode, ACE4_READ_DATA); + return _inner.read(inode, data, offset); + } + @Override public String readlink(Inode inode) throws IOException { checkAccess(inode, ACE4_READ_DATA); @@ -318,6 +325,12 @@ public WriteResult write(Inode inode, byte[] data, long offset, int count, Stabi return _inner.write(inode, data, offset, count, stabilityLevel); } + @Override + public WriteResult write(Inode inode, ByteBuffer data, long offset, StabilityLevel stabilityLevel) throws IOException { + checkAccess(inode, ACE4_WRITE_DATA); + return _inner.write(inode, data, offset, stabilityLevel); + } + @Override public Stat getattr(Inode inode) throws IOException { checkAccess(inode, ACE4_READ_ATTRIBUTES); diff --git a/core/src/main/java/org/dcache/nfs/vfs/VirtualFileSystem.java b/core/src/main/java/org/dcache/nfs/vfs/VirtualFileSystem.java index f4cae7d5c..371f5c5a3 100644 --- a/core/src/main/java/org/dcache/nfs/vfs/VirtualFileSystem.java +++ b/core/src/main/java/org/dcache/nfs/vfs/VirtualFileSystem.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009 - 2019 Deutsches Elektronen-Synchroton, + * Copyright (c) 2009 - 2020 Deutsches Elektronen-Synchroton, * Member of the Helmholtz Association, (DESY), HAMBURG, GERMANY * * This library is free software; you can redistribute it and/or modify @@ -20,6 +20,7 @@ package org.dcache.nfs.vfs; import java.io.IOException; +import java.nio.ByteBuffer; import javax.security.auth.Subject; import org.dcache.nfs.status.NotSuppException; import org.dcache.nfs.v4.NfsIdMapping; @@ -177,9 +178,31 @@ public interface VirtualFileSystem { * @return number of bytes read from the file, possibly zero. -1 if EOF is * reached. * @throws IOException + * @deprecated instead use the {@code read(Inode inode, ByteBuffer data, long offset)} */ + @Deprecated int read(Inode inode, byte[] data, long offset, int count) throws IOException; + /** + * Read data from file with a given inode into {@code data}. + * + * @param inode inode of the file to read from. + * @param data byte array for writing. + * @param offset file's position to read from. + * @return number of bytes read from the file, possibly zero. -1 if EOF is + * reached. + * @throws IOException + */ + default int read(Inode inode, ByteBuffer data, long offset) throws IOException { + ByteBuffer buf = ByteBuffer.allocate(data.remaining()); + int n = read(inode, buf.array(), offset, buf.remaining()); + if (n > 0) { + buf.limit(n); + data.put(buf); + } + return n; + } + /** * Get value of a symbolic link object. * @@ -222,12 +245,31 @@ public interface VirtualFileSystem { * @param stabilityLevel data stability level. * @return write result. * @throws IOException + * @deprecated instead use the {@code write(Inode inode, ByteBuffer data, long offset, StabilityLevel stabilityLevel)} */ + @Deprecated WriteResult write(Inode inode, byte[] data, long offset, int count, StabilityLevel stabilityLevel) throws IOException; + /** + * Write provided {@code data} into inode with a given stability level. + * + * @param inode inode of the file to write. + * @param data data to be written. + * @param offset the file position to begin writing at. + * @param stabilityLevel data stability level. + * @return write result. + * @throws IOException + */ + default WriteResult write(Inode inode, ByteBuffer data, long offset, StabilityLevel stabilityLevel) throws IOException { + int count = data.remaining(); + byte[] bytes = new byte[count]; + data.get(bytes); + return write(inode, bytes, offset, count, stabilityLevel); + } + /** * Flush data in {@code dirty} state to the stable storage. Typically - * follows {@link #write()} operation. + * follows {@link #write(Inode, ByteBuffer, long, StabilityLevel)} operation. * * @param inode inode of the file to commit. * @param offset the file position to start commit at. diff --git a/core/src/test/java/org/dcache/nfs/v4/OperationWRITETest.java b/core/src/test/java/org/dcache/nfs/v4/OperationWRITETest.java index a88b7c2d0..0a6b92b4e 100644 --- a/core/src/test/java/org/dcache/nfs/v4/OperationWRITETest.java +++ b/core/src/test/java/org/dcache/nfs/v4/OperationWRITETest.java @@ -54,7 +54,7 @@ public void testLeaseUpdateForV40Client() throws UnknownHostException, ChimeraNF NFSv4StateHandler stateHandler = mock(NFSv4StateHandler.class); when(vfs.getattr(any())).thenReturn(fileStat); - when(vfs.write(any(), any(), anyLong(), anyInt(), any())) + when(vfs.write(any(), any(), anyLong(), any())) .thenReturn(new VirtualFileSystem.WriteResult(VirtualFileSystem.StabilityLevel.UNSTABLE, 1)); COMPOUND4args writeArgs = new CompoundBuilder() @@ -80,7 +80,7 @@ public void testNoLeaseUpdateForV41Client() throws UnknownHostException, Chimera NFSv4StateHandler stateHandler = mock(NFSv4StateHandler.class); when(vfs.getattr(any())).thenReturn(fileStat); - when(vfs.write(any(), any(), anyLong(), anyInt(), any())) + when(vfs.write(any(), any(), anyLong(), any())) .thenReturn(new VirtualFileSystem.WriteResult(VirtualFileSystem.StabilityLevel.UNSTABLE, 1)); COMPOUND4args writeArgs = new CompoundBuilder() @@ -107,7 +107,7 @@ public void testReturnWriteVerifier() throws UnknownHostException, ChimeraNFSExc verifier4 verifier = mock(verifier4.class); when(vfs.getattr(any())).thenReturn(fileStat); - when(vfs.write(any(), any(), anyLong(), anyInt(), any())) + when(vfs.write(any(), any(), anyLong(), any())) .thenReturn(new VirtualFileSystem.WriteResult(VirtualFileSystem.StabilityLevel.UNSTABLE, 1)); COMPOUND4args writeArgs = new CompoundBuilder()