From d64c3fd4649d826d2d741771e2e4f4f6bc90a16d Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sat, 2 Jul 2016 01:59:28 -0700 Subject: [PATCH] posix: Return errDiskNotWritable during disk initialization. (#2048) It can happen that minio server might not have writable permissions on the export paths command line. Fixes #2035 --- fs-v1.go | 15 +++++++- object-common.go | 68 +++++++++++++++++++++++++++---------- posix-utils_windows_test.go | 2 +- posix.go | 9 +++-- posix_test.go | 44 ++++++++++++++++++++++++ server_test.go | 2 +- storage-errors.go | 3 ++ test-utils_test.go | 4 +-- xl-v1.go | 11 ++++-- 9 files changed, 131 insertions(+), 27 deletions(-) diff --git a/fs-v1.go b/fs-v1.go index 652e0a4da..0dd67ec73 100644 --- a/fs-v1.go +++ b/fs-v1.go @@ -85,8 +85,21 @@ func newFSObjects(disk string) (ObjectLayer, error) { return nil, err } + // Attempt to create `.minio`. + err = storage.MakeVol(minioMetaBucket) + if err != nil { + switch err { + // Ignore the errors. + case errVolumeExists, errDiskNotFound, errFaultyDisk: + default: + return nil, toObjectErr(err, minioMetaBucket) + } + } + // Runs house keeping code, like creating minioMetaBucket, cleaning up tmp files etc. - fsHouseKeeping(storage) + if err = fsHouseKeeping(storage); err != nil { + return nil, err + } // loading format.json from minioMetaBucket. // Note: The format.json content is ignored, reserved for future use. diff --git a/object-common.go b/object-common.go index 0eb18d9e1..638b15d3d 100644 --- a/object-common.go +++ b/object-common.go @@ -45,17 +45,10 @@ func registerShutdown(callback func()) { // House keeping code needed for FS. func fsHouseKeeping(storageDisk StorageAPI) error { - // Attempt to create `.minio`. - err := storageDisk.MakeVol(minioMetaBucket) - if err != nil { - if err != errVolumeExists && err != errDiskNotFound && err != errFaultyDisk { - return err - } - } // Cleanup all temp entries upon start. - err = cleanupDir(storageDisk, minioMetaBucket, tmpMetaPrefix) + err := cleanupDir(storageDisk, minioMetaBucket, tmpMetaPrefix) if err != nil { - return err + return toObjectErr(err, minioMetaBucket, tmpMetaPrefix) } return nil } @@ -70,8 +63,8 @@ func newStorageAPI(disk string) (storage StorageAPI, err error) { return newRPCClient(disk) } -// House keeping code needed for XL. -func xlHouseKeeping(storageDisks []StorageAPI) error { +// Initializes meta volume on all input storage disks. +func initMetaVolume(storageDisks []StorageAPI) error { // This happens for the first time, but keep this here since this // is the only place where it can be made expensive optimizing all // other calls. Create minio meta volume, if it doesn't exist yet. @@ -93,17 +86,58 @@ func xlHouseKeeping(storageDisks []StorageAPI) error { // Attempt to create `.minio`. err := disk.MakeVol(minioMetaBucket) - if err != nil && err != errVolumeExists && err != errDiskNotFound && err != errFaultyDisk { - errs[index] = err - return + if err != nil { + switch err { + // Ignored errors. + case errVolumeExists, errDiskNotFound, errFaultyDisk: + default: + errs[index] = err + } } + }(index, disk) + } + + // Wait for all cleanup to finish. + wg.Wait() + + // Return upon first error. + for _, err := range errs { + if err == nil { + continue + } + return toObjectErr(err, minioMetaBucket) + } + + // Return success here. + return nil +} + +// House keeping code needed for XL. +func xlHouseKeeping(storageDisks []StorageAPI) error { + // This happens for the first time, but keep this here since this + // is the only place where it can be made expensive optimizing all + // other calls. Create metavolume. + var wg = &sync.WaitGroup{} + + // Initialize errs to collect errors inside go-routine. + var errs = make([]error, len(storageDisks)) + + // Initialize all disks in parallel. + for index, disk := range storageDisks { + if disk == nil { + errs[index] = errDiskNotFound + continue + } + wg.Add(1) + go func(index int, disk StorageAPI) { + // Indicate this wait group is done. + defer wg.Done() + // Cleanup all temp entries upon start. - err = cleanupDir(disk, minioMetaBucket, tmpMetaPrefix) + err := cleanupDir(disk, minioMetaBucket, tmpMetaPrefix) if err != nil { errs[index] = err - return } - errs[index] = nil }(index, disk) } diff --git a/posix-utils_windows_test.go b/posix-utils_windows_test.go index d63d687bd..2fc30590d 100644 --- a/posix-utils_windows_test.go +++ b/posix-utils_windows_test.go @@ -144,7 +144,7 @@ func Test32kUNCPath(t *testing.T) { // The following calculation was derived empirically. It is not exactly MAX_PATH - len(longDiskName) // possibly due to expansion rules as mentioned here - // https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx#maxpath - remaining := 32767 - 25 - len(longDiskName) + remaining := 32767 - 25 - len(longDiskName) - 10 longDiskName = longDiskName + `\` + strings.Repeat("a", remaining) } err = mkdirAll(longDiskName, 0777) diff --git a/posix.go b/posix.go index fdde7cae4..e868d73af 100644 --- a/posix.go +++ b/posix.go @@ -218,8 +218,13 @@ func (s *posix) MakeVol(volume string) (err error) { } // Make a volume entry, with mode 0777 mkdir honors system umask. err = os.Mkdir(preparePath(volumeDir), 0777) - if err != nil && os.IsExist(err) { - return errVolumeExists + if err != nil { + if os.IsExist(err) { + return errVolumeExists + } else if os.IsPermission(err) { + return errDiskAccessDenied + } + return err } // Success return nil diff --git a/posix_test.go b/posix_test.go index c2b5d2f3c..5d69c5643 100644 --- a/posix_test.go +++ b/posix_test.go @@ -19,6 +19,7 @@ package main import ( "io/ioutil" "os" + "syscall" "testing" ) @@ -114,3 +115,46 @@ func TestReadAll(t *testing.T) { } } } + +// TestNewPosix all the cases handled in posix storage layer initialization. +func TestNewPosix(t *testing.T) { + // Temporary dir name. + tmpDirName := os.TempDir() + "/" + "minio-" + nextSuffix() + // Temporary file name. + tmpFileName := os.TempDir() + "/" + "minio-" + nextSuffix() + f, _ := os.Create(tmpFileName) + f.Close() + defer os.Remove(tmpFileName) + + // List of all tests for posix initialization. + testCases := []struct { + diskPath string + err error + }{ + // Validates input argument cannot be empty. + { + "", + errInvalidArgument, + }, + // Validates if the directory does not exist and + // gets automatically created. + { + tmpDirName, + nil, + }, + // Validates if the disk exists as file and returns error + // not a directory. + { + tmpFileName, + syscall.ENOTDIR, + }, + } + + // Validate all test cases. + for i, testCase := range testCases { + _, err := newPosix(testCase.diskPath) + if err != testCase.err { + t.Fatalf("Test %d failed wanted: %s, got: %s", i+1, err, testCase.err) + } + } +} diff --git a/server_test.go b/server_test.go index 62e0f3a84..7656f9825 100644 --- a/server_test.go +++ b/server_test.go @@ -1912,7 +1912,7 @@ func (s *TestSuiteFS) TestObjectValidMD5(c *C) { client = http.Client{} response, err = client.Do(request) c.Assert(err, IsNil) - // exepcting a successfull upload. + // exepcting a successful upload. c.Assert(response.StatusCode, Equals, http.StatusOK) objectName = "test-2-object" buffer1 = bytes.NewReader(data) diff --git a/storage-errors.go b/storage-errors.go index c92a82828..b322f8b42 100644 --- a/storage-errors.go +++ b/storage-errors.go @@ -33,6 +33,9 @@ var errDiskFull = errors.New("disk path full") // errDiskNotFount - cannot find the underlying configured disk anymore. var errDiskNotFound = errors.New("disk not found") +// errDiskAccessDenied - we don't have write permissions on disk. +var errDiskAccessDenied = errors.New("disk access denied") + // errFileNotFound - cannot find the file. var errFileNotFound = errors.New("file not found") diff --git a/test-utils_test.go b/test-utils_test.go index dc595bbee..1ba831ec9 100644 --- a/test-utils_test.go +++ b/test-utils_test.go @@ -69,12 +69,12 @@ const ( var randN uint32 var randmu sync.Mutex -// reseed - returns a new seed everytime the function is called. +// reseed - returns a new seed every time the function is called. func reseed() uint32 { return uint32(time.Now().UnixNano() + int64(os.Getpid())) } -// nextSuffix - provides a new unique suffix everytime the function is called. +// nextSuffix - provides a new unique suffix every time the function is called. func nextSuffix() string { randmu.Lock() r := randN diff --git a/xl-v1.go b/xl-v1.go index 3211f8eac..a346bbdaf 100644 --- a/xl-v1.go +++ b/xl-v1.go @@ -122,9 +122,6 @@ func newXLObjects(disks []string) (ObjectLayer, error) { } } - // Runs house keeping code, like creating minioMetaBucket, cleaning up tmp files etc. - xlHouseKeeping(storageDisks) - // Attempt to load all `format.json`. formatConfigs, sErrs := loadAllFormats(storageDisks) @@ -138,6 +135,9 @@ func newXLObjects(disks []string) (ObjectLayer, error) { // Handles different cases properly. switch reduceFormatErrs(sErrs, len(storageDisks)) { case errUnformattedDisk: + if err := initMetaVolume(storageDisks); err != nil { + return nil, fmt.Errorf("Unable to initialize '.minio' meta volume, %s", err) + } // All drives online but fresh, initialize format. if err := initFormatXL(storageDisks); err != nil { return nil, fmt.Errorf("Unable to initialize format, %s", err) @@ -153,6 +153,11 @@ func newXLObjects(disks []string) (ObjectLayer, error) { // FIXME. } + // Runs house keeping code, like t, cleaning up tmp files etc. + if err := xlHouseKeeping(storageDisks); err != nil { + return nil, err + } + // Load saved XL format.json and validate. newPosixDisks, err := loadFormatXL(storageDisks) if err != nil {