On Thu, Nov 14, 2013 at 9:08 PM, Haribabu kommi <haribabu.ko...@huawei.com> wrote: > Please find attached the patch, for adding a new option for pg_basebackup, > to specify a different directory for pg_xlog.
Sounds good! Here are the review comments: + printf(_(" --xlogdir=XLOGDIR location for the transaction log directory\n")); This message is not aligned well. - if (!streamwal || strcmp(filename + strlen(filename) - 8, "/pg_xlog") != 0) + if ((!streamwal && (strcmp(xlog_dir, "") == 0)) + || strcmp(filename + strlen(filename) - 8, "/pg_xlog") != 0) You need to update the source code comment. +#ifdef HAVE_SYMLINK + if (symlink(xlog_dir, linkloc) != 0) + { + fprintf(stderr, _("%s: could not create symbolic link \"%s\": %s\n"), + progname, linkloc, strerror(errno)); + exit(1); + } +#else + fprintf(stderr, _("%s: symlinks are not supported on this platform " + "cannot use xlogdir")); + exit(1); +#endif + } Is it better to call pg_free() at the end? Even if we don't do that, it would be almost harmless, though. Don't we need to prevent users from specifying the same directory in both --pgdata and --xlogdir? Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers