On Mon, Apr 6, 2015 at 1:41 PM, Michael Paquier wrote: > I guess that you are working on a patch? If not, you are looking for one?
Code-speaking, this gives the patch attached. I eliminated a bunch of newlines in the log messages that seemed really unnecessary to me, simplifying a bit the whole. While hacking this stuff, I noticed as well that pg_rewind could be called as root on non-Windows platform, that's dangerous from a security point of view as process manipulates files in PGDATA. Hence let's block that. On Windows, a restricted token should be used. Regards, -- Michael
From 32b0aeef2602f7388c7e9fca6290046d8d9dfe67 Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@otacoo.com> Date: Mon, 6 Apr 2015 17:18:21 +0900 Subject: [PATCH] Fix inconsistent error and process handling in pg_rewind pg_rewind was handling a couple of things differently compared to the other src/bin utilities: - pg_rewind manipulates files in a data folder, hence it should not run as root on Non-Windows platforms. On Windows, it should use a restricted token - Logging output needs to be flushed on stderr, not stdout - Logging messages should be prefixed with the application name - pg_fatal exits with error code of 1, but it was sometimes followed by subsequent logs, making this information lost in the process. --- src/bin/pg_rewind/copy_fetch.c | 22 +++++------ src/bin/pg_rewind/datapagemap.c | 4 +- src/bin/pg_rewind/file_ops.c | 30 +++++++------- src/bin/pg_rewind/filemap.c | 18 ++++----- src/bin/pg_rewind/libpq_fetch.c | 52 ++++++++++++------------- src/bin/pg_rewind/logging.c | 16 +++++--- src/bin/pg_rewind/logging.h | 1 + src/bin/pg_rewind/parsexlog.c | 20 +++++----- src/bin/pg_rewind/pg_rewind.c | 86 +++++++++++++++++++++++------------------ src/bin/pg_rewind/pg_rewind.h | 2 + src/bin/pg_rewind/timeline.c | 27 +++++-------- 11 files changed, 144 insertions(+), 134 deletions(-) diff --git a/src/bin/pg_rewind/copy_fetch.c b/src/bin/pg_rewind/copy_fetch.c index 887fec9..de5fe4a 100644 --- a/src/bin/pg_rewind/copy_fetch.c +++ b/src/bin/pg_rewind/copy_fetch.c @@ -58,7 +58,7 @@ recurse_dir(const char *datadir, const char *parentpath, xldir = opendir(fullparentpath); if (xldir == NULL) - pg_fatal("could not open directory \"%s\": %s\n", + pg_fatal("could not open directory \"%s\": %s", fullparentpath, strerror(errno)); while (errno = 0, (xlde = readdir(xldir)) != NULL) @@ -113,13 +113,13 @@ recurse_dir(const char *datadir, const char *parentpath, len = readlink(fullpath, link_target, sizeof(link_target) - 1); if (len == -1) - pg_fatal("readlink() failed on \"%s\": %s\n", + pg_fatal("readlink() failed on \"%s\": %s", fullpath, strerror(errno)); if (len == sizeof(link_target) - 1) { /* path was truncated */ - pg_fatal("symbolic link \"%s\" target path too long\n", + pg_fatal("symbolic link \"%s\" target path too long", fullpath); } @@ -132,18 +132,18 @@ recurse_dir(const char *datadir, const char *parentpath, if (strcmp(parentpath, "pg_tblspc") == 0) recurse_dir(datadir, path, callback); #else - pg_fatal("\"%s\" is a symbolic link, but symbolic links are not supported on this platform\n", + pg_fatal("\"%s\" is a symbolic link, but symbolic links are not supported on this platform", fullpath); #endif /* HAVE_READLINK */ } } if (errno) - pg_fatal("could not read directory \"%s\": %s\n", + pg_fatal("could not read directory \"%s\": %s", fullparentpath, strerror(errno)); if (closedir(xldir)) - pg_fatal("could not close archive location \"%s\": %s\n", + pg_fatal("could not close archive location \"%s\": %s", fullparentpath, strerror(errno)); } @@ -163,11 +163,11 @@ copy_file_range(const char *path, off_t begin, off_t end, bool trunc) srcfd = open(srcpath, O_RDONLY | PG_BINARY, 0); if (srcfd < 0) - pg_fatal("could not open source file \"%s\": %s\n", + pg_fatal("could not open source file \"%s\": %s", srcpath, strerror(errno)); if (lseek(srcfd, begin, SEEK_SET) == -1) - pg_fatal("could not seek in source file: %s\n", strerror(errno)); + pg_fatal("could not seek in source file: %s", strerror(errno)); open_target_file(path, trunc); @@ -184,17 +184,17 @@ copy_file_range(const char *path, off_t begin, off_t end, bool trunc) readlen = read(srcfd, buf, len); if (readlen < 0) - pg_fatal("could not read file \"%s\": %s\n", + pg_fatal("could not read file \"%s\": %s", srcpath, strerror(errno)); else if (readlen == 0) - pg_fatal("unexpected EOF while reading file \"%s\"\n", srcpath); + pg_fatal("unexpected EOF while reading file \"%s\"", srcpath); write_target_range(buf, begin, readlen); begin += readlen; } if (close(srcfd) != 0) - pg_fatal("error closing file \"%s\": %s\n", srcpath, strerror(errno)); + pg_fatal("error closing file \"%s\": %s", srcpath, strerror(errno)); } /* diff --git a/src/bin/pg_rewind/datapagemap.c b/src/bin/pg_rewind/datapagemap.c index 3477366..34868f9 100644 --- a/src/bin/pg_rewind/datapagemap.c +++ b/src/bin/pg_rewind/datapagemap.c @@ -13,6 +13,8 @@ #include "postgres_fe.h" #include "datapagemap.h" +#include "pg_rewind.h" +#include "logging.h" struct datapagemap_iterator { @@ -120,7 +122,7 @@ datapagemap_print(datapagemap_t *map) iter = datapagemap_iterate(map); while (datapagemap_next(iter, &blocknum)) - printf(" blk %u\n", blocknum); + pg_log(PG_DEBUG, "blk %u", blocknum); free(iter); } diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index 589a01a..4620064 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -61,7 +61,7 @@ open_target_file(const char *path, bool trunc) mode |= O_TRUNC; dstfd = open(dstpath, mode, 0600); if (dstfd < 0) - pg_fatal("could not open destination file \"%s\": %s\n", + pg_fatal("could not open destination file \"%s\": %s", dstpath, strerror(errno)); } @@ -75,7 +75,7 @@ close_target_file(void) return; if (close(dstfd) != 0) - pg_fatal("error closing destination file \"%s\": %s\n", + pg_fatal("error closing destination file \"%s\": %s", dstpath, strerror(errno)); dstfd = -1; @@ -96,7 +96,7 @@ write_target_range(char *buf, off_t begin, size_t size) return; if (lseek(dstfd, begin, SEEK_SET) == -1) - pg_fatal("could not seek in destination file \"%s\": %s\n", + pg_fatal("could not seek in destination file \"%s\": %s", dstpath, strerror(errno)); writeleft = size; @@ -107,7 +107,7 @@ write_target_range(char *buf, off_t begin, size_t size) writelen = write(dstfd, p, writeleft); if (writelen < 0) - pg_fatal("could not write file \"%s\": %s\n", + pg_fatal("could not write file \"%s\": %s", dstpath, strerror(errno)); p += writelen; @@ -156,7 +156,7 @@ create_target(file_entry_t *entry) case FILE_TYPE_REGULAR: /* can't happen. Regular files are created with open_target_file. */ - pg_fatal("invalid action (CREATE) for regular file\n"); + pg_fatal("invalid action (CREATE) for regular file"); break; } } @@ -171,7 +171,7 @@ remove_target_file(const char *path) snprintf(dstpath, sizeof(dstpath), "%s/%s", datadir_target, path); if (unlink(dstpath) != 0) - pg_fatal("could not remove file \"%s\": %s\n", + pg_fatal("could not remove file \"%s\": %s", dstpath, strerror(errno)); } @@ -188,11 +188,11 @@ truncate_target_file(const char *path, off_t newsize) fd = open(dstpath, O_WRONLY, 0); if (fd < 0) - pg_fatal("could not open file \"%s\" for truncation: %s\n", + pg_fatal("could not open file \"%s\" for truncation: %s", dstpath, strerror(errno)); if (ftruncate(fd, newsize) != 0) - pg_fatal("could not truncate file \"%s\" to %u bytes: %s\n", + pg_fatal("could not truncate file \"%s\" to %u bytes: %s", dstpath, (unsigned int) newsize, strerror(errno)); close(fd); @@ -208,7 +208,7 @@ create_target_dir(const char *path) snprintf(dstpath, sizeof(dstpath), "%s/%s", datadir_target, path); if (mkdir(dstpath, S_IRWXU) != 0) - pg_fatal("could not create directory \"%s\": %s\n", + pg_fatal("could not create directory \"%s\": %s", dstpath, strerror(errno)); } @@ -222,7 +222,7 @@ remove_target_dir(const char *path) snprintf(dstpath, sizeof(dstpath), "%s/%s", datadir_target, path); if (rmdir(dstpath) != 0) - pg_fatal("could not remove directory \"%s\": %s\n", + pg_fatal("could not remove directory \"%s\": %s", dstpath, strerror(errno)); } @@ -236,7 +236,7 @@ create_target_symlink(const char *path, const char *link) snprintf(dstpath, sizeof(dstpath), "%s/%s", datadir_target, path); if (symlink(link, dstpath) != 0) - pg_fatal("could not create symbolic link at \"%s\": %s\n", + pg_fatal("could not create symbolic link at \"%s\": %s", dstpath, strerror(errno)); } @@ -250,7 +250,7 @@ remove_target_symlink(const char *path) snprintf(dstpath, sizeof(dstpath), "%s/%s", datadir_target, path); if (unlink(dstpath) != 0) - pg_fatal("could not remove symbolic link \"%s\": %s\n", + pg_fatal("could not remove symbolic link \"%s\": %s", dstpath, strerror(errno)); } @@ -280,11 +280,11 @@ slurpFile(const char *datadir, const char *path, size_t *filesize) snprintf(fullpath, sizeof(fullpath), "%s/%s", datadir, path); if ((fd = open(fullpath, O_RDONLY | PG_BINARY, 0)) == -1) - pg_fatal("could not open file \"%s\" for reading: %s\n", + pg_fatal("could not open file \"%s\" for reading: %s", fullpath, strerror(errno)); if (fstat(fd, &statbuf) < 0) - pg_fatal("could not open file \"%s\" for reading: %s\n", + pg_fatal("could not open file \"%s\" for reading: %s", fullpath, strerror(errno)); len = statbuf.st_size; @@ -292,7 +292,7 @@ slurpFile(const char *datadir, const char *path, size_t *filesize) buffer = pg_malloc(len + 1); if (read(fd, buffer, len) != len) - pg_fatal("could not read file \"%s\": %s\n", + pg_fatal("could not read file \"%s\": %s", fullpath, strerror(errno)); close(fd); diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c index ee6e6db..83e9111 100644 --- a/src/bin/pg_rewind/filemap.c +++ b/src/bin/pg_rewind/filemap.c @@ -1,5 +1,5 @@ /*------------------------------------------------------------------------- - * +1;2c * * filemap.c * A data structure for keeping track of files that have changed. * @@ -93,7 +93,7 @@ process_remote_file(const char *path, file_type_t type, size_t newsize, * regular file */ if (type != FILE_TYPE_REGULAR && isRelDataFile(path)) - pg_fatal("data file in source \"%s\" is not a regular file\n", path); + pg_fatal("data file in source \"%s\" is not a regular file", path); snprintf(localpath, sizeof(localpath), "%s/%s", datadir_target, path); @@ -101,7 +101,7 @@ process_remote_file(const char *path, file_type_t type, size_t newsize, if (lstat(localpath, &statbuf) < 0) { if (errno != ENOENT) - pg_fatal("could not stat file \"%s\": %s\n", + pg_fatal("could not stat file \"%s\": %s", localpath, strerror(errno)); exists = false; @@ -115,7 +115,7 @@ process_remote_file(const char *path, file_type_t type, size_t newsize, if (exists && !S_ISDIR(statbuf.st_mode)) { /* it's a directory in target, but not in source. Strange.. */ - pg_fatal("\"%s\" is not a directory\n", localpath); + pg_fatal("\"%s\" is not a directory", localpath); } if (!exists) @@ -138,7 +138,7 @@ process_remote_file(const char *path, file_type_t type, size_t newsize, * It's a symbolic link in target, but not in source. * Strange.. */ - pg_fatal("\"%s\" is not a symbolic link\n", localpath); + pg_fatal("\"%s\" is not a symbolic link", localpath); } if (!exists) @@ -150,7 +150,7 @@ process_remote_file(const char *path, file_type_t type, size_t newsize, case FILE_TYPE_REGULAR: if (exists && !S_ISREG(statbuf.st_mode)) - pg_fatal("\"%s\" is not a regular file\n", localpath); + pg_fatal("\"%s\" is not a regular file", localpath); if (!exists || !isRelDataFile(path)) { @@ -266,7 +266,7 @@ process_local_file(const char *path, file_type_t type, size_t oldsize, if (map->nlist == 0) { /* should not happen */ - pg_fatal("remote file list is empty\n"); + pg_fatal("remote file list is empty"); } filemap_list_to_array(map); @@ -381,7 +381,7 @@ process_block_change(ForkNumber forknum, RelFileNode rnode, BlockNumber blkno) break; case FILE_ACTION_CREATE: - pg_fatal("unexpected page modification for directory or symbolic link \"%s\"\n", entry->path); + pg_fatal("unexpected page modification for directory or symbolic link \"%s\"", entry->path); } } else @@ -514,7 +514,7 @@ print_filemap(void) if (entry->action != FILE_ACTION_NONE || entry->pagemap.bitmapsize > 0) { - printf("%s (%s)\n", entry->path, action_to_str(entry->action)); + pg_log(PG_DEBUG, "%s (%s)", entry->path, action_to_str(entry->action)); if (entry->pagemap.bitmapsize > 0) datapagemap_print(&entry->pagemap); diff --git a/src/bin/pg_rewind/libpq_fetch.c b/src/bin/pg_rewind/libpq_fetch.c index 23c971a..b7accba 100644 --- a/src/bin/pg_rewind/libpq_fetch.c +++ b/src/bin/pg_rewind/libpq_fetch.c @@ -52,10 +52,10 @@ libpqConnect(const char *connstr) conn = PQconnectdb(connstr); if (PQstatus(conn) == CONNECTION_BAD) - pg_fatal("could not connect to remote server: %s\n", + pg_fatal("could not connect to remote server: %s", PQerrorMessage(conn)); - pg_log(PG_PROGRESS, "connected to remote server\n"); + pg_log(PG_PROGRESS, "connected to remote server"); /* * Check that the server is not in hot standby mode. There is no @@ -65,7 +65,7 @@ libpqConnect(const char *connstr) */ str = run_simple_query("SELECT pg_is_in_recovery()"); if (strcmp(str, "f") != 0) - pg_fatal("source server must not be in recovery mode\n"); + pg_fatal("source server must not be in recovery mode"); pg_free(str); /* @@ -75,7 +75,7 @@ libpqConnect(const char *connstr) */ str = run_simple_query("SHOW full_page_writes"); if (strcmp(str, "on") != 0) - pg_fatal("full_page_writes must be enabled in the source server\n"); + pg_fatal("full_page_writes must be enabled in the source server"); pg_free(str); } @@ -91,12 +91,12 @@ run_simple_query(const char *sql) res = PQexec(conn, sql); if (PQresultStatus(res) != PGRES_TUPLES_OK) - pg_fatal("error running query (%s) in source server: %s\n", + pg_fatal("error running query (%s) in source server: %s", sql, PQresultErrorMessage(res)); /* sanity check the result set */ if (PQnfields(res) != 1 || PQntuples(res) != 1 || PQgetisnull(res, 0, 0)) - pg_fatal("unexpected result set while running query\n"); + pg_fatal("unexpected result set while running query"); result = pg_strdup(PQgetvalue(res, 0, 0)); @@ -119,7 +119,7 @@ libpqGetCurrentXlogInsertLocation(void) val = run_simple_query("SELECT pg_current_xlog_insert_location()"); if (sscanf(val, "%X/%X", &hi, &lo) != 2) - pg_fatal("unexpected result \"%s\" while fetching current XLOG insert location\n", val); + pg_fatal("unexpected result \"%s\" while fetching current XLOG insert location", val); result = ((uint64) hi) << 32 | lo; @@ -167,12 +167,12 @@ libpqProcessFileList(void) res = PQexec(conn, sql); if (PQresultStatus(res) != PGRES_TUPLES_OK) - pg_fatal("unexpected result while fetching file list: %s\n", + pg_fatal("unexpected result while fetching file list: %s", PQresultErrorMessage(res)); /* sanity check the result set */ if (PQnfields(res) != 4) - pg_fatal("unexpected result set while fetching file list\n"); + pg_fatal("unexpected result set while fetching file list"); /* Read result to local variables */ for (i = 0; i < PQntuples(res); i++) @@ -210,12 +210,12 @@ receiveFileChunks(const char *sql) PGresult *res; if (PQsendQueryParams(conn, sql, 0, NULL, NULL, NULL, NULL, 1) != 1) - pg_fatal("could not send query: %s\n", PQerrorMessage(conn)); + pg_fatal("could not send query: %s", PQerrorMessage(conn)); pg_log(PG_DEBUG, "getting file chunks"); if (PQsetSingleRowMode(conn) != 1) - pg_fatal("could not set libpq connection to single row mode\n"); + pg_fatal("could not set libpq connection to single row mode"); while ((res = PQgetResult(conn)) != NULL) { @@ -234,19 +234,19 @@ receiveFileChunks(const char *sql) continue; /* final zero-row result */ default: - pg_fatal("unexpected result while fetching remote files: %s\n", + pg_fatal("unexpected result while fetching remote files: %s", PQresultErrorMessage(res)); } /* sanity check the result set */ if (PQnfields(res) != 3 || PQntuples(res) != 1) - pg_fatal("unexpected result set size while fetching remote files\n"); + pg_fatal("unexpected result set size while fetching remote files"); if (PQftype(res, 0) != TEXTOID && PQftype(res, 1) != INT4OID && PQftype(res, 2) != BYTEAOID) { - pg_fatal("unexpected data types in result set while fetching remote files: %u %u %u\n", + pg_fatal("unexpected data types in result set while fetching remote files: %u %u %u", PQftype(res, 0), PQftype(res, 1), PQftype(res, 2)); } @@ -254,18 +254,18 @@ receiveFileChunks(const char *sql) PQfformat(res, 1) != 1 && PQfformat(res, 2) != 1) { - pg_fatal("unexpected result format while fetching remote files\n"); + pg_fatal("unexpected result format while fetching remote files"); } if (PQgetisnull(res, 0, 0) || PQgetisnull(res, 0, 1) || PQgetisnull(res, 0, 2)) { - pg_fatal("unexpected NULL result while fetching remote files\n"); + pg_fatal("unexpected NULL result while fetching remote files"); } if (PQgetlength(res, 0, 1) != sizeof(int32)) - pg_fatal("unexpected result length while fetching remote files\n"); + pg_fatal("unexpected result length while fetching remote files"); /* Read result set to local variables */ memcpy(&chunkoff, PQgetvalue(res, 0, 1), sizeof(int32)); @@ -279,7 +279,7 @@ receiveFileChunks(const char *sql) chunk = PQgetvalue(res, 0, 2); - pg_log(PG_DEBUG, "received chunk for file \"%s\", off %d, len %d\n", + pg_log(PG_DEBUG, "received chunk for file \"%s\", off %d, len %d", filename, chunkoff, chunksize); open_target_file(filename, false); @@ -308,12 +308,12 @@ libpqGetFile(const char *filename, size_t *filesize) 1, NULL, paramValues, NULL, NULL, 1); if (PQresultStatus(res) != PGRES_TUPLES_OK) - pg_fatal("unexpected result while fetching remote file \"%s\": %s\n", + pg_fatal("unexpected result while fetching remote file \"%s\": %s", filename, PQresultErrorMessage(res)); /* sanity check the result set */ if (PQntuples(res) != 1 || PQgetisnull(res, 0, 0)) - pg_fatal("unexpected result set while fetching remote file \"%s\"\n", + pg_fatal("unexpected result set while fetching remote file \"%s\"", filename); /* Read result to local variables */ @@ -322,7 +322,7 @@ libpqGetFile(const char *filename, size_t *filesize) memcpy(result, PQgetvalue(res, 0, 0), len); result[len] = '\0'; - pg_log(PG_DEBUG, "fetched file \"%s\", length %d\n", filename, len); + pg_log(PG_DEBUG, "fetched file \"%s\", length %d", filename, len); if (filesize) *filesize = len; @@ -354,7 +354,7 @@ fetch_file_range(const char *path, unsigned int begin, unsigned int end) snprintf(linebuf, sizeof(linebuf), "%s\t%u\t%u\n", path, begin, len); if (PQputCopyData(conn, linebuf, strlen(linebuf)) != 1) - pg_fatal("error sending COPY data: %s\n", + pg_fatal("error sending COPY data: %s", PQerrorMessage(conn)); begin += len; @@ -380,14 +380,14 @@ libpq_executeFileMap(filemap_t *map) res = PQexec(conn, sql); if (PQresultStatus(res) != PGRES_COMMAND_OK) - pg_fatal("error creating temporary table: %s\n", + pg_fatal("error creating temporary table: %s", PQresultErrorMessage(res)); sql = "COPY fetchchunks FROM STDIN"; res = PQexec(conn, sql); if (PQresultStatus(res) != PGRES_COPY_IN) - pg_fatal("unexpected result while sending file list: %s\n", + pg_fatal("unexpected result while sending file list: %s", PQresultErrorMessage(res)); for (i = 0; i < map->narray; i++) @@ -428,13 +428,13 @@ libpq_executeFileMap(filemap_t *map) } if (PQputCopyEnd(conn, NULL) != 1) - pg_fatal("error sending end-of-COPY: %s\n", + pg_fatal("error sending end-of-COPY: %s", PQerrorMessage(conn)); while ((res = PQgetResult(conn)) != NULL) { if (PQresultStatus(res) != PGRES_COMMAND_OK) - pg_fatal("unexpected result while sending file list: %s\n", + pg_fatal("unexpected result while sending file list: %s", PQresultErrorMessage(res)); } diff --git a/src/bin/pg_rewind/logging.c b/src/bin/pg_rewind/logging.c index aba12d8..26d4f88 100644 --- a/src/bin/pg_rewind/logging.c +++ b/src/bin/pg_rewind/logging.c @@ -40,28 +40,32 @@ pg_log_v(eLogType type, const char *fmt, va_list ap) { case PG_DEBUG: if (debug) - printf("%s", _(message)); + fprintf(stderr, _("%s: %s\n"), progname, message); break; case PG_PROGRESS: if (showprogress) - printf("%s", _(message)); + fprintf(stderr, _("%s: %s\n"), progname, message); + break; + + case PG_STATUS: + fprintf(stderr, _("%s: %s\n"), progname, message); break; case PG_WARNING: - printf("%s", _(message)); + fprintf(stderr, _("%s: warning: %s\n"), progname, message); break; case PG_FATAL: - printf("\n%s", _(message)); - printf("%s", _("Failure, exiting\n")); + fprintf(stderr, _("\n%s: fatal: %s\n"), progname, message); + fprintf(stderr, _("Failure, exiting\n")); exit(1); break; default: break; } - fflush(stdout); + fflush(stderr); } diff --git a/src/bin/pg_rewind/logging.h b/src/bin/pg_rewind/logging.h index 0272a22..abf65dc 100644 --- a/src/bin/pg_rewind/logging.h +++ b/src/bin/pg_rewind/logging.h @@ -23,6 +23,7 @@ typedef enum { PG_DEBUG, PG_PROGRESS, + PG_STATUS, PG_WARNING, PG_FATAL } eLogType; diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c index 3cf96ab..78050e1 100644 --- a/src/bin/pg_rewind/parsexlog.c +++ b/src/bin/pg_rewind/parsexlog.c @@ -84,11 +84,11 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, TimeLineID tli, errptr = startpoint ? startpoint : xlogreader->EndRecPtr; if (errormsg) - pg_fatal("error reading WAL at %X/%X: %s\n", + pg_fatal("error reading WAL at %X/%X: %s", (uint32) (errptr >> 32), (uint32) (errptr), errormsg); else - pg_fatal("error reading WAL at %X/%X\n", + pg_fatal("error reading WAL at %X/%X", (uint32) (startpoint >> 32), (uint32) (startpoint)); } @@ -130,10 +130,10 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, TimeLineID tli) if (record == NULL) { if (errormsg) - pg_fatal("could not read WAL record at %X/%X: %s\n", + pg_fatal("could not read WAL record at %X/%X: %s", (uint32) (ptr >> 32), (uint32) (ptr), errormsg); else - pg_fatal("could not read WAL record at %X/%X\n", + pg_fatal("could not read WAL record at %X/%X", (uint32) (ptr >> 32), (uint32) (ptr)); } endptr = xlogreader->EndRecPtr; @@ -188,11 +188,11 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, TimeLineID tli, if (record == NULL) { if (errormsg) - pg_fatal("could not find previous WAL record at %X/%X: %s\n", + pg_fatal("could not find previous WAL record at %X/%X: %s", (uint32) (searchptr >> 32), (uint32) (searchptr), errormsg); else - pg_fatal("could not find previous WAL record at %X/%X\n", + pg_fatal("could not find previous WAL record at %X/%X", (uint32) (searchptr >> 32), (uint32) (searchptr)); } @@ -265,7 +265,7 @@ SimpleXLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr, if (xlogreadfd < 0) { - printf(_("could not open file \"%s\": %s\n"), xlogfpath, + pg_log(PG_WARNING, "could not open file \"%s\": %s", xlogfpath, strerror(errno)); return -1; } @@ -279,14 +279,14 @@ SimpleXLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr, /* Read the requested page */ if (lseek(xlogreadfd, (off_t) targetPageOff, SEEK_SET) < 0) { - printf(_("could not seek in file \"%s\": %s\n"), xlogfpath, + pg_log(PG_WARNING, "could not seek in file \"%s\": %s\n", xlogfpath, strerror(errno)); return -1; } if (read(xlogreadfd, readBuf, XLOG_BLCKSZ) != XLOG_BLCKSZ) { - printf(_("could not read from file \"%s\": %s\n"), xlogfpath, + pg_log(PG_WARNING, "could not read from file \"%s\": %s\n", xlogfpath, strerror(errno)); return -1; } @@ -357,7 +357,7 @@ extractPageInfo(XLogReaderState *record) * track that change. */ pg_fatal("WAL record modifies a relation, but record type is not recognized\n" - "lsn: %X/%X, rmgr: %s, info: %02X\n", + "lsn: %X/%X, rmgr: %s, info: %02X", (uint32) (record->ReadRecPtr >> 32), (uint32) (record->ReadRecPtr), RmgrNames[rmid], info); } diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index dda3a79..3c1c0d1 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -24,10 +24,11 @@ #include "access/xlog_internal.h" #include "catalog/catversion.h" #include "catalog/pg_control.h" +#include "common/restricted_token.h" #include "getopt_long.h" #include "storage/bufpage.h" -static void usage(const char *progname); +static void usage(void); static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, XLogRecPtr checkpointloc); @@ -41,7 +42,7 @@ static void findCommonAncestorTimeline(XLogRecPtr *recptr, TimeLineID *tli); static ControlFileData ControlFile_target; static ControlFileData ControlFile_source; -const char *progname; +const char *progname = NULL; /* Configuration options */ char *datadir_target = NULL; @@ -53,7 +54,7 @@ bool showprogress = false; bool dry_run = false; static void -usage(const char *progname) +usage(void) { printf(_("%s resynchronizes a cluster with another copy of the cluster.\n\n"), progname); printf(_("Usage:\n %s [OPTION]...\n\n"), progname); @@ -102,6 +103,7 @@ main(int argc, char **argv) TimeLineID endtli; ControlFileData ControlFile_new; + set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_rewind")); progname = get_progname(argv[0]); /* Process command-line arguments */ @@ -109,7 +111,7 @@ main(int argc, char **argv) { if (strcmp(argv[1], "--help") == 0 || strcmp(argv[1], "-?") == 0) { - usage(progname); + usage(); exit(0); } if (strcmp(argv[1], "--version") == 0 || strcmp(argv[1], "-V") == 0) @@ -154,25 +156,31 @@ main(int argc, char **argv) /* No source given? Show usage */ if (datadir_source == NULL && connstr_source == NULL) - { - pg_fatal("no source specified (--source-pgdata or --source-server)\n"); - pg_fatal("Try \"%s --help\" for more information.\n", progname); - exit(1); - } + pg_fatal("no source specified (--source-pgdata or --source-server)\n" + "Try \"%s --help\" for more information.", progname); if (datadir_target == NULL) - { - pg_fatal("no target data directory specified (--target-pgdata)\n"); - fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname); - exit(1); - } + pg_fatal("no target data directory specified (--target-pgdata)\n" + "Try \"%s --help\" for more information.\n", progname); if (argc != optind) - { - pg_fatal("%s: invalid arguments\n", progname); - fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname); - exit(1); - } + pg_fatal("invalid arguments\n" + "Try \"%s --help\" for more information.", progname); + + /* + * Don't allow pg_rewind to be run as root, to avoid overwriting the + * ownership of files in the data directory. We need only check for root + * -- any other user won't have sufficient permissions to modify files in + * the data directory. + */ +#ifndef WIN32 + if (geteuid() == 0) + pg_fatal("cannot be executed by \"root\"\n" + "You must run %s as the PostgreSQL superuser.\n", + progname); +#endif + + get_restricted_token(progname); /* Connect to remote server */ if (connstr_source) @@ -197,10 +205,11 @@ main(int argc, char **argv) * do. */ if (ControlFile_target.checkPointCopy.ThisTimeLineID == ControlFile_source.checkPointCopy.ThisTimeLineID) - pg_fatal("source and target cluster are on the same timeline\n"); + pg_fatal("source and target cluster are on the same timeline"); findCommonAncestorTimeline(&divergerec, &lastcommontli); - printf(_("The servers diverged at WAL position %X/%X on timeline %u.\n"), + pg_log(PG_STATUS, + "The servers diverged at WAL position %X/%X on timeline %u.", (uint32) (divergerec >> 32), (uint32) divergerec, lastcommontli); /* @@ -235,13 +244,14 @@ main(int argc, char **argv) if (!rewind_needed) { - printf(_("No rewind required.\n")); + pg_log(PG_STATUS, "No rewind required."); exit(0); } findLastCheckpoint(datadir_target, divergerec, lastcommontli, &chkptrec, &chkpttli, &chkptredo); - printf(_("Rewinding from last common checkpoint at %X/%X on timeline %u\n"), + pg_log(PG_STATUS, + "Rewinding from last common checkpoint at %X/%X on timeline %u", (uint32) (chkptrec >> 32), (uint32) chkptrec, chkpttli); @@ -249,9 +259,9 @@ main(int argc, char **argv) * Build the filemap, by comparing the remote and local data directories. */ filemap_create(); - pg_log(PG_PROGRESS, "reading source file list\n"); + pg_log(PG_PROGRESS, "reading source file list"); fetchRemoteFileList(); - pg_log(PG_PROGRESS, "reading target file list\n"); + pg_log(PG_PROGRESS, "reading target file list"); traverse_datadir(datadir_target, &process_local_file); /* @@ -261,7 +271,7 @@ main(int argc, char **argv) * XXX: If we supported rewinding a server that was not shut down cleanly, * we would need to replay until the end of WAL here. */ - pg_log(PG_PROGRESS, "reading WAL in target\n"); + pg_log(PG_PROGRESS, "reading WAL in target"); extractPageMap(datadir_target, chkptrec, lastcommontli, ControlFile_target.checkPoint); filemap_finalize(); @@ -278,7 +288,7 @@ main(int argc, char **argv) */ if (showprogress) { - pg_log(PG_PROGRESS, "Need to copy %lu MB (total source directory size is %lu MB)\n", + pg_log(PG_PROGRESS, "Need to copy %lu MB (total source directory size is %lu MB)", (unsigned long) (filemap->fetch_size / (1024 * 1024)), (unsigned long) (filemap->total_size / (1024 * 1024))); @@ -295,7 +305,7 @@ main(int argc, char **argv) progress_report(true); - pg_log(PG_PROGRESS, "\ncreating backup label and updating control file\n"); + pg_log(PG_PROGRESS, "\ncreating backup label and updating control file"); createBackupLabel(chkptredo, chkpttli, chkptrec); /* @@ -323,7 +333,7 @@ main(int argc, char **argv) ControlFile_new.state = DB_IN_ARCHIVE_RECOVERY; updateControlFile(&ControlFile_new); - printf(_("Done!\n")); + pg_log(PG_STATUS, "Done!"); return 0; } @@ -335,7 +345,7 @@ sanityChecks(void) /* Check system_id match */ if (ControlFile_target.system_identifier != ControlFile_source.system_identifier) - pg_fatal("source and target clusters are from different systems\n"); + pg_fatal("source and target clusters are from different systems"); /* check version */ if (ControlFile_target.pg_control_version != PG_CONTROL_VERSION || @@ -343,7 +353,7 @@ sanityChecks(void) ControlFile_target.catalog_version_no != CATALOG_VERSION_NO || ControlFile_source.catalog_version_no != CATALOG_VERSION_NO) { - pg_fatal("clusters are not compatible with this version of pg_rewind\n"); + pg_fatal("clusters are not compatible with this version of pg_rewind"); } /* @@ -353,7 +363,7 @@ sanityChecks(void) if (ControlFile_target.data_checksum_version != PG_DATA_CHECKSUM_VERSION && !ControlFile_target.wal_log_hints) { - pg_fatal("target server need to use either data checksums or \"wal_log_hints = on\"\n"); + pg_fatal("target server need to use either data checksums or \"wal_log_hints = on\""); } /* @@ -363,7 +373,7 @@ sanityChecks(void) * as long as it isn't running at the moment. */ if (ControlFile_target.state != DB_SHUTDOWNED) - pg_fatal("target server must be shut down cleanly\n"); + pg_fatal("target server must be shut down cleanly"); /* * When the source is a data directory, also require that the source @@ -371,7 +381,7 @@ sanityChecks(void) * limitation, but better safe than sorry. */ if (datadir_source && ControlFile_source.state != DB_SHUTDOWNED) - pg_fatal("source data directory must be shut down cleanly\n"); + pg_fatal("source data directory must be shut down cleanly"); } /* @@ -438,7 +448,7 @@ findCommonAncestorTimeline(XLogRecPtr *recptr, TimeLineID *tli) } } - pg_fatal("could not find common ancestor of the source and target cluster's timelines\n"); + pg_fatal("could not find common ancestor of the source and target cluster's timelines"); } @@ -478,7 +488,7 @@ createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, XLogRecPtr checkpo (uint32) (checkpointloc >> 32), (uint32) checkpointloc, strfbuf); if (len >= sizeof(buf)) - pg_fatal("backup label buffer too small\n"); /* shouldn't happen */ + pg_fatal("backup label buffer too small"); /* shouldn't happen */ /* TODO: move old file out of the way, if any. */ open_target_file("backup_label", true); /* BACKUP_LABEL_FILE */ @@ -500,7 +510,7 @@ checkControlFile(ControlFileData *ControlFile) /* And simply compare it */ if (!EQ_CRC32C(crc, ControlFile->crc)) - pg_fatal("unexpected control file CRC\n"); + pg_fatal("unexpected control file CRC"); } /* @@ -510,7 +520,7 @@ static void digestControlFile(ControlFileData *ControlFile, char *src, size_t size) { if (size != PG_CONTROL_SIZE) - pg_fatal("unexpected control file size %d, expected %d\n", + pg_fatal("unexpected control file size %d, expected %d", (int) size, PG_CONTROL_SIZE); memcpy(ControlFile, src, sizeof(ControlFileData)); diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h index e281369..ee7ba13 100644 --- a/src/bin/pg_rewind/pg_rewind.h +++ b/src/bin/pg_rewind/pg_rewind.h @@ -19,6 +19,8 @@ #include "storage/block.h" #include "storage/relfilenode.h" +extern const char *progname; + /* Configuration options */ extern char *datadir_target; extern char *datadir_source; diff --git a/src/bin/pg_rewind/timeline.c b/src/bin/pg_rewind/timeline.c index 07ca370..5547c53 100644 --- a/src/bin/pg_rewind/timeline.c +++ b/src/bin/pg_rewind/timeline.c @@ -9,6 +9,7 @@ */ #include "postgres_fe.h" +#include "logging.h" #include "pg_rewind.h" #include "access/timeline.h" @@ -73,22 +74,15 @@ rewind_parseTimeLineHistory(char *buffer, TimeLineID targetTLI, int *nentries) if (nfields < 1) { /* expect a numeric timeline ID as first field of line */ - printf(_("syntax error in history file: %s\n"), fline); - printf(_("Expected a numeric timeline ID.\n")); - exit(1); + pg_fatal("syntax error in history file: %s\n" + "Expected a numeric timeline ID.", fline); } if (nfields != 3) - { - printf(_("syntax error in history file: %s\n"), fline); - printf(_("Expected an XLOG switchpoint location.\n")); - exit(1); - } + pg_fatal("syntax error in history file: %s\n" + "Expected an XLOG switchpoint location.", fline); if (entries && tli <= lasttli) - { - printf(_("invalid data in history file: %s\n"), fline); - printf(_("Timeline IDs must be in increasing sequence.\n")); - exit(1); - } + pg_fatal("invalid data in history file: %s\n" + "Timeline IDs must be in increasing sequence.", fline); lasttli = tli; @@ -105,11 +99,8 @@ rewind_parseTimeLineHistory(char *buffer, TimeLineID targetTLI, int *nentries) } if (entries && targetTLI <= lasttli) - { - printf(_("invalid data in history file\n")); - printf(_("Timeline IDs must be less than child timeline's ID.\n")); - exit(1); - } + pg_fatal("invalid data in history file\n" + "Timeline IDs must be less than child timeline's ID,"); /* * Create one more entry for the "tip" of the timeline, which has no entry -- 2.3.5
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers