On Tue, Jan 11, 2022 at 04:41:58PM +0900, Michael Paquier wrote:
> On Sat, Jan 08, 2022 at 12:48:57PM -0600, Justin Pryzby wrote:
> > I fixed it by calling get_restricted_token() before parseCommandLine().
> > There's precedent for that in pg_regress (but the 3 other callers do it
> > differently).
> >
> > It seems more ideal to always call get_restricted_token sooner than later, 
> > but
> > for now I only changed pg_upgrade.  It's probably also better if
> > parseCommandLine() only parses the commandline, but for now I added on to 
> > the
> > logfile stuff that's already there.
>
> Well, the routine does a bit more than just parsing the options as it
> creates the directory infrastructure as well.  As you say, I think
> that it would be better to have the option parsing and the
> loading-into-structure portions in one routine, and the creation of
> the paths in a second one.  So, the new contents of the patch could
> just be moved in a new routine, after getting the restricted token.
> Moving get_restricted_token() before or after the option parsing as
> you do is not a big deal, but your patch is introducing in the
> existing routine more than what's currently done there as of HEAD.

I added mkdir() before the other stuff that messes with logfiles, because it
needs to happen before that.

Are you suggesting to change the pre-existing behavior of when logfiles are
created, like 0002 ?

> > Maybe the commandline argument should be callled something other than 
> > "logdir"
> > since it also outputs dumps there.  But the dumps are more or less not
> > user-facing.  But -d and -o are already used.  Maybe it shouldn't be
> > configurable at all?
> 
> If the choice of a short option becomes confusing, I'd be fine with
> just a long option, but -l is fine IMO.  Including the internal dumps
> in the directory is fine to me, and using a subdir, as you do, makes
> things more organized.
> 
> -             "--binary-upgrade %s -f %s",
> +             "--binary-upgrade %s -f %s/dump/%s",
> Some quotes seem to be missing here.

Yes, good catch

> Is it intentional to not use rmtree() here?  If you put all the data
> in the same directory, cleanup() gets simpler.

There's no reason not to.  We created the dir, and the user didn't specify to
preserve it.  It'd be their fault if they put something valuable there after
starting pg_upgrade.

-- 
Justin
>From 9b228bb1b529d0998340263dcb4238beeccd2ac9 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sat, 11 Dec 2021 19:19:50 -0600
Subject: [PATCH 1/2] pg_upgrade: write log files and dumps in subdir..

The subdir must not exist when pg_upgrade is started.
This avoids appending to logfiles which have pre-existing errors in them.
And allows more easily cleaning up after pg_upgrade.
---
 doc/src/sgml/ref/pgupgrade.sgml | 15 ++++++++++++-
 src/bin/pg_upgrade/.gitignore   |  3 ---
 src/bin/pg_upgrade/Makefile     |  4 +---
 src/bin/pg_upgrade/check.c      | 12 +++++++----
 src/bin/pg_upgrade/dump.c       |  6 ++++--
 src/bin/pg_upgrade/exec.c       |  5 ++++-
 src/bin/pg_upgrade/function.c   |  3 ++-
 src/bin/pg_upgrade/option.c     | 31 +++++++++++++++++++++++----
 src/bin/pg_upgrade/pg_upgrade.c | 38 ++++++++-------------------------
 src/bin/pg_upgrade/pg_upgrade.h |  1 +
 src/bin/pg_upgrade/server.c     |  6 ++++--
 11 files changed, 74 insertions(+), 50 deletions(-)

diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml
index c5ce732ee98..e6d730a2574 100644
--- a/doc/src/sgml/ref/pgupgrade.sgml
+++ b/doc/src/sgml/ref/pgupgrade.sgml
@@ -130,6 +130,19 @@ PostgreSQL documentation
       cluster</para></listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>-l</option></term>
+      <term><option>--logdir</option></term>
+      <listitem><para>Directory where SQL and log files will be written.
+      The directory will be created and must not exist before calling
+      <command>pg_upgrade</command>.
+      It will be removed after a successful upgrade unless
+      <option>--retain</option> is specified.
+      The default is <literal>pg_upgrade_output.d</literal> in the current
+      working directory.
+      </para></listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>-N</option></term>
       <term><option>--no-sync</option></term>
@@ -768,7 +781,7 @@ 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
+   as schema dumps, below the current working directory.  For security, be sure
    that that directory is not readable or writable by any other users.
   </para>
 
diff --git a/src/bin/pg_upgrade/.gitignore b/src/bin/pg_upgrade/.gitignore
index 2d3bfeaa502..7222f8a6b1f 100644
--- a/src/bin/pg_upgrade/.gitignore
+++ b/src/bin/pg_upgrade/.gitignore
@@ -1,9 +1,6 @@
 /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 44d06be5a61..ab21cee3601 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
+	       pg_upgrade_output.d/
 
 # 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 c2f6c57782a..80229f37dec 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -784,7 +784,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++)
@@ -861,7 +862,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 */
@@ -960,7 +962,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 */
@@ -1215,7 +1218,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 953873fa01d..895e4bfe83d 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/dump/%s\"",
 			  new_cluster.bindir, cluster_conn_opts(&old_cluster),
 			  log_opts.verbose ? "--verbose" : "",
+			  log_opts.basedir,
 			  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/dump/%s\" %s",
 						   new_cluster.bindir, cluster_conn_opts(&old_cluster),
 						   log_opts.verbose ? "--verbose" : "",
+						   log_opts.basedir,
 						   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 c0cbb1bec79..5d93d65285d 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/log/%s", log_opts.basedir, 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 e73a3090708..acc97fafef0 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 c894fdd685e..aa4c4d61edf 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -57,6 +57,7 @@ parseCommandLine(int argc, char *argv[])
 		{"socketdir", required_argument, NULL, 's'},
 		{"verbose", no_argument, NULL, 'v'},
 		{"clone", no_argument, NULL, 1},
+		{"logdir", required_argument, NULL, 'l'},
 
 		{NULL, 0, NULL, 0}
 	};
@@ -66,6 +67,7 @@ parseCommandLine(int argc, char *argv[])
 	FILE	   *fp;
 	char	  **filename;
 	time_t		run_time = time(NULL);
+	char		filename_path[MAXPGPATH];
 
 	user_opts.do_sync = true;
 	user_opts.transfer_mode = TRANSFER_MODE_COPY;
@@ -136,6 +138,10 @@ parseCommandLine(int argc, char *argv[])
 				user_opts.transfer_mode = TRANSFER_MODE_LINK;
 				break;
 
+			case 'l':
+				log_opts.basedir = pg_strdup(optarg);
+				break;
+
 			case 'N':
 				user_opts.do_sync = false;
 				break;
@@ -208,8 +214,23 @@ 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.basedir == NULL)
+		log_opts.basedir = "pg_upgrade_output.d";
+
+	if (mkdir(log_opts.basedir, 00700))
+		pg_fatal("could not create directory \"%s\": %m\n", log_opts.basedir);
+
+	snprintf(filename_path, sizeof(filename_path), "%s/log", log_opts.basedir);
+	if (mkdir(filename_path, 00700))
+		pg_fatal("could not create directory \"%s\": %m\n", filename_path);
+
+	snprintf(filename_path, sizeof(filename_path), "%s/dump", log_opts.basedir);
+	if (mkdir(filename_path, 00700))
+		pg_fatal("could not create directory \"%s\": %m\n", filename_path);
+
+	snprintf(filename_path, sizeof(filename_path), "%s/log/%s", log_opts.basedir, 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);
 
 	if (log_opts.verbose)
 		pg_log(PG_REPORT, "Running in verbose mode\n");
@@ -217,8 +238,10 @@ parseCommandLine(int argc, char *argv[])
 	/* 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);
+		snprintf(filename_path, sizeof(filename_path), "%s/log/%s",
+				log_opts.basedir, *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"
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 68b336e0a34..f90cd74be0e 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -84,10 +84,10 @@ main(int argc, char **argv)
 	/* Set default restrictive mask until new cluster permissions are read */
 	umask(PG_MODE_MASK_OWNER);
 
-	parseCommandLine(argc, argv);
-
 	get_restricted_token();
 
+	parseCommandLine(argc, argv);
+
 	adjust_data_dir(&old_cluster);
 	adjust_data_dir(&new_cluster);
 
@@ -305,8 +305,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/dump/%s\"",
 			  new_cluster.bindir, cluster_conn_opts(&new_cluster),
+			  log_opts.basedir,
 			  GLOBALS_DUMP_FILE);
 	check_ok();
 }
@@ -351,10 +352,11 @@ create_new_objects(void)
 				  true,
 				  true,
 				  "\"%s/pg_restore\" %s %s --exit-on-error --verbose "
-				  "--dbname postgres \"%s\"",
+				  "--dbname postgres \"%s/dump/%s\"",
 				  new_cluster.bindir,
 				  cluster_conn_opts(&new_cluster),
 				  create_opts,
+				  log_opts.basedir,
 				  sql_file_name);
 
 		break;					/* done once we've processed template1 */
@@ -388,10 +390,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/dump/%s\"",
 						   new_cluster.bindir,
 						   cluster_conn_opts(&new_cluster),
 						   create_opts,
+						   log_opts.basedir,
 						   sql_file_name);
 	}
 
@@ -688,28 +691,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 dd4a5437316..b06f188fe79 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -269,6 +269,7 @@ typedef struct
 	FILE	   *internal;		/* internal log FILE */
 	bool		verbose;		/* true -> be verbose in messages */
 	bool		retain;			/* retain log files on success */
+	char		*basedir;		/* Dir to create logfiles */
 } LogOpts;
 
 
diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.c
index f5d4656dec8..e2581e0a897 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/log/%s\" -D \"%s\" -o \"-p %d -b%s %s%s\" start",
+			 cluster->bindir,
+			 log_opts.basedir,
+			 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);
-- 
2.17.1

>From 37fe854259fe36b4dce0b1a824e5fa88fa064f9c Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Tue, 11 Jan 2022 13:06:32 -0600
Subject: [PATCH 2/2] f!move the mkdir and logfile stuff to a new function
 instead of rearranging get_restricted_token()

---
 src/bin/pg_upgrade/option.c     | 37 -------------------------
 src/bin/pg_upgrade/pg_upgrade.c | 48 ++++++++++++++++++++++++++++++++-
 2 files changed, 47 insertions(+), 38 deletions(-)

diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index aa4c4d61edf..767935d761f 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
@@ -64,10 +63,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);
-	char		filename_path[MAXPGPATH];
 
 	user_opts.do_sync = true;
 	user_opts.transfer_mode = TRANSFER_MODE_COPY;
@@ -217,41 +212,9 @@ parseCommandLine(int argc, char *argv[])
 	if (log_opts.basedir == NULL)
 		log_opts.basedir = "pg_upgrade_output.d";
 
-	if (mkdir(log_opts.basedir, 00700))
-		pg_fatal("could not create directory \"%s\": %m\n", log_opts.basedir);
-
-	snprintf(filename_path, sizeof(filename_path), "%s/log", log_opts.basedir);
-	if (mkdir(filename_path, 00700))
-		pg_fatal("could not create directory \"%s\": %m\n", filename_path);
-
-	snprintf(filename_path, sizeof(filename_path), "%s/dump", log_opts.basedir);
-	if (mkdir(filename_path, 00700))
-		pg_fatal("could not create directory \"%s\": %m\n", filename_path);
-
-	snprintf(filename_path, sizeof(filename_path), "%s/log/%s", log_opts.basedir, 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);
-
 	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++)
-	{
-		snprintf(filename_path, sizeof(filename_path), "%s/log/%s",
-				log_opts.basedir, *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);
-	}
-
 	/* 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 f90cd74be0e..5ed491f5c23 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -37,6 +37,8 @@
 
 #include "postgres_fe.h"
 
+#include <time.h>
+
 #ifdef HAVE_LANGINFO_H
 #include <langinfo.h>
 #endif
@@ -53,6 +55,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_dirs(void);
 static void setup(char *argv0, bool *live_check);
 static void cleanup(void);
 
@@ -84,9 +87,11 @@ main(int argc, char **argv)
 	/* Set default restrictive mask until new cluster permissions are read */
 	umask(PG_MODE_MASK_OWNER);
 
+	parseCommandLine(argc, argv);
+
 	get_restricted_token();
 
-	parseCommandLine(argc, argv);
+	make_dirs();
 
 	adjust_data_dir(&old_cluster);
 	adjust_data_dir(&new_cluster);
@@ -196,6 +201,47 @@ main(int argc, char **argv)
 	return 0;
 }
 
+static void
+make_dirs(void)
+{
+	FILE	*fp;
+	char	**filename;
+	time_t	run_time = time(NULL);
+	char	filename_path[MAXPGPATH];
+
+	if (mkdir(log_opts.basedir, 00700))
+		pg_fatal("could not create directory \"%s\": %m\n", log_opts.basedir);
+
+	snprintf(filename_path, sizeof(filename_path), "%s/log", log_opts.basedir);
+	if (mkdir(filename_path, 00700))
+		pg_fatal("could not create directory \"%s\": %m\n", filename_path);
+
+	snprintf(filename_path, sizeof(filename_path), "%s/dump", log_opts.basedir);
+	if (mkdir(filename_path, 00700))
+		pg_fatal("could not create directory \"%s\": %m\n", filename_path);
+
+	snprintf(filename_path, sizeof(filename_path), "%s/log/%s", log_opts.basedir, 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/log/%s",
+				log_opts.basedir, *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)
-- 
2.17.1

Reply via email to