From 9367cc461cda63698caa04ddb262405eef05890d Mon Sep 17 00:00:00 2001 From: David Steele Date: Wed, 27 Feb 2019 22:34:21 +0200 Subject: [PATCH] Migrate local command to C. The C local is only used for C commands in the main process. Some tweaking of the existing protocolGet() command was required. Originally the idea was to share the function for local and remote requests but the differences (as in Perl) were too great to make that practical. --- build/lib/pgBackRestBuild/Config/Data.pm | 1 + doc/xml/release.xml | 2 +- src/Makefile | 6 +- src/command/local/local.c | 35 +++ src/command/local/local.h | 12 + src/config/define.auto.c | 1 + src/main.c | 14 +- src/protocol/client.h | 9 - src/protocol/helper.c | 224 +++++++++++++++--- src/protocol/helper.h | 22 +- src/storage/driver/remote/storage.c | 6 +- src/storage/driver/remote/storage.h | 3 +- src/storage/helper.c | 3 +- test/define.yaml | 7 + .../Command/CommandArchiveGetPerlTest.pm | 5 +- test/src/module/command/localTest.c | 64 +++++ test/src/module/protocol/protocolTest.c | 42 ++-- 17 files changed, 383 insertions(+), 73 deletions(-) create mode 100644 src/command/local/local.c create mode 100644 src/command/local/local.h create mode 100644 test/src/module/command/localTest.c diff --git a/build/lib/pgBackRestBuild/Config/Data.pm b/build/lib/pgBackRestBuild/Config/Data.pm index c348c118b..0106fbc82 100644 --- a/build/lib/pgBackRestBuild/Config/Data.pm +++ b/build/lib/pgBackRestBuild/Config/Data.pm @@ -1825,6 +1825,7 @@ my %hConfigDefine = }, }, &CFGCMD_ARCHIVE_GET_ASYNC => {}, + &CFGCMD_LOCAL => {}, &CFGCMD_ARCHIVE_PUSH => { &CFGDEF_DEPEND => diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 21c1098ab..4e494c271 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -52,7 +52,7 @@ -

Migrate remote command to C.

+

Migrate local and remote commands to C.

 diff --git a/src/Makefile b/src/Makefile index 8328fddb4..fb05163af 100644 --- a/src/Makefile +++ b/src/Makefile @@ -65,6 +65,7 @@ SRCS = \ command/info/info.c \ command/command.c \ command/control/control.c \ + command/local/local.c \ command/remote/remote.c \ common/debug.c \ common/encode.c \ @@ -203,6 +204,9 @@ command/help/help.o: command/help/help.c common/assert.h common/debug.h common/e command/info/info.o: command/info/info.c command/archive/common.h command/info/info.h common/assert.h common/debug.h common/error.auto.h common/error.h common/ini.h common/io/filter/filter.h common/io/filter/group.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/json.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 crypto/crypto.h crypto/hash.h info/info.h info/infoArchive.h info/infoBackup.h info/infoPg.h perl/exec.h postgres/interface.h storage/fileRead.h storage/fileWrite.h storage/helper.h storage/info.h storage/storage.h $(CC) $(CFLAGS) -c command/info/info.c -o command/info/info.o +command/local/local.o: command/local/local.c 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 + $(CC) $(CFLAGS) -c command/local/local.c -o command/local/local.o + command/remote/remote.o: command/remote/remote.c 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/driver/remote/protocol.h $(CC) $(CFLAGS) -c command/remote/remote.c -o command/remote/remote.o @@ -380,7 +384,7 @@ info/infoManifest.o: info/infoManifest.c common/error.auto.h common/error.h comm info/infoPg.o: info/infoPg.c common/assert.h common/debug.h common/error.auto.h common/error.h common/ini.h common/io/filter/filter.h common/io/filter/group.h common/io/read.h common/io/write.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/json.h common/type/keyValue.h common/type/list.h common/type/string.h common/type/stringList.h common/type/variant.h common/type/variantList.h crypto/crypto.h crypto/hash.h info/info.h info/infoPg.h postgres/interface.h postgres/version.h storage/fileRead.h storage/fileWrite.h storage/helper.h storage/info.h storage/storage.h $(CC) $(CFLAGS) -c info/infoPg.c -o info/infoPg.o -main.o: main.c command/archive/get/get.h command/archive/push/push.h command/command.h command/help/help.h command/info/info.h command/remote/remote.h common/assert.h common/debug.h common/error.auto.h common/error.h common/exit.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/load.h perl/exec.h postgres/interface.h version.h +main.o: main.c command/archive/get/get.h command/archive/push/push.h command/command.h command/help/help.h command/info/info.h command/local/local.h command/remote/remote.h common/assert.h common/debug.h common/error.auto.h common/error.h common/exit.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/load.h perl/exec.h postgres/interface.h version.h $(CC) $(CFLAGS) -c main.c -o main.o perl/config.o: perl/config.c common/assert.h common/debug.h common/error.auto.h common/error.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/json.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 diff --git a/src/command/local/local.c b/src/command/local/local.c new file mode 100644 index 000000000..6b3558eda --- /dev/null +++ b/src/command/local/local.c @@ -0,0 +1,35 @@ +/*********************************************************************************************************************************** +Local Command +***********************************************************************************************************************************/ +#include "common/debug.h" +#include "common/io/handleRead.h" +#include "common/io/handleWrite.h" +#include "common/log.h" +#include "config/config.h" +#include "config/protocol.h" +#include "protocol/helper.h" +#include "protocol/server.h" + +/*********************************************************************************************************************************** +Remote command +***********************************************************************************************************************************/ +void +cmdLocal(int handleRead, int handleWrite) +{ + FUNCTION_LOG_VOID(logLevelDebug); + + MEM_CONTEXT_TEMP_BEGIN() + { + String *name = strNewFmt(PROTOCOL_SERVICE_LOCAL "-%d", cfgOptionInt(cfgOptProcess)); + IoRead *read = ioHandleReadIo(ioHandleReadNew(name, handleRead, (TimeMSec)(cfgOptionDbl(cfgOptProtocolTimeout) * 1000))); + ioReadOpen(read); + IoWrite *write = ioHandleWriteIo(ioHandleWriteNew(name, handleWrite)); + ioWriteOpen(write); + + ProtocolServer *server = protocolServerNew(name, PROTOCOL_SERVICE_LOCAL_STR, read, write); + protocolServerProcess(server); + } + MEM_CONTEXT_TEMP_END(); + + FUNCTION_LOG_RETURN_VOID(); +} diff --git a/src/command/local/local.h b/src/command/local/local.h new file mode 100644 index 000000000..093699392 --- /dev/null +++ b/src/command/local/local.h @@ -0,0 +1,12 @@ +/*********************************************************************************************************************************** +Local Command +***********************************************************************************************************************************/ +#ifndef COMMAND_LOCAL_LOCAL_H +#define COMMAND_LOCAL_LOCAL_H + +/*********************************************************************************************************************************** +Functions +***********************************************************************************************************************************/ +void cmdLocal(int handleRead, int handleWrite); + +#endif diff --git a/src/config/define.auto.c b/src/config/define.auto.c index b7f9ee634..cb27f62cb 100644 --- a/src/config/define.auto.c +++ b/src/config/define.auto.c @@ -3877,6 +3877,7 @@ static ConfigDefineOptionData configDefineOptionData[] = CFGDEFDATA_OPTION_LIST CFGDEFDATA_OPTION_COMMAND(cfgDefCmdArchiveGet) CFGDEFDATA_OPTION_COMMAND(cfgDefCmdArchiveGetAsync) CFGDEFDATA_OPTION_COMMAND(cfgDefCmdArchivePush) + CFGDEFDATA_OPTION_COMMAND(cfgDefCmdLocal) ) CFGDEFDATA_OPTION_OPTIONAL_LIST diff --git a/src/main.c b/src/main.c index b63e993fc..c5a336454 100644 --- a/src/main.c +++ b/src/main.c @@ -7,9 +7,10 @@ Main #include "command/archive/get/get.h" #include "command/archive/push/push.h" +#include "command/command.h" #include "command/help/help.h" #include "command/info/info.h" -#include "command/command.h" +#include "command/local/local.h" #include "command/remote/remote.h" #include "common/debug.h" #include "common/error.h" @@ -62,11 +63,18 @@ main(int argListSize, const char *argList[]) fflush(stdout); } + // Local command. Currently only implements a subset. + // ------------------------------------------------------------------------------------------------------------------------- + else if (cfgCommand() == cfgCmdLocal && strEqZ(cfgOptionStr(cfgOptCommand), cfgCommandName(cfgCmdArchiveGet))) + { + cmdLocal(STDIN_FILENO, STDOUT_FILENO); + } + // Remote command. Currently only implements a subset. // ------------------------------------------------------------------------------------------------------------------------- else if (cfgCommand() == cfgCmdRemote && - (strEqZ(cfgOptionStr(cfgOptCommand), cfgCommandName(cfgCmdInfo)) || - strEqZ(cfgOptionStr(cfgOptCommand), cfgCommandName(cfgCmdArchiveGet)))) + (strEqZ(cfgOptionStr(cfgOptCommand), cfgCommandName(cfgCmdArchiveGet)) || + strEqZ(cfgOptionStr(cfgOptCommand), cfgCommandName(cfgCmdInfo)))) { cmdRemote(STDIN_FILENO, STDOUT_FILENO); } diff --git a/src/protocol/client.h b/src/protocol/client.h index 7507f0cc5..a0f5e08e7 100644 --- a/src/protocol/client.h +++ b/src/protocol/client.h @@ -9,15 +9,6 @@ Object type ***********************************************************************************************************************************/ typedef struct ProtocolClient ProtocolClient; -/*********************************************************************************************************************************** -Remote tyoe enum -***********************************************************************************************************************************/ -typedef enum -{ - remoteTypeRepo, - remoteTypeDb, -} RemoteType; - #include "common/io/read.h" #include "common/io/write.h" #include "protocol/command.h" diff --git a/src/protocol/helper.c b/src/protocol/helper.c index e98268baf..ad0b46ac7 100644 --- a/src/protocol/helper.c +++ b/src/protocol/helper.c @@ -13,19 +13,47 @@ Protocol Helper /*********************************************************************************************************************************** Constants ***********************************************************************************************************************************/ +STRING_EXTERN(PROTOCOL_SERVICE_LOCAL_STR, PROTOCOL_SERVICE_LOCAL); STRING_EXTERN(PROTOCOL_SERVICE_REMOTE_STR, PROTOCOL_SERVICE_REMOTE); /*********************************************************************************************************************************** Local variables ***********************************************************************************************************************************/ +typedef struct ProtocolHelperClient +{ + Exec *exec; // Executed client + ProtocolClient *client; // Protocol client +} ProtocolHelperClient; + static struct { MemContext *memContext; // Mem context for protocol helper - Exec *remoteExec; // Executed remote - ProtocolClient *remote; // Remote protocol client + unsigned int clientRemoteSize; // Remote clients + ProtocolHelperClient *clientRemote; + + unsigned int clientLocalSize; // Local clients + ProtocolHelperClient *clientLocal; } protocolHelper; +/*********************************************************************************************************************************** +Init local mem context and data structure +***********************************************************************************************************************************/ +static void +protocolHelperInit(void) +{ + // In the protocol helper has not been initialized + if (protocolHelper.memContext == NULL) + { + // Create a mem context to store protocol objects + MEM_CONTEXT_BEGIN(memContextTop()) + { + protocolHelper.memContext = memContextNew("ProtocolHelper"); + } + MEM_CONTEXT_END(); + } +} + /*********************************************************************************************************************************** Is the repository local? ***********************************************************************************************************************************/ @@ -37,18 +65,110 @@ repoIsLocal(void) } /*********************************************************************************************************************************** -Get the command line required for protocol execution +Get the command line required for local protocol execution ***********************************************************************************************************************************/ static StringList * -protocolParam(RemoteType remoteType, unsigned int remoteId) +protocolLocalParam(ProtocolStorageType protocolStorageType, unsigned int protocolId) { FUNCTION_LOG_BEGIN(logLevelDebug); - FUNCTION_LOG_PARAM(ENUM, remoteType); - FUNCTION_LOG_PARAM(UINT, remoteId); + FUNCTION_LOG_PARAM(ENUM, protocolStorageType); + FUNCTION_LOG_PARAM(UINT, protocolId); FUNCTION_LOG_END(); - ASSERT(remoteType == remoteTypeRepo); // ??? Hard-coded until the function supports db remotes - ASSERT(remoteId == 1); // ??? Hard-coded until the function supports db remotes + ASSERT(protocolStorageType == protocolStorageTypeRepo); // ??? Hard-coded until the function supports pg remotes + + StringList *result = NULL; + + MEM_CONTEXT_TEMP_BEGIN() + { + // Option replacements + KeyValue *optionReplace = kvNew(); + + // Add the command option + kvPut(optionReplace, varNewStr(strNew(cfgOptionName(cfgOptCommand))), varNewStr(strNew(cfgCommandName(cfgCommand())))); + + // Add the process id -- used when more than one process will be called + kvPut(optionReplace, varNewStr(strNew(cfgOptionName(cfgOptProcess))), varNewInt(0)); + + // Add the host id -- for now this is hard-coded to 1 + kvPut(optionReplace, varNewStr(strNew(cfgOptionName(cfgOptHostId))), varNewInt(1)); + + // Add the type + kvPut(optionReplace, varNewStr(strNew(cfgOptionName(cfgOptType))), varNewStr(strNew("backup"))); + + result = strLstMove(cfgExecParam(cfgCmdLocal, optionReplace), MEM_CONTEXT_OLD()); + } + MEM_CONTEXT_TEMP_END(); + + FUNCTION_LOG_RETURN(STRING_LIST, result); +} + +/*********************************************************************************************************************************** +Get the local protocol client +***********************************************************************************************************************************/ +ProtocolClient * +protocolLocalGet(ProtocolStorageType protocolStorageType, unsigned int protocolId) +{ + FUNCTION_LOG_BEGIN(logLevelDebug); + FUNCTION_LOG_PARAM(ENUM, protocolStorageType); + FUNCTION_LOG_PARAM(UINT, protocolId); + FUNCTION_LOG_END(); + + protocolHelperInit(); + + // Allocate the client cache + if (protocolHelper.clientLocalSize == 0) + { + MEM_CONTEXT_BEGIN(protocolHelper.memContext) + { + protocolHelper.clientLocalSize = (unsigned int)cfgOptionInt(cfgOptProcessMax); + protocolHelper.clientLocal = (ProtocolHelperClient *)memNew( + protocolHelper.clientLocalSize * sizeof(ProtocolHelperClient)); + } + MEM_CONTEXT_END(); + } + + ASSERT(protocolId <= protocolHelper.clientLocalSize); + + // Create protocol object + ProtocolHelperClient *protocolHelperClient = &protocolHelper.clientLocal[protocolId - 1]; + + if (protocolHelperClient->client == NULL) + { + MEM_CONTEXT_BEGIN(protocolHelper.memContext) + { + // Execute the protocol command + protocolHelperClient->exec = execNew( + cfgExe(), protocolLocalParam(protocolStorageType, protocolId), + strNewFmt(PROTOCOL_SERVICE_LOCAL "-%u process", protocolId), + (TimeMSec)(cfgOptionDbl(cfgOptProtocolTimeout) * 1000)); + execOpen(protocolHelperClient->exec); + + // Create protocol object + protocolHelperClient->client = protocolClientNew( + strNewFmt(PROTOCOL_SERVICE_LOCAL "-%u protocol", protocolId), + PROTOCOL_SERVICE_LOCAL_STR, execIoRead(protocolHelperClient->exec), execIoWrite(protocolHelperClient->exec)); + + protocolClientMove(protocolHelperClient->client, execMemContext(protocolHelperClient->exec)); + } + MEM_CONTEXT_END(); + } + + FUNCTION_LOG_RETURN(PROTOCOL_CLIENT, protocolHelperClient->client); +} + +/*********************************************************************************************************************************** +Get the command line required for remote protocol execution +***********************************************************************************************************************************/ +static StringList * +protocolRemoteParam(ProtocolStorageType protocolStorageType, unsigned int protocolId) +{ + FUNCTION_LOG_BEGIN(logLevelDebug); + FUNCTION_LOG_PARAM(ENUM, protocolStorageType); + FUNCTION_LOG_PARAM(UINT, protocolId); + FUNCTION_LOG_END(); + + ASSERT(protocolStorageType == protocolStorageTypeRepo); // ??? Hard-coded until the function supports pg remotes // Fixed parameters for ssh command StringList *result = strLstNew(); @@ -69,7 +189,7 @@ protocolParam(RemoteType remoteType, unsigned int remoteId) // Append user/host strLstAdd(result, strNewFmt("%s@%s", strPtr(cfgOptionStr(cfgOptRepoHostUser)), strPtr(cfgOptionStr(cfgOptRepoHost)))); - // Append pgbackrest command + // Option replacements KeyValue *optionReplace = kvNew(); // Replace config options with the host versions @@ -82,7 +202,7 @@ protocolParam(RemoteType remoteType, unsigned int remoteId) if (cfgOptionSource(cfgOptRepoHostConfigPath) != cfgSourceDefault) kvPut(optionReplace, varNewStr(strNew(cfgOptionName(cfgOptConfigPath))), cfgOption(cfgOptRepoHostConfigPath)); - // If this is the local or remote command then we need to add the command option + // Add the command option kvPut(optionReplace, varNewStr(strNew(cfgOptionName(cfgOptCommand))), varNewStr(strNew(cfgCommandName(cfgCommand())))); // Add the process id -- used when more than one process will be called @@ -104,42 +224,55 @@ protocolParam(RemoteType remoteType, unsigned int remoteId) } /*********************************************************************************************************************************** -Get the protocol client +Get the remote protocol client ***********************************************************************************************************************************/ ProtocolClient * -protocolGet(RemoteType remoteType, unsigned int remoteId) +protocolRemoteGet(ProtocolStorageType protocolStorageType, unsigned int protocolId) { FUNCTION_LOG_BEGIN(logLevelDebug); - FUNCTION_LOG_PARAM(ENUM, remoteType); - FUNCTION_LOG_PARAM(UINT, remoteId); + FUNCTION_LOG_PARAM(ENUM, protocolStorageType); + FUNCTION_LOG_PARAM(UINT, protocolId); FUNCTION_LOG_END(); - // Create a mem context to store protocol objects - if (protocolHelper.memContext == NULL) + protocolHelperInit(); + + // Allocate the client cache + if (protocolHelper.clientRemoteSize == 0) { - MEM_CONTEXT_BEGIN(memContextTop()) + MEM_CONTEXT_BEGIN(protocolHelper.memContext) { - protocolHelper.memContext = memContextNew("ProtocolHelper"); + // The number of remotes allowed is the greater of allowed repo or db configs + 1 (0 is reserved for connections from + // the main process). Since these are static and only one will be true it presents a problem for coverage. We think + // that pg remotes will always be greater but we'll protect that assumption with an assertion. + ASSERT(cfgDefOptionIndexTotal(cfgDefOptPgPath) >= cfgDefOptionIndexTotal(cfgDefOptRepoPath)); + + protocolHelper.clientRemoteSize = cfgDefOptionIndexTotal(cfgDefOptPgPath) +1; + protocolHelper.clientRemote = (ProtocolHelperClient *)memNew( + protocolHelper.clientRemoteSize * sizeof(ProtocolHelperClient)); } MEM_CONTEXT_END(); } + ASSERT(protocolId < protocolHelper.clientRemoteSize); + // Create protocol object - if (protocolHelper.remote == NULL) + ProtocolHelperClient *protocolHelperClient = &protocolHelper.clientRemote[protocolId]; + + if (protocolHelperClient->client == NULL) { MEM_CONTEXT_BEGIN(protocolHelper.memContext) { // Execute the protocol command - protocolHelper.remoteExec = execNew( - cfgOptionStr(cfgOptCmdSsh), protocolParam(remoteType, remoteId), - strNewFmt("remote-%u process on '%s'", remoteId, strPtr(cfgOptionStr(cfgOptRepoHost))), + protocolHelperClient->exec = execNew( + cfgOptionStr(cfgOptCmdSsh), protocolRemoteParam(protocolStorageType, protocolId), + strNewFmt(PROTOCOL_SERVICE_REMOTE "-%u process on '%s'", protocolId, strPtr(cfgOptionStr(cfgOptRepoHost))), (TimeMSec)(cfgOptionDbl(cfgOptProtocolTimeout) * 1000)); - execOpen(protocolHelper.remoteExec); + execOpen(protocolHelperClient->exec); // Create protocol object - protocolHelper.remote = protocolClientNew( - strNewFmt("remote-%u protocol on '%s'", remoteId, strPtr(cfgOptionStr(cfgOptRepoHost))), - PROTOCOL_SERVICE_REMOTE_STR, execIoRead(protocolHelper.remoteExec), execIoWrite(protocolHelper.remoteExec)); + protocolHelperClient->client = protocolClientNew( + strNewFmt(PROTOCOL_SERVICE_REMOTE "-%u protocol on '%s'", protocolId, strPtr(cfgOptionStr(cfgOptRepoHost))), + PROTOCOL_SERVICE_REMOTE_STR, execIoRead(protocolHelperClient->exec), execIoWrite(protocolHelperClient->exec)); // Get cipher options from the remote if none are locally configured if (strEq(cfgOptionStr(cfgOptRepoCipherType), CIPHER_TYPE_NONE_STR)) @@ -149,7 +282,7 @@ protocolGet(RemoteType remoteType, unsigned int remoteId) varLstAdd(param, varNewStr(strNew(cfgOptionName(cfgOptRepoCipherType)))); varLstAdd(param, varNewStr(strNew(cfgOptionName(cfgOptRepoCipherPass)))); - VariantList *optionList = configProtocolOption(protocolHelper.remote, param); + VariantList *optionList = configProtocolOption(protocolHelperClient->client, param); if (!strEq(varStr(varLstGet(optionList, 0)), CIPHER_TYPE_NONE_STR)) { @@ -158,12 +291,12 @@ protocolGet(RemoteType remoteType, unsigned int remoteId) } } - protocolClientMove(protocolHelper.remote, execMemContext(protocolHelper.remoteExec)); + protocolClientMove(protocolHelperClient->client, execMemContext(protocolHelperClient->exec)); } MEM_CONTEXT_END(); } - FUNCTION_LOG_RETURN(PROTOCOL_CLIENT, protocolHelper.remote); + FUNCTION_LOG_RETURN(PROTOCOL_CLIENT, protocolHelperClient->client); } /*********************************************************************************************************************************** @@ -174,12 +307,37 @@ protocolFree(void) { FUNCTION_LOG_VOID(logLevelTrace); - if (protocolHelper.remote != NULL) + if (protocolHelper.memContext != NULL) { - protocolClientFree(protocolHelper.remote); - execFree(protocolHelper.remoteExec); + // Free remotes + for (unsigned int clientIdx = 0; clientIdx < protocolHelper.clientRemoteSize; clientIdx++) + { + ProtocolHelperClient *protocolHelperClient = &protocolHelper.clientRemote[clientIdx]; - protocolHelper.remote = NULL; + if (protocolHelperClient->client != NULL) + { + protocolClientFree(protocolHelperClient->client); + execFree(protocolHelperClient->exec); + + protocolHelperClient->client = NULL; + protocolHelperClient->exec = NULL; + } + } + + // Free locals + for (unsigned int clientIdx = 0; clientIdx < protocolHelper.clientLocalSize; clientIdx++) + { + ProtocolHelperClient *protocolHelperClient = &protocolHelper.clientLocal[clientIdx]; + + if (protocolHelperClient->client != NULL) + { + protocolClientFree(protocolHelperClient->client); + execFree(protocolHelperClient->exec); + + protocolHelperClient->client = NULL; + protocolHelperClient->exec = NULL; + } + } } FUNCTION_LOG_RETURN_VOID(); diff --git a/src/protocol/helper.h b/src/protocol/helper.h index 14edccc37..5ae4a1e0c 100644 --- a/src/protocol/helper.h +++ b/src/protocol/helper.h @@ -4,19 +4,39 @@ Protocol Helper #ifndef PROTOCOL_HELPER_H #define PROTOCOL_HELPER_H +/*********************************************************************************************************************************** +Protocol storage type enum +***********************************************************************************************************************************/ +typedef enum +{ + protocolStorageTypeRepo, + protocolStorageTypePg, +} ProtocolStorageType; + #include "protocol/client.h" /*********************************************************************************************************************************** Constants ***********************************************************************************************************************************/ +#define PROTOCOL_SERVICE_LOCAL "local" + STRING_DECLARE(PROTOCOL_SERVICE_LOCAL_STR); #define PROTOCOL_SERVICE_REMOTE "remote" STRING_DECLARE(PROTOCOL_SERVICE_REMOTE_STR); /*********************************************************************************************************************************** Functions ***********************************************************************************************************************************/ +ProtocolClient *protocolLocalGet(ProtocolStorageType protocolStorageType, unsigned int protocolId); +ProtocolClient *protocolRemoteGet(ProtocolStorageType protocolStorageType, unsigned int protocolId); + +/*********************************************************************************************************************************** +Getters +***********************************************************************************************************************************/ bool repoIsLocal(void); -ProtocolClient *protocolGet(RemoteType remoteType, unsigned int remoteId); + +/*********************************************************************************************************************************** +Destructor +***********************************************************************************************************************************/ void protocolFree(void); #endif diff --git a/src/storage/driver/remote/storage.c b/src/storage/driver/remote/storage.c index 07680f886..0157c10cb 100644 --- a/src/storage/driver/remote/storage.c +++ b/src/storage/driver/remote/storage.c @@ -30,14 +30,14 @@ New object ***********************************************************************************************************************************/ StorageDriverRemote * storageDriverRemoteNew( - mode_t modeFile, mode_t modePath, bool write, StoragePathExpressionCallback pathExpressionFunction, RemoteType remoteType, - unsigned int remoteId) + mode_t modeFile, mode_t modePath, bool write, StoragePathExpressionCallback pathExpressionFunction, ProtocolClient *client) { FUNCTION_LOG_BEGIN(logLevelDebug); FUNCTION_LOG_PARAM(MODE, modeFile); FUNCTION_LOG_PARAM(MODE, modePath); FUNCTION_LOG_PARAM(BOOL, write); FUNCTION_LOG_PARAM(FUNCTIONP, pathExpressionFunction); + FUNCTION_LOG_PARAM(PROTOCOL_CLIENT, client); FUNCTION_LOG_END(); ASSERT(modeFile != 0); @@ -51,7 +51,7 @@ storageDriverRemoteNew( this = memNew(sizeof(StorageDriverRemote)); this->memContext = MEM_CONTEXT_NEW(); - this->client = protocolGet(remoteType, remoteId); + this->client = client; // Create the storage interface this->interface = storageNewP( diff --git a/src/storage/driver/remote/storage.h b/src/storage/driver/remote/storage.h index e9ebb7cbf..6c74eaa22 100644 --- a/src/storage/driver/remote/storage.h +++ b/src/storage/driver/remote/storage.h @@ -23,8 +23,7 @@ Driver type constant Constructor ***********************************************************************************************************************************/ StorageDriverRemote *storageDriverRemoteNew( - mode_t modeFile, mode_t modePath, bool write, StoragePathExpressionCallback pathExpressionFunction, RemoteType remoteType, - unsigned int remoteId); + mode_t modeFile, mode_t modePath, bool write, StoragePathExpressionCallback pathExpressionFunction, ProtocolClient *client); /*********************************************************************************************************************************** Functions diff --git a/src/storage/helper.c b/src/storage/helper.c index def51b87b..21dc51d11 100644 --- a/src/storage/helper.c +++ b/src/storage/helper.c @@ -209,7 +209,8 @@ storageRepoGet(const String *type, bool write) { result = storageDriverRemoteInterface( storageDriverRemoteNew( - STORAGE_MODE_FILE_DEFAULT, STORAGE_MODE_PATH_DEFAULT, write, storageRepoPathExpression, remoteTypeRepo, 1)); + STORAGE_MODE_FILE_DEFAULT, STORAGE_MODE_PATH_DEFAULT, write, storageRepoPathExpression, + protocolRemoteGet(protocolStorageTypeRepo, 1))); } // For now treat posix and cifs drivers as if they are the same. This won't be true once the repository storage becomes // writable but for now it's OK. The assertion above should pop if we try to create writable repo storage. diff --git a/test/define.yaml b/test/define.yaml index debf78f10..87118600c 100644 --- a/test/define.yaml +++ b/test/define.yaml @@ -724,6 +724,13 @@ unit: coverage: command/info/info: full + # ---------------------------------------------------------------------------------------------------------------------------- + - name: local + total: 1 + + coverage: + command/local/local: full + # ---------------------------------------------------------------------------------------------------------------------------- - name: remote total: 1 diff --git a/test/lib/pgBackRestTest/Module/Command/CommandArchiveGetPerlTest.pm b/test/lib/pgBackRestTest/Module/Command/CommandArchiveGetPerlTest.pm index b341ef51f..8952aba28 100644 --- a/test/lib/pgBackRestTest/Module/Command/CommandArchiveGetPerlTest.pm +++ b/test/lib/pgBackRestTest/Module/Command/CommandArchiveGetPerlTest.pm @@ -298,9 +298,8 @@ sub run my $oGetAsync = new pgBackRest::Archive::Get::Async( $self->{strSpoolPath}, $self->backrestExe(), \@stryWal); - $self->optionTestSetBool(CFGOPT_ARCHIVE_ASYNC, true); $self->optionTestSet(CFGOPT_SPOOL_PATH, $self->{strRepoPath}); - $self->configTestLoad(CFGCMD_ARCHIVE_GET); + $self->configTestLoad(CFGCMD_ARCHIVE_GET_ASYNC); $oGetAsync->process(); @@ -372,7 +371,7 @@ sub run #--------------------------------------------------------------------------------------------------------------------------- $self->optionTestSet(CFGOPT_PROTOCOL_TIMEOUT, 30); $self->optionTestSet(CFGOPT_DB_TIMEOUT, 29); - $self->configTestLoad(CFGCMD_ARCHIVE_GET); + $self->configTestLoad(CFGCMD_ARCHIVE_GET_ASYNC); $oGetAsync->process(); } diff --git a/test/src/module/command/localTest.c b/test/src/module/command/localTest.c new file mode 100644 index 000000000..b3b251069 --- /dev/null +++ b/test/src/module/command/localTest.c @@ -0,0 +1,64 @@ +/*********************************************************************************************************************************** +Test Local Command +***********************************************************************************************************************************/ +#include "common/io/handleRead.h" +#include "common/io/handleWrite.h" +#include "protocol/client.h" +#include "protocol/server.h" + +#include "common/harnessConfig.h" +#include "common/harnessFork.h" + +/*********************************************************************************************************************************** +Test Run +***********************************************************************************************************************************/ +void +testRun(void) +{ + FUNCTION_HARNESS_VOID(); + + // ***************************************************************************************************************************** + if (testBegin("cmdLocal()")) + { + // Create pipes for testing. Read/write is from the perspective of the client. + int pipeRead[2]; + int pipeWrite[2]; + THROW_ON_SYS_ERROR(pipe(pipeRead) == -1, KernelError, "unable to read test pipe"); + THROW_ON_SYS_ERROR(pipe(pipeWrite) == -1, KernelError, "unable to write test pipe"); + + HARNESS_FORK_BEGIN() + { + HARNESS_FORK_CHILD_BEGIN(0, true) + { + StringList *argList = strLstNew(); + strLstAddZ(argList, "pgbackrest"); + strLstAddZ(argList, "--stanza=test1"); + strLstAddZ(argList, "--command=archive-get-async"); + strLstAddZ(argList, "--process=1"); + strLstAddZ(argList, "--type=backup"); + strLstAddZ(argList, "--host-id=1"); + strLstAddZ(argList, "local"); + harnessCfgLoad(strLstSize(argList), strLstPtr(argList)); + + cmdLocal(HARNESS_FORK_CHILD_READ(), HARNESS_FORK_CHILD_WRITE()); + } + HARNESS_FORK_CHILD_END(); + + HARNESS_FORK_PARENT_BEGIN() + { + IoRead *read = ioHandleReadIo(ioHandleReadNew(strNew("server read"), HARNESS_FORK_PARENT_READ_PROCESS(0), 2000)); + ioReadOpen(read); + IoWrite *write = ioHandleWriteIo(ioHandleWriteNew(strNew("server write"), HARNESS_FORK_PARENT_WRITE_PROCESS(0))); + ioWriteOpen(write); + + ProtocolClient *client = protocolClientNew(strNew("test"), PROTOCOL_SERVICE_LOCAL_STR, read, write); + protocolClientNoOp(client); + protocolClientFree(client); + } + HARNESS_FORK_PARENT_END(); + } + HARNESS_FORK_END(); + } + + FUNCTION_HARNESS_RESULT_VOID(); +} diff --git a/test/src/module/protocol/protocolTest.c b/test/src/module/protocol/protocolTest.c index 811e90cd8..fee401ba1 100644 --- a/test/src/module/protocol/protocolTest.c +++ b/test/src/module/protocol/protocolTest.c @@ -98,7 +98,7 @@ testRun(void) harnessCfgLoad(strLstSize(argList), strLstPtr(argList)); TEST_RESULT_STR( - strPtr(strLstJoin(protocolParam(remoteTypeRepo, 1), "|")), + strPtr(strLstJoin(protocolRemoteParam(protocolStorageTypeRepo, 1), "|")), strPtr( strNew( "-o|LogLevel=error|-o|Compression=no|-o|PasswordAuthentication=no|repo-host-user@repo-host" @@ -119,7 +119,7 @@ testRun(void) harnessCfgLoad(strLstSize(argList), strLstPtr(argList)); TEST_RESULT_STR( - strPtr(strLstJoin(protocolParam(remoteTypeRepo, 1), "|")), + strPtr(strLstJoin(protocolRemoteParam(protocolStorageTypeRepo, 1), "|")), strPtr( strNew( "-o|LogLevel=error|-o|Compression=no|-o|PasswordAuthentication=no|-p|444|repo-host-user@repo-host" @@ -161,12 +161,6 @@ testRun(void) // ***************************************************************************************************************************** if (testBegin("ProtocolClient")) { - // Create pipes for testing. Read/write is from the perspective of the client. - int pipeRead[2]; - int pipeWrite[2]; - THROW_ON_SYS_ERROR(pipe(pipeRead) == -1, KernelError, "unable to read test pipe"); - THROW_ON_SYS_ERROR(pipe(pipeWrite) == -1, KernelError, "unable to write test pipe"); - HARNESS_FORK_BEGIN() { HARNESS_FORK_CHILD_BEGIN(0, true) @@ -590,10 +584,12 @@ testRun(void) ProtocolClient *client = NULL; - TEST_ASSIGN(client, protocolGet(remoteTypeRepo, 1), "get protocol"); - TEST_RESULT_PTR(protocolGet(remoteTypeRepo, 1), client, "get cached protocol"); - TEST_RESULT_VOID(protocolFree(), "free protocol objects"); - TEST_RESULT_VOID(protocolFree(), "free protocol objects again"); + TEST_RESULT_VOID(protocolFree(), "free protocol objects before anything has been created"); + + TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypeRepo, 1), "get remote protocol"); + TEST_RESULT_PTR(protocolRemoteGet(protocolStorageTypeRepo, 1), client, "get remote cached protocol"); + TEST_RESULT_VOID(protocolFree(), "free remote protocol objects"); + TEST_RESULT_VOID(protocolFree(), "free remote protocol objects again"); // Start protocol with local encryption settings // ------------------------------------------------------------------------------------------------------------------------- @@ -616,10 +612,10 @@ testRun(void) harnessCfgLoad(strLstSize(argList), strLstPtr(argList)); TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptRepoCipherPass)), "acbd", "check cipher pass before"); - TEST_ASSIGN(client, protocolGet(remoteTypeRepo, 1), "get protocol"); + TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypeRepo, 1), "get remote protocol"); TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptRepoCipherPass)), "acbd", "check cipher pass after"); - TEST_RESULT_VOID(protocolFree(), "free protocol objects"); + TEST_RESULT_VOID(protocolFree(), "free remote protocol objects"); // Start protocol with remote encryption settings // ------------------------------------------------------------------------------------------------------------------------- @@ -642,10 +638,24 @@ testRun(void) harnessCfgLoad(strLstSize(argList), strLstPtr(argList)); TEST_RESULT_PTR(cfgOptionStr(cfgOptRepoCipherPass), NULL, "check cipher pass before"); - TEST_ASSIGN(client, protocolGet(remoteTypeRepo, 1), "get protocol"); + TEST_ASSIGN(client, protocolRemoteGet(protocolStorageTypeRepo, 1), "get remote protocol"); TEST_RESULT_STR(strPtr(cfgOptionStr(cfgOptRepoCipherPass)), "dcba", "check cipher pass after"); - TEST_RESULT_VOID(protocolFree(), "free protocol objects"); + // Start local protocol + // ------------------------------------------------------------------------------------------------------------------------- + argList = strLstNew(); + strLstAddZ(argList, "/usr/bin/pgbackrest"); + strLstAddZ(argList, "--stanza=db"); + strLstAddZ(argList, "--protocol-timeout=10"); + strLstAddZ(argList, "archive-get"); + harnessCfgLoad(strLstSize(argList), strLstPtr(argList)); + + TEST_ASSIGN(client, protocolLocalGet(protocolStorageTypeRepo, 1), "get local protocol"); + TEST_RESULT_PTR(protocolLocalGet(protocolStorageTypeRepo, 1), client, "get local cached protocol"); + TEST_RESULT_PTR(protocolHelper.clientLocal[0].client, client, "check location in cache"); + + TEST_RESULT_VOID(protocolFree(), "free local and remote protocol objects"); + TEST_RESULT_VOID(protocolFree(), "free local and remote protocol objects again"); } FUNCTION_HARNESS_RESULT_VOID();