Hint to set owner for tablespace directory

Hi!

I have noticed the novice users are stuck trying to create tablespace
over a directory whose owner is not the system postgres user. They
observed the message "could not set permissions on directory ...:
permission denied".

I want to add patch that prints hint to set required owner for the
tablespace directory if this is the cause of the problem (*errno ==
EPERM* after calling *chmod*).

--
Regards,
Maksim Milyutin

On Thu, Aug 23, 2018 at 02:24:25PM +0300, Maksim Milyutin wrote:
> I want to add patch that prints hint to set required owner for the
> tablespace directory if this is the cause of the problem (*errno == EPERM*
> after calling *chmod*).

Please do not forget to add this patch to the next commit fest.
--
Michael

On 08/24/2018 05:18 AM, Michael Paquier wrote:

> On Thu, Aug 23, 2018 at 02:24:25PM +0300, Maksim Milyutin wrote:
>> I want to add patch that prints hint to set required owner for the
>> tablespace directory if this is the cause of the problem (*errno == EPERM*
>> after calling *chmod*).
> Please do not forget to add this patch to the next commit fest.

Thanks, done.

--
Regards,
Maksim Milyutin

On 23/08/2018 13:24, Maksim Milyutin wrote:
> I have noticed the novice users are stuck trying to create tablespace
> over a directory whose owner is not the system postgres user. They
> observed the message "could not set permissions on directory ...:
> permission denied".
>
> I want to add patch that prints hint to set required owner for the
> tablespace directory if this is the cause of the problem (*errno ==
> EPERM* after calling *chmod*).

I think the hint is backwards. When you don't have permission to chmod
the tablespace directory, you probably want to fix the permissions on
the tablespace directory or its parent. But the hint says to run the
database server as a different user. That's a bit unusual.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Fri, Aug 24, 2018 at 3:05 PM, Maksim Milyutin <milyutinma(at)gmail(dot)com>
wrote:

> On 08/24/2018 05:18 AM, Michael Paquier wrote:
>
> On Thu, Aug 23, 2018 at 02:24:25PM +0300, Maksim Milyutin wrote:
>>
>>> I want to add patch that prints hint to set required owner for the
>>> tablespace directory if this is the cause of the problem (*errno ==
>>> EPERM*
>>> after calling *chmod*).
>>>
>> Please do not forget to add this patch to the next commit fest.
>>
>
> Thanks, done.

Adding to that this patch needs a rebase. And, please don't forget to run
'git diff --check', as it has some white-space issues.

--
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/

On 08/31/2018 11:55 AM, Rafia Sabih wrote:

>
> Adding to that this patch needs a rebase. And, please don't forget to
> run 'git diff --check', as it has some white-space issues.

Hmm, it's odd. Provided patch is fluently applied on the current master
HEAD (2e39f69) and *git diff --check* doesn't print any warnings.

--
Regards, Maksim Milyutin

On Fri, Aug 31, 2018 at 3:30 PM, Maksim Milyutin <milyutinma(at)gmail(dot)com>
wrote:

> On 08/31/2018 11:55 AM, Rafia Sabih wrote:
>
>
> Adding to that this patch needs a rebase. And, please don't forget to run
> 'git diff --check', as it has some white-space issues.
>
>
> git apply /home/edb/Desktop/patches/others/owner_tbsp_hint.patch
/home/edb/Desktop/patches/others/owner_tbsp_hint.patch:9: trailing
whitespace.
{
/home/edb/Desktop/patches/others/owner_tbsp_hint.patch:10: trailing
whitespace.
bool wrong_owner = (errno == EPERM);
/home/edb/Desktop/patches/others/owner_tbsp_hint.patch:11: trailing
whitespace.

/home/edb/Desktop/patches/others/owner_tbsp_hint.patch:16: trailing
whitespace.
location),
/home/edb/Desktop/patches/others/owner_tbsp_hint.patch:17: trailing
whitespace.
wrong_owner ? errhint("Install the PostgreSQL system
user as "
error: patch failed: src/backend/commands/tablespace.c:585
error: src/backend/commands/tablespace.c: patch does not apply

This is what I got. Please correct me if I am missng anything. I am using
centos 7, just an FYI.
--
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/

On 08/31/2018 01:12 PM, Rafia Sabih wrote:

>
>
> On Fri, Aug 31, 2018 at 3:30 PM, Maksim Milyutin <milyutinma(at)gmail(dot)com
> <mailto:milyutinma(at)gmail(dot)com>> wrote:
>
> On 08/31/2018 11:55 AM, Rafia Sabih wrote:
>
>>
>> Adding to that this patch needs a rebase. And, please don't
>> forget to run 'git diff --check', as it has some white-space issues.
>
>  git apply /home/edb/Desktop/patches/others/owner_tbsp_hint.patch
>  /home/edb/Desktop/patches/others/owner_tbsp_hint.patch:9: trailing
> whitespace.
>         {
> /home/edb/Desktop/patches/others/owner_tbsp_hint.patch:10: trailing
> whitespace.
>             bool wrong_owner = (errno == EPERM);

This is hex+ASCII display (from *hexdump -C owner_tbsp_hint.patch*
output) of code fragment above:

000001a0  0a 20 09 09 65 6c 73 65  0a 2b 09 09 7b 0a 2b 09  |.
..else.+..{.+.|
000001b0  09 09 62 6f 6f 6c 20 77  72 6f 6e 67 5f 6f 77 6e |..bool
wrong_own|
000001c0  65 72 20 3d 20 28 65 72  72 6e 6f 20 3d 3d 20 45 |er = (errno
== E|
000001d0  50 45 52 4d 29 3b 0a 2b  0a 20 09 09 09 65 72 65 |PERM);.+.
...ere|

Problem lines don't have trailing whitespaces, only line feed (0x0a) symbols

> This is what I got. Please correct me if I am missng anything. I am
> using centos 7,  just an FYI.

My git version is 2.7.4.

--
Regards, Maksim Milyutin

30.08.2018 19:52, Peter Eisentraut wrote:

> On 23/08/2018 13:24, Maksim Milyutin wrote:
>> I have noticed the novice users are stuck trying to create tablespace
>> over a directory whose owner is not the system postgres user. They
>> observed the message "could not set permissions on directory ...:
>> permission denied".
>>
>> I want to add patch that prints hint to set required owner for the
>> tablespace directory if this is the cause of the problem (*errno ==
>> EPERM* after calling *chmod*).
> I think the hint is backwards. When you don't have permission to chmod
> the tablespace directory, you probably want to fix the permissions on
> the tablespace directory or its parent.

According with man page of chmod(2) when permissions on parent
directories are insufficient the errno takes the EACCES value. However,
in Linux man page of chmod(2) there been mentioned that the process
could be not privileged (does not have the CAP_FOWNER capability) when
errno is EPERM.

> But the hint says to run the database server as a different user. That's a bit unusual.

In this case I propose to:
- replace my initial hint message to the guess what to do if errno ==
EPERM, smth like "You might need to install the PostgreSQL system user
as the owner of this directory";
- add another hint if errno == EACCES: "Fix permissions on the parent
directories".

Any thoughts?

--
Regards,
Maksim Milyutin

Maksim Milyutin <milyutinma(at)gmail(dot)com> writes:
> 30.08.2018 19:52, Peter Eisentraut wrote:
>> I think the hint is backwards. When you don't have permission to chmod
>> the tablespace directory, you probably want to fix the permissions on
>> the tablespace directory or its parent.

> In this case I propose to:
> - replace my initial hint message to the guess what to do if errno ==
> EPERM, smth like "You might need to install the PostgreSQL system user
> as the owner of this directory";
> - add another hint if errno == EACCES: "Fix permissions on the parent
> directories".

I agree with what I think Peter is saying: the hint should just recommend
fixing permissions on the directory, for either errno code. The more
detail you try to give, the more likely the hint will be wrong
... especially when you're dealing with errno codes that aren't all that
well standardized, as these aren't.

regards, tom lane

On Fri, Aug 31, 2018 at 4:11 PM, Maksim Milyutin <milyutinma(at)gmail(dot)com>
wrote:

> On 08/31/2018 01:12 PM, Rafia Sabih wrote:
>
>
>
> On Fri, Aug 31, 2018 at 3:30 PM, Maksim Milyutin <milyutinma(at)gmail(dot)com>
> wrote:
>
>> On 08/31/2018 11:55 AM, Rafia Sabih wrote:
>>
>>
>> Adding to that this patch needs a rebase. And, please don't forget to run
>> 'git diff --check', as it has some white-space issues.
>>
>>
>> git apply /home/edb/Desktop/patches/others/owner_tbsp_hint.patch
> /home/edb/Desktop/patches/others/owner_tbsp_hint.patch:9: trailing
> whitespace.
> {
> /home/edb/Desktop/patches/others/owner_tbsp_hint.patch:10: trailing
> whitespace.
> bool wrong_owner = (errno == EPERM);
>
>
> This is hex+ASCII display (from *hexdump -C owner_tbsp_hint.patch* output)
> of code fragment above:
>
> 000001a0 0a 20 09 09 65 6c 73 65 0a 2b 09 09 7b 0a 2b 09 |.
> ..else.+..{.+.|
> 000001b0 09 09 62 6f 6f 6c 20 77 72 6f 6e 67 5f 6f 77 6e |..bool
> wrong_own|
> 000001c0 65 72 20 3d 20 28 65 72 72 6e 6f 20 3d 3d 20 45 |er = (errno
> == E|
> 000001d0 50 45 52 4d 29 3b 0a 2b 0a 20 09 09 09 65 72 65 |PERM);.+.
> ...ere|
>
> Problem lines don't have trailing whitespaces, only line feed (0x0a)
> symbols
>
> This is what I got. Please correct me if I am missng anything. I am using
> centos 7, just an FYI.
>
>
> My git version is 2.7.4.
>
> I am using version 2.12.1, that might be the reason for the discrepancy.

--
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/

Thanks, that should be helpful.

-----
Currently Junior Marketing Specialist @ CrazyCall
--
Sent from: http://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html

On 08/31/2018 04:59 PM, Tom Lane wrote:

> Maksim Milyutin <milyutinma(at)gmail(dot)com> writes:
>> 30.08.2018 19:52, Peter Eisentraut wrote:
>>> I think the hint is backwards. When you don't have permission to chmod
>>> the tablespace directory, you probably want to fix the permissions on
>>> the tablespace directory or its parent.
>> In this case I propose to:
>> - replace my initial hint message to the guess what to do if errno ==
>> EPERM, smth like "You might need to install the PostgreSQL system user
>> as the owner of this directory";
>> - add another hint if errno == EACCES: "Fix permissions on the parent
>> directories".
> I agree with what I think Peter is saying: the hint should just recommend
> fixing permissions on the directory, for either errno code. The more
> detail you try to give, the more likely the hint will be wrong
> ... especially when you're dealing with errno codes that aren't all that
> well standardized, as these aren't.

OK. However I think it would be helpful to leave the mention about
setting necessary owner for tablespace directory. My final version of
hint is "You might need to fix permissions on this directory or its
parents or install the PostgreSQL system user as the owner of this
directory." And updated version of patch is attached.

--
Regards, Maksim Milyutin

On 07/09/2018 11:59, Maksim Milyutin wrote:
> OK. However I think it would be helpful to leave the mention about
> setting necessary owner for tablespace directory. My final version of
> hint is "You might need to fix permissions on this directory or its
> parents or install the PostgreSQL system user as the owner of this
> directory." And updated version of patch is attached.

You are still proposing to hint that the permissions on the tablespace
directory might be correct but that the main database instance is
running under the wrong user. I don't think that is a plausible
situation. Clearly changing the tablespace directory permissions is
much simpler than changing the entire database setup.

The normal error situation here is that the tablespace directory has
been created in a way that the "postgres" user can't access the way it
wants. I don't see how the existing messaging is insufficient in that case.

If anything, we should look to create some consistency with initdb. It
basically does the same thing when it creates a new data directory. I'm
not aware of a need to add more hints there either.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 09/07/2018 06:05 PM, Peter Eisentraut wrote:

> On 07/09/2018 11:59, Maksim Milyutin wrote:
>> OK. However I think it would be helpful to leave the mention about
>> setting necessary owner for tablespace directory. My final version of
>> hint is "You might need to fix permissions on this directory or its
>> parents or install the PostgreSQL system user as the owner of this
>> directory." And updated version of patch is attached.
> You are still proposing to hint that the permissions on the tablespace
> directory might be correct but that the main database instance is
> running under the wrong user.

Not exactly this way. The case that motivated me to make this patch was
the novice user after installing postgres from package (with setting up
postgres user and initializing PGDATA with right permissions) tried to
create the necessary tablespace directories from himself (the owner of
those directories was that user). The error message "could not set
permissions on directory ..." disoriented that user. The need to change
the owner of those directories came after careful reading of
documentation. I think it would be helpful to show the proposed hint to
more operatively resolve the problem.

--
Regards, Maksim Milyutin

On 07/09/2018 17:59, Maksim Milyutin wrote:
>> You are still proposing to hint that the permissions on the tablespace
>> directory might be correct but that the main database instance is
>> running under the wrong user.
> Not exactly this way. The case that motivated me to make this patch was
> the novice user after installing postgres from package (with setting up
> postgres user and initializing PGDATA with right permissions) tried to
> create the necessary tablespace directories from himself (the owner of
> those directories was that user). The error message "could not set
> permissions on directory ..." disoriented that user. The need to change
> the owner of those directories came after careful reading of
> documentation. I think it would be helpful to show the proposed hint to
> more operatively resolve the problem.

I think it might be worth clarifying the documentation instead. I'm
looking at the CREATE TABLESPACE reference page and it's not super clear
on first reading.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 11/09/2018 17:10, Peter Eisentraut wrote:
> On 07/09/2018 17:59, Maksim Milyutin wrote:
>> those directories was that user). The error message "could not set
>> permissions on directory ..." disoriented that user. The need to change
>> the owner of those directories came after careful reading of
>> documentation. I think it would be helpful to show the proposed hint to
>> more operatively resolve the problem.
>
> I think it might be worth clarifying the documentation instead. I'm
> looking at the CREATE TABLESPACE reference page and it's not super clear
> on first reading.

How about the attached patch?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 24/09/2018 14:50, Peter Eisentraut wrote:
> On 11/09/2018 17:10, Peter Eisentraut wrote:
>> On 07/09/2018 17:59, Maksim Milyutin wrote:
>>> those directories was that user). The error message "could not set
>>> permissions on directory ..." disoriented that user. The need to change
>>> the owner of those directories came after careful reading of
>>> documentation. I think it would be helpful to show the proposed hint to
>>> more operatively resolve the problem.
>>
>> I think it might be worth clarifying the documentation instead. I'm
>> looking at the CREATE TABLESPACE reference page and it's not super clear
>> on first reading.
>
> How about the attached patch?

I have committed this patch and will close this commitfest item. I
don't think the idea with the hints in the code was going anywhere.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

01.10.2018 15:15, Peter Eisentraut wrote:

> On 24/09/2018 14:50, Peter Eisentraut wrote:
>> On 11/09/2018 17:10, Peter Eisentraut wrote:
>>> On 07/09/2018 17:59, Maksim Milyutin wrote:
>>>> those directories was that user). The error message "could not set
>>>> permissions on directory ..." disoriented that user. The need to change
>>>> the owner of those directories came after careful reading of
>>>> documentation. I think it would be helpful to show the proposed hint to
>>>> more operatively resolve the problem.
>>> I think it might be worth clarifying the documentation instead. I'm
>>> looking at the CREATE TABLESPACE reference page and it's not super clear
>>> on first reading.
>> How about the attached patch?
> I have committed this patch and will close this commitfest item. I
> don't think the idea with the hints in the code was going anywhere.
>

Yes, thanks. Sorry for so delayed answer, I was busy. You have clearly
described the workflow to create tablespace (including *chown* command)
and in general it is enough to resolve the issue with  wrong owner of
tablespace directory.

--
Regards,
Maksim Milyutin