diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c index aa896b5..0fec73e 100644 --- a/contrib/pg_upgrade/check.c +++ b/contrib/pg_upgrade/check.c @@ -183,13 +183,10 @@ issue_warnings(char *sequence_script_file_name) if (sequence_script_file_name) { prep_status("Adjusting sequences"); - exec_prog(true, true, UTILITY_LOG_FILE, NULL, - SYSTEMQUOTE "\"%s/psql\" --echo-queries " - "--set ON_ERROR_STOP=on " - "--no-psqlrc --port %d --username \"%s\" " - "-f \"%s\" --dbname template1 >> \"%s\" 2>&1" SYSTEMQUOTE, + exec_prog(UTILITY_LOG_FILE, NULL, true, + "\"%s/psql\" " EXEC_PSQL_ARGS " --port %d --username \"%s\" -f \"%s\"", new_cluster.bindir, new_cluster.port, os_info.user, - sequence_script_file_name, UTILITY_LOG_FILE); + sequence_script_file_name); unlink(sequence_script_file_name); check_ok(); } diff --git a/contrib/pg_upgrade/dump.c b/contrib/pg_upgrade/dump.c index 07a3b54..cfc4017 100644 --- a/contrib/pg_upgrade/dump.c +++ b/contrib/pg_upgrade/dump.c @@ -23,12 +23,11 @@ generate_old_dump(void) * --binary-upgrade records the width of dropped columns in pg_class, and * restores the frozenid's for databases and relations. */ - exec_prog(true, true, UTILITY_LOG_FILE, NULL, - SYSTEMQUOTE "\"%s/pg_dumpall\" --port %d --username \"%s\" " - "--schema-only --binary-upgrade %s > \"%s\" 2>> \"%s\"" - SYSTEMQUOTE, new_cluster.bindir, old_cluster.port, os_info.user, + exec_prog(UTILITY_LOG_FILE, NULL, true, + "\"%s/pg_dumpall\" --port %d --username \"%s\" --schema-only --binary-upgrade %s -f %s", + new_cluster.bindir, old_cluster.port, os_info.user, log_opts.verbose ? "--verbose" : "", - ALL_DUMP_FILE, UTILITY_LOG_FILE); + ALL_DUMP_FILE); check_ok(); } diff --git a/contrib/pg_upgrade/exec.c b/contrib/pg_upgrade/exec.c index 6f993df..45fbea0 100644 --- a/contrib/pg_upgrade/exec.c +++ b/contrib/pg_upgrade/exec.c @@ -26,77 +26,77 @@ static int win32_check_directory_write_permissions(void); /* * exec_prog() + * Execute an external program with stdout/stderr redirected, and report + * errors * - * Formats a command from the given argument list and executes that - * command. If the command executes, exec_prog() returns 1 otherwise - * exec_prog() logs an error message and returns 0. Either way, the command - * line to be executed is saved to the specified log file. + * Formats a command from the given argument list, logs it to the log file, + * and attempts to execute that command. If the command executes + * successfully, exec_prog() returns true. * - * If throw_error is TRUE, this function will throw a PG_FATAL error - * instead of returning should an error occur. The command it appended - * to log_file; opt_log_file is used in error messages. + * If the command fails, an error message is saved to the specified log_file. + * If throw_error is true, this raises a PG_FATAL error and pg_upgrade + * terminates; otherwise it is just reported as PG_REPORT and exec_prog() + * returns false. */ -int -exec_prog(bool throw_error, bool is_priv, const char *log_file, - const char *opt_log_file, const char *fmt,...) +bool +exec_prog(const char *log_file, const char *opt_log_file, + bool throw_error, const char *fmt,...) { - va_list args; int result; - int retval; - char cmd[MAXPGPATH]; + int written; +#define MAXCMDLEN (2 * MAXPGPATH) + char cmd[MAXCMDLEN]; mode_t old_umask = 0; FILE *log; + va_list ap; - if (is_priv) - old_umask = umask(S_IRWXG | S_IRWXO); + old_umask = umask(S_IRWXG | S_IRWXO); - va_start(args, fmt); - vsnprintf(cmd, MAXPGPATH, fmt, args); - va_end(args); + written = strlcat(cmd, SYSTEMQUOTE, strlen(SYSTEMQUOTE)); + va_start(ap, fmt); + written += vsnprintf(cmd + written, MAXCMDLEN - written, fmt, ap); + va_end(ap); + snprintf(cmd + written, MAXCMDLEN - written, + " >> \"%s\" 2>&1" SYSTEMQUOTE, log_file); if ((log = fopen_priv(log_file, "a+")) == NULL) pg_log(PG_FATAL, "cannot write to log file %s\n", log_file); pg_log(PG_VERBOSE, "%s\n", cmd); fprintf(log, "command: %s\n", cmd); + /* - * In Windows, we must close then reopen the log file so the file is - * not open while the command is running, or we get a share violation. + * In Windows, we must close the log file at this point so the file is not + * open while the command is running, or we get a share violation. */ fclose(log); result = system(cmd); - if (is_priv) - umask(old_umask); + umask(old_umask); if (result != 0) { - char opt_string[MAXPGPATH]; - - /* Create string for optional second log file */ - if (opt_log_file) - snprintf(opt_string, sizeof(opt_string), " or \"%s\"", opt_log_file); - else - opt_string[0] = '\0'; - report_status(PG_REPORT, "*failure*"); fflush(stdout); pg_log(PG_VERBOSE, "There were problems executing \"%s\"\n", cmd); - pg_log(throw_error ? PG_FATAL : PG_REPORT, - "Consult the last few lines of \"%s\"%s for\n" - "the probable cause of the failure.\n", - log_file, opt_string); - retval = 1; + if (opt_log_file) + pg_log(throw_error ? PG_FATAL : PG_REPORT, + "Consult the last few lines of \"%s\" or \"%s\" for\n" + "the probable cause of the failure.\n", + log_file, opt_log_file); + else + pg_log(throw_error ? PG_FATAL : PG_REPORT, + "Consult the last few lines of \"%s\" for\n" + "the probable cause of the failure.\n", + log_file); } - else - retval = 0; if ((log = fopen_priv(log_file, "a+")) == NULL) pg_log(PG_FATAL, "cannot write to log file %s\n", log_file); fprintf(log, "\n\n"); fclose(log); - return retval; + return result == 0; } diff --git a/contrib/pg_upgrade/file.c b/contrib/pg_upgrade/file.c index 5509670..a5d92c6 100644 --- a/contrib/pg_upgrade/file.c +++ b/contrib/pg_upgrade/file.c @@ -103,10 +103,10 @@ copyAndUpdateFile(pageCnvCtx *pageConverter, /* * linkAndUpdateFile() * - * Creates a symbolic link between the given relation files. We use + * Creates a hard link between the given relation files. We use * this function to perform a true in-place update. If the on-disk * format of the new cluster is bit-for-bit compatible with the on-disk - * format of the old cluster, we can simply symlink each relation + * format of the old cluster, we can simply link each relation * instead of copying the data from the old cluster to the new cluster. */ const char * diff --git a/contrib/pg_upgrade/pg_upgrade.c b/contrib/pg_upgrade/pg_upgrade.c index eff1a08..c47c8bb 100644 --- a/contrib/pg_upgrade/pg_upgrade.c +++ b/contrib/pg_upgrade/pg_upgrade.c @@ -140,11 +140,10 @@ main(int argc, char **argv) * because there is no need to have the schema load use new oids. */ prep_status("Setting next OID for new cluster"); - exec_prog(true, true, UTILITY_LOG_FILE, NULL, - SYSTEMQUOTE "\"%s/pg_resetxlog\" -o %u \"%s\" >> \"%s\" 2>&1" - SYSTEMQUOTE, + exec_prog(UTILITY_LOG_FILE, NULL, true, + "\"%s/pg_resetxlog\" -o %u \"%s\"", new_cluster.bindir, old_cluster.controldata.chkpnt_nxtoid, - new_cluster.pgdata, UTILITY_LOG_FILE); + new_cluster.pgdata); check_ok(); create_script_for_cluster_analyze(&analyze_script_file_name); @@ -211,11 +210,10 @@ prepare_new_cluster(void) * --analyze so autovacuum doesn't update statistics later */ prep_status("Analyzing all rows in the new cluster"); - exec_prog(true, true, UTILITY_LOG_FILE, NULL, - SYSTEMQUOTE "\"%s/vacuumdb\" --port %d --username \"%s\" " - "--all --analyze %s >> \"%s\" 2>&1" SYSTEMQUOTE, + exec_prog(UTILITY_LOG_FILE, NULL, true, + "\"%s/vacuumdb\" --port %d --username \"%s\" --all --analyze %s", new_cluster.bindir, new_cluster.port, os_info.user, - log_opts.verbose ? "--verbose" : "", UTILITY_LOG_FILE); + log_opts.verbose ? "--verbose" : ""); check_ok(); /* @@ -225,11 +223,10 @@ prepare_new_cluster(void) * later. */ prep_status("Freezing all rows on the new cluster"); - exec_prog(true, true, UTILITY_LOG_FILE, NULL, - SYSTEMQUOTE "\"%s/vacuumdb\" --port %d --username \"%s\" " - "--all --freeze %s >> \"%s\" 2>&1" SYSTEMQUOTE, + exec_prog(UTILITY_LOG_FILE, NULL, true, + "\"%s/vacuumdb\" --port %d --username \"%s\" --all --freeze %s", new_cluster.bindir, new_cluster.port, os_info.user, - log_opts.verbose ? "--verbose" : "", UTILITY_LOG_FILE); + log_opts.verbose ? "--verbose" : ""); check_ok(); get_pg_database_relfilenode(&new_cluster); @@ -263,14 +260,10 @@ prepare_new_databases(void) * support functions in template1 but pg_dumpall creates database using * the template0 template. */ - exec_prog(true, true, RESTORE_LOG_FILE, NULL, - SYSTEMQUOTE "\"%s/psql\" --echo-queries " - "--set ON_ERROR_STOP=on " - /* --no-psqlrc prevents AUTOCOMMIT=off */ - "--no-psqlrc --port %d --username \"%s\" " - "-f \"%s\" --dbname template1 >> \"%s\" 2>&1" SYSTEMQUOTE, + exec_prog(RESTORE_LOG_FILE, NULL, true, + "\"%s/psql\" " EXEC_PSQL_ARGS " --port %d --username \"%s\" -f \"%s\"", new_cluster.bindir, new_cluster.port, os_info.user, - GLOBALS_DUMP_FILE, RESTORE_LOG_FILE); + GLOBALS_DUMP_FILE); check_ok(); /* we load this to get a current list of databases */ @@ -296,13 +289,10 @@ create_new_objects(void) check_ok(); prep_status("Restoring database schema to new cluster"); - exec_prog(true, true, RESTORE_LOG_FILE, NULL, - SYSTEMQUOTE "\"%s/psql\" --echo-queries " - "--set ON_ERROR_STOP=on " - "--no-psqlrc --port %d --username \"%s\" " - "-f \"%s\" --dbname template1 >> \"%s\" 2>&1" SYSTEMQUOTE, + exec_prog(RESTORE_LOG_FILE, NULL, true, + "\"%s/psql\" " EXEC_PSQL_ARGS " --port %d --username \"%s\" -f \"%s\"", new_cluster.bindir, new_cluster.port, os_info.user, - DB_DUMP_FILE, RESTORE_LOG_FILE); + DB_DUMP_FILE); check_ok(); /* regenerate now that we have objects in the databases */ @@ -331,16 +321,14 @@ copy_subdir_files(char *subdir) prep_status("Copying old %s to new server", subdir); - exec_prog(true, false, UTILITY_LOG_FILE, NULL, + exec_prog(UTILITY_LOG_FILE, NULL, true, #ifndef WIN32 - SYSTEMQUOTE "%s \"%s\" \"%s\" >> \"%s\" 2>&1" SYSTEMQUOTE, - "cp -Rf", + "cp -Rf \"%s\" \"%s\"", #else /* flags: everything, no confirm, quiet, overwrite read-only */ - SYSTEMQUOTE "%s \"%s\" \"%s\\\" >> \"%s\" 2>&1" SYSTEMQUOTE, - "xcopy /e /y /q /r", + "xcopy /e /y /q /r \"%s\" \"%s\\\"", #endif - old_path, new_path, UTILITY_LOG_FILE); + old_path, new_path); check_ok(); } @@ -353,22 +341,18 @@ copy_clog_xlog_xid(void) /* set the next transaction id of the new cluster */ prep_status("Setting next transaction ID for new cluster"); - exec_prog(true, true, UTILITY_LOG_FILE, NULL, - SYSTEMQUOTE - "\"%s/pg_resetxlog\" -f -x %u \"%s\" >> \"%s\" 2>&1" - SYSTEMQUOTE, new_cluster.bindir, - old_cluster.controldata.chkpnt_nxtxid, - new_cluster.pgdata, UTILITY_LOG_FILE); + exec_prog(UTILITY_LOG_FILE, NULL, true, + "\"%s/pg_resetxlog\" -f -x %u \"%s\"", + new_cluster.bindir, old_cluster.controldata.chkpnt_nxtxid, + new_cluster.pgdata); check_ok(); /* now reset the wal archives in the new cluster */ prep_status("Resetting WAL archives"); - exec_prog(true, true, UTILITY_LOG_FILE, NULL, - SYSTEMQUOTE - "\"%s/pg_resetxlog\" -l %s \"%s\" >> \"%s\" 2>&1" - SYSTEMQUOTE, new_cluster.bindir, + exec_prog(UTILITY_LOG_FILE, NULL, true, + "\"%s/pg_resetxlog\" -l %s \"%s\"", new_cluster.bindir, old_cluster.controldata.nextxlogfile, - new_cluster.pgdata, UTILITY_LOG_FILE); + new_cluster.pgdata); check_ok(); } diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h index affee7a..5ee9a34 100644 --- a/contrib/pg_upgrade/pg_upgrade.h +++ b/contrib/pg_upgrade/pg_upgrade.h @@ -316,10 +316,10 @@ void split_old_dump(void); /* exec.c */ -int -exec_prog(bool throw_error, bool is_priv, const char *log_file, - const char *opt_log_file, const char *cmd,...) -__attribute__((format(PG_PRINTF_ATTRIBUTE, 5, 6))); +#define EXEC_PSQL_ARGS "--echo-queries --set ON_ERROR_STOP=on --no-psqlrc --dbname=template1" +bool exec_prog(const char *log_file, const char *opt_log_file, + bool throw_error, const char *fmt,...) +__attribute__((format(PG_PRINTF_ATTRIBUTE, 4, 5))); void verify_directories(void); bool is_server_running(const char *datadir); diff --git a/contrib/pg_upgrade/server.c b/contrib/pg_upgrade/server.c index e94a897..8f2d3a6 100644 --- a/contrib/pg_upgrade/server.c +++ b/contrib/pg_upgrade/server.c @@ -143,7 +143,7 @@ start_postmaster(ClusterInfo *cluster) char cmd[MAXPGPATH]; PGconn *conn; bool exit_hook_registered = false; - int pg_ctl_return = 0; + bool pg_ctl_return = false; if (!exit_hook_registered) { @@ -159,22 +159,23 @@ start_postmaster(ClusterInfo *cluster) * not touch them. */ snprintf(cmd, sizeof(cmd), - SYSTEMQUOTE "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" " - "-o \"-p %d %s %s\" start >> \"%s\" 2>&1" SYSTEMQUOTE, + "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" -o \"-p %d %s %s\" start", cluster->bindir, SERVER_LOG_FILE, cluster->pgconfig, cluster->port, (cluster->controldata.cat_ver >= BINARY_UPGRADE_SERVER_FLAG_CAT_VER) ? "-b" : "-c autovacuum=off -c autovacuum_freeze_max_age=2000000000", - cluster->pgopts ? cluster->pgopts : "", SERVER_START_LOG_FILE); + cluster->pgopts ? cluster->pgopts : ""); /* * Don't throw an error right away, let connecting throw the error because * it might supply a reason for the failure. */ - pg_ctl_return = exec_prog(false, true, SERVER_START_LOG_FILE, - /* pass both file names if the differ */ - (strcmp(SERVER_LOG_FILE, SERVER_START_LOG_FILE) != 0) ? + pg_ctl_return = exec_prog(SERVER_START_LOG_FILE, + /* pass both file names if the differ */ + (strcmp(SERVER_LOG_FILE, + SERVER_START_LOG_FILE) != 0) ? SERVER_LOG_FILE : NULL, + false, "%s", cmd); /* Check to see if we can connect to the server; if not, report it. */ @@ -191,7 +192,7 @@ start_postmaster(ClusterInfo *cluster) PQfinish(conn); /* If the connection didn't fail, fail now */ - if (pg_ctl_return != 0) + if (!pg_ctl_return) pg_log(PG_FATAL, "pg_ctl failed to start the %s server, or connection failed\n", CLUSTER_NAME(cluster)); @@ -202,7 +203,6 @@ start_postmaster(ClusterInfo *cluster) void stop_postmaster(bool fast) { - char cmd[MAXPGPATH]; ClusterInfo *cluster; if (os_info.running_cluster == &old_cluster) @@ -212,14 +212,11 @@ stop_postmaster(bool fast) else return; /* no cluster running */ - snprintf(cmd, sizeof(cmd), - SYSTEMQUOTE "\"%s/pg_ctl\" -w -D \"%s\" -o \"%s\" " - "%s stop >> \"%s\" 2>&1" SYSTEMQUOTE, - cluster->bindir, cluster->pgconfig, - cluster->pgopts ? cluster->pgopts : "", - fast ? "-m fast" : "", SERVER_STOP_LOG_FILE); - - exec_prog(fast ? false : true, true, SERVER_STOP_LOG_FILE, NULL, "%s", cmd); + exec_prog(SERVER_STOP_LOG_FILE, NULL, !fast, + "\"%s/pg_ctl\" -w -D \"%s\" -o \"%s\" %s stop", + cluster->bindir, cluster->pgconfig, + cluster->pgopts ? cluster->pgopts : "", + fast ? "-m fast" : ""); os_info.running_cluster = NULL; }