Merge pull request #714 from harshavardhana/pr_out_move_atomic_file_writes_into_its_own_package_use_them_inside_quick_and_disk_packages

pkg/donut/disk/disk.go

@@ -18,7 +18,6 @@ package disk
 
 import (
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -27,6 +26,7 @@ import (
 	"syscall"
 
 	"github.com/minio/minio/pkg/iodine"
+	"github.com/minio/minio/pkg/utils/atomic"
 )
 
 // Disk container for disk parameters
@@ -36,36 +36,6 @@ type Disk struct {
 	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 == "" {
@@ -175,7 +145,7 @@ func (disk Disk) ListFiles(dirname string) ([]os.FileInfo, 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) {
+func (disk Disk) CreateFile(filename string) (*atomic.File, error) {
 	disk.lock.Lock()
 	defer disk.lock.Unlock()
 
@@ -188,16 +158,13 @@ func (disk Disk) CreateFile(filename string) (*File, error) {
 	if err := os.MkdirAll(filepath.Dir(filePath), 0700); err != nil {
 		return nil, iodine.New(err, nil)
 	}
-	f, err := ioutil.TempFile(filepath.Dir(filePath), filepath.Base(filePath))
+
+	f, err := atomic.FileCreate(filePath)
 	if err != nil {
 		return nil, iodine.New(err, 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
+	return f, nil
 }
 
 // OpenFile - read a file inside disk root path

pkg/quick/quick.go

@@ -31,6 +31,7 @@ import (
 
 	"github.com/fatih/structs"
 	"github.com/minio/minio/pkg/iodine"
+	"github.com/minio/minio/pkg/utils/atomic"
 )
 
 // Config - generic config interface functions
@@ -115,11 +116,10 @@ func (d config) Save(filename string) (err error) {
 		return iodine.New(err, nil)
 	}
 
-	file, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600)
+	file, err := atomic.FileCreate(filename)
 	if err != nil {
 		return iodine.New(err, nil)
 	}
-	defer file.Close()
 
 	if runtime.GOOS == "windows" {
 		jsonData = []byte(strings.Replace(string(jsonData), "\n", "\r\n", -1))
@@ -128,6 +128,10 @@ func (d config) Save(filename string) (err error) {
 	if err != nil {
 		return iodine.New(err, nil)
 	}
+
+	if err := file.Close(); err != nil {
+		return iodine.New(err, nil)
+	}
 	return nil
 }
 

pkg/utils/atomic/atomic.go

@@ -0,0 +1,74 @@
+/*
+ * Minio Client (C) 2015 Minio, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// NOTE - Rename() not guaranteed to be atomic on all filesystems which are not fully POSIX compatible
+
+// Package atomic provides atomic file write semantics by leveraging Rename's() atomicity.
+package atomic
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+
+	"github.com/minio/minio/pkg/iodine"
+)
+
+// File container provided for atomic file writes
+type File struct {
+	*os.File
+	file string
+}
+
+// Close the file replacing, returns an error if any
+func (f *File) Close() error {
+	// close the embedded fd
+	if err := f.File.Close(); err != nil {
+		return iodine.New(err, nil)
+	}
+	// atomic rename to final destination
+	if err := os.Rename(f.Name(), f.file); err != nil {
+		return iodine.New(err, nil)
+	}
+	return nil
+}
+
+// CloseAndPurge removes the temp file, closes the transaction and returns an error if any
+func (f *File) CloseAndPurge() error {
+	// close the embedded fd
+	if err := f.File.Close(); err != nil {
+		return iodine.New(err, nil)
+	}
+	if err := os.Remove(f.Name()); err != nil {
+		return err
+	}
+	return nil
+}
+
+// FileCreate creates a new file at filePath for atomic writes
+func FileCreate(filePath string) (*File, error) {
+	f, err := ioutil.TempFile(filepath.Dir(filePath), filepath.Base(filePath))
+	if err != nil {
+		return nil, iodine.New(err, nil)
+	}
+	if err := os.Chmod(f.Name(), 0600); err != nil {
+		if err := os.Remove(f.Name()); err != nil {
+			return nil, iodine.New(err, nil)
+		}
+		return nil, iodine.New(err, nil)
+	}
+	return &File{File: f, file: filePath}, nil
+}

pkg/utils/atomic/atomic_test.go

@@ -0,0 +1,58 @@
+/*
+ * Minio Client (C) 2015 Minio, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package atomic
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+
+	. "github.com/minio/check"
+)
+
+func Test(t *testing.T) { TestingT(t) }
+
+type MySuite struct{}
+
+var _ = Suite(&MySuite{})
+
+func (s *MySuite) TestAtomic(c *C) {
+	root, err := ioutil.TempDir("/tmp", "atomic-")
+	c.Assert(err, IsNil)
+	f, err := FileCreate(filepath.Join(root, "testfile"))
+	c.Assert(err, IsNil)
+	_, err = os.Stat(filepath.Join(root, "testfile"))
+	c.Assert(err, Not(IsNil))
+	err = f.Close()
+	c.Assert(err, IsNil)
+	_, err = os.Stat(filepath.Join(root, "testfile"))
+	c.Assert(err, IsNil)
+}
+
+func (s *MySuite) TestAtomicPurge(c *C) {
+	root, err := ioutil.TempDir("/tmp", "atomic-")
+	c.Assert(err, IsNil)
+	f, err := FileCreate(filepath.Join(root, "testfile"))
+	c.Assert(err, IsNil)
+	_, err = os.Stat(filepath.Join(root, "testfile"))
+	c.Assert(err, Not(IsNil))
+	err = f.CloseAndPurge()
+	c.Assert(err, IsNil)
+	err = f.Close()
+	c.Assert(err, Not(IsNil))
+}