xl/fs: isFunctions should only return boolean. (#1525)

log the unrecognize errors.

fs-objects-multipart.go

@@ -46,15 +46,17 @@ func (fs fsObjects) CompleteMultipartUpload(bucket string, object string, upload
 	if !IsValidBucketName(bucket) {
 		return "", BucketNameInvalid{Bucket: bucket}
 	}
+	// Verify whether the bucket exists.
+	if !isBucketExist(fs.storage, bucket) {
+		return "", BucketNotFound{Bucket: bucket}
+	}
 	if !IsValidObjectName(object) {
 		return "", ObjectNameInvalid{
 			Bucket: bucket,
 			Object: object,
 		}
 	}
-	if status, err := isUploadIDExists(fs.storage, bucket, object, uploadID); err != nil {
+	if !isUploadIDExists(fs.storage, bucket, object, uploadID) {
-		return "", err
-	} else if !status {
 		return "", InvalidUploadID{UploadID: uploadID}
 	}
 

fs-objects.go

@@ -90,11 +90,14 @@ func (fs fsObjects) DeleteBucket(bucket string) error {
 func (fs fsObjects) GetObject(bucket, object string, startOffset int64) (io.ReadCloser, error) {
 	// Verify if bucket is valid.
 	if !IsValidBucketName(bucket) {
-		return nil, (BucketNameInvalid{Bucket: bucket})
+		return nil, BucketNameInvalid{Bucket: bucket}
+	}
+	if !isBucketExist(fs.storage, bucket) {
+		return nil, BucketNotFound{Bucket: bucket}
 	}
 	// Verify if object is valid.
 	if !IsValidObjectName(object) {
-		return nil, (ObjectNameInvalid{Bucket: bucket, Object: object})
+		return nil, ObjectNameInvalid{Bucket: bucket, Object: object}
 	}
 	fileReader, err := fs.storage.ReadFile(bucket, object, startOffset)
 	if err != nil {
@@ -109,6 +112,9 @@ func (fs fsObjects) GetObjectInfo(bucket, object string) (ObjectInfo, error) {
 	if !IsValidBucketName(bucket) {
 		return ObjectInfo{}, (BucketNameInvalid{Bucket: bucket})
 	}
+	if !isBucketExist(fs.storage, bucket) {
+		return ObjectInfo{}, BucketNotFound{Bucket: bucket}
+	}
 	// Verify if object is valid.
 	if !IsValidObjectName(object) {
 		return ObjectInfo{}, (ObjectNameInvalid{Bucket: bucket, Object: object})
@@ -145,6 +151,9 @@ func (fs fsObjects) DeleteObject(bucket, object string) error {
 	if !IsValidBucketName(bucket) {
 		return BucketNameInvalid{Bucket: bucket}
 	}
+	if !isBucketExist(fs.storage, bucket) {
+		return BucketNotFound{Bucket: bucket}
+	}
 	if !IsValidObjectName(object) {
 		return ObjectNameInvalid{Bucket: bucket, Object: object}
 	}
@@ -154,6 +163,7 @@ func (fs fsObjects) DeleteObject(bucket, object string) error {
 	return nil
 }
 
+// ListObjects - list all objects.
 func (fs fsObjects) ListObjects(bucket, prefix, marker, delimiter string, maxKeys int) (ListObjectsInfo, error) {
 	return listObjectsCommon(fs, bucket, prefix, marker, delimiter, maxKeys)
 }

object-api-getobjectinfo_test.go

@@ -67,8 +67,8 @@ func testGetObjectInfo(obj ObjectLayer, instanceType string, t *testing.T) {
 		{"abcdefgh", "abc", ObjectInfo{}, BucketNotFound{Bucket: "abcdefgh"}, false},
 		{"ijklmnop", "efg", ObjectInfo{}, BucketNotFound{Bucket: "ijklmnop"}, false},
 		// Test cases with valid but non-existing bucket names and invalid object name (Test number 8-9).
-		{"abcdefgh", "", ObjectInfo{}, ObjectNameInvalid{Bucket: "abcdefgh", Object: ""}, false},
+		{"test-getobjectinfo", "", ObjectInfo{}, ObjectNameInvalid{Bucket: "test-getobjectinfo", Object: ""}, false},
-		{"ijklmnop", "", ObjectInfo{}, ObjectNameInvalid{Bucket: "ijklmnop", Object: ""}, false},
+		{"test-getobjectinfo", "", ObjectInfo{}, ObjectNameInvalid{Bucket: "test-getobjectinfo", Object: ""}, false},
 		// Test cases with non-existing object name with existing bucket (Test number 10-12).
 		{"test-getobjectinfo", "Africa", ObjectInfo{}, ObjectNotFound{Bucket: "test-getobjectinfo", Object: "Africa"}, false},
 		{"test-getobjectinfo", "Antartica", ObjectInfo{}, ObjectNotFound{Bucket: "test-getobjectinfo", Object: "Antartica"}, false},

object-api-multipart_test.go

@@ -149,7 +149,7 @@ func testObjectAPIPutObjectPart(obj ObjectLayer, instanceType string, t *testing
 		{"a", "obj", "", 1, "", "", 0, false, "", fmt.Errorf("%s", "Bucket name invalid: a")},
 		// Test case - 5.
 		// Case with invalid object names.
-		{"abc", "", "", 1, "", "", 0, false, "", fmt.Errorf("%s", "Object name invalid: abc#")},
+		{bucket, "", "", 1, "", "", 0, false, "", fmt.Errorf("%s", "Object name invalid: minio-bucket#")},
 		// Test case - 6.
 		// Valid object and bucket names but non-existent bucket.
 		{"abc", "def", "", 1, "", "", 0, false, "", fmt.Errorf("%s", "Bucket not found: abc")},

object-common-multipart.go

@@ -76,18 +76,16 @@ func newMultipartUploadCommon(storage StorageAPI, bucket string, object string)
 	if !IsValidBucketName(bucket) {
 		return "", BucketNameInvalid{Bucket: bucket}
 	}
+	// Verify whether the bucket exists.
+	if !isBucketExist(storage, bucket) {
+		return "", BucketNotFound{Bucket: bucket}
+	}
+
 	// Verify if object name is valid.
 	if !IsValidObjectName(object) {
 		return "", ObjectNameInvalid{Bucket: bucket, Object: object}
 	}
 
-	// Verify whether the bucket exists.
-	if isExist, err := isBucketExist(storage, bucket); err != nil {
-		return "", err
-	} else if !isExist {
-		return "", BucketNotFound{Bucket: bucket}
-	}
-
 	// Loops through until successfully generates a new unique upload id.
 	for {
 		uuid, err := uuid.New()
@@ -138,20 +136,14 @@ func putObjectPartCommon(storage StorageAPI, bucket string, object string, uploa
 	if !IsValidBucketName(bucket) {
 		return "", BucketNameInvalid{Bucket: bucket}
 	}
+	// Verify whether the bucket exists.
+	if !isBucketExist(storage, bucket) {
+		return "", BucketNotFound{Bucket: bucket}
+	}
 	if !IsValidObjectName(object) {
 		return "", ObjectNameInvalid{Bucket: bucket, Object: object}
 	}
-
+	if !isUploadIDExists(storage, bucket, object, uploadID) {
-	// Verify whether the bucket exists.
-	if isExist, err := isBucketExist(storage, bucket); err != nil {
-		return "", err
-	} else if !isExist {
-		return "", BucketNotFound{Bucket: bucket}
-	}
-
-	if status, err := isUploadIDExists(storage, bucket, object, uploadID); err != nil {
-		return "", err
-	} else if !status {
 		return "", InvalidUploadID{UploadID: uploadID}
 	}
 
@@ -251,12 +243,14 @@ func abortMultipartUploadCommon(storage StorageAPI, bucket, object, uploadID str
 	if !IsValidBucketName(bucket) {
 		return BucketNameInvalid{Bucket: bucket}
 	}
+	// Verify whether the bucket exists.
+	if !isBucketExist(storage, bucket) {
+		return BucketNotFound{Bucket: bucket}
+	}
 	if !IsValidObjectName(object) {
 		return ObjectNameInvalid{Bucket: bucket, Object: object}
 	}
-	if status, err := isUploadIDExists(storage, bucket, object, uploadID); err != nil {
+	if !isUploadIDExists(storage, bucket, object, uploadID) {
-		return err
-	} else if !status {
 		return InvalidUploadID{UploadID: uploadID}
 	}
 	return cleanupUploadedParts(storage, mpartMetaPrefix, bucket, object, uploadID)
@@ -393,11 +387,22 @@ func listMetaBucketMultipartFiles(layer ObjectLayer, prefixPath string, markerPa
 // listMultipartUploadsCommon - lists all multipart uploads, common
 // function for both object layers.
 func listMultipartUploadsCommon(layer ObjectLayer, bucket, prefix, keyMarker, uploadIDMarker, delimiter string, maxUploads int) (ListMultipartsInfo, error) {
+	var storage StorageAPI
+	switch l := layer.(type) {
+	case xlObjects:
+		storage = l.storage
+	case fsObjects:
+		storage = l.storage
+	}
+
 	result := ListMultipartsInfo{}
 	// Verify if bucket is valid.
 	if !IsValidBucketName(bucket) {
 		return ListMultipartsInfo{}, BucketNameInvalid{Bucket: bucket}
 	}
+	if !isBucketExist(storage, bucket) {
+		return ListMultipartsInfo{}, BucketNotFound{Bucket: bucket}
+	}
 	if !IsValidObjectPrefix(prefix) {
 		return ListMultipartsInfo{}, ObjectNameInvalid{Bucket: bucket, Object: prefix}
 	}
@@ -495,15 +500,17 @@ func listMultipartUploadsCommon(layer ObjectLayer, bucket, prefix, keyMarker, up
 func listObjectPartsCommon(storage StorageAPI, bucket, object, uploadID string, partNumberMarker, maxParts int) (ListPartsInfo, error) {
 	// Verify if bucket is valid.
 	if !IsValidBucketName(bucket) {
-		return ListPartsInfo{}, (BucketNameInvalid{Bucket: bucket})
+		return ListPartsInfo{}, BucketNameInvalid{Bucket: bucket}
+	}
+	// Verify whether the bucket exists.
+	if !isBucketExist(storage, bucket) {
+		return ListPartsInfo{}, BucketNotFound{Bucket: bucket}
 	}
 	if !IsValidObjectName(object) {
-		return ListPartsInfo{}, (ObjectNameInvalid{Bucket: bucket, Object: object})
+		return ListPartsInfo{}, ObjectNameInvalid{Bucket: bucket, Object: object}
 	}
-	if status, err := isUploadIDExists(storage, bucket, object, uploadID); err != nil {
+	if !isUploadIDExists(storage, bucket, object, uploadID) {
-		return ListPartsInfo{}, err
+		return ListPartsInfo{}, InvalidUploadID{UploadID: uploadID}
-	} else if !status {
-		return ListPartsInfo{}, (InvalidUploadID{UploadID: uploadID})
 	}
 	result := ListPartsInfo{}
 	entries, err := storage.ListDir(minioMetaBucket, path.Join(mpartMetaPrefix, bucket, object, uploadID))
@@ -547,16 +554,16 @@ func listObjectPartsCommon(storage StorageAPI, bucket, object, uploadID string,
 }
 
 // isUploadIDExists - verify if a given uploadID exists and is valid.
-func isUploadIDExists(storage StorageAPI, bucket, object, uploadID string) (bool, error) {
+func isUploadIDExists(storage StorageAPI, bucket, object, uploadID string) bool {
 	uploadIDPath := path.Join(mpartMetaPrefix, bucket, object, uploadID, incompleteFile)
 	st, err := storage.StatFile(minioMetaBucket, uploadIDPath)
 	if err != nil {
-		// Upload id does not exist.
 		if err == errFileNotFound {
-			return false, nil
+			return false
 		}
-		return false, err
+		log.Errorf("StatFile failed wtih %s", err)
+		return false
 	}
-	// Upload id exists and is a regular file.
+	log.Debugf("FileInfo: %v", st)
-	return st.Mode.IsRegular(), nil
+	return st.Mode.IsRegular()
 }

object-common.go

@@ -119,18 +119,16 @@ func putObjectCommon(storage StorageAPI, bucket string, object string, size int6
 	if !IsValidBucketName(bucket) {
 		return "", BucketNameInvalid{Bucket: bucket}
 	}
+	// Check whether the bucket exists.
+	if !isBucketExist(storage, bucket) {
+		return "", BucketNotFound{Bucket: bucket}
+	}
 	if !IsValidObjectName(object) {
 		return "", ObjectNameInvalid{
 			Bucket: bucket,
 			Object: object,
 		}
 	}
-	// Check whether the bucket exists.
-	if isExist, err := isBucketExist(storage, bucket); err != nil {
-		return "", err
-	} else if !isExist {
-		return "", BucketNotFound{Bucket: bucket}
-	}
 
 	tempObj := path.Join(tmpMetaPrefix, bucket, object)
 	fileWriter, err := storage.CreateFile(minioMetaBucket, tempObj)
@@ -195,12 +193,12 @@ func putObjectCommon(storage StorageAPI, bucket string, object string, size int6
 }
 
 func listObjectsCommon(layer ObjectLayer, bucket, prefix, marker, delimiter string, maxKeys int) (ListObjectsInfo, error) {
-	var disk StorageAPI
+	var storage StorageAPI
 	switch l := layer.(type) {
 	case xlObjects:
-		disk = l.storage
+		storage = l.storage
 	case fsObjects:
-		disk = l.storage
+		storage = l.storage
 	}
 
 	// Verify if bucket is valid.
@@ -209,9 +207,7 @@ func listObjectsCommon(layer ObjectLayer, bucket, prefix, marker, delimiter stri
 	}
 
 	// Verify whether the bucket exists.
-	if isExist, err := isBucketExist(disk, bucket); err != nil {
+	if !isBucketExist(storage, bucket) {
-		return ListObjectsInfo{}, err
-	} else if !isExist {
 		return ListObjectsInfo{}, BucketNotFound{Bucket: bucket}
 	}
 
@@ -319,13 +315,15 @@ func listObjectsCommon(layer ObjectLayer, bucket, prefix, marker, delimiter stri
 }
 
 // checks whether bucket exists.
-func isBucketExist(storage StorageAPI, bucketName string) (bool, error) {
+func isBucketExist(storage StorageAPI, bucketName string) bool {
 	// Check whether bucket exists.
-	if _, e := storage.StatVol(bucketName); e != nil {
+	_, err := storage.StatVol(bucketName)
-		if e == errVolumeNotFound {
+	if err != nil {
-			return false, nil
+		if err == errVolumeNotFound {
+			return false
 		}
-		return false, e
+		log.Errorf("StatVol failed with %s", err)
+		return false
 	}
-	return true, nil
+	return true
 }

posix.go

@@ -40,22 +40,32 @@ type fsStorage struct {
 }
 
 // isDirEmpty - returns whether given directory is empty or not.
-func isDirEmpty(dirname string) (status bool, err error) {
+func isDirEmpty(dirname string) bool {
 	f, err := os.Open(dirname)
-	if err == nil {
+	if err != nil {
-		defer f.Close()
+		log.Errorf("Unable to access directory %s, failed with %s", dirname, err)
-		if _, err = f.Readdirnames(1); err == io.EOF {
+		return false
-			status = true
-			err = nil
-		}
 	}
-	return status, err
+	defer f.Close()
+	// List one entry.
+	_, err = f.Readdirnames(1)
+	if err != nil {
+		if err == io.EOF {
+			// Returns true if we have reached EOF, directory is
+			// indeed empty.
+			return true
+		}
+		log.Errorf("Unable to list directory %s, failed with %s", dirname, err)
+		return false
+	}
+	// Directory is not empty.
+	return false
 }
 
 // Initialize a new storage disk.
 func newPosix(diskPath string) (StorageAPI, error) {
 	if diskPath == "" {
-		log.Debug("Disk cannot be empty")
+		log.Error("Disk cannot be empty")
 		return nil, errInvalidArgument
 	}
 	st, err := os.Stat(diskPath)
@@ -82,8 +92,7 @@ func newPosix(diskPath string) (StorageAPI, error) {
 	return fs, nil
 }
 
-// checkDiskFree verifies if disk path has sufficient minium free disk
+// checkDiskFree verifies if disk path has sufficient minium free disk space.
-// space.
 func checkDiskFree(diskPath string, minFreeDisk int64) (err error) {
 	di, err := disk.GetInfo(diskPath)
 	if err != nil {
@@ -527,18 +536,9 @@ func deleteFile(basePath, deletePath string) error {
 		}
 		return err
 	}
-	if pathSt.IsDir() {
+	if pathSt.IsDir() && !isDirEmpty(deletePath) {
 		// Verify if directory is empty.
-		empty, err := isDirEmpty(deletePath)
+		return nil
-		if err != nil {
-			log.WithFields(logrus.Fields{
-				"deletePath": deletePath,
-			}).Debugf("isDirEmpty failed with %s", err)
-			return err
-		}
-		if !empty {
-			return nil
-		}
 	}
 	// Attempt to remove path.
 	if err := os.Remove(deletePath); err != nil {

xl-objects-multipart.go

@@ -96,15 +96,17 @@ func (xl xlObjects) CompleteMultipartUpload(bucket string, object string, upload
 	if !IsValidBucketName(bucket) {
 		return "", BucketNameInvalid{Bucket: bucket}
 	}
+	// Verify whether the bucket exists.
+	if !isBucketExist(xl.storage, bucket) {
+		return "", BucketNotFound{Bucket: bucket}
+	}
 	if !IsValidObjectName(object) {
 		return "", ObjectNameInvalid{
 			Bucket: bucket,
 			Object: object,
 		}
 	}
-	if status, err := isUploadIDExists(xl.storage, bucket, object, uploadID); err != nil {
+	if !isUploadIDExists(xl.storage, bucket, object, uploadID) {
-		return "", err
-	} else if !status {
 		return "", InvalidUploadID{UploadID: uploadID}
 	}
 	var metadata = MultipartObjectInfo{}

xl-objects.go

@@ -139,6 +139,9 @@ func (xl xlObjects) GetObject(bucket, object string, startOffset int64) (io.Read
 	if !IsValidBucketName(bucket) {
 		return nil, BucketNameInvalid{Bucket: bucket}
 	}
+	if !isBucketExist(xl.storage, bucket) {
+		return nil, BucketNotFound{Bucket: bucket}
+	}
 	// Verify if object is valid.
 	if !IsValidObjectName(object) {
 		return nil, ObjectNameInvalid{Bucket: bucket, Object: object}
@@ -201,16 +204,14 @@ func (xl xlObjects) GetObjectInfo(bucket, object string) (ObjectInfo, error) {
 	if !IsValidBucketName(bucket) {
 		return ObjectInfo{}, BucketNameInvalid{Bucket: bucket}
 	}
+	// Check whether the bucket exists.
+	if !isBucketExist(xl.storage, bucket) {
+		return ObjectInfo{}, BucketNotFound{Bucket: bucket}
+	}
 	// Verify if object is valid.
 	if !IsValidObjectName(object) {
 		return ObjectInfo{}, ObjectNameInvalid{Bucket: bucket, Object: object}
 	}
-	// Check whether the bucket exists.
-	if isExist, err := isBucketExist(xl.storage, bucket); err != nil {
-		return ObjectInfo{}, err
-	} else if !isExist {
-		return ObjectInfo{}, BucketNotFound{Bucket: bucket}
-	}
 	fi, err := xl.storage.StatFile(bucket, object)
 	if err != nil {
 		if err != errFileNotFound {
@@ -265,6 +266,9 @@ func (xl xlObjects) DeleteObject(bucket, object string) error {
 	if !IsValidBucketName(bucket) {
 		return BucketNameInvalid{Bucket: bucket}
 	}
+	if !isBucketExist(xl.storage, bucket) {
+		return BucketNotFound{Bucket: bucket}
+	}
 	if !IsValidObjectName(object) {
 		return ObjectNameInvalid{Bucket: bucket, Object: object}
 	}