src/ports/pgcheckdir.c - Ignore dot directories...
Hello. This was bounced my way via IRC[1] and I'm kicking an updated version of the patch upstream for review and committing.
Currently src/port/pgcheckdir.c will reject non-empty directories, which is an issue during initdb(1) when PGDATA is also the mount point for filesystems that support snapshots (e.g. ZFS or UFS2). The original patch to the FreeBSD ports team excluded ".snap", but this seems limited.
Instead, it seems more correct to simply ignore all directories that begin with a dot character. I'm not aware of any special directories exposed by filesystems that aren't dot directories so this seems like a relatively futureproof solution, too.
Granted it's not hard to create a subdirectory, initdb there and move the contents of the files around, it's extra work that shouldn't be required. By UNIX convention, files/directories beginning with a dot are hidden anyway, and since PostgreSQL isn't using or creating any dot files or directories, this seems like the right trade off in usability.
Here's a quick reproduction of the problem along with the patch.
Attachments:
src-port-pgcheckdir.c.patchapplication/octet-stream; name=src-port-pgcheckdir.c.patchDownload
diff --git a/src/port/pgcheckdir.c b/src/port/pgcheckdir.c
index 3b8258c..63b68af 100644
--- a/src/port/pgcheckdir.c
+++ b/src/port/pgcheckdir.c
@@ -41,8 +41,7 @@ pg_check_dir(const char *dir)
while ((file = readdir(chkdir)) != NULL)
{
- if (strcmp(".", file->d_name) == 0 ||
- strcmp("..", file->d_name) == 0)
+ if (file->d_name[0] == '.' && file->d_type == DT_DIR)
{
/* skip this and parent directory */
continue;
On 02/05/2013 04:36 PM, Sean Chittenden wrote:
Hello. This was bounced my way via IRC[1] and I'm kicking an updated version of the patch upstream for review and committing.
Instead, it seems more correct to simply ignore all directories that begin with a dot character. I'm not aware of any special directories exposed by filesystems that aren't dot directories so this seems like a relatively futureproof solution, too.
lost+found
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Sean Chittenden <sean@chittenden.org> wrote:
Currently src/port/pgcheckdir.c will reject non-empty
directories, which is an issue during initdb(1) when PGDATA is
also the mount point for filesystems that support snapshots (e.g.
ZFS or UFS2).
Granted it's not hard to create a subdirectory, initdb there and
move the contents of the files around, it's extra work that
shouldn't be required.
I feel that it is very bad practice to use the mount point as the
PGDATA directory. It forcloses a lot of reasonable actions that
someone managing the database server might want to take.
It's hard to get enthusiastic about a patch to make bad practice
more convenient. I would rather add a sentence or two to the
initdb documentation recommending that a cluster not be created at
a mount point; it should be created in a directory underneath the
mount point.
-Kevin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hello. This was bounced my way via IRC[1] and I'm kicking an updated version of the patch upstream for review and committing.
Instead, it seems more correct to simply ignore all directories that begin with a dot character. I'm not aware of any special directories exposed by filesystems that aren't dot directories so this seems like a relatively futureproof solution, too.
lost+found
It's been a long time since I've seen that directory. Patch updated. -sc
--
Sean Chittenden
sean@chittenden.org
Attachments:
src-port-pgcheckdir.c.patchapplication/octet-stream; name=src-port-pgcheckdir.c.patchDownload
diff --git a/src/port/pgcheckdir.c b/src/port/pgcheckdir.c
index 3b8258c..f396739 100644
--- a/src/port/pgcheckdir.c
+++ b/src/port/pgcheckdir.c
@@ -41,8 +41,9 @@ pg_check_dir(const char *dir)
while ((file = readdir(chkdir)) != NULL)
{
- if (strcmp(".", file->d_name) == 0 ||
- strcmp("..", file->d_name) == 0)
+ if (file->d_type == DT_DIR && (
+ (file->d_name[0] == '.' ||
+ strcmp(file->d_name, "lost+found") == 0)))
{
/* skip this and parent directory */
continue;
On 02/05/2013 07:32 AM, Kevin Grittner wrote:
Sean Chittenden <sean@chittenden.org> wrote:
Currently src/port/pgcheckdir.c will reject non-empty
directories, which is an issue during initdb(1) when PGDATA is
also the mount point for filesystems that support snapshots (e.g.
ZFS or UFS2).
Granted it's not hard to create a subdirectory, initdb there and
move the contents of the files around, it's extra work that
shouldn't be required.I feel that it is very bad practice to use the mount point as the
PGDATA directory. It forcloses a lot of reasonable actions that
someone managing the database server might want to take.It's hard to get enthusiastic about a patch to make bad practice
more convenient. I would rather add a sentence or two to the
initdb documentation recommending that a cluster not be created at
a mount point; it should be created in a directory underneath the
mount point.
I tend to agree.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2/5/13 7:32 AM, Kevin Grittner wrote:
Sean Chittenden <sean@chittenden.org> wrote:
Currently src/port/pgcheckdir.c will reject non-empty
directories, which is an issue during initdb(1) when PGDATA is
also the mount point for filesystems that support snapshots (e.g.
ZFS or UFS2).
Granted it's not hard to create a subdirectory, initdb there and
move the contents of the files around, it's extra work that
shouldn't be required.I feel that it is very bad practice to use the mount point as the
PGDATA directory. It forcloses a lot of reasonable actions that
someone managing the database server might want to take.
Yes, a variant of this particular patch gets rejected about once every
18 months.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 02/05/2013 08:32 PM, Kevin Grittner wrote:
I would rather add a sentence or two to the
initdb documentation recommending that a cluster not be created at
a mount point; it should be created in a directory underneath the
mount point.
That makes a great deal of sense, actually. There's no meaningful
advantage to creating the cluster at the mountpoint root, and even if
you wanted to you could (on Linux) use bind mounts to make it at the
root of a mount without being at the root of a filesystem.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 5 February 2013 13:50, Craig Ringer <craig@2ndquadrant.com> wrote:
On 02/05/2013 08:32 PM, Kevin Grittner wrote:
I would rather add a sentence or two to the
initdb documentation recommending that a cluster not be created at
a mount point; it should be created in a directory underneath the
mount point.That makes a great deal of sense, actually. There's no meaningful
advantage to creating the cluster at the mountpoint root, and even if
you wanted to you could (on Linux) use bind mounts to make it at the
root of a mount without being at the root of a filesystem.
How about we allow Sean's patch insomuch as it can detect files
beginning with dots and throw an error message explaining what the
best practice is instead.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Currently src/port/pgcheckdir.c will reject non-empty
directories, which is an issue during initdb(1) when PGDATA is
also the mount point for filesystems that support snapshots (e.g.
ZFS or UFS2).Granted it's not hard to create a subdirectory, initdb there and
move the contents of the files around, it's extra work that
shouldn't be required.I feel that it is very bad practice to use the mount point as the
PGDATA directory. It forcloses a lot of reasonable actions that
someone managing the database server might want to take.
The only reason I have skin in this game is because pg_check_dir(xlog_dir) is called and I routinely have to work around that nuisance/nugget of joy. Again, just a nuisance.
It's common to create a ZFS dataset for specific applications (`zfs send ... | ssh zfs receive ...`). In SAN environments, mounting a LUN as PGDATA or pg_xlog isn't uncommon either.
I agree it's not ideal for some filesystems, but being overly protective doesn't buy us much either, because in some setups, it's entirely acceptable. If PostgreSQL had the ability to chroot(2) itself, I'd be very opposed to this patch, but as is, it's mostly harmless (the rev that didn't have lost+found, actually).
In thinking about it, I like ignoring the hidden directories and failing when lost+found is present because, IMO, filesystems where lost+found is going to be present are exactly the filesystems that shouldn't have PGDATA located at the top of a mount point.
Personally I'm a fan of having PGDATA's parent directory be its own dataset/filesystem as well, but that's because I want PGDATA's parent directory to include PostgreSQL's minor version number (e.g. zpool datasets: tank/pg tank/pg/data tank/pg/data/9.2), but I digress.
It's hard to get enthusiastic about a patch to make bad practice
more convenient. I would rather add a sentence or two to the
initdb documentation recommending that a cluster not be created at
a mount point; it should be created in a directory underneath the
mount point.
Giving filesystem advice is a large topic that I'm sure is covered someplace in the handbook. A general warning isn't a bad idea, however. *shrug*
Regardless, hopefully some of this is of interest to someone.
-sc
--
Sean Chittenden
sean@chittenden.org
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Sean Chittenden wrote:
In thinking about it, I like ignoring the hidden directories and failing when lost+found is present
because, IMO, filesystems where lost+found is going to be present are exactly the filesystems that
shouldn't have PGDATA located at the top of a mount point.
Huh? What's wrong with ext3 or ext4?
Yours,
Laurenz Albe
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Sean Chittenden <sean@chittenden.org> writes:
I agree it's not ideal for some filesystems, but being overly protective doesn't buy us much either, because in some setups, it's entirely acceptable.
No, it isn't. As several people have told you already, the idea of
letting a mount point be used directly as a data directory has been
suggested repeatedly, and rejected repeatedly, and this time is not
going to be any different. (Although I agree with Kevin that it's
about time we documented why not to do this.)
There are a couple of reasons why it's not good practice:
* mount-point directories really ought to be owned by root, or at least
by some user with more privilege than a DB server ought to have
* without a sub-directory, there's no simple cross-check to enforce that
the mount has actually happened. It's happened before that people have
had a server start up against a slow-to-mount NFS directory, and then
get completely confused when the mount did happen and the visible
database files got replaced. (The really nasty variants of this require
a startup script that will try to initdb automatically if it doesn't see
a database there.)
That's just what I can remember off the top of my head with insufficient
caffeine. If you check the archives for previous discussions you might
find some other good points.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Feb 5, 2013 at 08:49:17AM -0500, Peter Eisentraut wrote:
On 2/5/13 7:32 AM, Kevin Grittner wrote:
Sean Chittenden <sean@chittenden.org> wrote:
Currently src/port/pgcheckdir.c will reject non-empty
directories, which is an issue during initdb(1) when PGDATA is
also the mount point for filesystems that support snapshots (e.g.
ZFS or UFS2).
Granted it's not hard to create a subdirectory, initdb there and
move the contents of the files around, it's extra work that
shouldn't be required.I feel that it is very bad practice to use the mount point as the
PGDATA directory. It forcloses a lot of reasonable actions that
someone managing the database server might want to take.Yes, a variant of this particular patch gets rejected about once every
18 months.
Agreed. The attached patch modifies pg_check_dir() to report about
invisible and lost+found directory entries, and give more helpful
messages to the user.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
Attachments:
mount.difftext/x-diff; charset=us-asciiDownload
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
new file mode 100644
index 2ea3f6e..03eadcd
*** a/src/bin/initdb/initdb.c
--- b/src/bin/initdb/initdb.c
*************** setup_signals(void)
*** 3144,3150 ****
void
create_data_directory(void)
{
! switch (pg_check_dir(pg_data))
{
case 0:
/* PGDATA not there, must create it */
--- 3144,3152 ----
void
create_data_directory(void)
{
! int ret;
!
! switch ((ret = pg_check_dir(pg_data)))
{
case 0:
/* PGDATA not there, must create it */
*************** create_data_directory(void)
*** 3179,3193 ****
break;
case 2:
/* Present and not empty */
fprintf(stderr,
_("%s: directory \"%s\" exists but is not empty\n"),
progname, pg_data);
! fprintf(stderr,
! _("If you want to create a new database system, either remove or empty\n"
! "the directory \"%s\" or run %s\n"
! "with an argument other than \"%s\".\n"),
! pg_data, progname, pg_data);
exit(1); /* no further message needed */
default:
--- 3181,3205 ----
break;
case 2:
+ case 3:
+ case 4:
/* Present and not empty */
fprintf(stderr,
_("%s: directory \"%s\" exists but is not empty\n"),
progname, pg_data);
! if (ret == 2)
! fprintf(stderr,
! _("It contains a dot-prefixed/invisible file.\n"));
! else if (ret == 3)
! fprintf(stderr,
! _("It contains a lost+found directory.\n"
! "Using the top-level directory of a mount point is not recommended.\n"));
! if (ret != 3)
! fprintf(stderr,
! _("If you want to create a new database system, either remove or empty\n"
! "the directory \"%s\" or run %s\n"
! "with an argument other than \"%s\".\n"),
! pg_data, progname, pg_data);
exit(1); /* no further message needed */
default:
*************** create_xlog_symlink(void)
*** 3206,3211 ****
--- 3218,3224 ----
if (strcmp(xlog_dir, "") != 0)
{
char *linkloc;
+ int ret;
/* clean up xlog directory name, check it's absolute */
canonicalize_path(xlog_dir);
*************** create_xlog_symlink(void)
*** 3216,3222 ****
}
/* check if the specified xlog directory exists/is empty */
! switch (pg_check_dir(xlog_dir))
{
case 0:
/* xlog directory not there, must create it */
--- 3229,3235 ----
}
/* check if the specified xlog directory exists/is empty */
! switch ((ret = pg_check_dir(xlog_dir)))
{
case 0:
/* xlog directory not there, must create it */
*************** create_xlog_symlink(void)
*** 3255,3268 ****
break;
case 2:
/* Present and not empty */
fprintf(stderr,
_("%s: directory \"%s\" exists but is not empty\n"),
progname, xlog_dir);
! fprintf(stderr,
! _("If you want to store the transaction log there, either\n"
! "remove or empty the directory \"%s\".\n"),
! xlog_dir);
exit_nicely();
default:
--- 3268,3291 ----
break;
case 2:
+ case 3:
+ case 4:
/* Present and not empty */
fprintf(stderr,
_("%s: directory \"%s\" exists but is not empty\n"),
progname, xlog_dir);
! if (ret == 2)
! fprintf(stderr,
! _("It contains a dot-prefixed/invisible file.\n"));
! else if (ret == 3)
! fprintf(stderr,
! _("It contains a lost+found directory.\n"
! "Using the top-level directory of a mount point is not recommended.\n"));
! if (ret != 3)
! fprintf(stderr,
! _("If you want to store the transaction log there, either\n"
! "remove or empty the directory \"%s\".\n"),
! xlog_dir);
exit_nicely();
default:
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
new file mode 100644
index b6f7744..fb5a1bd
*** a/src/bin/pg_basebackup/pg_basebackup.c
--- b/src/bin/pg_basebackup/pg_basebackup.c
*************** verify_dir_is_empty_or_create(char *dirn
*** 371,376 ****
--- 371,378 ----
*/
return;
case 2:
+ case 3:
+ case 4:
/*
* Exists, not empty
diff --git a/src/port/pgcheckdir.c b/src/port/pgcheckdir.c
new file mode 100644
index 3b8258c..9d6f216
*** a/src/port/pgcheckdir.c
--- b/src/port/pgcheckdir.c
*************** pg_check_dir(const char *dir)
*** 47,55 ****
/* skip this and parent directory */
continue;
}
else
{
! result = 2; /* not empty */
break;
}
}
--- 47,68 ----
/* skip this and parent directory */
continue;
}
+ #ifndef WIN32
+ /* file starts with "." */
+ else if (file->d_name[0] == '.')
+ {
+ result = 2; /* not empty, invisible file */
+ break;
+ }
+ else if (strcmp("lost+found", file->d_name) == 0)
+ {
+ result = 3; /* not empty, mount point */
+ break;
+ }
+ #endif
else
{
! result = 4; /* not empty */
break;
}
}
Bruce Momjian <bruce@momjian.us> writes:
Agreed. The attached patch modifies pg_check_dir() to report about
invisible and lost+found directory entries, and give more helpful
messages to the user.
I'm not terribly thrilled with special-casing 'lost+found' like that,
since it's an extremely filesystem-dependent thing that even today
probably only applies to a minority of our installed platforms.
The special case for dotfiles might be useful, not because of any
connection to mount points but just because someone might forget
that such could be lurking in a directory that "looks empty".
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Feb 14, 2013 at 07:21:27PM -0500, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
Agreed. The attached patch modifies pg_check_dir() to report about
invisible and lost+found directory entries, and give more helpful
messages to the user.I'm not terribly thrilled with special-casing 'lost+found' like that,
since it's an extremely filesystem-dependent thing that even today
probably only applies to a minority of our installed platforms.The special case for dotfiles might be useful, not because of any
connection to mount points but just because someone might forget
that such could be lurking in a directory that "looks empty".
Yeah, I agree on both points. I am not sure the patch is worth it just
the dot output.
Want a crazy idea? '.' and '..' have different major device numbers on
the top directory of a mount point. We could test for that and
prevent/warn about creating data directories on top-level directories of
mount points.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Feb 14, 2013 at 07:21:27PM -0500, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
Agreed. The attached patch modifies pg_check_dir() to report about
invisible and lost+found directory entries, and give more helpful
messages to the user.I'm not terribly thrilled with special-casing 'lost+found' like that,
since it's an extremely filesystem-dependent thing that even today
probably only applies to a minority of our installed platforms.The special case for dotfiles might be useful, not because of any
connection to mount points but just because someone might forget
that such could be lurking in a directory that "looks empty".
I was ready to give up on this patch, but then I thought, what
percentage does lost+found and dot-file-only directories cover for mount
points? What other cases are there?
This updated version of the patch reports about dot files if they are
the _only_ files in the directory, and it suggests a top-level mount
point might be the cause.
Does this help?
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
Attachments:
mount.difftext/x-diff; charset=us-asciiDownload
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
new file mode 100644
index 2ea3f6e..b8faf9c
*** a/src/bin/initdb/initdb.c
--- b/src/bin/initdb/initdb.c
*************** void setup_signals(void);
*** 257,262 ****
--- 257,263 ----
void setup_text_search(void);
void create_data_directory(void);
void create_xlog_symlink(void);
+ void warn_on_mount_point(int error);
void initialize_data_directory(void);
*************** setup_signals(void)
*** 3144,3150 ****
void
create_data_directory(void)
{
! switch (pg_check_dir(pg_data))
{
case 0:
/* PGDATA not there, must create it */
--- 3145,3153 ----
void
create_data_directory(void)
{
! int ret;
!
! switch ((ret = pg_check_dir(pg_data)))
{
case 0:
/* PGDATA not there, must create it */
*************** create_data_directory(void)
*** 3179,3193 ****
break;
case 2:
/* Present and not empty */
fprintf(stderr,
_("%s: directory \"%s\" exists but is not empty\n"),
progname, pg_data);
! fprintf(stderr,
! _("If you want to create a new database system, either remove or empty\n"
! "the directory \"%s\" or run %s\n"
! "with an argument other than \"%s\".\n"),
! pg_data, progname, pg_data);
exit(1); /* no further message needed */
default:
--- 3182,3201 ----
break;
case 2:
+ case 3:
+ case 4:
/* Present and not empty */
fprintf(stderr,
_("%s: directory \"%s\" exists but is not empty\n"),
progname, pg_data);
! if (ret != 4)
! warn_on_mount_point(ret);
! else
! fprintf(stderr,
! _("If you want to create a new database system, either remove or empty\n"
! "the directory \"%s\" or run %s\n"
! "with an argument other than \"%s\".\n"),
! pg_data, progname, pg_data);
exit(1); /* no further message needed */
default:
*************** create_xlog_symlink(void)
*** 3206,3211 ****
--- 3214,3220 ----
if (strcmp(xlog_dir, "") != 0)
{
char *linkloc;
+ int ret;
/* clean up xlog directory name, check it's absolute */
canonicalize_path(xlog_dir);
*************** create_xlog_symlink(void)
*** 3216,3222 ****
}
/* check if the specified xlog directory exists/is empty */
! switch (pg_check_dir(xlog_dir))
{
case 0:
/* xlog directory not there, must create it */
--- 3225,3231 ----
}
/* check if the specified xlog directory exists/is empty */
! switch ((ret = pg_check_dir(xlog_dir)))
{
case 0:
/* xlog directory not there, must create it */
*************** create_xlog_symlink(void)
*** 3255,3268 ****
break;
case 2:
/* Present and not empty */
fprintf(stderr,
_("%s: directory \"%s\" exists but is not empty\n"),
progname, xlog_dir);
! fprintf(stderr,
! _("If you want to store the transaction log there, either\n"
! "remove or empty the directory \"%s\".\n"),
! xlog_dir);
exit_nicely();
default:
--- 3264,3282 ----
break;
case 2:
+ case 3:
+ case 4:
/* Present and not empty */
fprintf(stderr,
_("%s: directory \"%s\" exists but is not empty\n"),
progname, xlog_dir);
! if (ret != 4)
! warn_on_mount_point(ret);
! else
! fprintf(stderr,
! _("If you want to store the transaction log there, either\n"
! "remove or empty the directory \"%s\".\n"),
! xlog_dir);
exit_nicely();
default:
*************** create_xlog_symlink(void)
*** 3291,3296 ****
--- 3305,3325 ----
}
+ void
+ warn_on_mount_point(int error)
+ {
+ if (error == 2)
+ fprintf(stderr,
+ _("It contains a dot-prefixed/invisible file, perhaps due to it being a mount point.\n"));
+ else if (error == 3)
+ fprintf(stderr,
+ _("It contains a lost+found directory, perhaps due to it being a mount point.\n"));
+
+ fprintf(stderr,
+ _("Using the top-level directory of a mount point is not recommended.\n"));
+ }
+
+
void
initialize_data_directory(void)
{
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
new file mode 100644
index b6f7744..fb5a1bd
*** a/src/bin/pg_basebackup/pg_basebackup.c
--- b/src/bin/pg_basebackup/pg_basebackup.c
*************** verify_dir_is_empty_or_create(char *dirn
*** 371,376 ****
--- 371,378 ----
*/
return;
case 2:
+ case 3:
+ case 4:
/*
* Exists, not empty
diff --git a/src/port/pgcheckdir.c b/src/port/pgcheckdir.c
new file mode 100644
index 3b8258c..aee5997
*** a/src/port/pgcheckdir.c
--- b/src/port/pgcheckdir.c
*************** pg_check_dir(const char *dir)
*** 31,36 ****
--- 31,37 ----
int result = 1;
DIR *chkdir;
struct dirent *file;
+ bool dot_found = false;
errno = 0;
*************** pg_check_dir(const char *dir)
*** 47,61 ****
/* skip this and parent directory */
continue;
}
else
{
! result = 2; /* not empty */
break;
}
}
#ifdef WIN32
-
/*
* This fix is in mingw cvs (runtime/mingwex/dirent.c rev 1.4), but not in
* released version
--- 48,73 ----
/* skip this and parent directory */
continue;
}
+ #ifndef WIN32
+ /* file starts with "." */
+ else if (file->d_name[0] == '.')
+ {
+ dot_found = true;
+ }
+ else if (strcmp("lost+found", file->d_name) == 0)
+ {
+ result = 3; /* not empty, mount point */
+ break;
+ }
+ #endif
else
{
! result = 4; /* not empty */
break;
}
}
#ifdef WIN32
/*
* This fix is in mingw cvs (runtime/mingwex/dirent.c rev 1.4), but not in
* released version
*************** pg_check_dir(const char *dir)
*** 69,73 ****
--- 81,89 ----
if (errno != 0)
result = -1; /* some kind of I/O error? */
+ /* We report on dot-files if we _only_ find dot files */
+ if (result == 1 && dot_found)
+ result = 2;
+
return result;
}
On Fri, Feb 15, 2013 at 12:12:03PM -0500, Bruce Momjian wrote:
On Thu, Feb 14, 2013 at 07:21:27PM -0500, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
Agreed. The attached patch modifies pg_check_dir() to report about
invisible and lost+found directory entries, and give more helpful
messages to the user.I'm not terribly thrilled with special-casing 'lost+found' like that,
since it's an extremely filesystem-dependent thing that even today
probably only applies to a minority of our installed platforms.The special case for dotfiles might be useful, not because of any
connection to mount points but just because someone might forget
that such could be lurking in a directory that "looks empty".I was ready to give up on this patch, but then I thought, what
percentage does lost+found and dot-file-only directories cover for mount
points? What other cases are there?This updated version of the patch reports about dot files if they are
the _only_ files in the directory, and it suggests a top-level mount
point might be the cause.Does this help?
Applied.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers