diff --git a/pkg/donut/bucket.go b/pkg/donut/bucket.go index cd8e2c7c3..afb30505e 100644 --- a/pkg/donut/bucket.go +++ b/pkg/donut/bucket.go @@ -318,15 +318,15 @@ func (b bucket) writeObjectMetadata(objectName string, objMetadata ObjectMetadat if err != nil { return iodine.New(err, nil) } - for _, objMetadataWriter := range objMetadataWriters { - defer objMetadataWriter.Close() - } for _, objMetadataWriter := range objMetadataWriters { jenc := json.NewEncoder(objMetadataWriter) if err := jenc.Encode(&objMetadata); err != nil { return iodine.New(err, nil) } } + for _, objMetadataWriter := range objMetadataWriters { + objMetadataWriter.Close() + } return nil } diff --git a/pkg/donut/disk/disk.go b/pkg/donut/disk/disk.go index ad7ecdbb5..6db2bc962 100644 --- a/pkg/donut/disk/disk.go +++ b/pkg/donut/disk/disk.go @@ -18,10 +18,12 @@ package disk import ( "fmt" + "io/ioutil" "os" "path/filepath" "strconv" "strings" + "sync" "syscall" "github.com/minio/minio/pkg/iodine" @@ -29,10 +31,41 @@ import ( // Disk container for disk parameters type Disk struct { + lock *sync.Mutex path string fsInfo map[string]string } +// File container provided by disk for atomic file writes +type File struct { + *os.File + path string +} + +// Close the file replacing the configured file. +func (f *File) Close() error { + if err := f.File.Close(); err != nil { + return err + } + if err := os.Rename(f.Name(), f.path); err != nil { + return err + } + return nil +} + +// Abort closes the file and removes it instead of replacing the configured +// file. This is useful if after starting to write to the file you decide you +// don't want it anymore. +func (f *File) Abort() error { + if err := f.File.Close(); err != nil { + return err + } + if err := os.Remove(f.Name()); err != nil { + return err + } + return nil +} + // New - instantiate new disk func New(diskPath string) (Disk, error) { if diskPath == "" { @@ -51,6 +84,7 @@ func New(diskPath string) (Disk, error) { return Disk{}, iodine.New(err, nil) } disk := Disk{ + lock: &sync.Mutex{}, path: diskPath, fsInfo: make(map[string]string), } @@ -70,6 +104,9 @@ func (disk Disk) GetPath() string { // GetFSInfo - get disk filesystem and its usage information func (disk Disk) GetFSInfo() map[string]string { + disk.lock.Lock() + defer disk.lock.Unlock() + s := syscall.Statfs_t{} err := syscall.Statfs(disk.path, &s) if err != nil { @@ -84,11 +121,16 @@ func (disk Disk) GetFSInfo() map[string]string { // MakeDir - make a directory inside disk root path func (disk Disk) MakeDir(dirname string) error { + disk.lock.Lock() + defer disk.lock.Unlock() return os.MkdirAll(filepath.Join(disk.path, dirname), 0700) } // ListDir - list a directory inside disk root path, get only directories func (disk Disk) ListDir(dirname string) ([]os.FileInfo, error) { + disk.lock.Lock() + defer disk.lock.Unlock() + dir, err := os.Open(filepath.Join(disk.path, dirname)) if err != nil { return nil, iodine.New(err, nil) @@ -110,6 +152,9 @@ func (disk Disk) ListDir(dirname string) ([]os.FileInfo, error) { // ListFiles - list a directory inside disk root path, get only files func (disk Disk) ListFiles(dirname string) ([]os.FileInfo, error) { + disk.lock.Lock() + defer disk.lock.Unlock() + dir, err := os.Open(filepath.Join(disk.path, dirname)) if err != nil { return nil, iodine.New(err, nil) @@ -129,25 +174,37 @@ func (disk Disk) ListFiles(dirname string) ([]os.FileInfo, error) { return files, nil } -// CreateFile - create a file inside disk root path -func (disk Disk) CreateFile(filename string) (*os.File, error) { +// CreateFile - create a file inside disk root path, replies with custome disk.File which provides atomic writes +func (disk Disk) CreateFile(filename string) (*File, error) { + disk.lock.Lock() + defer disk.lock.Unlock() + if filename == "" { return nil, iodine.New(InvalidArgument{}, nil) } + filePath := filepath.Join(disk.path, filename) // Create directories if they don't exist if err := os.MkdirAll(filepath.Dir(filePath), 0700); err != nil { return nil, iodine.New(err, nil) } - dataFile, err := os.OpenFile(filePath, os.O_WRONLY|os.O_CREATE, 0600) + f, err := ioutil.TempFile(filepath.Dir(filePath), filepath.Base(filePath)) if err != nil { return nil, iodine.New(err, nil) } - return dataFile, nil + if err := os.Chmod(f.Name(), 0600); err != nil { + os.Remove(f.Name()) + return nil, iodine.New(err, nil) + } + + return &File{File: f, path: filePath}, nil } // OpenFile - read a file inside disk root path func (disk Disk) OpenFile(filename string) (*os.File, error) { + disk.lock.Lock() + defer disk.lock.Unlock() + if filename == "" { return nil, iodine.New(InvalidArgument{}, nil) } diff --git a/pkg/donut/disk/disk_test.go b/pkg/donut/disk/disk_test.go index 18cb5145d..93fc9e87f 100644 --- a/pkg/donut/disk/disk_test.go +++ b/pkg/donut/disk/disk_test.go @@ -61,18 +61,24 @@ func (s *MyDiskSuite) TestDiskCreateDir(c *C) { func (s *MyDiskSuite) TestDiskCreateFile(c *C) { f, err := s.disk.CreateFile("hello1") c.Assert(err, IsNil) - c.Assert(f.Name(), Equals, filepath.Join(s.path, "hello1")) - defer f.Close() + c.Assert(f.Name(), Not(Equals), filepath.Join(s.path, "hello1")) + // close renames the file + f.Close() + + // Openfile should be a success + _, err = s.disk.OpenFile("hello1") + c.Assert(err, IsNil) } func (s *MyDiskSuite) TestDiskOpenFile(c *C) { - f, err := s.disk.CreateFile("hello2") + f1, err := s.disk.CreateFile("hello2") c.Assert(err, IsNil) - c.Assert(f.Name(), Equals, filepath.Join(s.path, "hello2")) - defer f.Close() + c.Assert(f1.Name(), Not(Equals), filepath.Join(s.path, "hello2")) + // close renames the file + f1.Close() - f, err = s.disk.OpenFile("hello2") + f2, err := s.disk.OpenFile("hello2") c.Assert(err, IsNil) - c.Assert(f.Name(), Equals, filepath.Join(s.path, "hello2")) - defer f.Close() + c.Assert(f2.Name(), Equals, filepath.Join(s.path, "hello2")) + defer f2.Close() }