Re: PATCH: Configurable file mode mask

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: David Steele <david(at)pgmasters(dot)net>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Adam Brightwell <adam(dot)brightwell(at)crunchydata(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PATCH: Configurable file mode mask
Date: 2018-03-15 07:17:39
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, Mar 14, 2018 at 02:08:19PM -0400, David Steele wrote:
> OK, that being the case a new patch set is attached that sets the mode
> of the same as other files in PGDATA.


> I also cleaned up / corrected / added comments in various places.

Patches 1 and 2 look fine to me. The new umask calls in pg_rewind and
pg_resetwal could be split into a separate patch but that's a minor

When taking a base backup from a data folder which has group access,
then the tar data, as well as the untar'ed data, are still using
0600 as umask for files and 0700 for folders. Is that an expected
behavior? I would have imagined that sendFileWithContent() and
_tarWriteDir() should enforce the file mode to have group access if the
cluster has been initialized to work as such. Still as this is a
feature aimed at being used for custom backups, that's not really a
blocker I guess. Visibly there would be no need for a -g switch in
pg_basebackup as it is possible to guess from the received untar'ed
files what should be the permissions of the data based on what is
received in pg_basebackup.c. It would also be necessary to change the
permissions of pg_wal as this is created before receiving any files.

Speaking of which, we may want to switch the values used for st_mode to
what file_perm.h is giving in basebackup.c?

We should also replace the hardcoded 0700 value in pg_backup_directory.c
by what file_perm.h offers? I would recommend to not touch at mkdtemp.c
as this comes from NetBSD.

+=item $node->group_access()
+Does the data dir allow group access?
Nit: s/dir/directory/.

Indentation is weird in for some of the chmod calls
(tabs not spaces please).

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2018-03-15 07:46:19 Re: Transform for pl/perl
Previous Message Michael Paquier 2018-03-15 06:21:10 Re: PATCH: Configurable file mode mask