mirror of
https://github.com/octoleo/restic.git
synced 2024-12-27 04:32:40 +00:00
archiver: Use lstat before open/fstat
The previous code tried to be as efficient as possible and only do a single open() on an item to save, and then fstat() on the fd to find out what the item is (file, dir, other). For normal files, it would then start reading the data without opening the file again, so it could not be exchanged for e.g. a symlink. This behavior starts the watchdog on my machine when /dev is saved with restic, and after a few seconds, the machine reboots. This commit reverts the behavior to the strategy the old archiver code used: run lstat(), then decide what to do. For normal files, open the file and then run fstat() on the fd to verify it's still a normal file, then start reading the data. The downside is that for normal files we now do two stat() calls (lstat+fstat) instead of only one. On the upside, this does not start the watchdog. :)
This commit is contained in:
parent
8026e6fdfb
commit
2218ecd049
@ -7,7 +7,6 @@ import (
|
||||
"path"
|
||||
"runtime"
|
||||
"sort"
|
||||
"syscall"
|
||||
"time"
|
||||
|
||||
"github.com/restic/restic/internal/debug"
|
||||
@ -331,36 +330,19 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous
|
||||
|
||||
fn.absTarget = abstarget
|
||||
|
||||
var fi os.FileInfo
|
||||
var errFI error
|
||||
|
||||
file, errOpen := arch.FS.OpenFile(target, fs.O_RDONLY|fs.O_NOFOLLOW|fs.O_NONBLOCK, 0)
|
||||
if errOpen == nil {
|
||||
fi, errFI = file.Stat()
|
||||
}
|
||||
|
||||
fi, err := arch.FS.Lstat(target)
|
||||
if !arch.Select(abstarget, fi) {
|
||||
debug.Log("%v is excluded", target)
|
||||
if file != nil {
|
||||
_ = file.Close()
|
||||
}
|
||||
return FutureNode{}, true, nil
|
||||
}
|
||||
|
||||
if errOpen != nil {
|
||||
debug.Log(" open error %#v", errOpen)
|
||||
// test if the open failed because target is a symbolic link or a socket
|
||||
if e, ok := errOpen.(*os.PathError); ok && (e.Err == syscall.ELOOP || e.Err == syscall.ENXIO) {
|
||||
// in this case, redo the stat and carry on
|
||||
fi, errFI = arch.FS.Lstat(target)
|
||||
} else {
|
||||
return FutureNode{}, false, errors.Wrap(errOpen, "OpenFile")
|
||||
if err != nil {
|
||||
debug.Log("lstat() for %v returned error: %v", target, err)
|
||||
err = arch.error(abstarget, fi, err)
|
||||
if err != nil {
|
||||
return FutureNode{}, false, errors.Wrap(err, "Lstat")
|
||||
}
|
||||
}
|
||||
|
||||
if errFI != nil {
|
||||
_ = file.Close()
|
||||
return FutureNode{}, false, errors.Wrap(errFI, "Stat")
|
||||
return FutureNode{}, true, nil
|
||||
}
|
||||
|
||||
switch {
|
||||
@ -368,6 +350,39 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous
|
||||
debug.Log(" %v regular file", target)
|
||||
start := time.Now()
|
||||
|
||||
// reopen file and do an fstat() on the open file to check it is still
|
||||
// a file (and has not been exchanged for e.g. a symlink)
|
||||
file, err := arch.FS.OpenFile(target, fs.O_RDONLY|fs.O_NOFOLLOW|fs.O_NONBLOCK, 0)
|
||||
if err != nil {
|
||||
debug.Log("Openfile() for %v returned error: %v", target, err)
|
||||
err = arch.error(abstarget, fi, err)
|
||||
if err != nil {
|
||||
return FutureNode{}, false, errors.Wrap(err, "Lstat")
|
||||
}
|
||||
return FutureNode{}, true, nil
|
||||
}
|
||||
|
||||
fi, err = file.Stat()
|
||||
if err != nil {
|
||||
debug.Log("stat() on opened file %v returned error: %v", target, err)
|
||||
_ = file.Close()
|
||||
err = arch.error(abstarget, fi, err)
|
||||
if err != nil {
|
||||
return FutureNode{}, false, errors.Wrap(err, "Lstat")
|
||||
}
|
||||
return FutureNode{}, true, nil
|
||||
}
|
||||
|
||||
// make sure it's still a file
|
||||
if !fs.IsRegularFile(fi) {
|
||||
err = errors.Errorf("file %v changed type, refusing to archive")
|
||||
err = arch.error(abstarget, fi, err)
|
||||
if err != nil {
|
||||
return FutureNode{}, false, err
|
||||
}
|
||||
return FutureNode{}, true, nil
|
||||
}
|
||||
|
||||
// use previous node if the file hasn't changed
|
||||
if previous != nil && !fileChanged(fi, previous) {
|
||||
debug.Log("%v hasn't changed, returning old node", target)
|
||||
@ -386,8 +401,6 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous
|
||||
arch.CompleteItem(snPath, previous, node, stats, time.Since(start))
|
||||
})
|
||||
|
||||
file = nil
|
||||
|
||||
case fi.IsDir():
|
||||
debug.Log(" %v dir", target)
|
||||
|
||||
@ -400,7 +413,6 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous
|
||||
if err == nil {
|
||||
arch.CompleteItem(snItem, previous, fn.node, fn.stats, time.Since(start))
|
||||
} else {
|
||||
_ = file.Close()
|
||||
return FutureNode{}, false, err
|
||||
}
|
||||
|
||||
@ -413,18 +425,10 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous
|
||||
|
||||
fn.node, err = arch.nodeFromFileInfo(target, fi)
|
||||
if err != nil {
|
||||
_ = file.Close()
|
||||
return FutureNode{}, false, err
|
||||
}
|
||||
}
|
||||
|
||||
if file != nil {
|
||||
err = file.Close()
|
||||
if err != nil {
|
||||
return fn, false, errors.Wrap(err, "Close")
|
||||
}
|
||||
}
|
||||
|
||||
debug.Log("return after %.3f", time.Since(start).Seconds())
|
||||
|
||||
return fn, false, nil
|
||||
|
Loading…
Reference in New Issue
Block a user