From fbcfa90bb8ceb77f7763b6ae16bcbb777849ccee Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Tue, 7 Aug 2012 13:10:44 -0400 Subject: [PATCH] Fix pg_upgrade file share violation on Windows created by the commit 4741e9afb93f0d769655b2d18c2b73b86f281010. This was done by adding an optional second log file parameter to exec_prog(), and closing and reopening the log file between system() calls. Backpatch to 9.2. --- contrib/pg_upgrade/check.c | 2 +- contrib/pg_upgrade/dump.c | 2 +- contrib/pg_upgrade/exec.c | 31 ++++++++++++++++++++++++------- contrib/pg_upgrade/pg_upgrade.c | 16 ++++++++-------- contrib/pg_upgrade/pg_upgrade.h | 6 +++--- contrib/pg_upgrade/server.c | 9 ++++----- 6 files changed, 41 insertions(+), 25 deletions(-) diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c index 71d8f7514d..aa896b5823 100644 --- a/contrib/pg_upgrade/check.c +++ b/contrib/pg_upgrade/check.c @@ -183,7 +183,7 @@ issue_warnings(char *sequence_script_file_name) if (sequence_script_file_name) { prep_status("Adjusting sequences"); - exec_prog(true, true, UTILITY_LOG_FILE, + exec_prog(true, true, UTILITY_LOG_FILE, NULL, SYSTEMQUOTE "\"%s/psql\" --echo-queries " "--set ON_ERROR_STOP=on " "--no-psqlrc --port %d --username \"%s\" " diff --git a/contrib/pg_upgrade/dump.c b/contrib/pg_upgrade/dump.c index 571792b1d4..07a3b548a9 100644 --- a/contrib/pg_upgrade/dump.c +++ b/contrib/pg_upgrade/dump.c @@ -23,7 +23,7 @@ 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, + 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, diff --git a/contrib/pg_upgrade/exec.c b/contrib/pg_upgrade/exec.c index 0d6fb8d792..6f993df53a 100644 --- a/contrib/pg_upgrade/exec.c +++ b/contrib/pg_upgrade/exec.c @@ -33,18 +33,19 @@ static int win32_check_directory_write_permissions(void); * line to be executed is saved to the specified log file. * * If throw_error is TRUE, this function will throw a PG_FATAL error - * instead of returning should an error occur. + * instead of returning should an error occur. The command it appended + * to log_file; opt_log_file is used in error messages. */ int -exec_prog(bool throw_error, bool is_priv, - const char *log_file, const char *fmt,...) +exec_prog(bool throw_error, bool is_priv, const char *log_file, + const char *opt_log_file, const char *fmt,...) { va_list args; int result; int retval; char cmd[MAXPGPATH]; mode_t old_umask = 0; - FILE *log = fopen(log_file, "a+"); + FILE *log; if (is_priv) old_umask = umask(S_IRWXG | S_IRWXO); @@ -53,9 +54,15 @@ exec_prog(bool throw_error, bool is_priv, vsnprintf(cmd, MAXPGPATH, fmt, args); va_end(args); + 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); - fflush(log); + /* + * 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. + */ + fclose(log); result = system(cmd); @@ -64,18 +71,28 @@ exec_prog(bool throw_error, bool is_priv, 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\" for\n" + "Consult the last few lines of \"%s\"%s for\n" "the probable cause of the failure.\n", - log_file); + log_file, opt_string); retval = 1; } 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); diff --git a/contrib/pg_upgrade/pg_upgrade.c b/contrib/pg_upgrade/pg_upgrade.c index 9d26b8cf02..eff1a0872f 100644 --- a/contrib/pg_upgrade/pg_upgrade.c +++ b/contrib/pg_upgrade/pg_upgrade.c @@ -140,7 +140,7 @@ 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, + exec_prog(true, true, UTILITY_LOG_FILE, NULL, SYSTEMQUOTE "\"%s/pg_resetxlog\" -o %u \"%s\" >> \"%s\" 2>&1" SYSTEMQUOTE, new_cluster.bindir, old_cluster.controldata.chkpnt_nxtoid, @@ -211,7 +211,7 @@ 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, + exec_prog(true, true, UTILITY_LOG_FILE, NULL, SYSTEMQUOTE "\"%s/vacuumdb\" --port %d --username \"%s\" " "--all --analyze %s >> \"%s\" 2>&1" SYSTEMQUOTE, new_cluster.bindir, new_cluster.port, os_info.user, @@ -225,7 +225,7 @@ prepare_new_cluster(void) * later. */ prep_status("Freezing all rows on the new cluster"); - exec_prog(true, true, UTILITY_LOG_FILE, + exec_prog(true, true, UTILITY_LOG_FILE, NULL, SYSTEMQUOTE "\"%s/vacuumdb\" --port %d --username \"%s\" " "--all --freeze %s >> \"%s\" 2>&1" SYSTEMQUOTE, new_cluster.bindir, new_cluster.port, os_info.user, @@ -263,7 +263,7 @@ prepare_new_databases(void) * support functions in template1 but pg_dumpall creates database using * the template0 template. */ - exec_prog(true, true, RESTORE_LOG_FILE, + exec_prog(true, true, RESTORE_LOG_FILE, NULL, SYSTEMQUOTE "\"%s/psql\" --echo-queries " "--set ON_ERROR_STOP=on " /* --no-psqlrc prevents AUTOCOMMIT=off */ @@ -296,7 +296,7 @@ create_new_objects(void) check_ok(); prep_status("Restoring database schema to new cluster"); - exec_prog(true, true, RESTORE_LOG_FILE, + exec_prog(true, true, RESTORE_LOG_FILE, NULL, SYSTEMQUOTE "\"%s/psql\" --echo-queries " "--set ON_ERROR_STOP=on " "--no-psqlrc --port %d --username \"%s\" " @@ -331,7 +331,7 @@ copy_subdir_files(char *subdir) prep_status("Copying old %s to new server", subdir); - exec_prog(true, false, UTILITY_LOG_FILE, + exec_prog(true, false, UTILITY_LOG_FILE, NULL, #ifndef WIN32 SYSTEMQUOTE "%s \"%s\" \"%s\" >> \"%s\" 2>&1" SYSTEMQUOTE, "cp -Rf", @@ -353,7 +353,7 @@ 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, + exec_prog(true, true, UTILITY_LOG_FILE, NULL, SYSTEMQUOTE "\"%s/pg_resetxlog\" -f -x %u \"%s\" >> \"%s\" 2>&1" SYSTEMQUOTE, new_cluster.bindir, @@ -363,7 +363,7 @@ copy_clog_xlog_xid(void) /* now reset the wal archives in the new cluster */ prep_status("Resetting WAL archives"); - exec_prog(true, true, UTILITY_LOG_FILE, + exec_prog(true, true, UTILITY_LOG_FILE, NULL, SYSTEMQUOTE "\"%s/pg_resetxlog\" -l %s \"%s\" >> \"%s\" 2>&1" SYSTEMQUOTE, new_cluster.bindir, diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h index 9a231764df..affee7a9d9 100644 --- a/contrib/pg_upgrade/pg_upgrade.h +++ b/contrib/pg_upgrade/pg_upgrade.h @@ -317,9 +317,9 @@ void split_old_dump(void); /* exec.c */ int -exec_prog(bool throw_error, bool is_priv, - const char *log_file, const char *cmd,...) -__attribute__((format(PG_PRINTF_ATTRIBUTE, 4, 5))); +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))); 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 57f09d4d4a..e94a897c92 100644 --- a/contrib/pg_upgrade/server.c +++ b/contrib/pg_upgrade/server.c @@ -171,11 +171,10 @@ start_postmaster(ClusterInfo *cluster) * 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, + 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) ? - SERVER_LOG_FILE : - SERVER_LOG_FILE " or " SERVER_START_LOG_FILE, + (strcmp(SERVER_LOG_FILE, SERVER_START_LOG_FILE) != 0) ? + SERVER_LOG_FILE : NULL, "%s", cmd); /* Check to see if we can connect to the server; if not, report it. */ @@ -220,7 +219,7 @@ stop_postmaster(bool fast) cluster->pgopts ? cluster->pgopts : "", fast ? "-m fast" : "", SERVER_STOP_LOG_FILE); - exec_prog(fast ? false : true, true, SERVER_STOP_LOG_FILE, "%s", cmd); + exec_prog(fast ? false : true, true, SERVER_STOP_LOG_FILE, NULL, "%s", cmd); os_info.running_cluster = NULL; } -- 2.40.0