From 4570eddc2cc2491666f5a4742b2a5d1b2c9004f6 Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Fri, 10 Jun 2016 15:10:02 -0700 Subject: [PATCH 1/2] wal: PrivateFileMode/DirMode as in pkg/fileutil To make it consistent with pkg/fileutil --- wal/file_pipeline.go | 2 +- wal/repair.go | 2 +- wal/wal.go | 13 +++++-------- wal/wal_test.go | 3 ++- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/wal/file_pipeline.go b/wal/file_pipeline.go index 5b282b6cd..3412210a3 100644 --- a/wal/file_pipeline.go +++ b/wal/file_pipeline.go @@ -66,7 +66,7 @@ func (fp *filePipeline) Close() error { func (fp *filePipeline) alloc() (f *fileutil.LockedFile, err error) { // count % 2 so this file isn't the same as the one last published fpath := path.Join(fp.dir, fmt.Sprintf("%d.tmp", fp.count%2)) - if f, err = fileutil.LockFile(fpath, os.O_CREATE|os.O_WRONLY, 0600); err != nil { + if f, err = fileutil.LockFile(fpath, os.O_CREATE|os.O_WRONLY, fileutil.PrivateFileMode); err != nil { return nil, err } if err = fileutil.Preallocate(f.File, fp.size, true); err != nil { diff --git a/wal/repair.go b/wal/repair.go index 43b98135f..0a920e2d8 100644 --- a/wal/repair.go +++ b/wal/repair.go @@ -95,5 +95,5 @@ func openLast(dirpath string) (*fileutil.LockedFile, error) { return nil, err } last := path.Join(dirpath, names[len(names)-1]) - return fileutil.LockFile(last, os.O_RDWR, 0600) + return fileutil.LockFile(last, os.O_RDWR, fileutil.PrivateFileMode) } diff --git a/wal/wal.go b/wal/wal.go index f5913ffc0..f0e56a84f 100644 --- a/wal/wal.go +++ b/wal/wal.go @@ -41,9 +41,6 @@ const ( crcType snapshotType - // the owner can make/remove files inside the directory - privateDirMode = 0700 - // the expected size of each wal segment file. // the actual size might be bigger than it. segmentSizeBytes = 64 * 1000 * 1000 // 64MB @@ -100,12 +97,12 @@ func Create(dirpath string, metadata []byte) (*WAL, error) { return nil, err } } - if err := os.MkdirAll(tmpdirpath, privateDirMode); err != nil { + if err := os.MkdirAll(tmpdirpath, fileutil.PrivateDirMode); err != nil { return nil, err } p := path.Join(tmpdirpath, walName(0, 0)) - f, err := fileutil.LockFile(p, os.O_WRONLY|os.O_CREATE, 0600) + f, err := fileutil.LockFile(p, os.O_WRONLY|os.O_CREATE, fileutil.PrivateFileMode) if err != nil { return nil, err } @@ -177,7 +174,7 @@ func openAtIndex(dirpath string, snap walpb.Snapshot, write bool) (*WAL, error) for _, name := range names[nameIndex:] { p := path.Join(dirpath, name) if write { - l, err := fileutil.TryLockFile(p, os.O_RDWR, 0600) + l, err := fileutil.TryLockFile(p, os.O_RDWR, fileutil.PrivateFileMode) if err != nil { closeAll(rcs...) return nil, err @@ -185,7 +182,7 @@ func openAtIndex(dirpath string, snap walpb.Snapshot, write bool) (*WAL, error) ls = append(ls, l) rcs = append(rcs, l) } else { - rf, err := os.OpenFile(p, os.O_RDONLY, 0600) + rf, err := os.OpenFile(p, os.O_RDONLY, fileutil.PrivateFileMode) if err != nil { closeAll(rcs...) return nil, err @@ -373,7 +370,7 @@ func (w *WAL) cut() error { } newTail.Close() - if newTail, err = fileutil.LockFile(fpath, os.O_WRONLY, 0600); err != nil { + if newTail, err = fileutil.LockFile(fpath, os.O_WRONLY, fileutil.PrivateFileMode); err != nil { return err } if _, err = newTail.Seek(off, os.SEEK_SET); err != nil { diff --git a/wal/wal_test.go b/wal/wal_test.go index 270f6f6c3..17f1b7b9e 100644 --- a/wal/wal_test.go +++ b/wal/wal_test.go @@ -23,6 +23,7 @@ import ( "reflect" "testing" + "github.com/coreos/etcd/pkg/fileutil" "github.com/coreos/etcd/pkg/pbutil" "github.com/coreos/etcd/raft/raftpb" "github.com/coreos/etcd/wal/walpb" @@ -614,7 +615,7 @@ func TestRestartCreateWal(t *testing.T) { if err = os.Mkdir(p+".tmp", 0755); err != nil { t.Fatal(err) } - if _, err = os.OpenFile(path.Join(tmpdir, "test"), os.O_WRONLY|os.O_CREATE, 0600); err != nil { + if _, err = os.OpenFile(path.Join(tmpdir, "test"), os.O_WRONLY|os.O_CREATE, fileutil.PrivateFileMode); err != nil { t.Fatal(err) } From 47d5257622daf8a0077cc3e1742cb1574cb370f4 Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Fri, 10 Jun 2016 15:18:29 -0700 Subject: [PATCH 2/2] pkg/fileutil: expose PrivateFileMode/DirMode --- pkg/fileutil/fileutil.go | 11 ++++++----- pkg/fileutil/lock_plan9.go | 4 ++-- pkg/fileutil/lock_test.go | 8 ++++---- pkg/fileutil/purge.go | 2 +- pkg/fileutil/purge_test.go | 2 +- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/pkg/fileutil/fileutil.go b/pkg/fileutil/fileutil.go index 4cdb10d39..ee3acad96 100644 --- a/pkg/fileutil/fileutil.go +++ b/pkg/fileutil/fileutil.go @@ -25,9 +25,10 @@ import ( ) const ( - privateFileMode = 0600 - // owner can make/remove files inside the directory - privateDirMode = 0700 + // PrivateFileMode grants owner to read/write a file. + PrivateFileMode = 0600 + // PrivateDirMode grants owner to make/remove files inside the directory. + PrivateDirMode = 0700 ) var ( @@ -38,7 +39,7 @@ var ( // to dir. It returns nil if dir is writable. func IsDirWriteable(dir string) error { f := path.Join(dir, ".touch") - if err := ioutil.WriteFile(f, []byte(""), privateFileMode); err != nil { + if err := ioutil.WriteFile(f, []byte(""), PrivateFileMode); err != nil { return err } return os.Remove(f) @@ -62,7 +63,7 @@ func ReadDir(dirpath string) ([]string, error) { // TouchDirAll is similar to os.MkdirAll. It creates directories with 0700 permission if any directory // does not exists. TouchDirAll also ensures the given directory is writable. func TouchDirAll(dir string) error { - err := os.MkdirAll(dir, privateDirMode) + err := os.MkdirAll(dir, PrivateDirMode) if err != nil && err != os.ErrExist { return err } diff --git a/pkg/fileutil/lock_plan9.go b/pkg/fileutil/lock_plan9.go index 7432443bf..fee6a7c8f 100644 --- a/pkg/fileutil/lock_plan9.go +++ b/pkg/fileutil/lock_plan9.go @@ -21,7 +21,7 @@ import ( ) func TryLockFile(path string, flag int, perm os.FileMode) (*LockedFile, error) { - if err := os.Chmod(path, syscall.DMEXCL|0600); err != nil { + if err := os.Chmod(path, syscall.DMEXCL|PrivateFileMode); err != nil { return nil, err } f, err := os.Open(path, flag, perm) @@ -32,7 +32,7 @@ func TryLockFile(path string, flag int, perm os.FileMode) (*LockedFile, error) { } func LockFile(path string, flag int, perm os.FileMode) (*LockedFile, error) { - if err := os.Chmod(path, syscall.DMEXCL|0600); err != nil { + if err := os.Chmod(path, syscall.DMEXCL|PrivateFileMode); err != nil { return nil, err } for { diff --git a/pkg/fileutil/lock_test.go b/pkg/fileutil/lock_test.go index 2c4ed2da5..7c1dd8644 100644 --- a/pkg/fileutil/lock_test.go +++ b/pkg/fileutil/lock_test.go @@ -35,13 +35,13 @@ func TestLockAndUnlock(t *testing.T) { }() // lock the file - l, err := LockFile(f.Name(), os.O_WRONLY, 0600) + l, err := LockFile(f.Name(), os.O_WRONLY, PrivateFileMode) if err != nil { t.Fatal(err) } // try lock a locked file - if _, err = TryLockFile(f.Name(), os.O_WRONLY, 0600); err != ErrLocked { + if _, err = TryLockFile(f.Name(), os.O_WRONLY, PrivateFileMode); err != ErrLocked { t.Fatal(err) } @@ -51,7 +51,7 @@ func TestLockAndUnlock(t *testing.T) { } // try lock the unlocked file - dupl, err := TryLockFile(f.Name(), os.O_WRONLY, 0600) + dupl, err := TryLockFile(f.Name(), os.O_WRONLY, PrivateFileMode) if err != nil { t.Errorf("err = %v, want %v", err, nil) } @@ -59,7 +59,7 @@ func TestLockAndUnlock(t *testing.T) { // blocking on locked file locked := make(chan struct{}, 1) go func() { - bl, blerr := LockFile(f.Name(), os.O_WRONLY, 0600) + bl, blerr := LockFile(f.Name(), os.O_WRONLY, PrivateFileMode) if blerr != nil { t.Fatal(blerr) } diff --git a/pkg/fileutil/purge.go b/pkg/fileutil/purge.go index c051697e9..77d021a80 100644 --- a/pkg/fileutil/purge.go +++ b/pkg/fileutil/purge.go @@ -40,7 +40,7 @@ func PurgeFile(dirname string, suffix string, max uint, interval time.Duration, sort.Strings(newfnames) for len(newfnames) > int(max) { f := path.Join(dirname, newfnames[0]) - l, err := TryLockFile(f, os.O_WRONLY, 0600) + l, err := TryLockFile(f, os.O_WRONLY, PrivateFileMode) if err != nil { break } diff --git a/pkg/fileutil/purge_test.go b/pkg/fileutil/purge_test.go index 3cc622064..f3c10fc58 100644 --- a/pkg/fileutil/purge_test.go +++ b/pkg/fileutil/purge_test.go @@ -102,7 +102,7 @@ func TestPurgeFileHoldingLockFile(t *testing.T) { // create a purge barrier at 5 p := path.Join(dir, fmt.Sprintf("%d.test", 5)) - l, err := LockFile(p, os.O_WRONLY, 0600) + l, err := LockFile(p, os.O_WRONLY, PrivateFileMode) if err != nil { t.Fatal(err) }