On 06/04/2015 09:23 AM, Amit Kapila wrote:
Okay, as we both seem to agree that it can be mostly used in
tablespace symlinks context, so I have changed the name to
remove_tablespace_symlink() and moved the function to
tablespace.c. S_ISLINK check is used for non-windows code,
so not sure adding it here makes any real difference now that
we have made it specific to tablespace and we might need to
write small port specific code if we want to add S_ISLINK check.
Where is it used? I can't see it called at all in tablespace.c or xlog.c.
Perhaps I'm being overcautious, but here's more or less what I had in mind.
cheers
andrew
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 666fa37..8d75d0c 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -38,6 +38,7 @@
#include "catalog/catversion.h"
#include "catalog/pg_control.h"
#include "catalog/pg_database.h"
+#include "commands/tablespace.h"
#include "miscadmin.h"
#include "pgstat.h"
#include "postmaster/bgwriter.h"
@@ -6094,7 +6095,6 @@ StartupXLOG(void)
if (read_tablespace_map(&tablespaces))
{
ListCell *lc;
- struct stat st;
foreach(lc, tablespaces)
{
@@ -6105,27 +6105,9 @@ StartupXLOG(void)
/*
* Remove the existing symlink if any and Create the symlink
- * under PGDATA. We need to use rmtree instead of rmdir as
- * the link location might contain directories or files
- * corresponding to the actual path. Some tar utilities do
- * things that way while extracting symlinks.
+ * under PGDATA.
*/
- if (lstat(linkloc, &st) == 0 && S_ISDIR(st.st_mode))
- {
- if (!rmtree(linkloc, true))
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not remove directory \"%s\": %m",
- linkloc)));
- }
- else
- {
- if (unlink(linkloc) < 0 && errno != ENOENT)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not remove symbolic link \"%s\": %m",
- linkloc)));
- }
+ remove_tablespace_symlink(linkloc);
if (symlink(ti->path, linkloc) < 0)
ereport(ERROR,
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 4ec1aff..e8acf27 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -627,31 +627,9 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid)
/*
* In recovery, remove old symlink, in case it points to the wrong place.
- *
- * On Windows, junction points act like directories so we must be able to
- * apply rmdir; in general it seems best to make this code work like the
- * symlink removal code in destroy_tablespace_directories, except that
- * failure to remove is always an ERROR.
*/
if (InRecovery)
- {
- if (lstat(linkloc, &st) == 0 && S_ISDIR(st.st_mode))
- {
- if (rmdir(linkloc) < 0)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not remove directory \"%s\": %m",
- linkloc)));
- }
- else
- {
- if (unlink(linkloc) < 0 && errno != ENOENT)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not remove symbolic link \"%s\": %m",
- linkloc)));
- }
- }
+ remove_tablespace_symlink(linkloc);
/*
* Create the symlink under PGDATA
@@ -848,6 +826,61 @@ directory_is_empty(const char *path)
return true;
}
+/*
+ * remove_tablespace_symlink
+ *
+ * This function removes symlinks in pg_tblspc. On Windows, junction points
+ * act like directories so we must be able to apply rmdir. This function
+ * works like the symlink removal code in destroy_tablespace_directories,
+ * except that failure to remove is always an ERROR.
+ */
+void
+remove_tablespace_symlink(const char *linkloc)
+{
+ struct stat st;
+
+ if (lstat(linkloc, &st) != 0)
+ {
+ if (errno == ENOENT)
+ return;
+ else
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not stat \"%s\": %m",
+ linkloc)));
+ }
+
+ if (S_ISDIR(st.st_mode))
+ {
+ /*
+ * This will fail if the directory isn't empty, but not
+ * if it's a junction point.
+ */
+ if (rmdir(linkloc) < 0)
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not remove directory \"%s\": %m",
+ linkloc)));
+ }
+#ifdef S_ISLNK
+ else if (S_ISLNK(st.st_mode))
+ {
+ if (unlink(linkloc) < 0 && errno != ENOENT)
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not remove symbolic link \"%s\": %m",
+ linkloc)));
+ }
+#endif
+ else
+ {
+ /* Refuse to remove anything that's not a directory or symlink */
+ ereport(ERROR,
+ (ERRCODE_SYSTEM_ERROR,
+ errmsg("Not a directory or symbolic link: \"%s\"",
+ linkloc)));
+ }
+}
/*
* Rename a tablespace
diff --git a/src/include/commands/tablespace.h b/src/include/commands/tablespace.h
index 86b0477..6b928a5 100644
--- a/src/include/commands/tablespace.h
+++ b/src/include/commands/tablespace.h
@@ -56,6 +56,7 @@ extern Oid get_tablespace_oid(const char *tablespacename, bool missing_ok);
extern char *get_tablespace_name(Oid spc_oid);
extern bool directory_is_empty(const char *path);
+extern void remove_tablespace_symlink(const char *linkloc);
extern void tblspc_redo(XLogReaderState *rptr);
extern void tblspc_desc(StringInfo buf, XLogReaderState *rptr);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
