From f6f0807c86be78b4f926a55e32ae1779320584ec Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 24 Sep 2024 04:26:41 -0700 Subject: [PATCH] cleanup existing part.N's before renamePart() (#20466) this is a safety-net to avoid any unexpected parts to show up. --- cmd/erasure-multipart.go | 13 ++++++++----- cmd/xl-storage.go | 36 +++++++----------------------------- 2 files changed, 15 insertions(+), 34 deletions(-) diff --git a/cmd/erasure-multipart.go b/cmd/erasure-multipart.go index a5b6d4a98..c5726b216 100644 --- a/cmd/erasure-multipart.go +++ b/cmd/erasure-multipart.go @@ -526,6 +526,14 @@ func (er erasureObjects) NewMultipartUpload(ctx context.Context, bucket, object // renamePart - renames multipart part to its relevant location under uploadID. func (er erasureObjects) renamePart(ctx context.Context, disks []StorageAPI, srcBucket, srcEntry, dstBucket, dstEntry string, optsMeta []byte, writeQuorum int) ([]StorageAPI, error) { + paths := []string{ + dstEntry, + dstEntry + ".meta", + } + + // cleanup existing paths first across all drives. + er.cleanupMultipartPath(ctx, paths...) + g := errgroup.WithNErrs(len(disks)) // Rename file on all underlying storage disks. @@ -542,11 +550,6 @@ func (er erasureObjects) renamePart(ctx context.Context, disks []StorageAPI, src // Wait for all renames to finish. errs := g.Wait() - paths := []string{ - dstEntry, - dstEntry + ".meta", - } - err := reduceWriteQuorumErrs(ctx, errs, objectOpIgnoredErrs, writeQuorum) if err != nil { er.cleanupMultipartPath(ctx, paths...) diff --git a/cmd/xl-storage.go b/cmd/xl-storage.go index f1b51fe6b..77937210d 100644 --- a/cmd/xl-storage.go +++ b/cmd/xl-storage.go @@ -2921,7 +2921,8 @@ func (s *xlStorage) RenameData(ctx context.Context, srcVolume, srcPath string, f return res, nil } -// RenamePart - rename part path to destination path atomically. +// RenamePart - rename part path to destination path atomically, this is meant to be used +// only with multipart API func (s *xlStorage) RenamePart(ctx context.Context, srcVolume, srcPath, dstVolume, dstPath string, meta []byte) (err error) { srcVolumeDir, err := s.getVolDir(srcVolume) if err != nil { @@ -2952,46 +2953,23 @@ func (s *xlStorage) RenamePart(ctx context.Context, srcVolume, srcPath, dstVolum return err } } + srcIsDir := HasSuffix(srcPath, SlashSeparator) dstIsDir := HasSuffix(dstPath, SlashSeparator) - // Either src and dst have to be directories or files, else return error. - if !(srcIsDir && dstIsDir || !srcIsDir && !dstIsDir) { + // either source or destination is a directory return error. + if srcIsDir || dstIsDir { return errFileAccessDenied } + srcFilePath := pathutil.Join(srcVolumeDir, srcPath) if err = checkPathLength(srcFilePath); err != nil { return err } + dstFilePath := pathutil.Join(dstVolumeDir, dstPath) if err = checkPathLength(dstFilePath); err != nil { return err } - if srcIsDir { - // If source is a directory, we expect the destination to be non-existent but we - // we still need to allow overwriting an empty directory since it represents - // an object empty directory. - dirInfo, err := Lstat(dstFilePath) - if isSysErrIO(err) { - return errFaultyDisk - } - if err != nil { - if !osIsNotExist(err) { - return err - } - } else { - if !dirInfo.IsDir() { - return errFileAccessDenied - } - if err = Remove(dstFilePath); err != nil { - if isSysErrNotEmpty(err) || isSysErrNotDir(err) { - return errFileAccessDenied - } else if isSysErrIO(err) { - return errFaultyDisk - } - return err - } - } - } if err = renameAll(srcFilePath, dstFilePath, dstVolumeDir); err != nil { if isSysErrNotEmpty(err) || isSysErrNotDir(err) {