From f2df8ad480c649d25edc93d10836a2d8bfda5591 Mon Sep 17 00:00:00 2001 From: Zakhar Bessarab Date: Tue, 1 Aug 2023 11:18:04 +0400 Subject: [PATCH] vmbackupmanager: fixes for windows compatibility (#641) * app/vmbackupmanager/storage: fix path join for windows See: https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4704 Signed-off-by: Zakhar Bessarab * lib/backup: fixes for windows support - close dir before running os.RemoveAll. Windows FS does not allow to delete directory before all handles will be closed. - add path "normalization" for local FS to use the same format of paths for both *unix and Windows See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4704 Signed-off-by: Zakhar Bessarab --------- Signed-off-by: Zakhar Bessarab --- lib/backup/fscommon/fscommon.go | 10 +++++----- lib/backup/fsremote/fsremote.go | 5 +++++ lib/backup/fsremote/normilize_path_bsd.go | 12 ++++++++++++ lib/backup/fsremote/normilize_path_darwin.go | 9 +++++++++ lib/backup/fsremote/normilize_path_linux.go | 9 +++++++++ lib/backup/fsremote/normilize_path_solaris.go | 9 +++++++++ lib/backup/fsremote/normilize_path_windows.go | 11 +++++++++++ 7 files changed, 60 insertions(+), 5 deletions(-) create mode 100644 lib/backup/fsremote/normilize_path_bsd.go create mode 100644 lib/backup/fsremote/normilize_path_darwin.go create mode 100644 lib/backup/fsremote/normilize_path_linux.go create mode 100644 lib/backup/fsremote/normilize_path_solaris.go create mode 100644 lib/backup/fsremote/normilize_path_windows.go diff --git a/lib/backup/fscommon/fscommon.go b/lib/backup/fscommon/fscommon.go index e232e6ed69..0ba3e856ac 100644 --- a/lib/backup/fscommon/fscommon.go +++ b/lib/backup/fscommon/fscommon.go @@ -52,7 +52,7 @@ func appendFilesInternal(dst []string, d *os.File) ([]string, error) { // Process directory dst, err = AppendFiles(dst, path) if err != nil { - return nil, fmt.Errorf("cannot list %q: %w", path, err) + return nil, fmt.Errorf("cannot append files %q: %w", path, err) } continue } @@ -124,9 +124,6 @@ func removeEmptyDirs(dir string) (bool, error) { return false, err } ok, err := removeEmptyDirsInternal(d) - if err1 := d.Close(); err1 != nil { - err = err1 - } if err != nil { return false, err } @@ -157,7 +154,7 @@ func removeEmptyDirsInternal(d *os.File) (bool, error) { // Process directory ok, err := removeEmptyDirs(path) if err != nil { - return false, fmt.Errorf("cannot list %q: %w", path, err) + return false, fmt.Errorf("cannot remove empty dirs %q: %w", path, err) } if !ok { dirEntries++ @@ -221,6 +218,9 @@ func removeEmptyDirsInternal(d *os.File) (bool, error) { if dirEntries > 0 { return false, nil } + if err := d.Close(); err != nil { + return false, fmt.Errorf("cannot close %q: %w", dir, err) + } // Use os.RemoveAll() instead of os.Remove(), since the dir may contain special files such as flock.lock and backupnames.RestoreInProgressFilename, // which must be ignored. if err := os.RemoveAll(dir); err != nil { diff --git a/lib/backup/fsremote/fsremote.go b/lib/backup/fsremote/fsremote.go index 116d8d9990..45d5ff8f84 100644 --- a/lib/backup/fsremote/fsremote.go +++ b/lib/backup/fsremote/fsremote.go @@ -68,6 +68,7 @@ func (fs *FS) ListParts() ([]common.Part, error) { return nil, fmt.Errorf("cannot stat file %q for part %q: %w", file, p.Path, err) } p.ActualSize = uint64(fi.Size()) + p.Path = pathToCanonical(p.Path) parts = append(parts, p) } return parts, nil @@ -75,6 +76,7 @@ func (fs *FS) ListParts() ([]common.Part, error) { // DeletePart deletes the given part p from fs. func (fs *FS) DeletePart(p common.Part) error { + p.Path = canonicalPathToLocal(p.Path) path := fs.path(p) if err := os.Remove(path); err != nil { return fmt.Errorf("cannot remove %q: %w", path, err) @@ -95,6 +97,7 @@ func (fs *FS) CopyPart(srcFS common.OriginFS, p common.Part) error { if !ok { return fmt.Errorf("cannot perform server-side copying from %s to %s: both of them must be fsremote", srcFS, fs) } + p.Path = canonicalPathToLocal(p.Path) srcPath := src.path(p) dstPath := fs.path(p) if err := fs.mkdirAll(dstPath); err != nil { @@ -139,6 +142,7 @@ func (fs *FS) CopyPart(srcFS common.OriginFS, p common.Part) error { // DownloadPart download part p from fs to w. func (fs *FS) DownloadPart(p common.Part, w io.Writer) error { + p.Path = canonicalPathToLocal(p.Path) path := fs.path(p) r, err := os.Open(path) if err != nil { @@ -201,6 +205,7 @@ func (fs *FS) path(p common.Part) string { // // The function does nothing if the filePath doesn't exist. func (fs *FS) DeleteFile(filePath string) error { + filePath = canonicalPathToLocal(filePath) path := filepath.Join(fs.Dir, filePath) err := os.Remove(path) if err != nil && !os.IsNotExist(err) { diff --git a/lib/backup/fsremote/normilize_path_bsd.go b/lib/backup/fsremote/normilize_path_bsd.go new file mode 100644 index 0000000000..4fb42fee95 --- /dev/null +++ b/lib/backup/fsremote/normilize_path_bsd.go @@ -0,0 +1,12 @@ +//go:build freebsd || openbsd || dragonfly || netbsd +// +build freebsd openbsd dragonfly netbsd + +package fsremote + +func pathToCanonical(path string) string { + return path +} + +func canonicalPathToLocal(path string) string { + return path +} diff --git a/lib/backup/fsremote/normilize_path_darwin.go b/lib/backup/fsremote/normilize_path_darwin.go new file mode 100644 index 0000000000..5e9590684b --- /dev/null +++ b/lib/backup/fsremote/normilize_path_darwin.go @@ -0,0 +1,9 @@ +package fsremote + +func pathToCanonical(path string) string { + return path +} + +func canonicalPathToLocal(path string) string { + return path +} diff --git a/lib/backup/fsremote/normilize_path_linux.go b/lib/backup/fsremote/normilize_path_linux.go new file mode 100644 index 0000000000..5e9590684b --- /dev/null +++ b/lib/backup/fsremote/normilize_path_linux.go @@ -0,0 +1,9 @@ +package fsremote + +func pathToCanonical(path string) string { + return path +} + +func canonicalPathToLocal(path string) string { + return path +} diff --git a/lib/backup/fsremote/normilize_path_solaris.go b/lib/backup/fsremote/normilize_path_solaris.go new file mode 100644 index 0000000000..5e9590684b --- /dev/null +++ b/lib/backup/fsremote/normilize_path_solaris.go @@ -0,0 +1,9 @@ +package fsremote + +func pathToCanonical(path string) string { + return path +} + +func canonicalPathToLocal(path string) string { + return path +} diff --git a/lib/backup/fsremote/normilize_path_windows.go b/lib/backup/fsremote/normilize_path_windows.go new file mode 100644 index 0000000000..c1dfe71afa --- /dev/null +++ b/lib/backup/fsremote/normilize_path_windows.go @@ -0,0 +1,11 @@ +package fsremote + +import "strings" + +func pathToCanonical(path string) string { + return strings.ReplaceAll(path, "\\", "/") +} + +func canonicalPathToLocal(path string) string { + return strings.ReplaceAll(path, "/", "\\") +}