diff --git a/cmd/erasure-healfile.go b/cmd/erasure-healfile.go index 79d598535..b993416fd 100644 --- a/cmd/erasure-healfile.go +++ b/cmd/erasure-healfile.go @@ -95,13 +95,7 @@ func (s ErasureStorage) HealFile(staleDisks []StorageAPI, volume, path string, if int64(len(buffer)) != chunksize { buffer = make([]byte, chunksize) } - if !verifiers[i].IsVerified() { - _, err = disk.ReadFileWithVerify(volume, path, - chunkOffset, buffer, verifiers[i]) - } else { - _, err = disk.ReadFile(volume, path, - chunkOffset, buffer) - } + _, err = disk.ReadFile(volume, path, chunkOffset, buffer, verifiers[i]) if err != nil { // LOG FIXME: add a conditional log // for read failures, once per-disk diff --git a/cmd/erasure-readfile.go b/cmd/erasure-readfile.go index b054754cb..b66c09441 100644 --- a/cmd/erasure-readfile.go +++ b/cmd/erasure-readfile.go @@ -157,11 +157,6 @@ func erasureReadFromFile(disk StorageAPI, volume, path string, offset int64, buf errChan <- traceError(errDiskNotFound) return } - var err error - if !verifier.IsVerified() { - _, err = disk.ReadFileWithVerify(volume, path, offset, buffer, verifier) - } else { - _, err = disk.ReadFile(volume, path, offset, buffer) - } + _, err := disk.ReadFile(volume, path, offset, buffer, verifier) errChan <- err } diff --git a/cmd/erasure-readfile_test.go b/cmd/erasure-readfile_test.go index be1febfbc..a17a87532 100644 --- a/cmd/erasure-readfile_test.go +++ b/cmd/erasure-readfile_test.go @@ -27,11 +27,7 @@ import ( "github.com/minio/minio/pkg/bpool" ) -func (d badDisk) ReadFile(volume string, path string, offset int64, buf []byte) (n int64, err error) { - return 0, errFaultyDisk -} - -func (d badDisk) ReadFileWithVerify(volume string, path string, offset int64, buf []byte, verifier *BitrotVerifier) (n int64, err error) { +func (d badDisk) ReadFile(volume string, path string, offset int64, buf []byte, verifier *BitrotVerifier) (n int64, err error) { return 0, errFaultyDisk } diff --git a/cmd/naughty-disk_test.go b/cmd/naughty-disk_test.go index a018bad59..a8b2c36c2 100644 --- a/cmd/naughty-disk_test.go +++ b/cmd/naughty-disk_test.go @@ -115,18 +115,11 @@ func (d *naughtyDisk) ListDir(volume, path string) (entries []string, err error) return d.disk.ListDir(volume, path) } -func (d *naughtyDisk) ReadFile(volume string, path string, offset int64, buf []byte) (n int64, err error) { +func (d *naughtyDisk) ReadFile(volume string, path string, offset int64, buf []byte, verifier *BitrotVerifier) (n int64, err error) { if err := d.calcError(); err != nil { return 0, err } - return d.disk.ReadFile(volume, path, offset, buf) -} - -func (d *naughtyDisk) ReadFileWithVerify(volume, path string, offset int64, buf []byte, verifier *BitrotVerifier) (n int64, err error) { - if err := d.calcError(); err != nil { - return 0, err - } - return d.disk.ReadFileWithVerify(volume, path, offset, buf, verifier) + return d.disk.ReadFile(volume, path, offset, buf, verifier) } func (d *naughtyDisk) PrepareFile(volume, path string, length int64) error { diff --git a/cmd/posix.go b/cmd/posix.go index 2e531655e..29672e13b 100644 --- a/cmd/posix.go +++ b/cmd/posix.go @@ -533,25 +533,14 @@ func (s *posix) ReadAll(volume, path string) (buf []byte, err error) { // for io.EOF. // // If an EOF happens after reading some but not all the bytes, -// ReadFull returns ErrUnexpectedEOF. +// ReadFile returns ErrUnexpectedEOF. +// +// If the BitrotVerifier is not nil or not verified ReadFile +// tries to verify whether the disk has bitrot. // // Additionally ReadFile also starts reading from an offset. ReadFile // semantics are same as io.ReadFull. -func (s *posix) ReadFile(volume, path string, offset int64, buf []byte) (n int64, err error) { - return s.ReadFileWithVerify(volume, path, offset, buf, &BitrotVerifier{verified: true}) -} - -// ReadFileWithVerify is the same as ReadFile but with hashsum -// verification: the operation will fail if the hash verification -// fails. -// -// The `expectedHash` is the expected hex-encoded hash string for -// verification. With an empty expected hash string, hash verification -// is skipped. An empty HashAlgo defaults to `blake2b`. -// -// The function takes care to minimize the number of disk read -// operations. -func (s *posix) ReadFileWithVerify(volume, path string, offset int64, buffer []byte, verifier *BitrotVerifier) (n int64, err error) { +func (s *posix) ReadFile(volume, path string, offset int64, buffer []byte, verifier *BitrotVerifier) (n int64, err error) { defer func() { if err == syscall.EIO { atomic.AddInt32(&s.ioErrCount, 1) @@ -612,7 +601,7 @@ func (s *posix) ReadFileWithVerify(volume, path string, offset int64, buffer []b return 0, errIsNotRegular } - if !verifier.IsVerified() { + if verifier != nil && !verifier.IsVerified() { bufp := s.pool.Get().(*[]byte) defer s.pool.Put(bufp) diff --git a/cmd/posix_test.go b/cmd/posix_test.go index d366fce75..952705335 100644 --- a/cmd/posix_test.go +++ b/cmd/posix_test.go @@ -1074,7 +1074,7 @@ func TestPosixReadFile(t *testing.T) { var n int64 // Common read buffer. var buf = make([]byte, testCase.bufSize) - n, err = posixStorage.ReadFile(testCase.volume, testCase.fileName, testCase.offset, buf) + n, err = posixStorage.ReadFile(testCase.volume, testCase.fileName, testCase.offset, buf, nil) if err != nil && testCase.expectedErr != nil { // Validate if the type string of the errors are an exact match. if err.Error() != testCase.expectedErr.Error() { @@ -1135,7 +1135,7 @@ func TestPosixReadFile(t *testing.T) { if err == nil { // Common read buffer. var buf = make([]byte, 10) - if _, err = posixStorage.ReadFile("proc", "1/fd", 0, buf); err != errFileAccessDenied { + if _, err = posixStorage.ReadFile("proc", "1/fd", 0, buf, nil); err != errFileAccessDenied { t.Errorf("expected: %s, got: %s", errFileAccessDenied, err) } } @@ -1149,7 +1149,7 @@ func TestPosixReadFile(t *testing.T) { posixType.ioErrCount = int32(6) // Common read buffer. var buf = make([]byte, 10) - _, err = posixType.ReadFile("abc", "yes", 0, buf) + _, err = posixType.ReadFile("abc", "yes", 0, buf, nil) if err != errFaultyDisk { t.Fatalf("Expected \"Faulty Disk\", got: \"%s\"", err) } @@ -1183,8 +1183,8 @@ var posixReadFileWithVerifyTests = []struct { {file: "myobject", offset: 1000, length: 1001, algorithm: BLAKE2b512, expError: nil}, // 15 } -// TestPosixReadFileWithVerify - tests the posix level -// ReadFileWithVerify API. Only tests hashing related +// TestPosixReadFile with bitrot verification - tests the posix level +// ReadFile API with a BitrotVerifier. Only tests hashing related // functionality. Other functionality is tested with // TestPosixReadFile. func TestPosixReadFileWithVerify(t *testing.T) { @@ -1218,7 +1218,7 @@ func TestPosixReadFileWithVerify(t *testing.T) { } buffer := make([]byte, test.length) - n, err := posixStorage.ReadFileWithVerify(volume, test.file, int64(test.offset), buffer, NewBitrotVerifier(test.algorithm, h.Sum(nil))) + n, err := posixStorage.ReadFile(volume, test.file, int64(test.offset), buffer, NewBitrotVerifier(test.algorithm, h.Sum(nil))) switch { case err == nil && test.expError != nil: diff --git a/cmd/retry-storage.go b/cmd/retry-storage.go index 14a5d0ecb..a75e02e61 100644 --- a/cmd/retry-storage.go +++ b/cmd/retry-storage.go @@ -207,29 +207,18 @@ func (f *retryStorage) ReadAll(volume, path string) (buf []byte, err error) { } // ReadFile - a retryable implementation of reading at offset from a file. -func (f *retryStorage) ReadFile(volume, path string, offset int64, buffer []byte) (m int64, err error) { +func (f *retryStorage) ReadFile(volume, path string, offset int64, buffer []byte, verifier *BitrotVerifier) (m int64, err error) { if f.IsOffline() { return m, errDiskNotFound } - m, err = f.remoteStorage.ReadFile(volume, path, offset, buffer) + m, err = f.remoteStorage.ReadFile(volume, path, offset, buffer, verifier) if f.reInitUponDiskNotFound(err) { - m, err = f.remoteStorage.ReadFile(volume, path, offset, buffer) + m, err = f.remoteStorage.ReadFile(volume, path, offset, buffer, verifier) return m, retryToStorageErr(err) } return m, retryToStorageErr(err) } -// ReadFileWithVerify - a retryable implementation of reading at -// offset from a file with verification. -func (f retryStorage) ReadFileWithVerify(volume, path string, offset int64, buffer []byte, verifier *BitrotVerifier) (m int64, err error) { - - m, err = f.remoteStorage.ReadFileWithVerify(volume, path, offset, buffer, verifier) - if f.reInitUponDiskNotFound(err) { - m, err = f.remoteStorage.ReadFileWithVerify(volume, path, offset, buffer, verifier) - } - return m, retryToStorageErr(err) -} - // ListDir - a retryable implementation of listing directory entries. func (f *retryStorage) ListDir(volume, path string) (entries []string, err error) { if f.IsOffline() { diff --git a/cmd/retry-storage_test.go b/cmd/retry-storage_test.go index 263427720..28d9af5c0 100644 --- a/cmd/retry-storage_test.go +++ b/cmd/retry-storage_test.go @@ -290,7 +290,7 @@ func TestRetryStorage(t *testing.T) { for _, disk := range storageDisks { var buf2 = make([]byte, 5) var n int64 - if n, err = disk.ReadFile("existent", "path", 7, buf2); err != nil { + if n, err = disk.ReadFile("existent", "path", 7, buf2, nil); err != nil { t.Fatal(err) } if err != nil { @@ -312,11 +312,11 @@ func TestRetryStorage(t *testing.T) { var buf2 = make([]byte, 5) verifier := NewBitrotVerifier(SHA256, sha256Hash([]byte("Hello, World"))) var n int64 - if n, err = disk.ReadFileWithVerify("existent", "path", 7, buf2, verifier); err != nil { + if n, err = disk.ReadFile("existent", "path", 7, buf2, verifier); err != nil { t.Fatal(err) } if err != nil { - t.Error("Error in ReadFileWithVerify", err) + t.Error("Error in ReadFile with bitrot verification", err) } if n != 5 { t.Fatalf("Expected 5, got %d", n) diff --git a/cmd/storage-interface.go b/cmd/storage-interface.go index 52b1d1f16..597dd8cc8 100644 --- a/cmd/storage-interface.go +++ b/cmd/storage-interface.go @@ -40,8 +40,7 @@ type StorageAPI interface { // File operations. ListDir(volume, dirPath string) ([]string, error) - ReadFile(volume string, path string, offset int64, buf []byte) (n int64, err error) - ReadFileWithVerify(volume string, path string, offset int64, buf []byte, verifier *BitrotVerifier) (n int64, err error) + ReadFile(volume string, path string, offset int64, buf []byte, verifier *BitrotVerifier) (n int64, err error) PrepareFile(volume string, path string, len int64) (err error) AppendFile(volume string, path string, buf []byte) (err error) RenameFile(srcVolume, srcPath, dstVolume, dstPath string) error @@ -60,7 +59,7 @@ type storageReader struct { } func (r *storageReader) Read(p []byte) (n int, err error) { - nn, err := r.storage.ReadFile(r.volume, r.path, r.offset, p) + nn, err := r.storage.ReadFile(r.volume, r.path, r.offset, p, nil) r.offset += nn n = int(nn) diff --git a/cmd/storage-rpc-client.go b/cmd/storage-rpc-client.go index f5e627f2a..b39f11481 100644 --- a/cmd/storage-rpc-client.go +++ b/cmd/storage-rpc-client.go @@ -238,7 +238,7 @@ func (n *networkStorage) ReadAll(volume, path string) (buf []byte, err error) { } // ReadFile - reads a file at remote path and fills the buffer. -func (n *networkStorage) ReadFile(volume string, path string, offset int64, buffer []byte) (m int64, err error) { +func (n *networkStorage) ReadFile(volume string, path string, offset int64, buffer []byte, verifier *BitrotVerifier) (m int64, err error) { defer func() { if r := recover(); r != nil { // Recover any panic from allocation, and return error. @@ -246,41 +246,21 @@ func (n *networkStorage) ReadFile(volume string, path string, offset int64, buff } }() // Do not crash the server. - var result []byte - err = n.rpcClient.Call("Storage.ReadFileHandler", &ReadFileArgs{ - Vol: volume, - Path: path, - Offset: offset, - Buffer: buffer, - }, &result) - - // Copy results to buffer. - copy(buffer, result) - - // Return length of result, err if any. - return int64(len(result)), toStorageErr(err) -} - -// ReadFileWithVerify - reads a file at remote path and fills the buffer. -func (n *networkStorage) ReadFileWithVerify(volume string, path string, offset int64, buffer []byte, verifier *BitrotVerifier) (m int64, err error) { - - defer func() { - if r := recover(); r != nil { - // Recover any panic from allocation, and return error. - err = bytes.ErrTooLarge - } - }() // Do not crash the server. + args := ReadFileArgs{ + Vol: volume, + Path: path, + Offset: offset, + Buffer: buffer, + Verified: true, // mark read as verified by default + } + if verifier != nil { + args.Algo = verifier.algorithm + args.ExpectedHash = verifier.sum + args.Verified = verifier.IsVerified() + } var result []byte - err = n.rpcClient.Call("Storage.ReadFileWithVerifyHandler", - &ReadFileWithVerifyArgs{ - Vol: volume, - Path: path, - Offset: offset, - Buffer: buffer, - Algo: verifier.algorithm, - ExpectedHash: verifier.sum, - }, &result) + err = n.rpcClient.Call("Storage.ReadFileHandler", &args, &result) // Copy results to buffer. copy(buffer, result) diff --git a/cmd/storage-rpc-client_test.go b/cmd/storage-rpc-client_test.go index 54ba82bad..eda9b5150 100644 --- a/cmd/storage-rpc-client_test.go +++ b/cmd/storage-rpc-client_test.go @@ -379,7 +379,7 @@ func (s *TestRPCStorageSuite) testRPCStorageFileOps(t *testing.T) { t.Errorf("Expected `Hello, world`, got %s", string(buf)) } buf1 := make([]byte, 5) - n, err := storageDisk.ReadFile("myvol", "file1", 4, buf1) + n, err := storageDisk.ReadFile("myvol", "file1", 4, buf1, nil) if err != nil { t.Error("Unable to initiate ReadFile", err) } @@ -396,9 +396,9 @@ func (s *TestRPCStorageSuite) testRPCStorageFileOps(t *testing.T) { } verifier := NewBitrotVerifier(BLAKE2b512, blakeHash(buf)) buf2 := make([]byte, 2) - n, err = storageDisk.ReadFileWithVerify("myvol", "file1", 1, buf2, verifier) + n, err = storageDisk.ReadFile("myvol", "file1", 1, buf2, verifier) if err != nil { - t.Error("Error in ReadFileWithVerify", err) + t.Error("Error in ReadFile with bitrot verification", err) } if n != 2 { t.Errorf("Expected `2`, got %d", n) diff --git a/cmd/storage-rpc-server-datatypes.go b/cmd/storage-rpc-server-datatypes.go index 00c51254c..b417e3da1 100644 --- a/cmd/storage-rpc-server-datatypes.go +++ b/cmd/storage-rpc-server-datatypes.go @@ -59,30 +59,15 @@ type ReadFileArgs struct { // Data buffer read from the path at offset. Buffer []byte -} - -// ReadFileWithVerifyArgs represents read file RPC arguments. -type ReadFileWithVerifyArgs struct { - // Authentication token generated by Login. - AuthRPCArgs - - // Name of the volume. - Vol string - - // Name of the path. - Path string - - // Starting offset to start reading into Buffer. - Offset int64 - - // Data buffer read from the path at offset. - Buffer []byte // Algorithm used in bit-rot hash computation. Algo BitrotAlgorithm // Stored hash value used to compare with computed value. ExpectedHash []byte + + // Indicates whether the disk has already been verified + Verified bool } // PrepareFileArgs represents append file RPC arguments. diff --git a/cmd/storage-rpc-server.go b/cmd/storage-rpc-server.go index 8666d20ff..cc206b439 100644 --- a/cmd/storage-rpc-server.go +++ b/cmd/storage-rpc-server.go @@ -145,26 +145,13 @@ func (s *storageServer) ReadFileHandler(args *ReadFileArgs, reply *[]byte) (err if err = args.IsAuthenticated(); err != nil { return err } - - var n int64 - n, err = s.storage.ReadFile(args.Vol, args.Path, args.Offset, args.Buffer) - // Sending an error over the rpc layer, would cause unmarshalling to fail. In situations - // when we have short read i.e `io.ErrUnexpectedEOF` treat it as good condition and copy - // the buffer properly. - if err == io.ErrUnexpectedEOF { - // Reset to nil as good condition. - err = nil + var verifier *BitrotVerifier + if !args.Verified { + verifier = NewBitrotVerifier(args.Algo, args.ExpectedHash) } - *reply = args.Buffer[0:n] - return err -} -// ReadFileWithVerifyHandler - read file with verify handler is rpc wrapper to read file with verify. -func (s *storageServer) ReadFileWithVerifyHandler(args *ReadFileWithVerifyArgs, reply *[]byte) (err error) { - if err = args.IsAuthenticated(); err != nil { - return err - } - n, err := s.storage.ReadFileWithVerify(args.Vol, args.Path, args.Offset, args.Buffer, NewBitrotVerifier(args.Algo, args.ExpectedHash)) + var n int64 + n, err = s.storage.ReadFile(args.Vol, args.Path, args.Offset, args.Buffer, verifier) // Sending an error over the rpc layer, would cause unmarshalling to fail. In situations // when we have short read i.e `io.ErrUnexpectedEOF` treat it as good condition and copy // the buffer properly.