diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml index a1745a2642..729c886ac0 100644 --- a/doc/src/sgml/ref/pgupgrade.sgml +++ b/doc/src/sgml/ref/pgupgrade.sgml @@ -768,8 +768,8 @@ psql --username=postgres --file=script.sql postgres <para> <application>pg_upgrade</application> creates various working files, such - as schema dumps, in the current working directory. For security, be sure - that that directory is not readable or writable by any other users. + as schema dumps, stored within <literal>pg_upgrade_output.d</literal> in + the directory of the new cluster. </para> <para> diff --git a/src/bin/pg_upgrade/.gitignore b/src/bin/pg_upgrade/.gitignore index 2d3bfeaa50..939e50db6c 100644 --- a/src/bin/pg_upgrade/.gitignore +++ b/src/bin/pg_upgrade/.gitignore @@ -1,9 +1,7 @@ /pg_upgrade # Generated by test suite -/pg_upgrade_internal.log /delete_old_cluster.sh /delete_old_cluster.bat /reindex_hash.sql -/loadable_libraries.txt /log/ /tmp_check/ diff --git a/src/bin/pg_upgrade/Makefile b/src/bin/pg_upgrade/Makefile index 44d06be5a6..49b94f0ac7 100644 --- a/src/bin/pg_upgrade/Makefile +++ b/src/bin/pg_upgrade/Makefile @@ -45,9 +45,7 @@ uninstall: clean distclean maintainer-clean: rm -f pg_upgrade$(X) $(OBJS) rm -rf delete_old_cluster.sh log/ tmp_check/ \ - loadable_libraries.txt reindex_hash.sql \ - pg_upgrade_dump_globals.sql \ - pg_upgrade_dump_*.custom pg_upgrade_*.log + reindex_hash.sql # When $(MAKE) is present, make automatically infers that this is a # recursive make. which is not actually what we want here, as that diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 3d218c2ad2..019bcb6c7b 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -783,7 +783,8 @@ check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster) return; } - snprintf(output_path, sizeof(output_path), + snprintf(output_path, sizeof(output_path), "%s/%s", + log_opts.basedir, "contrib_isn_and_int8_pass_by_value.txt"); for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++) @@ -860,7 +861,8 @@ check_for_user_defined_postfix_ops(ClusterInfo *cluster) prep_status("Checking for user-defined postfix operators"); - snprintf(output_path, sizeof(output_path), + snprintf(output_path, sizeof(output_path), "%s/%s", + log_opts.basedir, "postfix_ops.txt"); /* Find any user defined postfix operators */ @@ -959,7 +961,8 @@ check_for_tables_with_oids(ClusterInfo *cluster) prep_status("Checking for tables WITH OIDS"); - snprintf(output_path, sizeof(output_path), + snprintf(output_path, sizeof(output_path), "%s/%s", + log_opts.basedir, "tables_with_oids.txt"); /* Find any tables declared WITH OIDS */ @@ -1214,7 +1217,8 @@ check_for_user_defined_encoding_conversions(ClusterInfo *cluster) prep_status("Checking for user-defined encoding conversions"); - snprintf(output_path, sizeof(output_path), + snprintf(output_path, sizeof(output_path), "%s/%s", + log_opts.basedir, "encoding_conversions.txt"); /* Find any user defined encoding conversions */ diff --git a/src/bin/pg_upgrade/dump.c b/src/bin/pg_upgrade/dump.c index 953873fa01..b69b4f9569 100644 --- a/src/bin/pg_upgrade/dump.c +++ b/src/bin/pg_upgrade/dump.c @@ -22,9 +22,10 @@ generate_old_dump(void) /* run new pg_dumpall binary for globals */ exec_prog(UTILITY_LOG_FILE, NULL, true, true, "\"%s/pg_dumpall\" %s --globals-only --quote-all-identifiers " - "--binary-upgrade %s -f %s", + "--binary-upgrade %s -f \"%s/%s\"", new_cluster.bindir, cluster_conn_opts(&old_cluster), log_opts.verbose ? "--verbose" : "", + log_opts.dumpdir, GLOBALS_DUMP_FILE); check_ok(); @@ -52,9 +53,10 @@ generate_old_dump(void) parallel_exec_prog(log_file_name, NULL, "\"%s/pg_dump\" %s --schema-only --quote-all-identifiers " - "--binary-upgrade --format=custom %s --file=\"%s\" %s", + "--binary-upgrade --format=custom %s --file=\"%s/%s\" %s", new_cluster.bindir, cluster_conn_opts(&old_cluster), log_opts.verbose ? "--verbose" : "", + log_opts.dumpdir, sql_file_name, escaped_connstr.data); termPQExpBuffer(&escaped_connstr); diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c index c0cbb1bec7..fadeea12ca 100644 --- a/src/bin/pg_upgrade/exec.c +++ b/src/bin/pg_upgrade/exec.c @@ -78,11 +78,12 @@ get_bin_version(ClusterInfo *cluster) * The code requires it be called first from the primary thread on Windows. */ bool -exec_prog(const char *log_file, const char *opt_log_file, +exec_prog(const char *log_filename, const char *opt_log_file, bool report_error, bool exit_on_error, const char *fmt,...) { int result = 0; int written; + char log_file[MAXPGPATH]; #define MAXCMDLEN (2 * MAXPGPATH) char cmd[MAXCMDLEN]; @@ -97,6 +98,8 @@ exec_prog(const char *log_file, const char *opt_log_file, mainThreadId = GetCurrentThreadId(); #endif + snprintf(log_file, MAXPGPATH, "%s/%s", log_opts.logdir, log_filename); + written = 0; va_start(ap, fmt); written += vsnprintf(cmd + written, MAXCMDLEN - written, fmt, ap); diff --git a/src/bin/pg_upgrade/function.c b/src/bin/pg_upgrade/function.c index e73a309070..ea785df771 100644 --- a/src/bin/pg_upgrade/function.c +++ b/src/bin/pg_upgrade/function.c @@ -128,7 +128,8 @@ check_loadable_libraries(void) prep_status("Checking for presence of required libraries"); - snprintf(output_path, sizeof(output_path), "loadable_libraries.txt"); + snprintf(output_path, sizeof(output_path), "%s/%s", + log_opts.basedir, "loadable_libraries.txt"); /* * Now we want to sort the library names into order. This avoids multiple diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c index c894fdd685..d2c82cc2bb 100644 --- a/src/bin/pg_upgrade/option.c +++ b/src/bin/pg_upgrade/option.c @@ -9,7 +9,6 @@ #include "postgres_fe.h" -#include <time.h> #ifdef WIN32 #include <io.h> #endif @@ -63,9 +62,6 @@ parseCommandLine(int argc, char *argv[]) int option; /* Command line option */ int optindex = 0; /* used by getopt_long */ int os_user_effective_id; - FILE *fp; - char **filename; - time_t run_time = time(NULL); user_opts.do_sync = true; user_opts.transfer_mode = TRANSFER_MODE_COPY; @@ -208,27 +204,9 @@ parseCommandLine(int argc, char *argv[]) if (optind < argc) pg_fatal("too many command-line arguments (first is \"%s\")\n", argv[optind]); - if ((log_opts.internal = fopen_priv(INTERNAL_LOG_FILE, "a")) == NULL) - pg_fatal("could not open log file \"%s\": %m\n", INTERNAL_LOG_FILE); - if (log_opts.verbose) pg_log(PG_REPORT, "Running in verbose mode\n"); - /* label start of upgrade in logfiles */ - for (filename = output_files; *filename != NULL; filename++) - { - if ((fp = fopen_priv(*filename, "a")) == NULL) - pg_fatal("could not write to log file \"%s\": %m\n", *filename); - - /* Start with newline because we might be appending to a file. */ - fprintf(fp, "\n" - "-----------------------------------------------------------------\n" - " pg_upgrade run on %s" - "-----------------------------------------------------------------\n\n", - ctime(&run_time)); - fclose(fp); - } - /* Turn off read-only mode; add prefix to PGOPTIONS? */ if (getenv("PGOPTIONS")) { diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c index 07defacd67..48a54170a7 100644 --- a/src/bin/pg_upgrade/pg_upgrade.c +++ b/src/bin/pg_upgrade/pg_upgrade.c @@ -38,6 +38,8 @@ #include "postgres_fe.h" +#include <time.h> + #ifdef HAVE_LANGINFO_H #include <langinfo.h> #endif @@ -54,6 +56,7 @@ static void prepare_new_globals(void); static void create_new_objects(void); static void copy_xact_xlog_xid(void); static void set_frozenxids(bool minmxid_only); +static void make_outputdirs(char *pgdata); static void setup(char *argv0, bool *live_check); static void cleanup(void); @@ -92,6 +95,22 @@ main(int argc, char **argv) adjust_data_dir(&old_cluster); adjust_data_dir(&new_cluster); + /* + * Set mask based on PGDATA permissions, needed for the creation of the + * output directories with correct permissions. + */ + if (!GetDataDirectoryCreatePerm(new_cluster.pgdata)) + pg_fatal("could not read permissions of directory \"%s\": %s\n", + new_cluster.pgdata, strerror(errno)); + + umask(pg_mode_mask); + + /* + * This needs to happen after adjusting the data directory of the new + * cluster in adjust_data_dir(). + */ + make_outputdirs(new_cluster.pgdata); + setup(argv[0], &live_check); output_check_banner(live_check); @@ -103,13 +122,6 @@ main(int argc, char **argv) check_cluster_compatibility(live_check); - /* Set mask based on PGDATA permissions */ - if (!GetDataDirectoryCreatePerm(new_cluster.pgdata)) - pg_fatal("could not read permissions of directory \"%s\": %s\n", - new_cluster.pgdata, strerror(errno)); - - umask(pg_mode_mask); - check_and_dump_old_cluster(live_check); @@ -197,6 +209,56 @@ main(int argc, char **argv) return 0; } +/* + * Create and assign proper permissions to the set of output directories + * used to store any data generated internally, filling in log_opts in + * the process. + */ +static void +make_outputdirs(char *pgdata) +{ + FILE *fp; + char **filename; + time_t run_time = time(NULL); + char filename_path[MAXPGPATH]; + + log_opts.basedir = (char *) pg_malloc(MAXPGPATH); + snprintf(log_opts.basedir, MAXPGPATH, "%s/%s", pgdata, BASE_OUTPUTDIR); + log_opts.dumpdir = (char *) pg_malloc(MAXPGPATH); + snprintf(log_opts.dumpdir, MAXPGPATH, "%s/%s", pgdata, LOG_OUTPUTDIR); + log_opts.logdir = (char *) pg_malloc(MAXPGPATH); + snprintf(log_opts.logdir, MAXPGPATH, "%s/%s", pgdata, DUMP_OUTPUTDIR); + + if (mkdir(log_opts.basedir, pg_dir_create_mode)) + pg_fatal("could not create directory \"%s\": %m\n", log_opts.basedir); + if (mkdir(log_opts.dumpdir, pg_dir_create_mode)) + pg_fatal("could not create directory \"%s\": %m\n", log_opts.dumpdir); + if (mkdir(log_opts.logdir, pg_dir_create_mode)) + pg_fatal("could not create directory \"%s\": %m\n", log_opts.logdir); + + snprintf(filename_path, sizeof(filename_path), "%s/%s", log_opts.logdir, + INTERNAL_LOG_FILE); + if ((log_opts.internal = fopen_priv(filename_path, "a")) == NULL) + pg_fatal("could not open log file \"%s\": %m\n", filename_path); + + /* label start of upgrade in logfiles */ + for (filename = output_files; *filename != NULL; filename++) + { + snprintf(filename_path, sizeof(filename_path), "%s/%s", + log_opts.logdir, *filename); + if ((fp = fopen_priv(filename_path, "a")) == NULL) + pg_fatal("could not write to log file \"%s\": %m\n", filename_path); + + /* Start with newline because we might be appending to a file. */ + fprintf(fp, "\n" + "-----------------------------------------------------------------\n" + " pg_upgrade run on %s" + "-----------------------------------------------------------------\n\n", + ctime(&run_time)); + fclose(fp); + } +} + static void setup(char *argv0, bool *live_check) @@ -306,8 +368,9 @@ prepare_new_globals(void) prep_status("Restoring global objects in the new cluster"); exec_prog(UTILITY_LOG_FILE, NULL, true, true, - "\"%s/psql\" " EXEC_PSQL_ARGS " %s -f \"%s\"", + "\"%s/psql\" " EXEC_PSQL_ARGS " %s -f \"%s/%s\"", new_cluster.bindir, cluster_conn_opts(&new_cluster), + log_opts.dumpdir, GLOBALS_DUMP_FILE); check_ok(); } @@ -352,10 +415,11 @@ create_new_objects(void) true, true, "\"%s/pg_restore\" %s %s --exit-on-error --verbose " - "--dbname postgres \"%s\"", + "--dbname postgres \"%s/%s\"", new_cluster.bindir, cluster_conn_opts(&new_cluster), create_opts, + log_opts.dumpdir, sql_file_name); break; /* done once we've processed template1 */ @@ -389,10 +453,11 @@ create_new_objects(void) parallel_exec_prog(log_file_name, NULL, "\"%s/pg_restore\" %s %s --exit-on-error --verbose " - "--dbname template1 \"%s\"", + "--dbname template1 \"%s/%s\"", new_cluster.bindir, cluster_conn_opts(&new_cluster), create_opts, + log_opts.dumpdir, sql_file_name); } @@ -689,28 +754,5 @@ cleanup(void) /* Remove dump and log files? */ if (!log_opts.retain) - { - int dbnum; - char **filename; - - for (filename = output_files; *filename != NULL; filename++) - unlink(*filename); - - /* remove dump files */ - unlink(GLOBALS_DUMP_FILE); - - if (old_cluster.dbarr.dbs) - for (dbnum = 0; dbnum < old_cluster.dbarr.ndbs; dbnum++) - { - char sql_file_name[MAXPGPATH], - log_file_name[MAXPGPATH]; - DbInfo *old_db = &old_cluster.dbarr.dbs[dbnum]; - - snprintf(sql_file_name, sizeof(sql_file_name), DB_DUMP_FILE_MASK, old_db->db_oid); - unlink(sql_file_name); - - snprintf(log_file_name, sizeof(log_file_name), DB_DUMP_LOG_FILE_MASK, old_db->db_oid); - unlink(log_file_name); - } - } + rmtree(log_opts.basedir, true); } diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h index 1db8e3f0fb..0aca0a77aa 100644 --- a/src/bin/pg_upgrade/pg_upgrade.h +++ b/src/bin/pg_upgrade/pg_upgrade.h @@ -26,6 +26,14 @@ #define GLOBALS_DUMP_FILE "pg_upgrade_dump_globals.sql" #define DB_DUMP_FILE_MASK "pg_upgrade_dump_%u.custom" +/* + * Base directories that include all the files generated internally, + * from the root path of the new cluster. + */ +#define BASE_OUTPUTDIR "pg_upgrade_output.d" +#define LOG_OUTPUTDIR BASE_OUTPUTDIR "/log" +#define DUMP_OUTPUTDIR BASE_OUTPUTDIR "/dump" + #define DB_DUMP_LOG_FILE_MASK "pg_upgrade_dump_%u.log" #define SERVER_LOG_FILE "pg_upgrade_server.log" #define UTILITY_LOG_FILE "pg_upgrade_utility.log" @@ -262,6 +270,10 @@ typedef struct FILE *internal; /* internal log FILE */ bool verbose; /* true -> be verbose in messages */ bool retain; /* retain log files on success */ + /* Set of internal directories for output files */ + char *basedir; /* Base output directory */ + char *dumpdir; /* Dumps */ + char *logdir; /* Log files */ } LogOpts; diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.c index f5d4656dec..7878d233de 100644 --- a/src/bin/pg_upgrade/server.c +++ b/src/bin/pg_upgrade/server.c @@ -238,8 +238,10 @@ start_postmaster(ClusterInfo *cluster, bool report_and_exit_on_error) * vacuumdb --freeze actually freezes the tuples. */ snprintf(cmd, sizeof(cmd), - "\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" -o \"-p %d -b%s %s%s\" start", - cluster->bindir, SERVER_LOG_FILE, cluster->pgconfig, cluster->port, + "\"%s/pg_ctl\" -w -l \"%s/%s\" -D \"%s\" -o \"-p %d -b%s %s%s\" start", + cluster->bindir, + log_opts.logdir, + SERVER_LOG_FILE, cluster->pgconfig, cluster->port, (cluster == &new_cluster) ? " -c synchronous_commit=off -c fsync=off -c full_page_writes=off -c vacuum_defer_cleanup_age=0" : "", cluster->pgopts ? cluster->pgopts : "", socket_string);