fix: log if there is readDir() failure with ListBuckets (#15461)

This is actionable and must be logged.

Bonus: also honor umask by using 0o666 for all Open() syscalls.
This commit is contained in:
Harshavardhana 2022-08-04 07:23:05 -07:00 committed by GitHub
parent 2871cb5775
commit 3bd9615d0e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 40 additions and 20 deletions

View file

@ -110,7 +110,7 @@ func formatCacheGetVersion(r io.ReadSeeker) (string, error) {
// Creates a new cache format.json if unformatted.
func createFormatCache(fsFormatPath string, format *formatCacheV1) error {
// open file using READ & WRITE permission
file, err := os.OpenFile(fsFormatPath, os.O_RDWR|os.O_CREATE, 0o600)
file, err := os.OpenFile(fsFormatPath, os.O_RDWR|os.O_CREATE, 0o666)
if err != nil {
return err
}
@ -155,7 +155,7 @@ func loadFormatCache(ctx context.Context, drives []string) ([]*formatCacheV2, bo
migrating := false
for i, drive := range drives {
cacheFormatPath := pathJoin(drive, minioMetaBucket, formatConfigFile)
f, err := os.OpenFile(cacheFormatPath, os.O_RDWR, 0)
f, err := os.OpenFile(cacheFormatPath, os.O_RDWR, 0o666)
if err != nil {
if osIsNotExist(err) {
continue
@ -478,7 +478,7 @@ func migrateOldCache(ctx context.Context, c *diskCache) error {
func migrateCacheFormatJSON(cacheFormatPath string) error {
// now migrate format.json
f, err := os.OpenFile(cacheFormatPath, os.O_RDWR, 0)
f, err := os.OpenFile(cacheFormatPath, os.O_RDWR, 0o666)
if err != nil {
return err
}

View file

@ -165,7 +165,7 @@ func formatFSMigrate(ctx context.Context, wlk *lock.LockedFile, fsPath string) e
func createFormatFS(fsFormatPath string) error {
// Attempt a write lock on formatConfigFile `format.json`
// file stored in minioMetaBucket(.minio.sys) directory.
lk, err := lock.TryLockedOpenFile(fsFormatPath, os.O_RDWR|os.O_CREATE, 0o600)
lk, err := lock.TryLockedOpenFile(fsFormatPath, os.O_RDWR|os.O_CREATE, 0o666)
if err != nil {
return err
}
@ -256,7 +256,7 @@ func initFormatFS(ctx context.Context, fsPath string) (rlk *lock.RLockedFile, er
// Hold write lock during migration so that we do not disturb any
// minio processes running in parallel.
var wlk *lock.LockedFile
wlk, err = lock.TryLockedOpenFile(fsFormatPath, os.O_RDWR, 0)
wlk, err = lock.TryLockedOpenFile(fsFormatPath, os.O_RDWR, 0o666)
if err == lock.ErrAlreadyLocked {
// Lock already present, sleep and attempt again.
time.Sleep(100 * time.Millisecond)
@ -357,7 +357,7 @@ func formatFSFixDeploymentID(ctx context.Context, fsFormatPath string) error {
case <-ctx.Done():
return fmt.Errorf("Initializing FS format stopped gracefully")
default:
wlk, err = lock.TryLockedOpenFile(fsFormatPath, os.O_RDWR, 0)
wlk, err = lock.TryLockedOpenFile(fsFormatPath, os.O_RDWR, 0o666)
if err == lock.ErrAlreadyLocked {
// Lock already present, sleep and attempt again
logger.Info("Another minio process(es) might be holding a lock to the file %s. Please kill that minio process(es) (elapsed %s)\n", fsFormatPath, getElapsedTime())

View file

@ -517,7 +517,7 @@ func (fs *FSObjects) ListObjectParts(ctx context.Context, bucket, object, upload
}
}
rc, _, err := fsOpenFile(ctx, pathJoin(uploadIDDir, fs.metaJSONFile), 0)
rc, _, err := fsOpenFile(ctx, pathJoin(uploadIDDir, fs.metaJSONFile), 0o666)
if err != nil {
if err == errFileNotFound || err == errFileAccessDenied {
return result, InvalidUploadID{Bucket: bucket, Object: object, UploadID: uploadID}

View file

@ -867,7 +867,7 @@ func (fs *FSObjects) getObjectInfoNoFSLock(ctx context.Context, bucket, object s
// Read `fs.json` to perhaps contend with
// parallel Put() operations.
rc, _, err := fsOpenFile(ctx, fsMetaPath, 0)
rc, _, err := fsOpenFile(ctx, fsMetaPath, 0o666)
if err == nil {
fsMetaBuf, rerr := ioutil.ReadAll(rc)
rc.Close()

View file

@ -152,12 +152,24 @@ func parseDirEnt(buf []byte) (consumed int, name []byte, typ os.FileMode, err er
// the directory itself, if the dirPath doesn't exist this function doesn't return
// an error.
func readDirFn(dirPath string, fn func(name string, typ os.FileMode) error) error {
f, err := Open(dirPath)
f, err := OpenFile(dirPath, readMode, 0o666)
if err != nil {
if osErrToFileErr(err) == errFileNotFound {
return nil
}
return osErrToFileErr(err)
if !osIsPermission(err) {
return osErrToFileErr(err)
}
// There may be permission error when dirPath
// is at the root of the disk mount that may
// not have the permissions to avoid 'noatime'
f, err = Open(dirPath)
if err != nil {
if osErrToFileErr(err) == errFileNotFound {
return nil
}
return osErrToFileErr(err)
}
}
defer f.Close()
@ -234,12 +246,14 @@ func readDirFn(dirPath string, fn func(name string, typ os.FileMode) error) erro
func readDirWithOpts(dirPath string, opts readDirOpts) (entries []string, err error) {
f, err := OpenFile(dirPath, readMode, 0o666)
if err != nil {
if osIsPermission(err) {
f, err = Open(dirPath)
if err != nil {
return nil, osErrToFileErr(err)
}
} else {
if !osIsPermission(err) {
return nil, osErrToFileErr(err)
}
// There may be permission error when dirPath
// is at the root of the disk mount that may
// not have the permissions to avoid 'noatime'
f, err = Open(dirPath)
if err != nil {
return nil, osErrToFileErr(err)
}
}

View file

@ -751,18 +751,24 @@ func (s *xlStorage) MakeVol(ctx context.Context, volume string) error {
}
// ListVols - list volumes.
func (s *xlStorage) ListVols(context.Context) (volsInfo []VolInfo, err error) {
return listVols(s.diskPath)
func (s *xlStorage) ListVols(ctx context.Context) (volsInfo []VolInfo, err error) {
return listVols(ctx, s.diskPath)
}
// List all the volumes from diskPath.
func listVols(dirPath string) ([]VolInfo, error) {
func listVols(ctx context.Context, dirPath string) ([]VolInfo, error) {
if err := checkPathLength(dirPath); err != nil {
return nil, err
}
entries, err := readDir(dirPath)
if err != nil {
return nil, errDiskNotFound
logger.LogIf(ctx, err)
if errors.Is(err, errFileAccessDenied) {
return nil, errDiskAccessDenied
} else if errors.Is(err, errFileNotFound) {
return nil, errDiskNotFound
}
return nil, err
}
volsInfo := make([]VolInfo, 0, len(entries))
for _, entry := range entries {