From a474ba54c5c9c7bdba6ffa3e92671bb9565889f6 Mon Sep 17 00:00:00 2001 From: David Steele Date: Sun, 26 May 2019 12:41:15 -0400 Subject: [PATCH] Refactoring path support in the storage module. Not all storage types support paths as a physical thing that must be created/destroyed. Add a feature to determine which drivers use paths and simplify the driver API as much as possible given that knowledge and by implementing as much path logic as possible in the Storage object. Remove the ignoreMissing parameter from pathSync() since it is not used and makes little sense. Create a standard list of error messages for the drivers to use and apply them where the code was modified -- there is plenty of work still to be done here. --- libc/Makefile.PL | 1 - src/Makefile.in | 12 +- src/command/restore/file.c | 1 - src/storage/posix/common.c | 98 -------------- src/storage/posix/common.h | 17 --- src/storage/posix/read.c | 18 ++- src/storage/posix/storage.c | 147 +++++++++++---------- src/storage/posix/storage.intern.h | 2 +- src/storage/posix/write.c | 35 ++--- src/storage/remote/protocol.c | 19 +-- src/storage/remote/protocol.h | 2 + src/storage/remote/storage.c | 36 ++--- src/storage/s3/storage.c | 152 ++++++++++------------ src/storage/storage.c | 65 ++++++--- src/storage/storage.h | 32 +++-- src/storage/storage.intern.h | 43 +++++- test/define.yaml | 5 +- test/expect/mock-archive-001.log | 8 +- test/src/module/command/archiveGetTest.c | 18 +-- test/src/module/command/archivePushTest.c | 4 +- test/src/module/command/infoTest.c | 15 ++- test/src/module/config/parseTest.c | 11 +- test/src/module/info/infoArchiveTest.c | 9 +- test/src/module/info/infoBackupTest.c | 9 +- test/src/module/info/infoTest.c | 39 +++--- test/src/module/storage/posixTest.c | 129 ++++++------------ test/src/module/storage/remoteTest.c | 71 +++++----- test/src/module/storage/s3Test.c | 9 +- 28 files changed, 468 insertions(+), 539 deletions(-) delete mode 100644 src/storage/posix/common.c delete mode 100644 src/storage/posix/common.h diff --git a/libc/Makefile.PL b/libc/Makefile.PL index 8cc8d558d8..0632ea1864 100644 --- a/libc/Makefile.PL +++ b/libc/Makefile.PL @@ -87,7 +87,6 @@ my @stryCFile = 'config/parse.c', 'perl/config.c', 'postgres/pageChecksum.c', - 'storage/posix/common.c', 'storage/posix/read.c', 'storage/posix/storage.c', 'storage/posix/write.c', diff --git a/src/Makefile.in b/src/Makefile.in index 9ef55ef629..c3a00c285d 100644 --- a/src/Makefile.in +++ b/src/Makefile.in @@ -138,7 +138,6 @@ SRCS = \ protocol/parallelJob.c \ protocol/server.c \ storage/cifs/storage.c \ - storage/posix/common.c \ storage/posix/read.c \ storage/posix/storage.c \ storage/posix/write.c \ @@ -231,7 +230,7 @@ command/local/local.o: command/local/local.c build.auto.h command/archive/get/pr command/remote/remote.o: command/remote/remote.c build.auto.h common/assert.h common/debug.h common/error.auto.h common/error.h common/io/filter/filter.h common/io/filter/group.h common/io/handleRead.h common/io/handleWrite.h common/io/read.h common/io/write.h common/lock.h common/log.h common/logLevel.h common/memContext.h common/stackTrace.h common/time.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h config/config.auto.h config/config.h config/define.auto.h config/define.h config/protocol.h protocol/client.h protocol/command.h protocol/helper.h protocol/server.h storage/remote/protocol.h $(CC) $(CFLAGS) $(CMAKE) -c command/remote/remote.c -o command/remote/remote.o -command/restore/file.o: command/restore/file.c build.auto.h command/restore/file.h common/assert.h common/compress/gzip/common.h common/compress/gzip/decompress.h common/crypto/cipherBlock.h common/crypto/common.h common/crypto/hash.h common/debug.h common/error.auto.h common/error.h common/io/filter/filter.h common/io/filter/group.h common/io/filter/size.h common/io/io.h common/io/read.h common/io/write.h common/lock.h common/log.h common/logLevel.h common/memContext.h common/stackTrace.h common/time.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h config/config.auto.h config/config.h config/define.auto.h config/define.h storage/helper.h storage/info.h storage/posix/common.h storage/read.h storage/storage.h storage/write.h +command/restore/file.o: command/restore/file.c build.auto.h command/restore/file.h common/assert.h common/compress/gzip/common.h common/compress/gzip/decompress.h common/crypto/cipherBlock.h common/crypto/common.h common/crypto/hash.h common/debug.h common/error.auto.h common/error.h common/io/filter/filter.h common/io/filter/group.h common/io/filter/size.h common/io/io.h common/io/read.h common/io/write.h common/lock.h common/log.h common/logLevel.h common/memContext.h common/stackTrace.h common/time.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h config/config.auto.h config/config.h config/define.auto.h config/define.h storage/helper.h storage/info.h storage/read.h storage/storage.h storage/write.h $(CC) $(CFLAGS) $(CMAKE) -c command/restore/file.c -o command/restore/file.o command/restore/protocol.o: command/restore/protocol.c build.auto.h command/restore/file.h command/restore/protocol.h common/assert.h common/crypto/common.h common/debug.h common/error.auto.h common/error.h common/io/filter/filter.h common/io/filter/group.h common/io/io.h common/io/read.h common/io/write.h common/lock.h common/log.h common/logLevel.h common/memContext.h common/stackTrace.h common/time.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h config/config.auto.h config/config.h config/define.auto.h config/define.h protocol/server.h storage/helper.h storage/info.h storage/read.h storage/storage.h storage/write.h @@ -483,16 +482,13 @@ storage/cifs/storage.o: storage/cifs/storage.c build.auto.h common/assert.h comm storage/helper.o: storage/helper.c build.auto.h common/assert.h common/debug.h common/error.auto.h common/error.h common/io/filter/filter.h common/io/filter/group.h common/io/read.h common/io/read.intern.h common/io/write.h common/io/write.intern.h common/lock.h common/log.h common/logLevel.h common/memContext.h common/regExp.h common/stackTrace.h common/time.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h config/config.auto.h config/config.h config/define.auto.h config/define.h protocol/client.h protocol/command.h protocol/helper.h storage/cifs/storage.h storage/helper.h storage/info.h storage/posix/storage.h storage/read.h storage/read.intern.h storage/remote/storage.h storage/s3/storage.h storage/storage.h storage/storage.intern.h storage/write.h storage/write.intern.h version.h $(CC) $(CFLAGS) $(CMAKE) -c storage/helper.c -o storage/helper.o -storage/posix/common.o: storage/posix/common.c build.auto.h common/assert.h common/debug.h common/error.auto.h common/error.h common/logLevel.h common/memContext.h common/stackTrace.h common/type/buffer.h common/type/convert.h common/type/string.h storage/posix/common.h - $(CC) $(CFLAGS) $(CMAKE) -c storage/posix/common.c -o storage/posix/common.o - -storage/posix/read.o: storage/posix/read.c build.auto.h common/assert.h common/debug.h common/error.auto.h common/error.h common/io/filter/filter.h common/io/filter/group.h common/io/read.h common/io/read.intern.h common/io/write.h common/io/write.intern.h common/log.h common/logLevel.h common/macro.h common/memContext.h common/object.h common/stackTrace.h common/time.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h storage/info.h storage/posix/common.h storage/posix/read.h storage/posix/storage.h storage/posix/storage.intern.h storage/read.h storage/read.intern.h storage/storage.h storage/storage.intern.h storage/write.h storage/write.intern.h version.h +storage/posix/read.o: storage/posix/read.c build.auto.h common/assert.h common/debug.h common/error.auto.h common/error.h common/io/filter/filter.h common/io/filter/group.h common/io/read.h common/io/read.intern.h common/io/write.h common/io/write.intern.h common/log.h common/logLevel.h common/macro.h common/memContext.h common/object.h common/stackTrace.h common/time.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h storage/info.h storage/posix/read.h storage/posix/storage.h storage/posix/storage.intern.h storage/read.h storage/read.intern.h storage/storage.h storage/storage.intern.h storage/write.h storage/write.intern.h version.h $(CC) $(CFLAGS) $(CMAKE) -c storage/posix/read.c -o storage/posix/read.o -storage/posix/storage.o: storage/posix/storage.c build.auto.h common/assert.h common/debug.h common/error.auto.h common/error.h common/io/filter/filter.h common/io/filter/group.h common/io/read.h common/io/read.intern.h common/io/write.h common/io/write.intern.h common/log.h common/logLevel.h common/macro.h common/memContext.h common/object.h common/regExp.h common/stackTrace.h common/time.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h storage/info.h storage/posix/common.h storage/posix/read.h storage/posix/storage.h storage/posix/storage.intern.h storage/posix/write.h storage/read.h storage/read.intern.h storage/storage.h storage/storage.intern.h storage/write.h storage/write.intern.h version.h +storage/posix/storage.o: storage/posix/storage.c build.auto.h common/assert.h common/debug.h common/error.auto.h common/error.h common/io/filter/filter.h common/io/filter/group.h common/io/read.h common/io/read.intern.h common/io/write.h common/io/write.intern.h common/log.h common/logLevel.h common/macro.h common/memContext.h common/object.h common/regExp.h common/stackTrace.h common/time.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h storage/info.h storage/posix/read.h storage/posix/storage.h storage/posix/storage.intern.h storage/posix/write.h storage/read.h storage/read.intern.h storage/storage.h storage/storage.intern.h storage/write.h storage/write.intern.h version.h $(CC) $(CFLAGS) $(CMAKE) -c storage/posix/storage.c -o storage/posix/storage.o -storage/posix/write.o: storage/posix/write.c build.auto.h common/assert.h common/debug.h common/error.auto.h common/error.h common/io/filter/filter.h common/io/filter/group.h common/io/read.h common/io/read.intern.h common/io/write.h common/io/write.intern.h common/log.h common/logLevel.h common/macro.h common/memContext.h common/object.h common/stackTrace.h common/time.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h storage/info.h storage/posix/common.h storage/posix/storage.h storage/posix/storage.intern.h storage/posix/write.h storage/read.h storage/read.intern.h storage/storage.h storage/storage.intern.h storage/write.h storage/write.intern.h version.h +storage/posix/write.o: storage/posix/write.c build.auto.h common/assert.h common/debug.h common/error.auto.h common/error.h common/io/filter/filter.h common/io/filter/group.h common/io/read.h common/io/read.intern.h common/io/write.h common/io/write.intern.h common/log.h common/logLevel.h common/macro.h common/memContext.h common/object.h common/stackTrace.h common/time.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h storage/info.h storage/posix/storage.h storage/posix/storage.intern.h storage/posix/write.h storage/read.h storage/read.intern.h storage/storage.h storage/storage.intern.h storage/write.h storage/write.intern.h version.h $(CC) $(CFLAGS) $(CMAKE) -c storage/posix/write.c -o storage/posix/write.o storage/read.o: storage/read.c build.auto.h common/assert.h common/debug.h common/error.auto.h common/error.h common/io/filter/filter.h common/io/filter/group.h common/io/read.h common/io/read.intern.h common/log.h common/logLevel.h common/macro.h common/memContext.h common/object.h common/stackTrace.h common/type/buffer.h common/type/convert.h common/type/keyValue.h common/type/string.h common/type/variant.h common/type/variantList.h storage/read.h storage/read.intern.h diff --git a/src/command/restore/file.c b/src/command/restore/file.c index 6209b1bac9..eed4a66a64 100644 --- a/src/command/restore/file.c +++ b/src/command/restore/file.c @@ -18,7 +18,6 @@ Restore File #include "common/io/io.h" #include "common/log.h" #include "config/config.h" -#include "storage/posix/common.h" #include "storage/helper.h" /*********************************************************************************************************************************** diff --git a/src/storage/posix/common.c b/src/storage/posix/common.c deleted file mode 100644 index 43f0ae446b..0000000000 --- a/src/storage/posix/common.c +++ /dev/null @@ -1,98 +0,0 @@ -/*********************************************************************************************************************************** -Posix Common File Routines -***********************************************************************************************************************************/ -#include "build.auto.h" - -#include -#include - -#include "common/debug.h" -#include "storage/posix/common.h" - -/*********************************************************************************************************************************** -Open a file - -Returns the handle of the open file, or -1 for reads if the file is missing and -1 for writes if the path is mssing. -***********************************************************************************************************************************/ -int -storagePosixFileOpen(const String *name, int flags, mode_t mode, bool ignoreMissing, bool file, const char *purpose) -{ - FUNCTION_TEST_BEGIN(); - FUNCTION_TEST_PARAM(STRING, name); - FUNCTION_TEST_PARAM(INT, flags); - FUNCTION_TEST_PARAM(MODE, mode); - FUNCTION_TEST_PARAM(BOOL, ignoreMissing); - FUNCTION_TEST_PARAM(BOOL, file); // Is this a file or a path? - FUNCTION_TEST_PARAM(STRINGZ, purpose); - FUNCTION_TEST_END(); - - ASSERT(name != NULL); - ASSERT(purpose != NULL); - - int result = -1; - - result = open(strPtr(name), flags, mode); - - if (result == -1) - { - if (errno != ENOENT || !ignoreMissing) - { - THROWP_SYS_ERROR_FMT( - errno == ENOENT ? (file ? &FileMissingError : &PathMissingError) : (file ? &FileOpenError : &PathOpenError), - "unable to open '%s' for %s", strPtr(name), purpose); - } - } - - FUNCTION_TEST_RETURN(result); -} - -/*********************************************************************************************************************************** -Sync a file/directory handle -***********************************************************************************************************************************/ -void -storagePosixFileSync(int handle, const String *name, bool file, bool closeOnError) -{ - FUNCTION_TEST_BEGIN(); - FUNCTION_TEST_PARAM(INT, handle); - FUNCTION_TEST_PARAM(STRING, name); - FUNCTION_TEST_PARAM(BOOL, file); // Is this a file or a path? - FUNCTION_TEST_PARAM(BOOL, closeOnError); - FUNCTION_TEST_END(); - - ASSERT(handle != -1); - ASSERT(name != NULL); - - if (fsync(handle) == -1) - { - int errNo = errno; - - // Close if requested but don't report errors -- we want to report the sync error instead - if (closeOnError) - close(handle); - - THROWP_SYS_ERROR_CODE_FMT(errNo, file ? &FileSyncError : &PathSyncError, "unable to sync '%s'", strPtr(name)); - } - - FUNCTION_TEST_RETURN_VOID(); -} - -/*********************************************************************************************************************************** -Close a file/directory handle -***********************************************************************************************************************************/ -void -storagePosixFileClose(int handle, const String *name, bool file) -{ - FUNCTION_TEST_BEGIN(); - FUNCTION_TEST_PARAM(INT, handle); - FUNCTION_TEST_PARAM(STRING, name); - FUNCTION_TEST_PARAM(BOOL, file); // Is this a file or a path? - FUNCTION_TEST_END(); - - ASSERT(handle != -1); - ASSERT(name != NULL); - - if (close(handle) == -1) - THROWP_SYS_ERROR_FMT(file ? &FileCloseError : &PathCloseError, "unable to close '%s'", strPtr(name)); - - FUNCTION_TEST_RETURN_VOID(); -} diff --git a/src/storage/posix/common.h b/src/storage/posix/common.h deleted file mode 100644 index 962bf9243f..0000000000 --- a/src/storage/posix/common.h +++ /dev/null @@ -1,17 +0,0 @@ -/*********************************************************************************************************************************** -Posix Common File Routines -***********************************************************************************************************************************/ -#ifndef STORAGE_POSIX_COMMON_H -#define STORAGE_POSIX_COMMON_H - -#include "common/error.h" -#include "common/type/string.h" - -/*********************************************************************************************************************************** -Functions -***********************************************************************************************************************************/ -int storagePosixFileOpen(const String *name, int flags, mode_t mode, bool ignoreMissing, bool file, const char *purpose); -void storagePosixFileSync(int handle, const String *name, bool file, bool closeOnError); -void storagePosixFileClose(int handle, const String *name, bool file); - -#endif diff --git a/src/storage/posix/read.c b/src/storage/posix/read.c index 3c7021ca0d..bf6071748c 100644 --- a/src/storage/posix/read.c +++ b/src/storage/posix/read.c @@ -11,7 +11,6 @@ Posix Storage Read #include "common/log.h" #include "common/memContext.h" #include "common/object.h" -#include "storage/posix/common.h" #include "storage/posix/read.h" #include "storage/posix/storage.intern.h" #include "storage/read.intern.h" @@ -46,7 +45,7 @@ Close the file handle OBJECT_DEFINE_FREE_RESOURCE_BEGIN(STORAGE_READ_POSIX, LOG, logLevelTrace) { if (this->handle != -1) - storagePosixFileClose(this->handle, this->interface.name, true); + THROW_ON_SYS_ERROR_FMT(close(this->handle) == -1, FileCloseError, STORAGE_ERROR_READ_CLOSE, strPtr(this->interface.name)); } OBJECT_DEFINE_FREE_RESOURCE_END(LOG); @@ -67,9 +66,20 @@ storageReadPosixOpen(THIS_VOID) bool result = false; - // Open the file and handle errors - this->handle = storagePosixFileOpen(this->interface.name, O_RDONLY, 0, this->interface.ignoreMissing, true, "read"); + // Open the file + this->handle = open(strPtr(this->interface.name), O_RDONLY, 0); + // Handle errors + if (this->handle == -1) + { + if (errno == ENOENT) + { + if (!this->interface.ignoreMissing) + THROW_FMT(FileMissingError, STORAGE_ERROR_READ_MISSING, strPtr(this->interface.name)); + } + else + THROW_SYS_ERROR_FMT(FileOpenError, STORAGE_ERROR_READ_OPEN, strPtr(this->interface.name)); + } // On success set free callback to ensure file handle is freed if (this->handle != -1) { diff --git a/src/storage/posix/storage.c b/src/storage/posix/storage.c index 488acc82a6..ea34892f1f 100644 --- a/src/storage/posix/storage.c +++ b/src/storage/posix/storage.c @@ -18,7 +18,6 @@ Posix Storage #include "common/log.h" #include "common/memContext.h" #include "common/regExp.h" -#include "storage/posix/common.h" #include "storage/posix/read.h" #include "storage/posix/storage.intern.h" #include "storage/posix/write.h" @@ -34,7 +33,7 @@ Object type struct StoragePosix { MemContext *memContext; // Object memory context - bool syncPath; // Will paths be synced? + StorageInterface interface; // Storage interface }; /*********************************************************************************************************************************** @@ -75,14 +74,13 @@ storagePosixExists(THIS_VOID, const String *path) File/path info ***********************************************************************************************************************************/ static StorageInfo -storagePosixInfo(THIS_VOID, const String *file, bool ignoreMissing, bool followLink) +storagePosixInfo(THIS_VOID, const String *file, bool followLink) { THIS(StoragePosix); FUNCTION_LOG_BEGIN(logLevelTrace); FUNCTION_LOG_PARAM(STORAGE_POSIX, this); FUNCTION_LOG_PARAM(STRING, file); - FUNCTION_LOG_PARAM(BOOL, ignoreMissing); FUNCTION_LOG_PARAM(BOOL, followLink); FUNCTION_LOG_END(); @@ -96,8 +94,8 @@ storagePosixInfo(THIS_VOID, const String *file, bool ignoreMissing, bool followL if ((followLink ? stat(strPtr(file), &statFile) : lstat(strPtr(file), &statFile)) == -1) { - if (errno != ENOENT || !ignoreMissing) - THROW_SYS_ERROR_FMT(FileOpenError, "unable to get info for '%s'", strPtr(file)); + if (errno != ENOENT) + THROW_SYS_ERROR_FMT(FileOpenError, STORAGE_ERROR_INFO, strPtr(file)); } // On success load info into a structure else @@ -170,7 +168,7 @@ storagePosixInfoListEntry( { String *pathInfo = strEqZ(name, ".") ? strDup(path) : strNewFmt("%s/%s", strPtr(path), strPtr(name)); - StorageInfo storageInfo = storagePosixInfo(this, pathInfo, true, false); + StorageInfo storageInfo = storagePosixInfo(this, pathInfo, false); if (storageInfo.exists) { @@ -185,14 +183,13 @@ storagePosixInfoListEntry( } static bool -storagePosixInfoList(THIS_VOID, const String *path, bool errorOnMissing, StorageInfoListCallback callback, void *callbackData) +storagePosixInfoList(THIS_VOID, const String *path, StorageInfoListCallback callback, void *callbackData) { THIS(StoragePosix); FUNCTION_LOG_BEGIN(logLevelTrace); FUNCTION_LOG_PARAM(STORAGE_POSIX, this); FUNCTION_LOG_PARAM(STRING, path); - FUNCTION_LOG_PARAM(BOOL, errorOnMissing); FUNCTION_LOG_PARAM(FUNCTIONP, callback); FUNCTION_LOG_PARAM_P(VOID, callbackData); FUNCTION_LOG_END(); @@ -201,25 +198,24 @@ storagePosixInfoList(THIS_VOID, const String *path, bool errorOnMissing, Storage ASSERT(path != NULL); ASSERT(callback != NULL); - DIR *dir = NULL; bool result = false; - TRY_BEGIN() + // Open the directory for read + DIR *dir = opendir(strPtr(path)); + + // If the directory could not be opened process errors and report missing directories + if (dir == NULL) { - // Open the directory for read - dir = opendir(strPtr(path)); + if (errno != ENOENT) + THROW_SYS_ERROR_FMT(PathOpenError, STORAGE_ERROR_LIST_INFO, strPtr(path)); + } + else + { + // Directory was found + result = true; - // If the directory could not be opened process errors but ignore missing directories when specified - if (!dir) - { - if (errorOnMissing || errno != ENOENT) - THROW_SYS_ERROR_FMT(PathOpenError, "unable to open path '%s' for read", strPtr(path)); - } - else + TRY_BEGIN() { - // Directory was found - result = true; - MEM_CONTEXT_TEMP_BEGIN() { // Read the directory entries @@ -236,13 +232,12 @@ storagePosixInfoList(THIS_VOID, const String *path, bool errorOnMissing, Storage } MEM_CONTEXT_TEMP_END(); } - } - FINALLY() - { - if (dir != NULL) + FINALLY() + { closedir(dir); + } + TRY_END(); } - TRY_END(); FUNCTION_LOG_RETURN(BOOL, result); } @@ -251,14 +246,13 @@ storagePosixInfoList(THIS_VOID, const String *path, bool errorOnMissing, Storage Get a list of files from a directory ***********************************************************************************************************************************/ static StringList * -storagePosixList(THIS_VOID, const String *path, bool errorOnMissing, const String *expression) +storagePosixList(THIS_VOID, const String *path, const String *expression) { THIS(StoragePosix); FUNCTION_LOG_BEGIN(logLevelTrace); FUNCTION_LOG_PARAM(STORAGE_POSIX, this); FUNCTION_LOG_PARAM(STRING, path); - FUNCTION_LOG_PARAM(BOOL, errorOnMissing); FUNCTION_LOG_PARAM(STRING, expression); FUNCTION_LOG_END(); @@ -276,8 +270,8 @@ storagePosixList(THIS_VOID, const String *path, bool errorOnMissing, const Strin // If the directory could not be opened process errors but ignore missing directories when specified if (!dir) { - if (errorOnMissing || errno != ENOENT) - THROW_SYS_ERROR_FMT(PathOpenError, "unable to open path '%s' for read", strPtr(path)); + if (errno != ENOENT) + THROW_SYS_ERROR_FMT(PathOpenError, STORAGE_ERROR_LIST, strPtr(path)); } else { @@ -380,7 +374,7 @@ storagePosixMove(THIS_VOID, StorageRead *source, StorageWrite *destination) String *sourcePath = strPath(sourceFile); if (!strEq(destinationPath, sourcePath)) - storagePosixPathSync(this, sourcePath, false); + storagePosixPathSync(this, sourcePath); } } } @@ -439,8 +433,8 @@ storagePosixNewWrite( FUNCTION_LOG_RETURN( STORAGE_WRITE, storageWritePosixNew( - this, file, modeFile, modePath, user, group, timeModified, createPath, syncFile, this->syncPath ? syncPath : false, - atomic)); + this, file, modeFile, modePath, user, group, timeModified, createPath, syncFile, + this->interface.pathSync != NULL ? syncPath : false, atomic)); } /*********************************************************************************************************************************** @@ -516,28 +510,29 @@ storagePosixPathExists(THIS_VOID, const String *path) /*********************************************************************************************************************************** Remove a path ***********************************************************************************************************************************/ -static void -storagePosixPathRemove(THIS_VOID, const String *path, bool errorOnMissing, bool recurse) +static bool +storagePosixPathRemove(THIS_VOID, const String *path, bool recurse) { THIS(StoragePosix); FUNCTION_LOG_BEGIN(logLevelTrace); FUNCTION_LOG_PARAM(STORAGE_POSIX, this); FUNCTION_LOG_PARAM(STRING, path); - FUNCTION_LOG_PARAM(BOOL, errorOnMissing); FUNCTION_LOG_PARAM(BOOL, recurse); FUNCTION_LOG_END(); ASSERT(this != NULL); ASSERT(path != NULL); + bool result = true; + MEM_CONTEXT_TEMP_BEGIN() { // Recurse if requested if (recurse) { // Get a list of files in this path - StringList *fileList = storagePosixList(this, path, errorOnMissing, NULL); + StringList *fileList = storagePosixList(this, path, NULL); // Only continue if the path exists if (fileList != NULL) @@ -552,10 +547,10 @@ storagePosixPathRemove(THIS_VOID, const String *path, bool errorOnMissing, bool { // These errors indicate that the entry is actually a path so we'll try to delete it that way if (errno == EPERM || errno == EISDIR) // {uncovered_branch - no EPERM on tested systems} - storagePosixPathRemove(this, file, false, true); + storagePosixPathRemove(this, file, true); // Else error else - THROW_SYS_ERROR_FMT(PathRemoveError, "unable to remove path/file '%s'", strPtr(file)); + THROW_SYS_ERROR_FMT(PathRemoveError, STORAGE_ERROR_PATH_REMOVE_FILE, strPtr(file)); } } } @@ -564,46 +559,59 @@ storagePosixPathRemove(THIS_VOID, const String *path, bool errorOnMissing, bool // Delete the path if (rmdir(strPtr(path)) == -1) { - if (errorOnMissing || errno != ENOENT) - THROW_SYS_ERROR_FMT(PathRemoveError, "unable to remove path '%s'", strPtr(path)); + if (errno != ENOENT) + THROW_SYS_ERROR_FMT(PathRemoveError, STORAGE_ERROR_PATH_REMOVE, strPtr(path)); + + // Path does not exist + result = false; } } MEM_CONTEXT_TEMP_END(); - FUNCTION_LOG_RETURN_VOID(); + FUNCTION_LOG_RETURN(BOOL, result); } /*********************************************************************************************************************************** Sync a path ***********************************************************************************************************************************/ void -storagePosixPathSync(THIS_VOID, const String *path, bool ignoreMissing) +storagePosixPathSync(THIS_VOID, const String *path) { THIS(StoragePosix); FUNCTION_LOG_BEGIN(logLevelTrace); FUNCTION_LOG_PARAM(STORAGE_POSIX, this); FUNCTION_LOG_PARAM(STRING, path); - FUNCTION_LOG_PARAM(BOOL, ignoreMissing); FUNCTION_LOG_END(); ASSERT(this != NULL); ASSERT(path != NULL); - if (this->syncPath) - { - // Open directory and handle errors - int handle = storagePosixFileOpen(path, O_RDONLY, 0, ignoreMissing, false, "sync"); + // Open directory and handle errors + int handle = open(strPtr(path), O_RDONLY, 0); - // On success - if (handle != -1) + // Handle errors + if (handle == -1) + { + if (errno == ENOENT) + THROW_FMT(PathMissingError, STORAGE_ERROR_PATH_SYNC_MISSING, strPtr(path)); + else + THROW_SYS_ERROR_FMT(PathOpenError, STORAGE_ERROR_PATH_SYNC_OPEN, strPtr(path)); + } + else + { + // Attempt to sync the directory + if (fsync(handle) == -1) { - // Attempt to sync the directory - storagePosixFileSync(handle, path, false, true); + int errNo = errno; - // Close the directory - storagePosixFileClose(handle, path, false); + // Close the handle to free resources but don't check for failure + close(handle); + + THROW_SYS_ERROR_CODE_FMT(errNo, PathSyncError, STORAGE_ERROR_PATH_SYNC, strPtr(path)); } + + THROW_ON_SYS_ERROR_FMT(close(handle) == -1, PathCloseError, STORAGE_ERROR_PATH_SYNC_CLOSE, strPtr(path)); } FUNCTION_LOG_RETURN_VOID(); @@ -642,7 +650,7 @@ New object Storage * storagePosixNewInternal( const String *type, const String *path, mode_t modeFile, mode_t modePath, bool write, - StoragePathExpressionCallback pathExpressionFunction, bool syncPath) + StoragePathExpressionCallback pathExpressionFunction, bool pathSync) { FUNCTION_LOG_BEGIN(logLevelDebug); FUNCTION_LOG_PARAM(STRING, type); @@ -651,7 +659,7 @@ storagePosixNewInternal( FUNCTION_LOG_PARAM(MODE, modePath); FUNCTION_LOG_PARAM(BOOL, write); FUNCTION_LOG_PARAM(FUNCTIONP, pathExpressionFunction); - FUNCTION_LOG_PARAM(BOOL, syncPath); + FUNCTION_LOG_PARAM(BOOL, pathSync); FUNCTION_LOG_END(); ASSERT(type != NULL); @@ -666,14 +674,17 @@ storagePosixNewInternal( { StoragePosix *driver = memNew(sizeof(StoragePosix)); driver->memContext = MEM_CONTEXT_NEW(); - driver->syncPath = syncPath; - - this = storageNewP( - type, path, modeFile, modePath, write, pathExpressionFunction, driver, .exists = storagePosixExists, - .info = storagePosixInfo, .infoList = storagePosixInfoList, .list = storagePosixList, .move = storagePosixMove, - .newRead = storagePosixNewRead, .newWrite = storagePosixNewWrite, .pathCreate = storagePosixPathCreate, - .pathExists = storagePosixPathExists, .pathRemove = storagePosixPathRemove, .pathSync = storagePosixPathSync, - .remove = storagePosixRemove); + + driver->interface = (StorageInterface) + { + .feature = (1 << storageFeaturePath), .exists = storagePosixExists, .info = storagePosixInfo, + .infoList = storagePosixInfoList, .list = storagePosixList, .move = storagePosixMove, .newRead = storagePosixNewRead, + .newWrite = storagePosixNewWrite, .pathCreate = storagePosixPathCreate, .pathExists = storagePosixPathExists, + .pathRemove = storagePosixPathRemove, .pathSync = pathSync ? storagePosixPathSync : NULL, + .remove = storagePosixRemove + }; + + this = storageNew(type, path, modeFile, modePath, write, pathExpressionFunction, driver, driver->interface); } MEM_CONTEXT_NEW_END(); @@ -693,7 +704,5 @@ storagePosixNew( FUNCTION_LOG_END(); FUNCTION_LOG_RETURN( - STORAGE, - storagePosixNewInternal( - STORAGE_POSIX_TYPE_STR, path, modeFile, modePath, write, pathExpressionFunction, true)); + STORAGE, storagePosixNewInternal(STORAGE_POSIX_TYPE_STR, path, modeFile, modePath, write, pathExpressionFunction, true)); } diff --git a/src/storage/posix/storage.intern.h b/src/storage/posix/storage.intern.h index 46ba7b4861..a9d0bc60c4 100644 --- a/src/storage/posix/storage.intern.h +++ b/src/storage/posix/storage.intern.h @@ -18,7 +18,7 @@ Storage *storagePosixNewInternal( Functions ***********************************************************************************************************************************/ void storagePosixPathCreate(THIS_VOID, const String *path, bool errorOnExists, bool noParentCreate, mode_t mode); -void storagePosixPathSync(THIS_VOID, const String *path, bool ignoreMissing); +void storagePosixPathSync(THIS_VOID, const String *path); /*********************************************************************************************************************************** Macros for function logging diff --git a/src/storage/posix/write.c b/src/storage/posix/write.c index 5476d526f3..78328e165e 100644 --- a/src/storage/posix/write.c +++ b/src/storage/posix/write.c @@ -15,7 +15,6 @@ Posix Storage File write #include "common/log.h" #include "common/memContext.h" #include "common/object.h" -#include "storage/posix/common.h" #include "storage/posix/storage.intern.h" #include "storage/posix/write.h" #include "storage/write.intern.h" @@ -58,8 +57,7 @@ Close file handle ***********************************************************************************************************************************/ OBJECT_DEFINE_FREE_RESOURCE_BEGIN(STORAGE_WRITE_POSIX, LOG, logLevelTrace) { - if (this->handle != -1) - storagePosixFileClose(this->handle, this->interface.name, true); + THROW_ON_SYS_ERROR_FMT(close(this->handle) == -1, FileCloseError, STORAGE_ERROR_WRITE_CLOSE, strPtr(this->nameTmp)); } OBJECT_DEFINE_FREE_RESOURCE_END(LOG); @@ -78,19 +76,26 @@ storageWritePosixOpen(THIS_VOID) ASSERT(this != NULL); ASSERT(this->handle == -1); - // Open the file and handle errors - this->handle = storagePosixFileOpen( - this->nameTmp, FILE_OPEN_FLAGS, this->interface.modeFile, this->interface.createPath, true, FILE_OPEN_PURPOSE); + // Open the file + this->handle = open(strPtr(this->nameTmp), FILE_OPEN_FLAGS, this->interface.modeFile); - // If path is missing - if (this->handle == -1) + // Attempt the create the path if it is missing + if (this->handle == -1 && errno == ENOENT && this->interface.createPath) { - // Create the path + // Create the path storagePosixPathCreate(this->storage, this->path, false, false, this->interface.modePath); - // Try the open again - this->handle = storagePosixFileOpen( - this->nameTmp, FILE_OPEN_FLAGS, this->interface.modeFile, false, true, FILE_OPEN_PURPOSE); + // Open file again + this->handle = open(strPtr(this->nameTmp), FILE_OPEN_FLAGS, this->interface.modeFile); + } + + // Handle errors + if (this->handle == -1) + { + if (errno == ENOENT) + THROW_FMT(FileMissingError, STORAGE_ERROR_WRITE_MISSING, strPtr(this->interface.name)); + else + THROW_SYS_ERROR_FMT(FileOpenError, STORAGE_ERROR_WRITE_OPEN, strPtr(this->interface.name)); } // Set free callback to ensure file handle is freed @@ -169,11 +174,11 @@ storageWritePosixClose(THIS_VOID) { // Sync the file if (this->interface.syncFile) - storagePosixFileSync(this->handle, this->nameTmp, true, false); + THROW_ON_SYS_ERROR_FMT(fsync(this->handle) == -1, FileSyncError, STORAGE_ERROR_WRITE_SYNC, strPtr(this->nameTmp)); // Close the file - storagePosixFileClose(this->handle, this->nameTmp, true); memContextCallbackClear(this->memContext); + THROW_ON_SYS_ERROR_FMT(close(this->handle) == -1, FileCloseError, STORAGE_ERROR_WRITE_CLOSE, strPtr(this->nameTmp)); this->handle = -1; // Update modified time @@ -198,7 +203,7 @@ storageWritePosixClose(THIS_VOID) // Sync the path if (this->interface.syncPath) - storagePosixPathSync(this->storage, this->path, false); + storagePosixPathSync(this->storage, this->path); } FUNCTION_LOG_RETURN_VOID(); diff --git a/src/storage/remote/protocol.c b/src/storage/remote/protocol.c index a555692daa..20b1fb8715 100644 --- a/src/storage/remote/protocol.c +++ b/src/storage/remote/protocol.c @@ -16,6 +16,7 @@ Remote Storage Protocol Handler Constants ***********************************************************************************************************************************/ STRING_EXTERN(PROTOCOL_COMMAND_STORAGE_EXISTS_STR, PROTOCOL_COMMAND_STORAGE_EXISTS); +STRING_EXTERN(PROTOCOL_COMMAND_STORAGE_FEATURE_STR, PROTOCOL_COMMAND_STORAGE_FEATURE); STRING_EXTERN(PROTOCOL_COMMAND_STORAGE_LIST_STR, PROTOCOL_COMMAND_STORAGE_LIST); STRING_EXTERN(PROTOCOL_COMMAND_STORAGE_OPEN_READ_STR, PROTOCOL_COMMAND_STORAGE_OPEN_READ); STRING_EXTERN(PROTOCOL_COMMAND_STORAGE_OPEN_WRITE_STR, PROTOCOL_COMMAND_STORAGE_OPEN_WRITE); @@ -68,6 +69,10 @@ storageRemoteProtocol(const String *command, const VariantList *paramList, Proto protocolServerResponse(server, VARBOOL( // The unusual line break is to make coverage happy -- not sure why interface.exists(driver, storagePathNP(storage, varStr(varLstGet(paramList, 0)))))); } + else if (strEq(command, PROTOCOL_COMMAND_STORAGE_FEATURE_STR)) + { + protocolServerResponse(server, varNewUInt64(interface.feature)); + } else if (strEq(command, PROTOCOL_COMMAND_STORAGE_LIST_STR)) { protocolServerResponse( @@ -75,8 +80,7 @@ storageRemoteProtocol(const String *command, const VariantList *paramList, Proto varNewVarLst( varLstNewStrLst( interface.list( - driver, storagePathNP(storage, varStr(varLstGet(paramList, 0))), varBool(varLstGet(paramList, 1)), - varStr(varLstGet(paramList, 2)))))); + driver, storagePathNP(storage, varStr(varLstGet(paramList, 0))), varStr(varLstGet(paramList, 1)))))); } else if (strEq(command, PROTOCOL_COMMAND_STORAGE_OPEN_READ_STR)) { @@ -187,15 +191,14 @@ storageRemoteProtocol(const String *command, const VariantList *paramList, Proto } else if (strEq(command, PROTOCOL_COMMAND_STORAGE_PATH_REMOVE_STR)) { - interface.pathRemove - (driver, storagePathNP(storage, varStr(varLstGet(paramList, 0))), varBool(varLstGet(paramList, 1)), - varBool(varLstGet(paramList, 2))); - - protocolServerResponse(server, NULL); + protocolServerResponse(server, + varNewBool( + interface.pathRemove( + driver, storagePathNP(storage, varStr(varLstGet(paramList, 0))), varBool(varLstGet(paramList, 1))))); } else if (strEq(command, PROTOCOL_COMMAND_STORAGE_PATH_SYNC_STR)) { - interface.pathSync(driver, storagePathNP(storage, varStr(varLstGet(paramList, 0))), varBool(varLstGet(paramList, 1))); + interface.pathSync(driver, storagePathNP(storage, varStr(varLstGet(paramList, 0)))); protocolServerResponse(server, NULL); } diff --git a/src/storage/remote/protocol.h b/src/storage/remote/protocol.h index 658eb67bd6..82095f3b0c 100644 --- a/src/storage/remote/protocol.h +++ b/src/storage/remote/protocol.h @@ -15,6 +15,8 @@ Constants #define PROTOCOL_COMMAND_STORAGE_EXISTS "storageExists" STRING_DECLARE(PROTOCOL_COMMAND_STORAGE_EXISTS_STR); +#define PROTOCOL_COMMAND_STORAGE_FEATURE "storageFeature" + STRING_DECLARE(PROTOCOL_COMMAND_STORAGE_FEATURE_STR); #define PROTOCOL_COMMAND_STORAGE_LIST "storageList" STRING_DECLARE(PROTOCOL_COMMAND_STORAGE_LIST_STR); #define PROTOCOL_COMMAND_STORAGE_OPEN_READ "storageOpenRead" diff --git a/src/storage/remote/storage.c b/src/storage/remote/storage.c index e520f27d8a..8379fb2006 100644 --- a/src/storage/remote/storage.c +++ b/src/storage/remote/storage.c @@ -60,14 +60,13 @@ storageRemoteExists(THIS_VOID, const String *path) File/path info ***********************************************************************************************************************************/ static StorageInfo -storageRemoteInfo(THIS_VOID, const String *file, bool ignoreMissing, bool followLink) +storageRemoteInfo(THIS_VOID, const String *file, bool followLink) { THIS(StorageRemote); FUNCTION_LOG_BEGIN(logLevelDebug); FUNCTION_LOG_PARAM(STORAGE_REMOTE, this); FUNCTION_LOG_PARAM(STRING, file); - FUNCTION_LOG_PARAM(BOOL, ignoreMissing); FUNCTION_LOG_PARAM(BOOL, followLink); FUNCTION_LOG_END(); @@ -83,19 +82,17 @@ storageRemoteInfo(THIS_VOID, const String *file, bool ignoreMissing, bool follow Get a list of files from a directory ***********************************************************************************************************************************/ static StringList * -storageRemoteList(THIS_VOID, const String *path, bool errorOnMissing, const String *expression) +storageRemoteList(THIS_VOID, const String *path, const String *expression) { THIS(StorageRemote); FUNCTION_LOG_BEGIN(logLevelDebug); FUNCTION_LOG_PARAM(STORAGE_REMOTE, this); FUNCTION_LOG_PARAM(STRING, path); - FUNCTION_LOG_PARAM(BOOL, errorOnMissing); FUNCTION_LOG_PARAM(STRING, expression); FUNCTION_LOG_END(); ASSERT(this != NULL); - ASSERT(!errorOnMissing); StringList *result = NULL; @@ -103,7 +100,6 @@ storageRemoteList(THIS_VOID, const String *path, bool errorOnMissing, const Stri { ProtocolCommand *command = protocolCommandNew(PROTOCOL_COMMAND_STORAGE_LIST_STR); protocolCommandParamAdd(command, VARSTR(path)); - protocolCommandParamAdd(command, VARBOOL(errorOnMissing)); protocolCommandParamAdd(command, VARSTR(expression)); result = strLstMove(strLstNewVarLst(varVarLst(protocolClientExecute(this->client, command, true))), MEM_CONTEXT_OLD()); @@ -231,47 +227,46 @@ storageRemotePathExists(THIS_VOID, const String *path) /*********************************************************************************************************************************** Remove a path ***********************************************************************************************************************************/ -static void -storageRemotePathRemove(THIS_VOID, const String *path, bool errorOnMissing, bool recurse) +static bool +storageRemotePathRemove(THIS_VOID, const String *path, bool recurse) { THIS(StorageRemote); FUNCTION_LOG_BEGIN(logLevelDebug); FUNCTION_LOG_PARAM(STORAGE_REMOTE, this); FUNCTION_LOG_PARAM(STRING, path); - FUNCTION_LOG_PARAM(BOOL, errorOnMissing); FUNCTION_LOG_PARAM(BOOL, recurse); FUNCTION_LOG_END(); ASSERT(this != NULL); ASSERT(path != NULL); + bool result = false; + MEM_CONTEXT_TEMP_BEGIN() { ProtocolCommand *command = protocolCommandNew(PROTOCOL_COMMAND_STORAGE_PATH_REMOVE_STR); protocolCommandParamAdd(command, VARSTR(path)); - protocolCommandParamAdd(command, VARBOOL(errorOnMissing)); protocolCommandParamAdd(command, VARBOOL(recurse)); - protocolClientExecute(this->client, command, false); + result = varBool(protocolClientExecute(this->client, command, true)); } MEM_CONTEXT_TEMP_END(); - FUNCTION_LOG_RETURN_VOID(); + FUNCTION_LOG_RETURN(BOOL, result); } /*********************************************************************************************************************************** Sync a path. There's no need for this on S3 so just return success. ***********************************************************************************************************************************/ static void -storageRemotePathSync(THIS_VOID, const String *path, bool ignoreMissing) +storageRemotePathSync(THIS_VOID, const String *path) { THIS(StorageRemote); FUNCTION_LOG_BEGIN(logLevelTrace); FUNCTION_LOG_PARAM(STORAGE_REMOTE, this); FUNCTION_LOG_PARAM(STRING, path); - FUNCTION_LOG_PARAM(BOOL, ignoreMissing); FUNCTION_LOG_END(); ASSERT(this != NULL); @@ -281,7 +276,6 @@ storageRemotePathSync(THIS_VOID, const String *path, bool ignoreMissing) { ProtocolCommand *command = protocolCommandNew(PROTOCOL_COMMAND_STORAGE_PATH_SYNC_STR); protocolCommandParamAdd(command, VARSTR(path)); - protocolCommandParamAdd(command, VARBOOL(ignoreMissing)); protocolClientExecute(this->client, command, false); } @@ -347,8 +341,18 @@ storageRemoteNew( driver->memContext = MEM_CONTEXT_NEW(); driver->client = client; + uint64_t feature = 0; + + // Get storage features from the remote + MEM_CONTEXT_TEMP_BEGIN() + { + feature = varUInt64( + protocolClientExecute(driver->client, protocolCommandNew(PROTOCOL_COMMAND_STORAGE_FEATURE_STR), true)); + } + MEM_CONTEXT_TEMP_END(); + this = storageNewP( - STORAGE_REMOTE_TYPE_STR, NULL, modeFile, modePath, write, pathExpressionFunction, driver, + STORAGE_REMOTE_TYPE_STR, NULL, modeFile, modePath, write, pathExpressionFunction, driver, .feature = feature, .exists = storageRemoteExists, .info = storageRemoteInfo, .list = storageRemoteList, .newRead = storageRemoteNewRead, .newWrite = storageRemoteNewWrite, .pathCreate = storageRemotePathCreate, .pathExists = storageRemotePathExists, .pathRemove = storageRemotePathRemove, .pathSync = storageRemotePathSync, .remove = storageRemoteRemove); diff --git a/src/storage/s3/storage.c b/src/storage/s3/storage.c index 34598f365c..7f79280f64 100644 --- a/src/storage/s3/storage.c +++ b/src/storage/s3/storage.c @@ -393,20 +393,18 @@ storageS3Exists(THIS_VOID, const String *path) Get a list of files from a directory ***********************************************************************************************************************************/ static StringList * -storageS3List(THIS_VOID, const String *path, bool errorOnMissing, const String *expression) +storageS3List(THIS_VOID, const String *path, const String *expression) { THIS(StorageS3); FUNCTION_LOG_BEGIN(logLevelDebug); FUNCTION_LOG_PARAM(STORAGE_S3, this); FUNCTION_LOG_PARAM(STRING, path); - FUNCTION_LOG_PARAM(BOOL, errorOnMissing); FUNCTION_LOG_PARAM(STRING, expression); FUNCTION_LOG_END(); ASSERT(this != NULL); ASSERT(path != NULL); - ASSERT(!errorOnMissing); StringList *result = NULL; @@ -575,120 +573,112 @@ storageS3NewWrite( /*********************************************************************************************************************************** Remove a path ***********************************************************************************************************************************/ -static void -storageS3PathRemove(THIS_VOID, const String *path, bool errorOnMissing, bool recurse) +static bool +storageS3PathRemove(THIS_VOID, const String *path, bool recurse) { THIS(StorageS3); FUNCTION_LOG_BEGIN(logLevelDebug); FUNCTION_LOG_PARAM(STORAGE_S3, this); FUNCTION_LOG_PARAM(STRING, path); - FUNCTION_LOG_PARAM(BOOL, errorOnMissing); FUNCTION_LOG_PARAM(BOOL, recurse); FUNCTION_LOG_END(); ASSERT(this != NULL); ASSERT(path != NULL); - ASSERT(!errorOnMissing); - // S3 doesn't have paths that need to be deleted so nothing to do unless recursing - if (recurse) + MEM_CONTEXT_TEMP_BEGIN() { - MEM_CONTEXT_TEMP_BEGIN() - { - const String *continuationToken = NULL; + const String *continuationToken = NULL; - // Build the base prefix by stripping off the initial / - const String *basePrefix; + // Build the base prefix by stripping off the initial / + const String *basePrefix; - if (strSize(path) == 1) - basePrefix = EMPTY_STR; - else - basePrefix = strNewFmt("%s/", strPtr(strSub(path, 1))); + if (strSize(path) == 1) + basePrefix = EMPTY_STR; + else + basePrefix = strNewFmt("%s/", strPtr(strSub(path, 1))); - // Loop as long as a continuation token returned - do + // Loop as long as a continuation token returned + do + { + // Use an inner mem context here because we could potentially be retrieving millions of files so it is a good idea to + // free memory at regular intervals + MEM_CONTEXT_TEMP_BEGIN() { - // Use an inner mem context here because we could potentially be retrieving millions of files so it is a good idea - // to free memory at regular intervals - MEM_CONTEXT_TEMP_BEGIN() - { - HttpQuery *query = httpQueryNew(); + HttpQuery *query = httpQueryNew(); - // Add continuation token from the prior loop if any - if (continuationToken != NULL) - httpQueryAdd(query, S3_QUERY_CONTINUATION_TOKEN_STR, continuationToken); + // Add continuation token from the prior loop if any + if (continuationToken != NULL) + httpQueryAdd(query, S3_QUERY_CONTINUATION_TOKEN_STR, continuationToken); - // Use list type 2 - httpQueryAdd(query, S3_QUERY_LIST_TYPE_STR, S3_QUERY_VALUE_LIST_TYPE_2_STR); + // Use list type 2 + httpQueryAdd(query, S3_QUERY_LIST_TYPE_STR, S3_QUERY_VALUE_LIST_TYPE_2_STR); - // Don't specified empty prefix because it is the default - if (!strEmpty(basePrefix)) - httpQueryAdd(query, S3_QUERY_PREFIX_STR, basePrefix); + // Don't specified empty prefix because it is the default + if (!strEmpty(basePrefix)) + httpQueryAdd(query, S3_QUERY_PREFIX_STR, basePrefix); - XmlNode *xmlRoot = xmlDocumentRoot( - xmlDocumentNewBuf( - storageS3Request(this, HTTP_VERB_GET_STR, FSLASH_STR, query, NULL, true, false).response)); + XmlNode *xmlRoot = xmlDocumentRoot( + xmlDocumentNewBuf(storageS3Request(this, HTTP_VERB_GET_STR, FSLASH_STR, query, NULL, true, false).response)); - // Get file list to delete - XmlNodeList *fileList = xmlNodeChildList(xmlRoot, S3_XML_TAG_CONTENTS_STR); - XmlDocument *delete = NULL; + // Get file list to delete + XmlNodeList *fileList = xmlNodeChildList(xmlRoot, S3_XML_TAG_CONTENTS_STR); + XmlDocument *delete = NULL; - for (unsigned int fileIdx = 0; fileIdx < xmlNodeLstSize(fileList); fileIdx++) + for (unsigned int fileIdx = 0; fileIdx < xmlNodeLstSize(fileList); fileIdx++) + { + // If there is something to delete then create the request + if (delete == NULL) { - // If there is something to delete then create the request - if (delete == NULL) - { - delete = xmlDocumentNew(S3_XML_TAG_DELETE_STR); - xmlNodeContentSet(xmlNodeAdd(xmlDocumentRoot(delete), S3_XML_TAG_QUIET_STR), TRUE_STR); - } - - // Add to delete list - xmlNodeContentSet( - xmlNodeAdd(xmlNodeAdd(xmlDocumentRoot(delete), S3_XML_TAG_OBJECT_STR), S3_XML_TAG_KEY_STR), - xmlNodeContent(xmlNodeChild(xmlNodeLstGet(fileList, fileIdx), S3_XML_TAG_KEY_STR, true))); + delete = xmlDocumentNew(S3_XML_TAG_DELETE_STR); + xmlNodeContentSet(xmlNodeAdd(xmlDocumentRoot(delete), S3_XML_TAG_QUIET_STR), TRUE_STR); } - // If there is something to delete then send the request - if (delete != NULL) + // Add to delete list + xmlNodeContentSet( + xmlNodeAdd(xmlNodeAdd(xmlDocumentRoot(delete), S3_XML_TAG_OBJECT_STR), S3_XML_TAG_KEY_STR), + xmlNodeContent(xmlNodeChild(xmlNodeLstGet(fileList, fileIdx), S3_XML_TAG_KEY_STR, true))); + } + + // If there is something to delete then send the request + if (delete != NULL) + { + // Delete file list + Buffer *xml = storageS3Request( + this, HTTP_VERB_POST_STR, FSLASH_STR, httpQueryAdd(httpQueryNew(), S3_QUERY_DELETE_STR, EMPTY_STR), + xmlDocumentBuf(delete), true, false).response; + + // Nothing is returned when there are no errors + if (xml != NULL) { - // Delete file list - Buffer *xml = storageS3Request( - this, HTTP_VERB_POST_STR, FSLASH_STR, httpQueryAdd(httpQueryNew(), S3_QUERY_DELETE_STR, EMPTY_STR), - xmlDocumentBuf(delete), true, false).response; + XmlNodeList *errorList = xmlNodeChildList(xmlDocumentRoot(xmlDocumentNewBuf(xml)), S3_XML_TAG_ERROR_STR); - // Nothing is returned when there are no errors - if (xml != NULL) + if (xmlNodeLstSize(errorList) > 0) { - XmlNodeList *errorList = xmlNodeChildList( - xmlDocumentRoot(xmlDocumentNewBuf(xml)), S3_XML_TAG_ERROR_STR); - - if (xmlNodeLstSize(errorList) > 0) - { - XmlNode *error = xmlNodeLstGet(errorList, 0); - - THROW_FMT( - FileRemoveError, "unable to remove '%s': [%s] %s", - strPtr(xmlNodeContent(xmlNodeChild(error, S3_XML_TAG_KEY_STR, true))), - strPtr(xmlNodeContent(xmlNodeChild(error, S3_XML_TAG_CODE_STR, true))), - strPtr(xmlNodeContent(xmlNodeChild(error, S3_XML_TAG_MESSAGE_STR, true)))); - } + XmlNode *error = xmlNodeLstGet(errorList, 0); + + THROW_FMT( + FileRemoveError, STORAGE_ERROR_PATH_REMOVE_FILE ": [%s] %s", + strPtr(xmlNodeContent(xmlNodeChild(error, S3_XML_TAG_KEY_STR, true))), + strPtr(xmlNodeContent(xmlNodeChild(error, S3_XML_TAG_CODE_STR, true))), + strPtr(xmlNodeContent(xmlNodeChild(error, S3_XML_TAG_MESSAGE_STR, true)))); } } - - // Get the continuation token and store it in the outer temp context - memContextSwitch(MEM_CONTEXT_OLD()); - continuationToken = xmlNodeContent(xmlNodeChild(xmlRoot, S3_XML_TAG_NEXT_CONTINUATION_TOKEN_STR, false)); - memContextSwitch(MEM_CONTEXT_TEMP()); } - MEM_CONTEXT_TEMP_END(); + + // Get the continuation token and store it in the outer temp context + memContextSwitch(MEM_CONTEXT_OLD()); + continuationToken = xmlNodeContent(xmlNodeChild(xmlRoot, S3_XML_TAG_NEXT_CONTINUATION_TOKEN_STR, false)); + memContextSwitch(MEM_CONTEXT_TEMP()); } - while (continuationToken != NULL); + MEM_CONTEXT_TEMP_END(); } - MEM_CONTEXT_TEMP_END(); + while (continuationToken != NULL); } + MEM_CONTEXT_TEMP_END(); - FUNCTION_LOG_RETURN_VOID(); + FUNCTION_LOG_RETURN(BOOL, true); } /*********************************************************************************************************************************** diff --git a/src/storage/storage.c b/src/storage/storage.c index 7c728a4541..e7b9ce69f8 100644 --- a/src/storage/storage.c +++ b/src/storage/storage.c @@ -250,7 +250,11 @@ storageInfo(const Storage *this, const String *fileExp, StorageInfoParam param) String *file = storagePathNP(this, fileExp); // Call driver function - result = this->interface.info(this->driver, file, param.ignoreMissing, param.followLink); + result = this->interface.info(this->driver, file, param.followLink); + + // Error if the file missing and not ignoring + if (!result.exists && !param.ignoreMissing) + THROW_SYS_ERROR_FMT(FileOpenError, STORAGE_ERROR_INFO_MISSING, strPtr(file)); // Dup the strings into the calling context memContextSwitch(MEM_CONTEXT_OLD()); @@ -282,6 +286,7 @@ storageInfoList( ASSERT(this != NULL); ASSERT(callback != NULL); ASSERT(this->interface.infoList != NULL); + ASSERT(!param.errorOnMissing || storageFeature(this, storageFeaturePath)); bool result = false; @@ -291,7 +296,10 @@ storageInfoList( String *path = storagePathNP(this, pathExp); // Call driver function - result = this->interface.infoList(this->driver, path, param.errorOnMissing, callback, callbackData); + result = this->interface.infoList(this->driver, path, callback, callbackData); + + if (!result && param.errorOnMissing) + THROW_FMT(PathOpenError, STORAGE_ERROR_LIST_INFO_MISSING, strPtr(path)); } MEM_CONTEXT_TEMP_END(); @@ -314,6 +322,7 @@ storageList(const Storage *this, const String *pathExp, StorageListParam param) ASSERT(this != NULL); ASSERT(!param.errorOnMissing || !param.nullOnMissing); + ASSERT(!param.errorOnMissing || storageFeature(this, storageFeaturePath)); StringList *result = NULL; @@ -323,12 +332,20 @@ storageList(const Storage *this, const String *pathExp, StorageListParam param) String *path = storagePathNP(this, pathExp); // Get the list - result = this->interface.list(this->driver, path, param.errorOnMissing, param.expression); + result = this->interface.list(this->driver, path, param.expression); - // Build an empty list if the directory does not exist by default. This makes the logic in calling functions simpler - // when they don't care if the path is missing. - if (result == NULL && !param.nullOnMissing) - result = strLstNew(); + // If the path does not exist + if (result == NULL) + { + // Error if requested + if (param.errorOnMissing) + THROW_FMT(PathOpenError, STORAGE_ERROR_LIST_MISSING, strPtr(path)); + + // Build an empty list if the directory does not exist by default. This makes the logic in calling functions simpler + // when they don't care if the path is missing. + if (!param.nullOnMissing) + result = strLstNew(); + } // Move list up to the old context result = strLstMove(result, MEM_CONTEXT_OLD()); @@ -372,7 +389,7 @@ storageMove(const Storage *this, StorageRead *source, StorageWrite *destination) // the move did not succeed. This will need updating when drivers other than Posix/CIFS are implemented becaue there's // no way to get coverage on it now. if (storageWriteSyncPath(destination)) - this->interface.pathSync(this->driver, strPath(storageReadName(source)), false); + this->interface.pathSync(this->driver, strPath(storageReadName(source))); } } MEM_CONTEXT_TEMP_END(); @@ -572,7 +589,7 @@ storagePathCreate(const Storage *this, const String *pathExp, StoragePathCreateP FUNCTION_LOG_END(); ASSERT(this != NULL); - ASSERT(this->interface.pathCreate != NULL); + ASSERT(this->interface.pathCreate != NULL && storageFeature(this, storageFeaturePath)); ASSERT(this->write); // It doesn't make sense to combine these parameters because if we are creating missing parent paths why error when they exist? @@ -633,6 +650,8 @@ storagePathRemove(const Storage *this, const String *pathExp, StoragePathRemoveP ASSERT(this != NULL); ASSERT(this->write); + ASSERT(!param.errorOnMissing || storageFeature(this, storageFeaturePath)); + ASSERT(param.recurse || storageFeature(this, storageFeaturePath)); MEM_CONTEXT_TEMP_BEGIN() { @@ -640,7 +659,8 @@ storagePathRemove(const Storage *this, const String *pathExp, StoragePathRemoveP String *path = storagePathNP(this, pathExp); // Call driver function - this->interface.pathRemove(this->driver, path, param.errorOnMissing, param.recurse); + if (!this->interface.pathRemove(this->driver, path, param.recurse) && param.errorOnMissing) + THROW_FMT(PathRemoveError, STORAGE_ERROR_PATH_REMOVE_MISSING, strPtr(path)); } MEM_CONTEXT_TEMP_END(); @@ -650,12 +670,11 @@ storagePathRemove(const Storage *this, const String *pathExp, StoragePathRemoveP /*********************************************************************************************************************************** Sync a path ***********************************************************************************************************************************/ -void storagePathSync(const Storage *this, const String *pathExp, StoragePathSyncParam param) +void storagePathSync(const Storage *this, const String *pathExp) { FUNCTION_LOG_BEGIN(logLevelDebug); FUNCTION_LOG_PARAM(STORAGE, this); FUNCTION_LOG_PARAM(STRING, pathExp); - FUNCTION_LOG_PARAM(BOOL, param.ignoreMissing); FUNCTION_LOG_END(); ASSERT(this != NULL); @@ -666,11 +685,7 @@ void storagePathSync(const Storage *this, const String *pathExp, StoragePathSync { MEM_CONTEXT_TEMP_BEGIN() { - // Build the path - String *path = storagePathNP(this, pathExp); - - // Call driver function - this->interface.pathSync(this->driver, path, param.ignoreMissing); + this->interface.pathSync(this->driver, storagePathNP(this, pathExp)); } MEM_CONTEXT_TEMP_END(); } @@ -741,6 +756,22 @@ storageDriver(const Storage *this) FUNCTION_TEST_RETURN(this->driver); } +/*********************************************************************************************************************************** +Is the feature supported by this storage? +***********************************************************************************************************************************/ +bool +storageFeature(const Storage *this, StorageFeature feature) +{ + FUNCTION_TEST_BEGIN(); + FUNCTION_LOG_PARAM(STORAGE, this); + FUNCTION_LOG_PARAM(ENUM, feature); + FUNCTION_TEST_END(); + + ASSERT(this != NULL); + + FUNCTION_TEST_RETURN(this->interface.feature >> feature & 1); +} + /*********************************************************************************************************************************** Get the storage interface ***********************************************************************************************************************************/ diff --git a/src/storage/storage.h b/src/storage/storage.h index 1f221bf3f8..aa7e2df21e 100644 --- a/src/storage/storage.h +++ b/src/storage/storage.h @@ -22,6 +22,19 @@ typedef struct Storage Storage; #include "storage/read.h" #include "storage/write.h" +/*********************************************************************************************************************************** +Storage feature +***********************************************************************************************************************************/ +typedef enum +{ + // Does the storage support paths/directories as something that needs to be created and deleted? Object stores (e.g. S3) often + // do not have paths/directories -- they are only inferred by the object name. Therefore it doesn't make sense to create or + // remove directories since this implies something is happening on the storage and in the case of objects stores it would be a + // noop. We'll error on any path operation (e.g. pathExists(), pathCreate(), non-recursive removes, error on missing paths, + // etc.) for storage that does not support paths. + storageFeaturePath, +} StorageFeature; + /*********************************************************************************************************************************** storageCopy ***********************************************************************************************************************************/ @@ -45,6 +58,14 @@ typedef struct StorageExistsParam bool storageExists(const Storage *this, const String *pathExp, StorageExistsParam param); +/*********************************************************************************************************************************** +storageFeature +***********************************************************************************************************************************/ +#define storageFeatureNP(this, feature) \ + storageFeature(this, feature) + +bool storageFeature(const Storage *this, StorageFeature feature); + /*********************************************************************************************************************************** storageGet ***********************************************************************************************************************************/ @@ -211,17 +232,10 @@ void storagePathRemove(const Storage *this, const String *pathExp, StoragePathRe /*********************************************************************************************************************************** storagePathSync ***********************************************************************************************************************************/ -typedef struct StoragePathSync -{ - bool ignoreMissing; -} StoragePathSyncParam; - -#define storagePathSyncP(this, pathExp, ...) \ - storagePathSync(this, pathExp, (StoragePathSyncParam){__VA_ARGS__}) #define storagePathSyncNP(this, pathExp) \ - storagePathSync(this, pathExp, (StoragePathSyncParam){0}) + storagePathSync(this, pathExp) -void storagePathSync(const Storage *this, const String *pathExp, StoragePathSyncParam param); +void storagePathSync(const Storage *this, const String *pathExp); /*********************************************************************************************************************************** storagePut diff --git a/src/storage/storage.intern.h b/src/storage/storage.intern.h index fc7827c21c..0f4f955dae 100644 --- a/src/storage/storage.intern.h +++ b/src/storage/storage.intern.h @@ -14,6 +14,36 @@ Default file and path modes #define STORAGE_MODE_FILE_DEFAULT 0640 #define STORAGE_MODE_PATH_DEFAULT 0750 +/*********************************************************************************************************************************** +Error messages +***********************************************************************************************************************************/ +#define STORAGE_ERROR_READ_CLOSE "unable to close file '%s' after read" +#define STORAGE_ERROR_READ_OPEN "unable to open file '%s' for read" +#define STORAGE_ERROR_READ_MISSING "unable to open missing file '%s' for read" + +#define STORAGE_ERROR_INFO "unable to get info for path/file '%s'" +#define STORAGE_ERROR_INFO_MISSING "unable to get info for missing path/file '%s'" + +#define STORAGE_ERROR_LIST "unable to list files for path '%s'" +#define STORAGE_ERROR_LIST_MISSING "unable to list files for missing path '%s'" + +#define STORAGE_ERROR_LIST_INFO "unable to list file info for path '%s'" +#define STORAGE_ERROR_LIST_INFO_MISSING "unable to list file info for missing path '%s'" + +#define STORAGE_ERROR_PATH_REMOVE "unable to remove path '%s'" +#define STORAGE_ERROR_PATH_REMOVE_FILE "unable to remove file '%s'" +#define STORAGE_ERROR_PATH_REMOVE_MISSING "unable to remove missing path '%s'" + +#define STORAGE_ERROR_PATH_SYNC "unable to sync path '%s'" +#define STORAGE_ERROR_PATH_SYNC_CLOSE "unable to close path '%s' after sync" +#define STORAGE_ERROR_PATH_SYNC_OPEN "unable to open path '%s' for sync" +#define STORAGE_ERROR_PATH_SYNC_MISSING "unable to sync missing path '%s'" + +#define STORAGE_ERROR_WRITE_CLOSE "unable to close file '%s' after write" +#define STORAGE_ERROR_WRITE_OPEN "unable to open file '%s' for write" +#define STORAGE_ERROR_WRITE_MISSING "unable to open file '%s' for write in missing path" +#define STORAGE_ERROR_WRITE_SYNC "unable to sync file '%s' after write" + /*********************************************************************************************************************************** Path expression callback function type - used to modify paths based on expressions enclosed in <> ***********************************************************************************************************************************/ @@ -24,10 +54,13 @@ Constructor ***********************************************************************************************************************************/ typedef struct StorageInterface { + // Features implemented by the storage driver + uint64_t feature; + bool (*exists)(void *driver, const String *path); - StorageInfo (*info)(void *driver, const String *path, bool ignoreMissing, bool followLink); - bool (*infoList)(void *driver, const String *file, bool ignoreMissing, StorageInfoListCallback callback, void *callbackData); - StringList *(*list)(void *driver, const String *path, bool errorOnMissing, const String *expression); + StorageInfo (*info)(void *driver, const String *path, bool followLink); + bool (*infoList)(void *driver, const String *file, StorageInfoListCallback callback, void *callbackData); + StringList *(*list)(void *driver, const String *path, const String *expression); bool (*move)(void *driver, StorageRead *source, StorageWrite *destination); StorageRead *(*newRead)(void *driver, const String *file, bool ignoreMissing); StorageWrite *(*newWrite)( @@ -35,8 +68,8 @@ typedef struct StorageInterface time_t timeModified, bool createPath, bool syncFile, bool syncPath, bool atomic); void (*pathCreate)(void *driver, const String *path, bool errorOnExists, bool noParentCreate, mode_t mode); bool (*pathExists)(void *driver, const String *path); - void (*pathRemove)(void *driver, const String *path, bool errorOnMissing, bool recurse); - void (*pathSync)(void *driver, const String *path, bool ignoreMissing); + bool (*pathRemove)(void *driver, const String *path, bool recurse); + void (*pathSync)(void *driver, const String *path); void (*remove)(void *driver, const String *file, bool errorOnMissing); } StorageInterface; diff --git a/test/define.yaml b/test/define.yaml index 8bf95b599c..00828c011c 100644 --- a/test/define.yaml +++ b/test/define.yaml @@ -511,10 +511,9 @@ unit: # ---------------------------------------------------------------------------------------------------------------------------- - name: posix - total: 21 + total: 20 coverage: - storage/posix/common: full storage/posix/read: full storage/posix/storage: full storage/posix/write: full @@ -525,7 +524,7 @@ unit: # ---------------------------------------------------------------------------------------------------------------------------- - name: remote - total: 10 + total: 11 perlReq: true coverage: diff --git a/test/expect/mock-archive-001.log b/test/expect/mock-archive-001.log index 3e03ff88c2..da0319839d 100644 --- a/test/expect/mock-archive-001.log +++ b/test/expect/mock-archive-001.log @@ -5,8 +5,8 @@ run 001 - rmt 0, s3 0, enc 1 ------------------------------------------------------------------------------------------------------------------------------------ P00 INFO: archive-push command begin [BACKREST-VERSION]: [[TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000001] --no-compress --compress-level=3 --config=[TEST_PATH]/db-master/pgbackrest.conf --db-timeout=45 --lock-path=[TEST_PATH]/db-master/lock --log-level-console=detail --log-level-file=trace --log-level-stderr=off --log-path=[TEST_PATH]/db-master/log --log-subprocess --no-log-timestamp --pg1-path=[TEST_PATH]/db-master/db/base --protocol-timeout=60 --repo1-cipher-pass= --repo1-cipher-type=aes-256-cbc --repo1-path=[TEST_PATH]/db-master/repo --stanza=db P00 ERROR: [055]: unable to load info file '[TEST_PATH]/db-master/repo/archive/db/archive.info' or '[TEST_PATH]/db-master/repo/archive/db/archive.info.copy': - FileMissingError: unable to open '[TEST_PATH]/db-master/repo/archive/db/archive.info' for read: [2] No such file or directory - FileMissingError: unable to open '[TEST_PATH]/db-master/repo/archive/db/archive.info.copy' for read: [2] No such file or directory + FileMissingError: unable to open missing file '[TEST_PATH]/db-master/repo/archive/db/archive.info' for read + FileMissingError: unable to open missing file '[TEST_PATH]/db-master/repo/archive/db/archive.info.copy' for read HINT: archive.info cannot be opened but is required to push/get WAL segments. HINT: is archive_command configured correctly in postgresql.conf? HINT: has a stanza-create been performed? @@ -17,8 +17,8 @@ P00 INFO: archive-push command end: aborted with exception [055] ------------------------------------------------------------------------------------------------------------------------------------ P00 INFO: archive-get command begin [BACKREST-VERSION]: [000000010000000100000001, [TEST_PATH]/db-master/db/base/pg_xlog/RECOVERYXLOG] --no-compress --compress-level=3 --config=[TEST_PATH]/db-master/pgbackrest.conf --db-timeout=45 --lock-path=[TEST_PATH]/db-master/lock --log-level-console=detail --log-level-file=trace --log-level-stderr=off --log-path=[TEST_PATH]/db-master/log --log-subprocess --no-log-timestamp --pg1-path=[TEST_PATH]/db-master/db/base --protocol-timeout=60 --repo1-cipher-pass= --repo1-cipher-type=aes-256-cbc --repo1-path=[TEST_PATH]/db-master/repo --stanza=db P00 ERROR: [055]: unable to load info file '[TEST_PATH]/db-master/repo/archive/db/archive.info' or '[TEST_PATH]/db-master/repo/archive/db/archive.info.copy': - FileMissingError: unable to open '[TEST_PATH]/db-master/repo/archive/db/archive.info' for read: [2] No such file or directory - FileMissingError: unable to open '[TEST_PATH]/db-master/repo/archive/db/archive.info.copy' for read: [2] No such file or directory + FileMissingError: unable to open missing file '[TEST_PATH]/db-master/repo/archive/db/archive.info' for read + FileMissingError: unable to open missing file '[TEST_PATH]/db-master/repo/archive/db/archive.info.copy' for read HINT: archive.info cannot be opened but is required to push/get WAL segments. HINT: is archive_command configured correctly in postgresql.conf? HINT: has a stanza-create been performed? diff --git a/test/src/module/command/archiveGetTest.c b/test/src/module/command/archiveGetTest.c index d7fb3907a0..3c4c0f9c94 100644 --- a/test/src/module/command/archiveGetTest.c +++ b/test/src/module/command/archiveGetTest.c @@ -269,7 +269,7 @@ testRun(void) TEST_ERROR_FMT( queueNeed(strNew("000000010000000100000001"), false, queueSize, walSegmentSize, PG_VERSION_92), - PathOpenError, "unable to open path '%s/spool/archive/test1/in' for read: [2] No such file or directory", testPath()); + PathOpenError, "unable to list files for missing path '%s/spool/archive/test1/in'", testPath()); // ------------------------------------------------------------------------------------------------------------------------- storagePathCreateNP(storageSpoolWrite(), strNew(STORAGE_SPOOL_ARCHIVE_IN)); @@ -544,16 +544,16 @@ testRun(void) TEST_ERROR_FMT( cmdArchiveGet(), FileMissingError, "unable to load info file '%s/archive/test1/archive.info' or '%s/archive/test1/archive.info.copy':\n" - "FileMissingError: unable to open '%s/archive/test1/archive.info' for read: [2] No such file or directory\n" - "FileMissingError: unable to open '%s/archive/test1/archive.info.copy' for read: [2] No such file or" - " directory\n" + "FileMissingError: " STORAGE_ERROR_READ_MISSING "\n" + "FileMissingError: " STORAGE_ERROR_READ_MISSING "\n" "HINT: archive.info cannot be opened but is required to push/get WAL segments.\n" "HINT: is archive_command configured correctly in postgresql.conf?\n" "HINT: has a stanza-create been performed?\n" "HINT: use --no-archive-check to disable archive checks during backup if you have an alternate archiving" " scheme.", strPtr(cfgOptionStr(cfgOptRepoPath)), strPtr(cfgOptionStr(cfgOptRepoPath)), - strPtr(cfgOptionStr(cfgOptRepoPath)), strPtr(cfgOptionStr(cfgOptRepoPath))); + strPtr(strNewFmt("%s/archive/test1/archive.info", strPtr(cfgOptionStr(cfgOptRepoPath)))), + strPtr(strNewFmt("%s/archive/test1/archive.info.copy", strPtr(cfgOptionStr(cfgOptRepoPath))))); } HARNESS_FORK_CHILD_END(); } @@ -575,16 +575,16 @@ testRun(void) TEST_ERROR_FMT( cmdArchiveGet(), FileMissingError, "unable to load info file '%s/archive/test1/archive.info' or '%s/archive/test1/archive.info.copy':\n" - "FileMissingError: unable to open '%s/archive/test1/archive.info' for read: [2] No such file or directory\n" - "FileMissingError: unable to open '%s/archive/test1/archive.info.copy' for read: [2] No such file or" - " directory\n" + "FileMissingError: " STORAGE_ERROR_READ_MISSING "\n" + "FileMissingError: " STORAGE_ERROR_READ_MISSING "\n" "HINT: archive.info cannot be opened but is required to push/get WAL segments.\n" "HINT: is archive_command configured correctly in postgresql.conf?\n" "HINT: has a stanza-create been performed?\n" "HINT: use --no-archive-check to disable archive checks during backup if you have an alternate archiving" " scheme.", strPtr(cfgOptionStr(cfgOptRepoPath)), strPtr(cfgOptionStr(cfgOptRepoPath)), - strPtr(cfgOptionStr(cfgOptRepoPath)), strPtr(cfgOptionStr(cfgOptRepoPath))); + strPtr(strNewFmt("%s/archive/test1/archive.info", strPtr(cfgOptionStr(cfgOptRepoPath)))), + strPtr(strNewFmt("%s/archive/test1/archive.info.copy", strPtr(cfgOptionStr(cfgOptRepoPath))))); } HARNESS_FORK_CHILD_END(); } diff --git a/test/src/module/command/archivePushTest.c b/test/src/module/command/archivePushTest.c index 66c9c6f2ef..288281a615 100644 --- a/test/src/module/command/archivePushTest.c +++ b/test/src/module/command/archivePushTest.c @@ -611,8 +611,8 @@ testRun(void) "P00 INFO: push 2 WAL file(s) to archive: 000000010000000100000001...000000010000000100000002\n" "P01 DETAIL: pushed WAL file '000000010000000100000001' to the archive\n" "P01 WARN: could not push WAL file '000000010000000100000002' to the archive (will be retried): " - "[55] raised from local-1 protocol: unable to open '%s/pg/pg_xlog/000000010000000100000002' for read: " - "[2] No such file or directory", testPath()))); + "[55] raised from local-1 protocol: " STORAGE_ERROR_READ_MISSING, + strPtr(strNewFmt("%s/pg/pg_xlog/000000010000000100000002", testPath()))))); TEST_RESULT_BOOL( storageExistsNP( diff --git a/test/src/module/command/infoTest.c b/test/src/module/command/infoTest.c index 5bcc63c9ee..f8de4b790a 100644 --- a/test/src/module/command/infoTest.c +++ b/test/src/module/command/infoTest.c @@ -95,13 +95,15 @@ testRun(void) TEST_ERROR_FMT(infoRender(), FileMissingError, "unable to load info file '%s/archive.info' or '%s/archive.info.copy':\n" - "FileMissingError: unable to open '%s/archive.info' for read: [2] No such file or directory\n" - "FileMissingError: unable to open '%s/archive.info.copy' for read: [2] No such file or directory\n" + "FileMissingError: " STORAGE_ERROR_READ_MISSING "\n" + "FileMissingError: " STORAGE_ERROR_READ_MISSING "\n" "HINT: archive.info cannot be opened but is required to push/get WAL segments.\n" "HINT: is archive_command configured correctly in postgresql.conf?\n" "HINT: has a stanza-create been performed?\n" "HINT: use --no-archive-check to disable archive checks during backup if you have an alternate archiving scheme.", - strPtr(archiveStanza1Path), strPtr(archiveStanza1Path), strPtr(archiveStanza1Path), strPtr(archiveStanza1Path)); + strPtr(archiveStanza1Path), strPtr(archiveStanza1Path), + strPtr(strNewFmt("%s/archive.info", strPtr(archiveStanza1Path))), + strPtr(strNewFmt("%s/archive.info.copy", strPtr(archiveStanza1Path)))); // backup.info/archive.info files exist, mismatched db ids, no backup:current section so no valid backups // Only the current db information from the db:history will be processed. @@ -740,11 +742,12 @@ testRun(void) "unable to load info file '%s/backup.info' or '%s/backup.info.copy':\n" "CryptoError: '%s/backup.info' cipher header invalid\n" "HINT: Is or was the repo encrypted?\n" - "FileMissingError: unable to open '%s/backup.info.copy' for read: [2] No such file or directory\n" + "FileMissingError: " STORAGE_ERROR_READ_MISSING "\n" "HINT: backup.info cannot be opened and is required to perform a backup.\n" "HINT: has a stanza-create been performed?\n" - "HINT: use option --stanza if encryption settings are different for the stanza than the global settings" - ,strPtr(backupStanza2Path), strPtr(backupStanza2Path), strPtr(backupStanza2Path), strPtr(backupStanza2Path)); + "HINT: use option --stanza if encryption settings are different for the stanza than the global settings", + strPtr(backupStanza2Path), strPtr(backupStanza2Path), strPtr(backupStanza2Path), + strPtr(strNewFmt("%s/backup.info.copy", strPtr(backupStanza2Path)))); } //****************************************************************************************************************************** diff --git a/test/src/module/config/parseTest.c b/test/src/module/config/parseTest.c index 3753fa25e7..f8c65e721e 100644 --- a/test/src/module/config/parseTest.c +++ b/test/src/module/config/parseTest.c @@ -1,6 +1,8 @@ /*********************************************************************************************************************************** Test Configuration Parse ***********************************************************************************************************************************/ +#include "storage/storage.intern.h" + #define TEST_BACKREST_EXE "pgbackrest" #define TEST_COMMAND_ARCHIVE_GET "archive-get" @@ -174,16 +176,15 @@ testRun(void) TEST_ERROR( cfgFileLoad(parseOptionList, backupCmdDefConfigValue, backupCmdDefConfigInclPathValue, oldConfigDefault), PathOpenError, - "unable to open path '/BOGUS' for read: [2] No such file or directory"); + "unable to list files for missing path '/BOGUS'"); // --config-include-path valid, --config invalid (does not exist) parseOptionList[cfgOptConfigIncludePath].valueList = strLstAdd(strLstNew(), configIncludePath); parseOptionList[cfgOptConfig].valueList = strLstAdd(strLstNew(), strNewFmt("%s/%s", testPath(), BOGUS_STR)); - TEST_ERROR( - cfgFileLoad(parseOptionList, backupCmdDefConfigValue, - backupCmdDefConfigInclPathValue, oldConfigDefault), FileMissingError, - strPtr(strNewFmt("unable to open '%s/%s' for read: [2] No such file or directory", testPath(), BOGUS_STR))); + TEST_ERROR_FMT( + cfgFileLoad(parseOptionList, backupCmdDefConfigValue, backupCmdDefConfigInclPathValue, oldConfigDefault), + FileMissingError, STORAGE_ERROR_READ_MISSING, strPtr(strNewFmt("%s/BOGUS", testPath()))); strLstFree(parseOptionList[cfgOptConfig].valueList); strLstFree(parseOptionList[cfgOptConfigIncludePath].valueList); diff --git a/test/src/module/info/infoArchiveTest.c b/test/src/module/info/infoArchiveTest.c index 2863f4b920..7771861ccb 100644 --- a/test/src/module/info/infoArchiveTest.c +++ b/test/src/module/info/infoArchiveTest.c @@ -1,6 +1,8 @@ /*********************************************************************************************************************************** Test Archive Info Handler ***********************************************************************************************************************************/ +#include "storage/storage.intern.h" + #include "common/harnessInfo.h" /*********************************************************************************************************************************** @@ -21,13 +23,14 @@ testRun(void) TEST_ERROR_FMT( infoArchiveNew(storageLocal(), fileName, true, cipherTypeNone, NULL), FileMissingError, "unable to load info file '%s/test.ini' or '%s/test.ini.copy':\n" - "FileMissingError: unable to open '%s/test.ini' for read: [2] No such file or directory\n" - "FileMissingError: unable to open '%s/test.ini.copy' for read: [2] No such file or directory\n" + "FileMissingError: " STORAGE_ERROR_READ_MISSING "\n" + "FileMissingError: " STORAGE_ERROR_READ_MISSING "\n" "HINT: archive.info cannot be opened but is required to push/get WAL segments.\n" "HINT: is archive_command configured correctly in postgresql.conf?\n" "HINT: has a stanza-create been performed?\n" "HINT: use --no-archive-check to disable archive checks during backup if you have an alternate archiving scheme.", - testPath(), testPath(), testPath(), testPath()); + testPath(), testPath(), strPtr(strNewFmt("%s/test.ini", testPath())), + strPtr(strNewFmt("%s/test.ini.copy", testPath()))); //-------------------------------------------------------------------------------------------------------------------------- content = strNew diff --git a/test/src/module/info/infoBackupTest.c b/test/src/module/info/infoBackupTest.c index 95c0a93828..b0ba2bd4ba 100644 --- a/test/src/module/info/infoBackupTest.c +++ b/test/src/module/info/infoBackupTest.c @@ -1,6 +1,8 @@ /*********************************************************************************************************************************** Test Backup Info Handler ***********************************************************************************************************************************/ +#include "storage/storage.intern.h" + #include "common/harnessInfo.h" /*********************************************************************************************************************************** @@ -24,11 +26,12 @@ testRun(void) TEST_ERROR_FMT( infoBackupNew(storageLocal(), fileName, false, cipherTypeNone, NULL), FileMissingError, "unable to load info file '%s/test.ini' or '%s/test.ini.copy':\n" - "FileMissingError: unable to open '%s/test.ini' for read: [2] No such file or directory\n" - "FileMissingError: unable to open '%s/test.ini.copy' for read: [2] No such file or directory\n" + "FileMissingError: " STORAGE_ERROR_READ_MISSING "\n" + "FileMissingError: " STORAGE_ERROR_READ_MISSING "\n" "HINT: backup.info cannot be opened and is required to perform a backup.\n" "HINT: has a stanza-create been performed?", - testPath(), testPath(), testPath(), testPath()); + testPath(), testPath(), strPtr(strNewFmt("%s/test.ini", testPath())), + strPtr(strNewFmt("%s/test.ini.copy", testPath()))); // File exists, ignoreMissing=false, no backup:current section //-------------------------------------------------------------------------------------------------------------------------- diff --git a/test/src/module/info/infoTest.c b/test/src/module/info/infoTest.c index 5f3a50430a..09e1ec02eb 100644 --- a/test/src/module/info/infoTest.c +++ b/test/src/module/info/infoTest.c @@ -41,14 +41,13 @@ testRun(void) // Info files missing and at least one is required //-------------------------------------------------------------------------------------------------------------------------- - TEST_ERROR( + TEST_ERROR_FMT( infoNewLoad(storageLocal(), fileName, cipherTypeNone, NULL, NULL), FileMissingError, - strPtr( - strNewFmt( - "unable to load info file '%s/test.ini' or '%s/test.ini.copy':\n" - "FileMissingError: unable to open '%s/test.ini' for read: [2] No such file or directory\n" - "FileMissingError: unable to open '%s/test.ini.copy' for read: [2] No such file or directory", - testPath(), testPath(), testPath(), testPath()))); + "unable to load info file '%s/test.ini' or '%s/test.ini.copy':\n" + "FileMissingError: " STORAGE_ERROR_READ_MISSING "\n" + "FileMissingError: " STORAGE_ERROR_READ_MISSING, + testPath(), testPath(), strPtr(strNewFmt("%s/test.ini", testPath())), + strPtr(strNewFmt("%s/test.ini.copy", testPath()))); // Only copy exists and one is required //-------------------------------------------------------------------------------------------------------------------------- @@ -122,14 +121,12 @@ testRun(void) TEST_RESULT_VOID( storagePutNP(storageNewWriteNP(storageLocalWrite(), fileName), BUFSTR(content)), "put invalid br format to file"); - TEST_ERROR( + TEST_ERROR_FMT( infoNewLoad(storageLocal(), fileName, cipherTypeNone, NULL, NULL), FormatError, - strPtr( - strNewFmt( - "unable to load info file '%s/test.ini' or '%s/test.ini.copy':\n" - "FormatError: invalid format in '%s/test.ini', expected 5 but found 4\n" - "FileMissingError: unable to open '%s/test.ini.copy' for read: [2] No such file or directory", - testPath(), testPath(), testPath(), testPath()))); + "unable to load info file '%s/test.ini' or '%s/test.ini.copy':\n" + "FormatError: invalid format in '%s/test.ini', expected 5 but found 4\n" + "FileMissingError: " STORAGE_ERROR_READ_MISSING, + testPath(), testPath(), testPath(), strPtr(strNewFmt("%s/test.ini.copy", testPath()))); content = strNew ( @@ -226,15 +223,13 @@ testRun(void) // Encryption error //-------------------------------------------------------------------------------------------------------------------------- storageRemoveNP(storageLocalWrite(), fileName); - TEST_ERROR( + TEST_ERROR_FMT( infoNewLoad(storageLocal(), fileName, cipherTypeAes256Cbc, strNew("12345678"), NULL), CryptoError, - strPtr( - strNewFmt( - "unable to load info file '%s/test.ini' or '%s/test.ini.copy':\n" - "FileMissingError: unable to open '%s/test.ini' for read: [2] No such file or directory\n" - "CryptoError: '%s/test.ini.copy' cipher header invalid\n" - "HINT: Is or was the repo encrypted?", - testPath(), testPath(), testPath(), testPath()))); + "unable to load info file '%s/test.ini' or '%s/test.ini.copy':\n" + "FileMissingError: " STORAGE_ERROR_READ_MISSING "\n" + "CryptoError: '%s/test.ini.copy' cipher header invalid\n" + "HINT: Is or was the repo encrypted?", + testPath(), testPath(), strPtr(strNewFmt("%s/test.ini", testPath())), testPath()); storageRemoveNP(storageLocalWrite(), fileNameCopy); diff --git a/test/src/module/storage/posixTest.c b/test/src/module/storage/posixTest.c index 4a8206f7f0..e3bf983b3c 100644 --- a/test/src/module/storage/posixTest.c +++ b/test/src/module/storage/posixTest.c @@ -78,53 +78,6 @@ testRun(void) // Write file for testing if storage is read-only String *writeFile = strNewFmt("%s/writefile", testPath()); - // ***************************************************************************************************************************** - if (testBegin("storagePosixFile*()")) - { - TEST_CREATE_NOPERM(); - - TEST_ERROR_FMT( - storagePosixFileOpen(pathNoPerm, O_RDONLY, 0, false, false, "test"), PathOpenError, - "unable to open '%s' for test: [13] Permission denied", strPtr(pathNoPerm)); - - // ------------------------------------------------------------------------------------------------------------------------- - String *fileName = strNewFmt("%s/test.file", testPath()); - - TEST_ERROR_FMT( - storagePosixFileOpen(fileName, O_RDONLY, 0, false, true, "read"), FileMissingError, - "unable to open '%s' for read: [2] No such file or directory", strPtr(fileName)); - - TEST_RESULT_INT(storagePosixFileOpen(fileName, O_RDONLY, 0, true, true, "read"), -1, "missing file ignored"); - - // ------------------------------------------------------------------------------------------------------------------------- - int handle = -1; - - TEST_RESULT_INT(system(strPtr(strNewFmt("touch %s", strPtr(fileName)))), 0, "create read file"); - TEST_ASSIGN(handle, storagePosixFileOpen(fileName, O_RDONLY, 0, false, true, "read"), "open read file"); - - // ------------------------------------------------------------------------------------------------------------------------- - TEST_ERROR_FMT( - storagePosixFileSync(-99, fileName, false, false), PathSyncError, - "unable to sync '%s': [9] Bad file descriptor", strPtr(fileName)); - TEST_ERROR_FMT( - storagePosixFileSync(-99, fileName, true, true), FileSyncError, - "unable to sync '%s': [9] Bad file descriptor", strPtr(fileName)); - - TEST_RESULT_VOID(storagePosixFileSync(handle, fileName, true, false), "sync file"); - - // ------------------------------------------------------------------------------------------------------------------------- - TEST_ERROR_FMT( - storagePosixFileClose(-99, fileName, true), FileCloseError, - "unable to close '%s': [9] Bad file descriptor", strPtr(fileName)); - TEST_ERROR_FMT( - storagePosixFileClose(-99, fileName, false), PathCloseError, - "unable to close '%s': [9] Bad file descriptor", strPtr(fileName)); - - TEST_RESULT_VOID(storagePosixFileClose(handle, fileName, true), "close file"); - - TEST_RESULT_INT(system(strPtr(strNewFmt("rm %s", strPtr(fileName)))), 0, "remove read file"); - } - // ***************************************************************************************************************************** if (testBegin("storageNew() and storageFree()")) { @@ -207,15 +160,15 @@ testRun(void) TEST_CREATE_NOPERM(); TEST_ERROR_FMT( - storageInfoNP(storageTest, fileNoPerm), FileOpenError, - "unable to get info for '%s': [13] Permission denied", strPtr(fileNoPerm)); + storageInfoNP(storageTest, fileNoPerm), FileOpenError, STORAGE_ERROR_INFO ": [13] Permission denied", + strPtr(fileNoPerm)); // ------------------------------------------------------------------------------------------------------------------------- String *fileName = strNewFmt("%s/fileinfo", testPath()); TEST_ERROR_FMT( - storageInfoNP(storageTest, fileName), FileOpenError, - "unable to get info for '%s': [2] No such file or directory", strPtr(fileName)); + storageInfoNP(storageTest, fileName), FileOpenError, STORAGE_ERROR_INFO_MISSING ": [2] No such file or directory", + strPtr(fileName)); // ------------------------------------------------------------------------------------------------------------------------- StorageInfo info = {0}; @@ -302,19 +255,19 @@ testRun(void) // ------------------------------------------------------------------------------------------------------------------------- TEST_ERROR_FMT( storageInfoListP(storageTest, strNew(BOGUS_STR), (StorageInfoListCallback)1, NULL, .errorOnMissing = true), - PathOpenError, "unable to open path '%s/BOGUS' for read: [2] No such file or directory", testPath()); + PathOpenError, STORAGE_ERROR_LIST_INFO_MISSING, strPtr(strNewFmt("%s/BOGUS", testPath()))); TEST_RESULT_BOOL( storageInfoListNP(storageTest, strNew(BOGUS_STR), (StorageInfoListCallback)1, NULL), false, "ignore missing dir"); TEST_ERROR_FMT( storageInfoListNP(storageTest, pathNoPerm, (StorageInfoListCallback)1, NULL), PathOpenError, - "unable to open path '%s' for read: [13] Permission denied", strPtr(pathNoPerm)); + STORAGE_ERROR_LIST_INFO ": [13] Permission denied", strPtr(pathNoPerm)); // Should still error even when ignore missing TEST_ERROR_FMT( storageInfoListNP(storageTest, pathNoPerm, (StorageInfoListCallback)1, NULL), PathOpenError, - "unable to open path '%s' for read: [13] Permission denied", strPtr(pathNoPerm)); + STORAGE_ERROR_LIST_INFO ": [13] Permission denied", strPtr(pathNoPerm)); // ------------------------------------------------------------------------------------------------------------------------- testStorageInfoListSize = 0; @@ -341,8 +294,8 @@ testRun(void) // ------------------------------------------------------------------------------------------------------------------------- TEST_ERROR_FMT( - storageListP(storageTest, strNew(BOGUS_STR), .errorOnMissing = true), PathOpenError, - "unable to open path '%s/BOGUS' for read: [2] No such file or directory", testPath()); + storageListP(storageTest, strNew(BOGUS_STR), .errorOnMissing = true), PathOpenError, STORAGE_ERROR_LIST_MISSING, + strPtr(strNewFmt("%s/BOGUS", testPath()))); TEST_RESULT_PTR(storageListP(storageTest, strNew(BOGUS_STR), .nullOnMissing = true), NULL, "null for missing dir"); TEST_RESULT_UINT(strLstSize(storageListNP(storageTest, strNew(BOGUS_STR))), 0, "empty list for missing dir"); @@ -350,12 +303,12 @@ testRun(void) // ------------------------------------------------------------------------------------------------------------------------- TEST_ERROR_FMT( storageListNP(storageTest, pathNoPerm), PathOpenError, - "unable to open path '%s' for read: [13] Permission denied", strPtr(pathNoPerm)); + STORAGE_ERROR_LIST ": [13] Permission denied", strPtr(pathNoPerm)); // Should still error even when ignore missing TEST_ERROR_FMT( storageListNP(storageTest, pathNoPerm), PathOpenError, - "unable to open path '%s' for read: [13] Permission denied", strPtr(pathNoPerm)); + STORAGE_ERROR_LIST ": [13] Permission denied", strPtr(pathNoPerm)); // ------------------------------------------------------------------------------------------------------------------------- TEST_RESULT_VOID( @@ -379,9 +332,7 @@ testRun(void) StorageRead *source = storageNewReadNP(storageTest, sourceFile); StorageWrite *destination = storageNewWriteNP(storageTest, destinationFile); - TEST_ERROR_FMT( - storageCopyNP(source, destination), FileMissingError, - "unable to open '%s' for read: [2] No such file or directory", strPtr(sourceFile)); + TEST_ERROR_FMT(storageCopyNP(source, destination), FileMissingError, STORAGE_ERROR_READ_MISSING, strPtr(sourceFile)); // ------------------------------------------------------------------------------------------------------------------------- source = storageNewReadP(storageTest, sourceFile, .ignoreMissing = true); @@ -558,7 +509,7 @@ testRun(void) TEST_ERROR_FMT( storagePathRemoveP(storageTest, pathRemove1, .errorOnMissing = true), PathRemoveError, - "unable to remove path '%s': [2] No such file or directory", strPtr(pathRemove1)); + STORAGE_ERROR_PATH_REMOVE_MISSING, strPtr(pathRemove1)); TEST_RESULT_VOID(storagePathRemoveP(storageTest, pathRemove1, .recurse = true), "ignore missing path"); // ------------------------------------------------------------------------------------------------------------------------- @@ -567,18 +518,18 @@ testRun(void) TEST_RESULT_INT(system(strPtr(strNewFmt("sudo mkdir -p -m 700 %s", strPtr(pathRemove2)))), 0, "create noperm paths"); TEST_ERROR_FMT( - storagePathRemoveNP(storageTest, pathRemove2), PathRemoveError, - "unable to remove path '%s': [13] Permission denied", strPtr(pathRemove2)); + storagePathRemoveNP(storageTest, pathRemove2), PathRemoveError, STORAGE_ERROR_PATH_REMOVE ": [13] Permission denied", + strPtr(pathRemove2)); TEST_ERROR_FMT( storagePathRemoveP(storageTest, pathRemove2, .recurse = true), PathOpenError, - "unable to open path '%s' for read: [13] Permission denied", strPtr(pathRemove2)); + STORAGE_ERROR_LIST ": [13] Permission denied", strPtr(pathRemove2)); // ------------------------------------------------------------------------------------------------------------------------- TEST_RESULT_INT(system(strPtr(strNewFmt("sudo chmod 777 %s", strPtr(pathRemove1)))), 0, "top path can be removed"); TEST_ERROR_FMT( storagePathRemoveP(storageTest, pathRemove2, .recurse = true), PathOpenError, - "unable to open path '%s' for read: [13] Permission denied", strPtr(pathRemove2)); + STORAGE_ERROR_LIST ": [13] Permission denied", strPtr(pathRemove2)); // ------------------------------------------------------------------------------------------------------------------------- String *fileRemove = strNewFmt("%s/remove.txt", strPtr(pathRemove2)); @@ -591,7 +542,7 @@ testRun(void) TEST_ERROR_FMT( storagePathRemoveP(storageTest, pathRemove1, .recurse = true), PathRemoveError, - "unable to remove path/file '%s': [13] Permission denied", strPtr(fileRemove)); + STORAGE_ERROR_PATH_REMOVE_FILE ": [13] Permission denied", strPtr(fileRemove)); // ------------------------------------------------------------------------------------------------------------------------- TEST_RESULT_INT(system(strPtr(strNewFmt("sudo chmod 777 %s", strPtr(pathRemove2)))), 0, "bottom path can be removed"); @@ -616,17 +567,20 @@ testRun(void) TEST_CREATE_NOPERM(); TEST_ERROR_FMT( - storagePathSyncNP(storageTest, fileNoPerm), PathOpenError, - "unable to open '%s' for sync: [13] Permission denied", strPtr(fileNoPerm)); + storagePathSyncNP(storageTest, fileNoPerm), PathOpenError, STORAGE_ERROR_PATH_SYNC_OPEN ": [13] Permission denied", + strPtr(fileNoPerm)); // ------------------------------------------------------------------------------------------------------------------------- String *pathName = strNewFmt("%s/testpath", testPath()); TEST_ERROR_FMT( - storagePathSyncNP(storageTest, pathName), PathMissingError, - "unable to open '%s' for sync: [2] No such file or directory", strPtr(pathName)); + storagePathSyncNP(storageTest, pathName), PathMissingError, STORAGE_ERROR_PATH_SYNC_MISSING, strPtr(pathName)); - TEST_RESULT_VOID(storagePathSyncP(storageTest, pathName, .ignoreMissing = true), "ignore missing path"); + // ------------------------------------------------------------------------------------------------------------------------- + TEST_ERROR_FMT( + storagePathSyncNP( + storagePosixNew(strNew("/"), STORAGE_MODE_FILE_DEFAULT, STORAGE_MODE_PATH_DEFAULT, true, NULL), strNew("/proc")), + PathSyncError, STORAGE_ERROR_PATH_SYNC ": [22] Invalid argument", "/proc"); // ------------------------------------------------------------------------------------------------------------------------- TEST_RESULT_VOID(storagePathCreateNP(storageTest, pathName), "create path to sync"); @@ -640,9 +594,7 @@ testRun(void) String *fileName = strNewFmt("%s/readtest.txt", testPath()); TEST_ASSIGN(file, storageNewReadNP(storageTest, fileName), "new read file (defaults)"); - TEST_ERROR_FMT( - ioReadOpen(storageReadIo(file)), FileMissingError, - "unable to open '%s' for read: [2] No such file or directory", strPtr(fileName)); + TEST_ERROR_FMT(ioReadOpen(storageReadIo(file)), FileMissingError, STORAGE_ERROR_READ_MISSING, strPtr(fileName)); // ------------------------------------------------------------------------------------------------------------------------- TEST_RESULT_INT(system(strPtr(strNewFmt("touch %s", strPtr(fileName)))), 0, "create read file"); @@ -668,8 +620,8 @@ testRun(void) TEST_ASSIGN(file, storageNewWriteP(storageTest, fileNoPerm, .noAtomic = true), "new write file (defaults)"); TEST_ERROR_FMT( - ioWriteOpen(storageWriteIo(file)), FileOpenError, - "unable to open '%s' for write: [13] Permission denied", strPtr(fileNoPerm)); + ioWriteOpen(storageWriteIo(file)), FileOpenError, STORAGE_ERROR_WRITE_OPEN ": [13] Permission denied", + strPtr(fileNoPerm)); TEST_ASSIGN(file, storageNewWriteP(storageTest, fileName, .user = strNew("bogus")), "new write file (bogus user)"); TEST_ERROR(ioWriteOpen(storageWriteIo(file)), UserMissingError, "unable to find user 'bogus'"); @@ -815,16 +767,13 @@ testRun(void) // ------------------------------------------------------------------------------------------------------------------------- TEST_ASSIGN(file, storageNewReadNP(storageTest, fileNoPerm), "new no perm read file"); TEST_ERROR_FMT( - ioReadOpen(storageReadIo(file)), FileOpenError, - "unable to open '%s' for read: [13] Permission denied", strPtr(fileNoPerm)); + ioReadOpen(storageReadIo(file)), FileOpenError, STORAGE_ERROR_READ_OPEN ": [13] Permission denied", strPtr(fileNoPerm)); // ------------------------------------------------------------------------------------------------------------------------- String *fileName = strNewFmt("%s/test.file", testPath()); TEST_ASSIGN(file, storageNewReadNP(storageTest, fileName), "new missing read file"); - TEST_ERROR_FMT( - ioReadOpen(storageReadIo(file)), FileMissingError, - "unable to open '%s' for read: [2] No such file or directory", strPtr(fileName)); + TEST_ERROR_FMT(ioReadOpen(storageReadIo(file)), FileMissingError, STORAGE_ERROR_READ_MISSING, strPtr(fileName)); // ------------------------------------------------------------------------------------------------------------------------- TEST_ASSIGN(file, storageNewReadP(storageTest, fileName, .ignoreMissing = true), "new missing read file"); @@ -924,16 +873,14 @@ testRun(void) // ------------------------------------------------------------------------------------------------------------------------- TEST_ASSIGN(file, storageNewWriteP(storageTest, fileNoPerm, .noAtomic = true), "new write file"); TEST_ERROR_FMT( - ioWriteOpen(storageWriteIo(file)), FileOpenError, - "unable to open '%s' for write: [13] Permission denied", strPtr(fileNoPerm)); + ioWriteOpen(storageWriteIo(file)), FileOpenError, STORAGE_ERROR_WRITE_OPEN ": [13] Permission denied", + strPtr(fileNoPerm)); // ------------------------------------------------------------------------------------------------------------------------- String *fileName = strNewFmt("%s/sub1/test.file", testPath()); TEST_ASSIGN(file, storageNewWriteP(storageTest, fileName, .noCreatePath = true, .noAtomic = true), "new write file"); - TEST_ERROR_FMT( - ioWriteOpen(storageWriteIo(file)), FileMissingError, - "unable to open '%s' for write: [2] No such file or directory", strPtr(fileName)); + TEST_ERROR_FMT(ioWriteOpen(storageWriteIo(file)), FileMissingError, STORAGE_ERROR_WRITE_MISSING, strPtr(fileName)); // ------------------------------------------------------------------------------------------------------------------------- String *fileTmp = strNewFmt("%s.pgbackrest.tmp", strPtr(fileName)); @@ -952,15 +899,15 @@ testRun(void) storageWritePosix(storageWriteDriver(file), buffer), FileWriteError, "unable to write '%s.pgbackrest.tmp': [9] Bad file descriptor", strPtr(fileName)); TEST_ERROR_FMT( - storageWritePosixClose(storageWriteDriver(file)), FileSyncError, - "unable to sync '%s.pgbackrest.tmp': [9] Bad file descriptor", strPtr(fileName)); + storageWritePosixClose(storageWriteDriver(file)), FileSyncError, STORAGE_ERROR_WRITE_SYNC ": [9] Bad file descriptor", + strPtr(fileTmp)); // Disable file sync so close() can be reached ((StorageWritePosix *)file->driver)->interface.syncFile = false; TEST_ERROR_FMT( - storageWritePosixClose(storageWriteDriver(file)), FileCloseError, - "unable to close '%s.pgbackrest.tmp': [9] Bad file descriptor", strPtr(fileName)); + storageWritePosixClose(storageWriteDriver(file)), FileCloseError, STORAGE_ERROR_WRITE_CLOSE ": [9] Bad file descriptor", + strPtr(fileTmp)); // Set file handle to -1 so the close on free with not fail ((StorageWritePosix *)file->driver)->handle = -1; diff --git a/test/src/module/storage/remoteTest.c b/test/src/module/storage/remoteTest.c index 3a5aa61c80..23e75f4543 100644 --- a/test/src/module/storage/remoteTest.c +++ b/test/src/module/storage/remoteTest.c @@ -41,6 +41,28 @@ testRun(void) bufUsedSet(serverWrite, 0); + // ***************************************************************************************************************************** + if (testBegin("storageNew()")) + { + Storage *storageRemote = NULL; + TEST_ASSIGN(storageRemote, storageRepoGet(strNew(STORAGE_TYPE_POSIX), false), "get remote repo storage"); + TEST_RESULT_UINT(storageInterface(storageRemote).feature, storageInterface(storageTest).feature, " check features"); + + // Check protocol function directly + // ------------------------------------------------------------------------------------------------------------------------- + TEST_RESULT_BOOL( + storageRemoteProtocol(PROTOCOL_COMMAND_STORAGE_FEATURE_STR, varLstNew(), server), true, "protocol feature"); + TEST_RESULT_STR( + strPtr(strNewBuf(serverWrite)), strPtr(strNewFmt("{\"out\":%" PRIu64 "}\n", storageInterface(storageTest).feature)), + "check result"); + + bufUsedSet(serverWrite, 0); + + // Check invalid protocol function + // ------------------------------------------------------------------------------------------------------------------------- + TEST_RESULT_BOOL(storageRemoteProtocol(strNew(BOGUS_STR), varLstNew(), server), false, "invalid function"); + } + // ***************************************************************************************************************************** if (testBegin("storageExists()")) { @@ -88,17 +110,12 @@ testRun(void) // ------------------------------------------------------------------------------------------------------------------------- VariantList *paramList = varLstNew(); varLstAdd(paramList, NULL); - varLstAdd(paramList, varNewBool(false)); varLstAdd(paramList, varNewStr(strNew("^testy$"))); TEST_RESULT_BOOL(storageRemoteProtocol(PROTOCOL_COMMAND_STORAGE_LIST_STR, paramList, server), true, "protocol list"); TEST_RESULT_STR(strPtr(strNewBuf(serverWrite)), "{\"out\":[\"testy\"]}\n", "check result"); bufUsedSet(serverWrite, 0); - - // Check invalid protocol function - // ------------------------------------------------------------------------------------------------------------------------- - TEST_RESULT_BOOL(storageRemoteProtocol(strNew(BOGUS_STR), paramList, server), false, "invalid function"); } // ***************************************************************************************************************************** @@ -115,13 +132,10 @@ testRun(void) bufUsedSet(contentBuf, bufSize(contentBuf)); - TEST_ERROR( + TEST_ERROR_FMT( strPtr(strNewBuf(storageGetNP(storageNewReadNP(storageRemote, strNew("test.txt"))))), FileMissingError, - strPtr( - strNewFmt( - "raised from remote-0 protocol on 'localhost': unable to open '%s/repo/test.txt' for read:" - " [2] No such file or directory", - testPath()))); + "raised from remote-0 protocol on 'localhost': " STORAGE_ERROR_READ_MISSING, + strPtr(strNewFmt("%s/repo/test.txt", testPath()))); storagePutNP(storageNewWriteNP(storageTest, strNew("repo/test.txt")), contentBuf); @@ -403,23 +417,13 @@ testRun(void) // ------------------------------------------------------------------------------------------------------------------------- VariantList *paramList = varLstNew(); varLstAdd(paramList, varNewStr(path)); // path - varLstAdd(paramList, varNewBool(true)); // errorOnMissing - varLstAdd(paramList, varNewBool(false)); // recurse - - TEST_ERROR_FMT( - storageRemoteProtocol(PROTOCOL_COMMAND_STORAGE_PATH_REMOVE_STR, paramList, server), PathRemoveError, - "raised from remote-0 protocol on 'localhost': unable to remove path '%s/repo/testpath': " - "[2] No such file or directory", testPath()); - - paramList = varLstNew(); - varLstAdd(paramList, varNewStr(path)); // path - varLstAdd(paramList, varNewBool(false)); // errorOnMissing - varLstAdd(paramList, varNewBool(true)); // recurse + varLstAdd(paramList, varNewBool(true)); // recurse TEST_RESULT_BOOL( storageRemoteProtocol(PROTOCOL_COMMAND_STORAGE_PATH_REMOVE_STR, paramList, server), true, - "protocol path remove - no error on missing"); - TEST_RESULT_STR(strPtr(strNewBuf(serverWrite)), "{}\n", "check result"); + " protocol path remove missing"); + TEST_RESULT_STR(strPtr(strNewBuf(serverWrite)), "{\"out\":false}\n", " check result"); + bufUsedSet(serverWrite, 0); // Write the path and file to the repo and test the protocol @@ -430,7 +434,8 @@ testRun(void) storageRemoteProtocol(PROTOCOL_COMMAND_STORAGE_PATH_REMOVE_STR, paramList, server), true, " protocol path recurse remove"); TEST_RESULT_BOOL(storagePathExistsNP(storageTest, strNewFmt("repo/%s", strPtr(path))), false, " recurse path removed"); - TEST_RESULT_STR(strPtr(strNewBuf(serverWrite)), "{}\n", " check result"); + TEST_RESULT_STR(strPtr(strNewBuf(serverWrite)), "{\"out\":true}\n", " check result"); + bufUsedSet(serverWrite, 0); } @@ -510,20 +515,10 @@ testRun(void) paramList = varLstNew(); varLstAdd(paramList, varNewStr(strNew("anewpath"))); - varLstAdd(paramList, varNewBool(false)); // ignoreMissing TEST_ERROR_FMT( storageRemoteProtocol(PROTOCOL_COMMAND_STORAGE_PATH_SYNC_STR, paramList, server), PathMissingError, - "raised from remote-0 protocol on 'localhost': unable to open '%s/repo/anewpath' for sync: " - "[2] No such file or directory", testPath()); - - paramList = varLstNew(); - varLstAdd(paramList, varNewStr(strNew("anewpath"))); - varLstAdd(paramList, varNewBool(true)); // ignoreMissing - TEST_RESULT_BOOL( - storageRemoteProtocol(PROTOCOL_COMMAND_STORAGE_PATH_SYNC_STR, paramList, server), true, - "protocol path sync - ignore missing"); - TEST_RESULT_STR(strPtr(strNewBuf(serverWrite)), "{}\n", " check result"); - bufUsedSet(serverWrite, 0); + "raised from remote-0 protocol on 'localhost': " STORAGE_ERROR_PATH_SYNC_MISSING, + strPtr(strNewFmt("%s/repo/anewpath", testPath()))); } // ***************************************************************************************************************************** diff --git a/test/src/module/storage/s3Test.c b/test/src/module/storage/s3Test.c index 44a31ebd31..c9c18abd2b 100644 --- a/test/src/module/storage/s3Test.c +++ b/test/src/module/storage/s3Test.c @@ -739,7 +739,8 @@ testRun(void) // storageDriverList() // ------------------------------------------------------------------------------------------------------------------------- TEST_ERROR( - storageListP(s3, strNew("/"), .errorOnMissing = true), AssertError, "assertion '!errorOnMissing' failed"); + storageListP(s3, strNew("/"), .errorOnMissing = true), AssertError, + "assertion '!param.errorOnMissing || storageFeature(this, storageFeaturePath)' failed"); TEST_ERROR(storageListNP(s3, strNew("/")), ProtocolError, "S3 request failed with 344: Another bad status\n" "*** URI/Query ***:\n" @@ -764,13 +765,15 @@ testRun(void) // storageDriverPathRemove() // ------------------------------------------------------------------------------------------------------------------------- - TEST_RESULT_VOID(storagePathRemoveNP(s3, strNew("/")), "do nothing when no recurse"); + TEST_ERROR( + storagePathRemoveNP(s3, strNew("/")), AssertError, + "assertion 'param.recurse || storageFeature(this, storageFeaturePath)' failed"); TEST_RESULT_VOID(storagePathRemoveP(s3, strNew("/"), .recurse = true), "remove root path"); TEST_RESULT_VOID(storagePathRemoveP(s3, strNew("/path"), .recurse = true), "nothing to do in empty subpath"); TEST_RESULT_VOID(storagePathRemoveP(s3, strNew("/path/to"), .recurse = true), "delete with continuation"); TEST_ERROR( storagePathRemoveP(s3, strNew("/path"), .recurse = true), FileRemoveError, - "unable to remove 'sample2.txt': [AccessDenied] Access Denied"); + "unable to remove file 'sample2.txt': [AccessDenied] Access Denied"); // storageDriverRemove() // -------------------------------------------------------------------------------------------------------------------------