Skip to content

Commit

Permalink
vmbackupmanager: fixes for windows compatibility (#641)
Browse files Browse the repository at this point in the history
* app/vmbackupmanager/storage: fix path join for windows

See: #4704

Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com>

* 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 #4704

Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com>

---------

Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com>
  • Loading branch information
zekker6 authored and valyala committed Aug 11, 2023
1 parent bc2145d commit 9efc077
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 5 deletions.
10 changes: 5 additions & 5 deletions lib/backup/fscommon/fscommon.go
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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++
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions lib/backup/fsremote/fsremote.go
Expand Up @@ -68,13 +68,15 @@ 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
}

// 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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down
12 changes: 12 additions & 0 deletions 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
}
9 changes: 9 additions & 0 deletions 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
}
9 changes: 9 additions & 0 deletions 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
}
9 changes: 9 additions & 0 deletions 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
}
11 changes: 11 additions & 0 deletions 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, "/", "\\")
}

0 comments on commit 9efc077

Please sign in to comment.