You've already forked pgbackrest
mirror of
https://github.com/pgbackrest/pgbackrest.git
synced 2025-07-13 01:00:23 +02:00
Restore works when PGDATA is a link.
Make the restore clean process look more like manifest build, i.e. do cleanup of each target root directory outside the main cleanup callback. This means some code duplication but removes the logic handling "dot" paths. Add tests for both restore and backup (which already worked but was not tested).
This commit is contained in:
@ -764,32 +764,28 @@ restoreCleanInfoListCallback(void *data, const StorageInfo *info)
|
||||
return;
|
||||
}
|
||||
|
||||
// Is this the . path, i.e. the root path for this list?
|
||||
bool dotPath = info->type == storageTypePath && strEq(info->name, DOT_STR);
|
||||
|
||||
// If this is not a delta then error because the directory is expected to be empty. Ignore the . path.
|
||||
if (!cleanData->delta)
|
||||
// Skip all . paths because they have already been cleaned on the previous level of recursion
|
||||
if (strEq(info->name, DOT_STR))
|
||||
{
|
||||
if (!dotPath)
|
||||
{
|
||||
THROW_FMT(
|
||||
PathNotEmptyError,
|
||||
"unable to restore to path '%s' because it contains files\n"
|
||||
"HINT: try using --delta if this is what you intended.",
|
||||
strPtr(cleanData->targetPath));
|
||||
}
|
||||
|
||||
FUNCTION_TEST_RETURN_VOID();
|
||||
return;
|
||||
}
|
||||
|
||||
// If this is not a delta then error because the directory is expected to be empty. Ignore the . path.
|
||||
if (!cleanData->delta)
|
||||
{
|
||||
THROW_FMT(
|
||||
PathNotEmptyError,
|
||||
"unable to restore to path '%s' because it contains files\n"
|
||||
"HINT: try using --delta if this is what you intended.",
|
||||
strPtr(cleanData->targetPath));
|
||||
}
|
||||
|
||||
// Construct the name used to find this file/link/path in the manifest
|
||||
const String *manifestName = dotPath ?
|
||||
cleanData->targetName : strNewFmt("%s/%s", strPtr(cleanData->targetName), strPtr(info->name));
|
||||
const String *manifestName = strNewFmt("%s/%s", strPtr(cleanData->targetName), strPtr(info->name));
|
||||
|
||||
// Construct the path of this file/link/path in the PostgreSQL data directory
|
||||
const String *pgPath = dotPath ?
|
||||
cleanData->targetPath : strNewFmt("%s/%s", strPtr(cleanData->targetPath), strPtr(info->name));
|
||||
const String *pgPath = strNewFmt("%s/%s", strPtr(cleanData->targetPath), strPtr(info->name));
|
||||
|
||||
switch (info->type)
|
||||
{
|
||||
@ -841,23 +837,18 @@ restoreCleanInfoListCallback(void *data, const StorageInfo *info)
|
||||
if (manifestPath != NULL)
|
||||
{
|
||||
// Check ownership/permissions
|
||||
if (dotPath)
|
||||
{
|
||||
restoreCleanOwnership(pgPath, manifestPath->user, manifestPath->group, info->userId, info->groupId, false);
|
||||
restoreCleanMode(pgPath, manifestPath->mode, info);
|
||||
}
|
||||
// Recurse into the path
|
||||
else
|
||||
{
|
||||
RestoreCleanCallbackData cleanDataSub = *cleanData;
|
||||
cleanDataSub.targetName = strNewFmt("%s/%s", strPtr(cleanData->targetName), strPtr(info->name));
|
||||
cleanDataSub.targetPath = strNewFmt("%s/%s", strPtr(cleanData->targetPath), strPtr(info->name));
|
||||
cleanDataSub.basePath = false;
|
||||
restoreCleanOwnership(pgPath, manifestPath->user, manifestPath->group, info->userId, info->groupId, false);
|
||||
restoreCleanMode(pgPath, manifestPath->mode, info);
|
||||
|
||||
storageInfoListP(
|
||||
storageLocalWrite(), cleanDataSub.targetPath, restoreCleanInfoListCallback, &cleanDataSub,
|
||||
.errorOnMissing = true, .sortOrder = sortOrderAsc);
|
||||
}
|
||||
// Recurse into the path
|
||||
RestoreCleanCallbackData cleanDataSub = *cleanData;
|
||||
cleanDataSub.targetName = strNewFmt("%s/%s", strPtr(cleanData->targetName), strPtr(info->name));
|
||||
cleanDataSub.targetPath = strNewFmt("%s/%s", strPtr(cleanData->targetPath), strPtr(info->name));
|
||||
cleanDataSub.basePath = false;
|
||||
|
||||
storageInfoListP(
|
||||
storageLocalWrite(), cleanDataSub.targetPath, restoreCleanInfoListCallback, &cleanDataSub,
|
||||
.errorOnMissing = true, .sortOrder = sortOrderAsc);
|
||||
}
|
||||
else
|
||||
{
|
||||
@ -1051,6 +1042,14 @@ restoreCleanBuild(Manifest *manifest)
|
||||
if (delta)
|
||||
LOG_INFO_FMT("remove invalid files/links/paths from '%s'", strPtr(cleanData->targetPath));
|
||||
|
||||
// Check target ownership/permissions
|
||||
const ManifestPath *manifestPath = manifestPathFind(cleanData->manifest, cleanData->targetName);
|
||||
StorageInfo info = storageInfoP(storageLocal(), cleanData->targetPath, .followLink = true);
|
||||
|
||||
restoreCleanOwnership(
|
||||
cleanData->targetPath, manifestPath->user, manifestPath->group, info.userId, info.groupId, false);
|
||||
restoreCleanMode(cleanData->targetPath, manifestPath->mode, &info);
|
||||
|
||||
// Clean the target
|
||||
storageInfoListP(
|
||||
storageLocalWrite(), cleanData->targetPath, restoreCleanInfoListCallback, cleanData, .errorOnMissing = true,
|
||||
|
@ -2226,6 +2226,11 @@ testRun(void)
|
||||
strLstAddZ(argList, "--" CFGOPT_ARCHIVE_COPY);
|
||||
harnessCfgLoad(cfgCmdBackup, argList);
|
||||
|
||||
// Move pg1-path and put a link in its place. This tests that backup works when pg1-path is a symlink yet should be
|
||||
// completely invisible in the manifest and logging.
|
||||
TEST_SYSTEM_FMT("mv %s %s-data", strPtr(pg1Path), strPtr(pg1Path));
|
||||
TEST_SYSTEM_FMT("ln -s %s-data %s ", strPtr(pg1Path), strPtr(pg1Path));
|
||||
|
||||
// Zeroed file which passes page checksums
|
||||
Buffer *relation = bufNew(PG_PAGE_SIZE_DEFAULT);
|
||||
memset(bufPtr(relation), 0, bufSize(relation));
|
||||
|
@ -2268,6 +2268,11 @@ testRun(void)
|
||||
strLstAddZ(argList, "--db-include=16384");
|
||||
harnessCfgLoad(cfgCmdRestore, argList);
|
||||
|
||||
// Move pg1-path and put a link in its place. This tests that restore works when pg1-path is a symlink yet should be
|
||||
// completely invisible in the manifest and logging.
|
||||
TEST_SYSTEM_FMT("mv %s %s-data", strPtr(pgPath), strPtr(pgPath));
|
||||
TEST_SYSTEM_FMT("ln -s %s-data %s ", strPtr(pgPath), strPtr(pgPath));
|
||||
|
||||
// Write recovery.conf so we don't get a preserve warning
|
||||
storagePutP(storageNewWriteP(storagePgWrite(), PG_FILE_RECOVERYCONF_STR), BUFSTRDEF("Some Settings"));
|
||||
|
||||
|
Reference in New Issue
Block a user