pg_upgrade error checking improvement - Mailing list pgsql-hackers

The attached, applied patch improves pg_upgrade error reporting if the
bin or data directories do not exist or are not directories.  Previously
the error message was not clear.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/exec.c b/contrib/pg_upgrade/exec.c
new file mode 100644
index 7c69175..a8f455d
*** a/contrib/pg_upgrade/exec.c
--- b/contrib/pg_upgrade/exec.c
*************** is_server_running(const char *datadir)
*** 71,77 ****
      if ((fd = open(path, O_RDONLY, 0)) < 0)
      {
          if (errno != ENOENT)
!             pg_log(PG_FATAL, "could not open file \"%s\" for reading\n",
                     path);

          return false;
--- 71,78 ----
      if ((fd = open(path, O_RDONLY, 0)) < 0)
      {
          if (errno != ENOENT)
!             /* issue a warning but continue so we can throw a clearer error later */
!             pg_log(PG_WARNING, "could not open file \"%s\" for reading\n",
                     path);

          return false;
*************** void
*** 94,99 ****
--- 95,102 ----
  verify_directories(void)
  {

+     prep_status("Checking current, bin, and data directories");
+
      if (access(".", R_OK | W_OK
  #ifndef WIN32
      /*
*************** verify_directories(void)
*** 107,126 ****
          pg_log(PG_FATAL,
          "You must have read and write access in the current directory.\n");

-     prep_status("Checking old data directory (%s)", old_cluster.pgdata);
-     check_data_dir(old_cluster.pgdata);
-     check_ok();
-
-     prep_status("Checking old bin directory (%s)", old_cluster.bindir);
      check_bin_dir(&old_cluster);
!     check_ok();
!
!     prep_status("Checking new data directory (%s)", new_cluster.pgdata);
!     check_data_dir(new_cluster.pgdata);
!     check_ok();
!
!     prep_status("Checking new bin directory (%s)", new_cluster.bindir);
      check_bin_dir(&new_cluster);
      check_ok();
  }

--- 110,119 ----
          pg_log(PG_FATAL,
          "You must have read and write access in the current directory.\n");

      check_bin_dir(&old_cluster);
!     check_data_dir(old_cluster.pgdata);
      check_bin_dir(&new_cluster);
+     check_data_dir(new_cluster.pgdata);
      check_ok();
  }

*************** check_data_dir(const char *pg_data)
*** 139,163 ****
  {
      char        subDirName[MAXPGPATH];
      int            subdirnum;
!     const char *requiredSubdirs[] = {"base", "global", "pg_clog",
          "pg_multixact", "pg_subtrans", "pg_tblspc", "pg_twophase",
!     "pg_xlog"};

      for (subdirnum = 0;
           subdirnum < sizeof(requiredSubdirs) / sizeof(requiredSubdirs[0]);
           ++subdirnum)
      {
          struct stat statBuf;
-
          snprintf(subDirName, sizeof(subDirName), "%s/%s", pg_data,
                   requiredSubdirs[subdirnum]);

          if (stat(subDirName, &statBuf) != 0)
              report_status(PG_FATAL, "check for %s failed:  %s\n",
!                           requiredSubdirs[subdirnum], getErrorText(errno));
          else if (!S_ISDIR(statBuf.st_mode))
              report_status(PG_FATAL, "%s is not a directory\n",
!                           requiredSubdirs[subdirnum]);
      }
  }

--- 132,156 ----
  {
      char        subDirName[MAXPGPATH];
      int            subdirnum;
!     /* start check with top-most directory */
!     const char *requiredSubdirs[] = {"", "base", "global", "pg_clog",
          "pg_multixact", "pg_subtrans", "pg_tblspc", "pg_twophase",
!         "pg_xlog"};

      for (subdirnum = 0;
           subdirnum < sizeof(requiredSubdirs) / sizeof(requiredSubdirs[0]);
           ++subdirnum)
      {
          struct stat statBuf;
          snprintf(subDirName, sizeof(subDirName), "%s/%s", pg_data,
                   requiredSubdirs[subdirnum]);

          if (stat(subDirName, &statBuf) != 0)
              report_status(PG_FATAL, "check for %s failed:  %s\n",
!                           subDirName, getErrorText(errno));
          else if (!S_ISDIR(statBuf.st_mode))
              report_status(PG_FATAL, "%s is not a directory\n",
!                           subDirName);
      }
  }

*************** check_data_dir(const char *pg_data)
*** 173,178 ****
--- 166,181 ----
  static void
  check_bin_dir(ClusterInfo *cluster)
  {
+     struct stat statBuf;
+
+     /* check bindir */
+     if (stat(cluster->bindir, &statBuf) != 0)
+         report_status(PG_FATAL, "check for %s failed:  %s\n",
+                       cluster->bindir, getErrorText(errno));
+     else if (!S_ISDIR(statBuf.st_mode))
+             report_status(PG_FATAL, "%s is not a directory\n",
+                           cluster->bindir);
+
      validate_exec(cluster->bindir, "postgres");
      validate_exec(cluster->bindir, "pg_ctl");
      validate_exec(cluster->bindir, "pg_resetxlog");
*************** validate_exec(const char *dir, const cha
*** 211,221 ****
       */
      if (stat(path, &buf) < 0)
          pg_log(PG_FATAL, "check for %s failed - %s\n",
!                cmdName, getErrorText(errno));
!
!     if (!S_ISREG(buf.st_mode))
          pg_log(PG_FATAL, "check for %s failed - not an executable file\n",
!                cmdName);

      /*
       * Ensure that the file is both executable and readable (required for
--- 214,223 ----
       */
      if (stat(path, &buf) < 0)
          pg_log(PG_FATAL, "check for %s failed - %s\n",
!                path, getErrorText(errno));
!     else if (!S_ISREG(buf.st_mode))
          pg_log(PG_FATAL, "check for %s failed - not an executable file\n",
!                path);

      /*
       * Ensure that the file is both executable and readable (required for
*************** validate_exec(const char *dir, const cha
*** 227,233 ****
      if ((buf.st_mode & S_IRUSR) == 0)
  #endif
          pg_log(PG_FATAL, "check for %s failed - cannot read file (permission denied)\n",
!                cmdName);

  #ifndef WIN32
      if (access(path, X_OK) != 0)
--- 229,235 ----
      if ((buf.st_mode & S_IRUSR) == 0)
  #endif
          pg_log(PG_FATAL, "check for %s failed - cannot read file (permission denied)\n",
!                path);

  #ifndef WIN32
      if (access(path, X_OK) != 0)
*************** validate_exec(const char *dir, const cha
*** 235,239 ****
      if ((buf.st_mode & S_IXUSR) == 0)
  #endif
          pg_log(PG_FATAL, "check for %s failed - cannot execute (permission denied)\n",
!                cmdName);
  }
--- 237,241 ----
      if ((buf.st_mode & S_IXUSR) == 0)
  #endif
          pg_log(PG_FATAL, "check for %s failed - cannot execute (permission denied)\n",
!                path);
  }