Add a check to check if disk is writable (#5662)

This check is a pre-emptive check to return
error early before we attempt to use the disk
for any other operations later.

refer #5645

cmd/fs-v1.go

@@ -24,11 +24,9 @@ import (
 	"io/ioutil"
 	"os"
 	"path"
-	"path/filepath"
 	"reflect"
 	"sort"
 	"sync"
-	"syscall"
 	"time"
 
 	"github.com/minio/minio-go/pkg/policy"
@@ -104,33 +102,7 @@ func NewFSObjectLayer(fsPath string) (ObjectLayer, error) {
 	}
 
 	var err error
-	// Disallow relative paths, figure out absolute paths.
+	if fsPath, err = checkPathValid(fsPath); err != nil {
-	fsPath, err = filepath.Abs(fsPath)
-	if err != nil {
-		return nil, err
-	}
-
-	fi, err := os.Stat((fsPath))
-	if err == nil {
-		if !fi.IsDir() {
-			return nil, syscall.ENOTDIR
-		}
-	}
-	if os.IsNotExist(err) {
-		// Disk not found create it.
-		err = os.MkdirAll(fsPath, 0777)
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	di, err := getDiskInfo((fsPath))
-	if err != nil {
-		return nil, err
-	}
-
-	// Check if disk has minimum required total space.
-	if err = checkDiskMinTotal(di); err != nil {
 		return nil, err
 	}
 

cmd/posix.go

@@ -72,6 +72,51 @@ func checkPathLength(pathName string) error {
 	return nil
 }
 
+func checkPathValid(path string) (string, error) {
+	if path == "" {
+		return path, errInvalidArgument
+	}
+
+	var err error
+	// Disallow relative paths, figure out absolute paths.
+	path, err = filepath.Abs(path)
+	if err != nil {
+		return path, err
+	}
+
+	fi, err := os.Stat(path)
+	if err != nil && !os.IsNotExist(err) {
+		return path, err
+	}
+	if os.IsNotExist(err) {
+		// Disk not found create it.
+		if err = os.MkdirAll(path, 0777); err != nil {
+			return path, err
+		}
+	}
+	if fi != nil && !fi.IsDir() {
+		return path, syscall.ENOTDIR
+	}
+
+	di, err := getDiskInfo(path)
+	if err != nil {
+		return path, err
+	}
+	if err = checkDiskMinTotal(di); err != nil {
+		return path, err
+	}
+
+	// check if backend is writable.
+	file, err := os.Create(pathJoin(path, ".writable-check.tmp"))
+	if err != nil {
+		return path, err
+	}
+	file.Close()
+
+	err = os.Remove(pathJoin(path, ".writable-check.tmp"))
+	return path, err
+}
+
 // isDirEmpty - returns whether given directory is empty or not.
 func isDirEmpty(dirname string) bool {
 	f, err := os.Open((dirname))
@@ -98,17 +143,13 @@ func isDirEmpty(dirname string) bool {
 
 // Initialize a new storage disk.
 func newPosix(path string) (StorageAPI, error) {
-	if path == "" {
+	var err error
-		return nil, errInvalidArgument
+	if path, err = checkPathValid(path); err != nil {
-	}
-	// Disallow relative paths, figure out absolute paths.
-	diskPath, err := filepath.Abs(path)
-	if err != nil {
 		return nil, err
 	}
 
 	st := &posix{
-		diskPath: diskPath,
+		diskPath: path,
 		// 1MiB buffer pool for posix internal operations.
 		pool: sync.Pool{
 			New: func() interface{} {
@@ -117,30 +158,6 @@ func newPosix(path string) (StorageAPI, error) {
 			},
 		},
 	}
-	fi, err := os.Stat((diskPath))
-	if err == nil {
-		if !fi.IsDir() {
-			return nil, syscall.ENOTDIR
-		}
-	}
-	if os.IsNotExist(err) {
-		// Disk not found create it.
-		err = os.MkdirAll(diskPath, 0777)
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	di, err := getDiskInfo((diskPath))
-	if err != nil {
-		return nil, err
-	}
-
-	// Check if disk has minimum required total space.
-	if err = checkDiskMinTotal(di); err != nil {
-		return nil, err
-	}
-
 	st.connected = true
 
 	// Success.
@@ -435,7 +452,10 @@ func (s *posix) DeleteVol(volume string) (err error) {
 			return errVolumeNotFound
 		} else if isSysErrNotEmpty(err) {
 			return errVolumeNotEmpty
+		} else if os.IsPermission(err) {
+			return errDiskAccessDenied
 		}
+
 		return err
 	}
 	return nil
@@ -678,7 +698,7 @@ func (s *posix) createFile(volume, path string) (f *os.File, err error) {
 
 	// Verify if the file already exists and is not of regular type.
 	var st os.FileInfo
-	if st, err = os.Stat((filePath)); err == nil {
+	if st, err = os.Stat(filePath); err == nil {
 		if !st.Mode().IsRegular() {
 			return nil, errIsNotRegular
 		}
@@ -690,11 +710,13 @@ func (s *posix) createFile(volume, path string) (f *os.File, err error) {
 		}
 	}
 
-	w, err := os.OpenFile((filePath), os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0666)
+	w, err := os.OpenFile(filePath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0666)
 	if err != nil {
 		// File path cannot be verified since one of the parents is a file.
 		if isSysErrNotDir(err) {
 			return nil, errFileAccessDenied
+		} else if os.IsPermission(err) {
+			return nil, errFileAccessDenied
 		}
 		return nil, err
 	}

cmd/posix_test.go

@@ -389,12 +389,27 @@ func TestPosixMakeVol(t *testing.T) {
 		if err = os.Chmod(permDeniedDir, 0400); err != nil {
 			t.Fatalf("Unable to change permission to temporary directory %v. %v", permDeniedDir, err)
 		}
+
 		// Initialize posix storage layer for permission denied error.
+		_, err = newPosix(permDeniedDir)
+		if err != nil && !os.IsPermission(err) {
+			t.Fatalf("Unable to initialize posix, %s", err)
+		}
+
+		if err = os.Chmod(permDeniedDir, 0755); err != nil {
+			t.Fatalf("Unable to change permission to temporary directory %v. %v", permDeniedDir, err)
+		}
+
 		posixStorage, err = newPosix(permDeniedDir)
 		if err != nil {
 			t.Fatalf("Unable to initialize posix, %s", err)
 		}
 
+		// change backend permissions for MakeVol error.
+		if err = os.Chmod(permDeniedDir, 0400); err != nil {
+			t.Fatalf("Unable to change permission to temporary directory %v. %v", permDeniedDir, err)
+		}
+
 		if err := posixStorage.MakeVol("test-vol"); err != errDiskAccessDenied {
 			t.Fatalf("expected: %s, got: %s", errDiskAccessDenied, err)
 		}
@@ -491,13 +506,28 @@ func TestPosixDeleteVol(t *testing.T) {
 		if err = os.Chmod(permDeniedDir, 0400); err != nil {
 			t.Fatalf("Unable to change permission to temporary directory %v. %v", permDeniedDir, err)
 		}
+
 		// Initialize posix storage layer for permission denied error.
+		_, err = newPosix(permDeniedDir)
+		if err != nil && !os.IsPermission(err) {
+			t.Fatalf("Unable to initialize posix, %s", err)
+		}
+
+		if err = os.Chmod(permDeniedDir, 0755); err != nil {
+			t.Fatalf("Unable to change permission to temporary directory %v. %v", permDeniedDir, err)
+		}
+
 		posixStorage, err = newPosix(permDeniedDir)
 		if err != nil {
 			t.Fatalf("Unable to initialize posix, %s", err)
 		}
 
-		if err = posixStorage.DeleteVol("bin"); !os.IsPermission(err) {
+		// change backend permissions for MakeVol error.
+		if err = os.Chmod(permDeniedDir, 0400); err != nil {
+			t.Fatalf("Unable to change permission to temporary directory %v. %v", permDeniedDir, err)
+		}
+
+		if err = posixStorage.DeleteVol("mybucket"); err != errDiskAccessDenied {
 			t.Fatalf("expected: Permission error, got: %s", err)
 		}
 	}
@@ -774,14 +804,22 @@ func TestPosixPosixListDir(t *testing.T) {
 		defer removePermDeniedFile(permDeniedDir)
 
 		// Initialize posix storage layer for permission denied error.
+		_, err = newPosix(permDeniedDir)
+		if err != nil && !os.IsPermission(err) {
+			t.Fatalf("Unable to initialize posix, %s", err)
+		}
+
+		if err = os.Chmod(permDeniedDir, 0755); err != nil {
+			t.Fatalf("Unable to change permission to temporary directory %v. %v", permDeniedDir, err)
+		}
+
 		posixStorage, err = newPosix(permDeniedDir)
 		if err != nil {
 			t.Fatalf("Unable to initialize posix, %s", err)
 		}
 
-		errMsg := fmt.Sprintf("stat %v: permission denied", slashpath.Join(permDeniedDir, "mybucket"))
+		if err = posixStorage.DeleteFile("mybucket", "myobject"); err != errFileAccessDenied {
-		if err = posixStorage.DeleteFile("mybucket", "myobject"); err.Error() != errMsg {
+			t.Errorf("expected: %s, got: %s", errFileAccessDenied, err)
-			t.Errorf("expected: %s, got: %s", errMsg, err)
 		}
 	}
 
@@ -912,14 +950,22 @@ func TestPosixDeleteFile(t *testing.T) {
 		defer removePermDeniedFile(permDeniedDir)
 
 		// Initialize posix storage layer for permission denied error.
+		_, err = newPosix(permDeniedDir)
+		if err != nil && !os.IsPermission(err) {
+			t.Fatalf("Unable to initialize posix, %s", err)
+		}
+
+		if err = os.Chmod(permDeniedDir, 0755); err != nil {
+			t.Fatalf("Unable to change permission to temporary directory %v. %v", permDeniedDir, err)
+		}
+
 		posixStorage, err = newPosix(permDeniedDir)
 		if err != nil {
 			t.Fatalf("Unable to initialize posix, %s", err)
 		}
 
-		errMsg := fmt.Sprintf("stat %v: permission denied", slashpath.Join(permDeniedDir, "mybucket"))
+		if err = posixStorage.DeleteFile("mybucket", "myobject"); err != errFileAccessDenied {
-		if err = posixStorage.DeleteFile("mybucket", "myobject"); err.Error() != errMsg {
+			t.Errorf("expected: %s, got: %s", errFileAccessDenied, err)
-			t.Errorf("expected: %s, got: %s", errMsg, err)
 		}
 	}
 
@@ -1124,20 +1170,31 @@ func TestPosixReadFile(t *testing.T) {
 	}
 
 	// TestPosix for permission denied.
-	if runtime.GOOS == "linux" {
+	if runtime.GOOS != globalWindowsOSName {
+		permDeniedDir := createPermDeniedFile(t)
+		defer removePermDeniedFile(permDeniedDir)
+
 		// Initialize posix storage layer for permission denied error.
-		posixStorage, err = newPosix("/")
+		_, err = newPosix(permDeniedDir)
-		if err != nil {
+		if err != nil && !os.IsPermission(err) {
-			t.Errorf("Unable to initialize posix, %s", err)
+			t.Fatalf("Unable to initialize posix, %s", err)
 		}
-		if err == nil {
+
+		if err = os.Chmod(permDeniedDir, 0755); err != nil {
+			t.Fatalf("Unable to change permission to temporary directory %v. %v", permDeniedDir, err)
+		}
+
+		posixStorage, err = newPosix(permDeniedDir)
+		if err != nil {
+			t.Fatalf("Unable to initialize posix, %s", err)
+		}
+
 		// Common read buffer.
 		var buf = make([]byte, 10)
-			if _, err = posixStorage.ReadFile("proc", "1/fd", 0, buf, nil); err != errFileAccessDenied {
+		if _, err = posixStorage.ReadFile("mybucket", "myobject", 0, buf, nil); err != errFileAccessDenied {
 			t.Errorf("expected: %s, got: %s", errFileAccessDenied, err)
 		}
 	}
-	}
 
 	// TestPosixing for faulty disk.
 	// setting ioErrCnt to 6.
@@ -1295,15 +1352,25 @@ func TestPosixAppendFile(t *testing.T) {
 
 		var posixPermStorage StorageAPI
 		// Initialize posix storage layer for permission denied error.
+		_, err = newPosix(permDeniedDir)
+		if err != nil && !os.IsPermission(err) {
+			t.Fatalf("Unable to initialize posix, %s", err)
+		}
+
+		if err = os.Chmod(permDeniedDir, 0755); err != nil {
+			t.Fatalf("Unable to change permission to temporary directory %v. %v", permDeniedDir, err)
+		}
+
 		posixPermStorage, err = newPosix(permDeniedDir)
 		if err != nil {
 			t.Fatalf("Unable to initialize posix, %s", err)
 		}
 
-		if err = posixPermStorage.AppendFile("mybucket", "myobject", []byte("hello, world")); !os.IsPermission(err) {
+		if err = posixPermStorage.AppendFile("mybucket", "myobject", []byte("hello, world")); err != errFileAccessDenied {
 			t.Fatalf("expected: Permission error, got: %s", err)
 		}
 	}
+
 	// TestPosix case with invalid volume name.
 	// A valid volume name should be atleast of size 3.
 	err = posixStorage.AppendFile("bn", "yes", []byte("hello, world"))
@@ -1386,12 +1453,21 @@ func TestPosixPrepareFile(t *testing.T) {
 
 		var posixPermStorage StorageAPI
 		// Initialize posix storage layer for permission denied error.
+		_, err = newPosix(permDeniedDir)
+		if err != nil && !os.IsPermission(err) {
+			t.Fatalf("Unable to initialize posix, %s", err)
+		}
+
+		if err = os.Chmod(permDeniedDir, 0755); err != nil {
+			t.Fatalf("Unable to change permission to temporary directory %v. %v", permDeniedDir, err)
+		}
+
 		posixPermStorage, err = newPosix(permDeniedDir)
 		if err != nil {
 			t.Fatalf("Unable to initialize posix, %s", err)
 		}
 
-		if err = posixPermStorage.PrepareFile("mybucket", "myobject", 16); !os.IsPermission(err) {
+		if err = posixPermStorage.PrepareFile("mybucket", "myobject", 16); err != errFileAccessDenied {
 			t.Fatalf("expected: Permission error, got: %s", err)
 		}
 	}