On 05/12/2015 10:33 AM, Heikki Linnakangas wrote:
On 05/12/2015 04:42 PM, Andrew Dunstan wrote:
+
+ /*
+ * 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.
+ */
+ 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)));
+ }
+
That's scary. What tar utilitiy replaces the symlink with a non-empty
directory?
What if you call pg_start_backup() and take the backup with a utility
that follows symlinks? I wouldn't recommend using such a tool, but
with this patch, it will zap all the tablespaces. Before this, you at
least got a working database and could read out all the data or fix
the symlinks afterwards.
Why is the RelationCacheInitFileRemove() call moved?
I will let Amit comment on this.
XLogRecPtr
do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID
*starttli_p,
- char **labelfile)
+ char **labelfile, DIR *tblspcdir, List **tablespaces,
+ char **tblspcmapfile, bool infotbssize,
+ bool needtblspcmapfile)
Why does this need to take tblspcdir as argument? Wouldn't it make
more sense to open the directory inside do_pg_start_backup?
Good point. It does look cleaner to do that.
In the BASE_BACKUP command, is there any reason to not always include
the tablespace map? IOW, do we really need the TABLESPACE_MAP option?
I assume it was done for minimal disturbance, i.e. that's where we need
it. Amit, comments?
cheers
andrew
--
Sent via pgsql-committers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
