Merge lp:~chipaca/snappy/fsync into lp:~snappy-dev/snappy/snappy-moved-to-github

Proposed by John Lenton
Status: Merged
Approved by: Michael Vogt
Approved revision: 432
Merged at revision: 431
Proposed branch: lp:~chipaca/snappy/fsync
Merge into: lp:~snappy-dev/snappy/snappy-moved-to-github
Diff against target: 119 lines (+57/-11)
2 files modified
helpers/helpers.go (+37/-8)
partition/bootloader_uboot.go (+20/-3)
To merge this branch: bzr merge lp:~chipaca/snappy/fsync
Reviewer Review Type Date Requested Status
Michael Vogt (community) Approve
Review via email: mp+257726@code.launchpad.net

Commit message

do fsync after "atomic" file writes

Description of the change

This might make mounting boot "sync" unnecessary.

To post a comment you must log in.
lp:~chipaca/snappy/fsync updated
432. By John Lenton

oops

Revision history for this message
Michael Vogt (mvo) wrote :

Thanks for doing this work, its really great! One comment/question inline.

Revision history for this message
Michael Vogt (mvo) wrote :

One more comment from Steve.

Revision history for this message
John Lenton (chipaca) :
Revision history for this message
Michael Vogt (mvo) :
Revision history for this message
Michael Vogt (mvo) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'helpers/helpers.go'
--- helpers/helpers.go 2015-04-23 11:19:42 +0000
+++ helpers/helpers.go 2015-04-29 09:10:20 +0000
@@ -23,7 +23,6 @@
23 "encoding/hex"23 "encoding/hex"
24 "fmt"24 "fmt"
25 "io"25 "io"
26 "io/ioutil"
27 "math/rand"26 "math/rand"
28 "os"27 "os"
29 "os/exec"28 "os/exec"
@@ -247,15 +246,45 @@
247246
248// AtomicWriteFile updates the filename atomically and works otherwise247// AtomicWriteFile updates the filename atomically and works otherwise
249// exactly like io/ioutil.WriteFile()248// exactly like io/ioutil.WriteFile()
250func AtomicWriteFile(filename string, data []byte, perm os.FileMode) error {249func AtomicWriteFile(filename string, data []byte, perm os.FileMode) (err error) {
251 tmp := filename + ".new"250 tmp := filename + ".new"
252251
253 if err := ioutil.WriteFile(tmp, data, perm); err != nil {252 // XXX: if go switches to use aio_fsync, we need to open the dir for writing
254 os.Remove(tmp)253 dir, err := os.Open(filepath.Dir(filename))
255 return err254 if err != nil {
256 }255 return err
257256 }
258 return os.Rename(tmp, filename)257 defer dir.Close()
258
259 fd, err := os.OpenFile(tmp, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, perm)
260 if err != nil {
261 return err
262 }
263 defer func() {
264 e := fd.Close()
265 if err == nil {
266 err = e
267 }
268 if err != nil {
269 os.Remove(tmp)
270 }
271 }()
272
273 // according to the docs, Write returns a non-nil error when n !=
274 // len(b), so don't worry about short writes.
275 if _, err := fd.Write(data); err != nil {
276 return err
277 }
278
279 if err := fd.Sync(); err != nil {
280 return err
281 }
282
283 if err := os.Rename(tmp, filename); err != nil {
284 return err
285 }
286
287 return dir.Sync()
259}288}
260289
261// CurrentHomeDir returns the homedir of the current user. It looks at290// CurrentHomeDir returns the homedir of the current user. It looks at
262291
=== modified file 'partition/bootloader_uboot.go'
--- partition/bootloader_uboot.go 2015-03-26 09:05:27 +0000
+++ partition/bootloader_uboot.go 2015-04-29 09:10:20 +0000
@@ -175,7 +175,12 @@
175 return err175 return err
176 }176 }
177177
178 defer file.Close()178 defer func() {
179 e := file.Close()
180 if err == nil {
181 err = e
182 }
183 }()
179184
180 writer := bufio.NewWriter(file)185 writer := bufio.NewWriter(file)
181186
@@ -184,7 +189,12 @@
184 return err189 return err
185 }190 }
186 }191 }
187 return writer.Flush()192
193 if err := writer.Flush(); err != nil {
194 return err
195 }
196
197 return file.Sync()
188}198}
189199
190func (u *uboot) MarkCurrentBootSuccessful() (err error) {200func (u *uboot) MarkCurrentBootSuccessful() (err error) {
@@ -310,6 +320,13 @@
310func atomicFileUpdate(file string, lines []string) (err error) {320func atomicFileUpdate(file string, lines []string) (err error) {
311 tmpFile := fmt.Sprintf("%s.NEW", file)321 tmpFile := fmt.Sprintf("%s.NEW", file)
312322
323 // XXX: if go switches to use aio_fsync, we need to open the dir for writing
324 dir, err := os.Open(filepath.Dir(file))
325 if err != nil {
326 return err
327 }
328 defer dir.Close()
329
313 if err := writeLines(lines, tmpFile); err != nil {330 if err := writeLines(lines, tmpFile); err != nil {
314 return err331 return err
315 }332 }
@@ -319,7 +336,7 @@
319 return err336 return err
320 }337 }
321338
322 return nil339 return dir.Sync()
323}340}
324341
325// Rewrite the specified file, applying the specified set of changes.342// Rewrite the specified file, applying the specified set of changes.

Subscribers

People subscribed via source and target branches