pg_upgrade failure on Windows Server
Hi,
It is been observed that pg_upgrade fail to upgrade on Windows Server
machine if being run as Administrator user (other local users works fine),
It fail with the following error message i.e.
""c:\pg93\bin/pg_ctl" -w -l "pg_upgrade_server.log" -D "c:\pgdata93" -o
"-p 3333 -b -c autovacuum_multixact_freeze_max_age=2000000000 -c
synchronous_commit=off -c fsync=off -c full_page_writes=off" start >>
"pg_upgrade_server_start.log" 2>&1"
*failure*
There were problems executing """c:\pg93\bin/pg_ctl" -w -l
"pg_upgrade_server.log" -D "c:\pgdata93" -o "-p 3333 -b -c
autovacuum_multixact_freeze_max_age=2000000000 -c synchronous_commit=off
-c fsync=off -c full_page_writes=off" start >>
"pg_upgrade_server_start.log" 2>&1""
Consult the last few lines of "pg_upgrade_server_start.log" or
"pg_upgrade_server.log" for
the probable cause of the failure.
connection to database failed: could not connect to server: Connection
refused (0x0000274D/10061)
Is the server running on host "localhost" (127.0.0.1) and accepting
TCP/IP connections on port 3333?could not connect to new postmaster started with the command:
"c:\pg93\bin/pg_ctl" -w -l "pg_upgrade_server.log" -D "c:\pgdata93" -o "-p
3333 -b -c autovacuum_multixact_freeze_max_age=2000000000 -c
synchronous_commit=off -c fsync=off -c full_page_writes=off" start
Failure, exiting
pg_upgrade_server.log
PANIC: could not open control file "global/pg_control": Permission denied
"global/pg_control” file in data directory owned by
"BUILTIN\Administrators” that was owned by “Administrator” user before
pg_upgrade. I have verified it on Windows Server 2003/2008 R2/2012.
Steps to reproduce the issue :
1. Login as Administrator on Windows Server 2012 or Windows Server 2008
2. On command prompt run "c:\pg92\bin\initdb -D c:\pgdata92"
3. On command prompt run "c:\pg93\bin\initdb -D c:\pgdata93"
4. On command prompt run "c:\pg93\bin\pg_upgrade -d c:\pgdata92 -D
c:\pgdata93 -b c:\pg92\bin -B c:\pg93\bin -p 2222 -P 3333 -v"
Issue is reproducible with PG 9.2, 9.3 and 9.4. It is caused by
pg_resetxlog utility (It is internally used by pg_upgrade) that fail to
keep correct file permissions/ownership (as required by dbserver process)
while rewriting "global/pg_control” in data directory. pg_resetxlog utility
don’t sandbox and make changes in data directory, when it is being run as
Administrator ; it create files that are owned by "BUILTIN\Administrators”
group. pg_ctl fails because it is using CreateRestrictedProcess() to
sandbox and give up administrator privileges that make it fail to open
"global/pg_control” file (that is now owned by "BUILTIN\Administrators”
group) in write mode i.e.
2014-12-19 03:19:34 PST PANIC: 42501: could not open control file
"global/pg_control": Permission denied
2014-12-19 03:19:34 PST LOCATION: ReadControlFile,
src\backend\access\transam\xlog.c:4458
I think Domain users that are members of Administrators group suffer from
same problem. On Windows Server 2003, changing Group Policy “System
objects: Default owner for objects created by members of the Administrators
group” to “Object Creator” fixes the problem. Microsoft obsolete this Group
Policy in later version of Windows Server. I have tried a workaround (
http://support.microsoft.com/kb/947721) on Windows Server 2008 R2 but it
seems did not worked for me. As it is default behavior for Windows Server,
I would suggest to use CreateRestrictedProcess function (sandbox or
restricted tokens approach) in pg_resetxlog utility or explicitly change
ownership to current user instead of "BUILTIN\Administrators” group if
required (If you agree with the approach I can submit patch for this).
Please do let me know if I missed something or more information is
required. Thanks.
Regards,
Muhammad Asif Naeem
Hi,
When pg_upgrade is being run by Administrator user to upgrade database from
Windows Server (2003/2008/2012), pg_upgrade fail with permission denied
error. When an application (run by Administrator) create files, the default
ownership for the newly created file/folder goes to administrators group
rather than Administrator user.
https://www.microsoft.com/resources/documentation/windows/xp/all/proddocs/en-us/634.mspx?mfr=true
System objects: Default owner for objects created by members of the
administrators group
Description
Determines whether the Administrators group or an object creator is the
default owner of any system objects that are created.
Default: Administrators group (on servers).
Above setting is available in Windows Server 2003, If I change this policy
setting from “Administrators group" to "object creator", pg_upgrade work
without issue. Unfortunately it got obsolete in later version of Windows
Server. On Windows Server 2008, there seems workaround available i.e.
http://support.microsoft.com/kb/947721 but it did not worked for me.
PFA patch. To run pg_upgrade (and its child processes) without
administrative privileges (when available), I have copied restricted token
logic from src/bin/initdb/initdb.c, that enables pg_upgrade to rerun itself
with removed Administrators group privileges, this way it create files with
the ownership of user running pg_upgrade. With the patch applied,
pg_upgrade (or its child process) don’t create files with administrators
group owner and works fine without issue.
As pg_resetxlog change file in data directory, it do suffer from same
permission issue, it also require similar changes. PFA for pg_resetxlog
fix. Please do let me know if I missed something or more information is
required. Thanks.
Regards,
Muhammad Asif Naeem
On Mon, Jan 5, 2015 at 5:34 AM, Asif Naeem <anaeem.it@gmail.com> wrote:
Show quoted text
Hi,
It is been observed that pg_upgrade fail to upgrade on Windows Server
machine if being run as Administrator user (other local users works fine),
It fail with the following error message i.e.""c:\pg93\bin/pg_ctl" -w -l "pg_upgrade_server.log" -D "c:\pgdata93" -o
"-p 3333 -b -c autovacuum_multixact_freeze_max_age=2000000000 -c
synchronous_commit=off -c fsync=off -c full_page_writes=off" start >>
"pg_upgrade_server_start.log" 2>&1"
*failure*
There were problems executing """c:\pg93\bin/pg_ctl" -w -l
"pg_upgrade_server.log" -D "c:\pgdata93" -o "-p 3333 -b -c
autovacuum_multixact_freeze_max_age=2000000000 -c synchronous_commit=off
-c fsync=off -c full_page_writes=off" start >>
"pg_upgrade_server_start.log" 2>&1""
Consult the last few lines of "pg_upgrade_server_start.log" or
"pg_upgrade_server.log" for
the probable cause of the failure.
connection to database failed: could not connect to server: Connection
refused (0x0000274D/10061)
Is the server running on host "localhost" (127.0.0.1) and
accepting
TCP/IP connections on port 3333?could not connect to new postmaster started with the command:
"c:\pg93\bin/pg_ctl" -w -l "pg_upgrade_server.log" -D "c:\pgdata93" -o
"-p 3333 -b -c autovacuum_multixact_freeze_max_age=2000000000 -c
synchronous_commit=off -c fsync=off -c full_page_writes=off" start
Failure, exitingpg_upgrade_server.log
PANIC: could not open control file "global/pg_control": Permission denied
"global/pg_control” file in data directory owned by
"BUILTIN\Administrators” that was owned by “Administrator” user before
pg_upgrade. I have verified it on Windows Server 2003/2008 R2/2012.Steps to reproduce the issue :
1. Login as Administrator on Windows Server 2012 or Windows Server 2008
2. On command prompt run "c:\pg92\bin\initdb -D c:\pgdata92"
3. On command prompt run "c:\pg93\bin\initdb -D c:\pgdata93"
4. On command prompt run "c:\pg93\bin\pg_upgrade -d c:\pgdata92 -D
c:\pgdata93 -b c:\pg92\bin -B c:\pg93\bin -p 2222 -P 3333 -v"Issue is reproducible with PG 9.2, 9.3 and 9.4. It is caused by
pg_resetxlog utility (It is internally used by pg_upgrade) that fail to
keep correct file permissions/ownership (as required by dbserver process)
while rewriting "global/pg_control” in data directory. pg_resetxlog utility
don’t sandbox and make changes in data directory, when it is being run as
Administrator ; it create files that are owned by "BUILTIN\Administrators”
group. pg_ctl fails because it is using CreateRestrictedProcess() to
sandbox and give up administrator privileges that make it fail to open
"global/pg_control” file (that is now owned by "BUILTIN\Administrators”
group) in write mode i.e.2014-12-19 03:19:34 PST PANIC: 42501: could not open control file
"global/pg_control": Permission denied
2014-12-19 03:19:34 PST LOCATION: ReadControlFile,
src\backend\access\transam\xlog.c:4458I think Domain users that are members of Administrators group suffer from
same problem. On Windows Server 2003, changing Group Policy “System
objects: Default owner for objects created by members of the Administrators
group” to “Object Creator” fixes the problem. Microsoft obsolete this Group
Policy in later version of Windows Server. I have tried a workaround (
http://support.microsoft.com/kb/947721) on Windows Server 2008 R2 but it
seems did not worked for me. As it is default behavior for Windows Server,
I would suggest to use CreateRestrictedProcess function (sandbox or
restricted tokens approach) in pg_resetxlog utility or explicitly change
ownership to current user instead of "BUILTIN\Administrators” group if
required (If you agree with the approach I can submit patch for this).
Please do let me know if I missed something or more information is
required. Thanks.Regards,
Muhammad Asif Naeem
On Tue, Jan 20, 2015 at 10:41:41PM +0500, Asif Naeem wrote:
Above setting is available in Windows Server 2003, If I change this policy
setting from “Administrators group" to "object creator", pg_upgrade work
without issue. Unfortunately it got obsolete in later version of Windows
Server. On Windows Server 2008, there seems workaround available i.e. http://
support.microsoft.com/kb/947721 but it did not worked for me.PFA patch. To run pg_upgrade (and its child processes) without administrative
privileges (when available), I have copied restricted token logic from src/bin/
initdb/initdb.c, that enables pg_upgrade to rerun itself with removed
Administrators group privileges, this way it create files with the ownership of
user running pg_upgrade. With the patch applied, pg_upgrade (or its child
process) don’t create files with administrators group owner and works fine
without issue.As pg_resetxlog change file in data directory, it do suffer from same
permission issue, it also require similar changes. PFA for pg_resetxlog fix.
Please do let me know if I missed something or more information is required.
Thanks.
This sounds like the exact right patch. However, since it has a lot of
Windows-specific code, and we don't have any Windows experts, I am not
sure how this can be applied.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Bruce Momjian wrote:
This sounds like the exact right patch. However, since it has a lot of
Windows-specific code, and we don't have any Windows experts, I am not
sure how this can be applied.
Are you saying we will remove the Windows port? That sounds awesome,
thanks! If you need help, I will volunteer on the spot, just LMK.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Wed, Jan 21, 2015 at 03:31:42PM -0300, Alvaro Herrera wrote:
Bruce Momjian wrote:
This sounds like the exact right patch. However, since it has a lot of
Windows-specific code, and we don't have any Windows experts, I am not
sure how this can be applied.Are you saying we will remove the Windows port? That sounds awesome,
thanks! If you need help, I will volunteer on the spot, just LMK.
:-)
Well, I _am_ saying that historically patches that touch the innards of
the Windows API are rarely applied as we can't evaluate or maintain the
code. I can probably come up with an example if you want. The
Windows-specif code we do carry is required and was developed by people
that are no longer as involved.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Thu, Jan 22, 2015 at 12:06 AM, Bruce Momjian <bruce@momjian.us> wrote:
On Wed, Jan 21, 2015 at 03:31:42PM -0300, Alvaro Herrera wrote:
Bruce Momjian wrote:
This sounds like the exact right patch. However, since it has a lot
of
Windows-specific code, and we don't have any Windows experts, I am not
sure how this can be applied.Are you saying we will remove the Windows port? That sounds awesome,
thanks! If you need help, I will volunteer on the spot, just LMK.:-)
Well, I _am_ saying that historically patches that touch the innards of
the Windows API are rarely applied as we can't evaluate or maintain the
code. I can probably come up with an example if you want.
I think it is true to a great extent that Windows patches receive less
attention, however in many cases the patch finally do get committed.
I think the right thing for this patch is that Author should submit it to
next CF, so that it could be tracked and reviewed, once it is reviewed
by some one having Windows access, it should be taken care by
Committer.
The
Windows-specif code we do carry is required and was developed by people
that are no longer as involved.
I have seen many a times that once committer's (those who are not generally
involved in Windows development) get reasonable confidence about patch
and the review done for the same, they commit the patch. It happens both
for the patches where I was Author and where I was Reviewer, although I
agree that it takes more time.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On Thu, Jan 22, 2015 at 12:31 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Thu, Jan 22, 2015 at 12:06 AM, Bruce Momjian <bruce@momjian.us> wrote:
On Wed, Jan 21, 2015 at 03:31:42PM -0300, Alvaro Herrera wrote:
Bruce Momjian wrote:
This sounds like the exact right patch. However, since it has a lot
of
Windows-specific code, and we don't have any Windows experts, I am not
sure how this can be applied.Are you saying we will remove the Windows port? That sounds awesome,
thanks! If you need help, I will volunteer on the spot, just LMK.:-)
Well, I _am_ saying that historically patches that touch the innards of
the Windows API are rarely applied as we can't evaluate or maintain the
code. I can probably come up with an example if you want.I think it is true to a great extent that Windows patches receive less
attention, however in many cases the patch finally do get committed.
I think the right thing for this patch is that Author should submit it to
next CF, so that it could be tracked and reviewed, once it is reviewed
by some one having Windows access, it should be taken care by
Committer.
Adding it to the next CF would be a good first step. I got some access
to some 2k3 and 2k8 boxes, so I think that I could give it a shot.
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Thank you. I have added it to next commitfest
<https://commitfest.postgresql.org/4/114/>.
On Thu, Jan 22, 2015 at 9:38 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:
Show quoted text
On Thu, Jan 22, 2015 at 12:31 PM, Amit Kapila <amit.kapila16@gmail.com>
wrote:On Thu, Jan 22, 2015 at 12:06 AM, Bruce Momjian <bruce@momjian.us>
wrote:
On Wed, Jan 21, 2015 at 03:31:42PM -0300, Alvaro Herrera wrote:
Bruce Momjian wrote:
This sounds like the exact right patch. However, since it has a lot
of
Windows-specific code, and we don't have any Windows experts, I amnot
sure how this can be applied.
Are you saying we will remove the Windows port? That sounds awesome,
thanks! If you need help, I will volunteer on the spot, just LMK.:-)
Well, I _am_ saying that historically patches that touch the innards of
the Windows API are rarely applied as we can't evaluate or maintain the
code. I can probably come up with an example if you want.I think it is true to a great extent that Windows patches receive less
attention, however in many cases the patch finally do get committed.
I think the right thing for this patch is that Author should submit it to
next CF, so that it could be tracked and reviewed, once it is reviewed
by some one having Windows access, it should be taken care by
Committer.Adding it to the next CF would be a good first step. I got some access
to some 2k3 and 2k8 boxes, so I think that I could give it a shot.
--
Michael
On Thu, Jan 22, 2015 at 09:01:39AM +0530, Amit Kapila wrote:
The
Windows-specif code we do carry is required and was developed by people
that are no longer as involved.I have seen many a times that once committer's (those who are not generally
involved in Windows development) get reasonable�confidence about patch
and the review done for the same, they commit the patch.� It happens both
for the patches where I was Author and where I was Reviewer, although I
agree that it takes�more time.
Here is Windows change to properly detach the server process that never
got implemented as no Windows expert developed or tested a patch, but
test code was posted:
/messages/by-id/53759381.4000205@cubiclesoft.com
Here is another unapplied Windows patch for preventing Control-C from
closing the server:
/messages/by-id/20140410214426.GI6917@momjian.us
Again, I would love to say we handle Windows patches well, but we don't.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On 2015-01-22 11:24:31 -0500, Bruce Momjian wrote:
On Thu, Jan 22, 2015 at 09:01:39AM +0530, Amit Kapila wrote:
The
Windows-specif code we do carry is required and was developed by people
that are no longer as involved.I have seen many a times that once committer's (those who are not generally
involved in Windows development) get reasonable�confidence about patch
and the review done for the same, they commit the patch.� It happens both
for the patches where I was Author and where I was Reviewer, although I
agree that it takes�more time.Here is Windows change to properly detach the server process that never
got implemented as no Windows expert developed or tested a patch, but
test code was posted:/messages/by-id/53759381.4000205@cubiclesoft.com
Here is another unapplied Windows patch for preventing Control-C from
closing the server:/messages/by-id/20140410214426.GI6917@momjian.us
Again, I would love to say we handle Windows patches well, but we don't.
I unfortunately don't the issue is that specific to windows.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Thu, Jan 22, 2015 at 05:26:57PM +0100, Andres Freund wrote:
Again, I would love to say we handle Windows patches well, but we don't.
I unfortunately don't the issue is that specific to windows.
Yes, any case where small number of people understand the problem is an
issue.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Thu, Jan 22, 2015 at 4:07 PM, Asif Naeem <anaeem.it@gmail.com> wrote:
Thank you. I have added it to next commitfest.
Sorry for the late reply.
So I have been looking at both patches, and I would definitely be
useful to authorize the use of restricted tokens for both pg_upgrade
and pg_resetxlog. The patch for pg_resetxlog should be rebased on
latest HEAD because there is a small diff with get_restricted_token()
though.
The portion for initdb to make it use restricted tokens has been added
some time ago with fc9c20e, and the one of pg_ctl with a25cd81, both
of them in 2006, so instead of duplicating again the logic for
pg_upgrade and pg_resetxlog, shouldn't we create a common routine in
src/common and link to it all the frontend binaries that need it?
Hence, Asif, could you refactor first the existing code and make it
use a new API in libpqcommon? Let's say in
src/common/restricted_token.c or a better name than the one I just
gave out. Then, you could plug-in this new common routine with
pg_upgrade and pg_resetxlog in a second patch.
Regards,
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Fri, Jan 23, 2015 at 1:26 AM, Andres Freund <andres@2ndquadrant.com> wrote:
On 2015-01-22 11:24:31 -0500, Bruce Momjian wrote:
On Thu, Jan 22, 2015 at 09:01:39AM +0530, Amit Kapila wrote:
The
Windows-specif code we do carry is required and was developed by people
that are no longer as involved.I have seen many a times that once committer's (those who are not generally
involved in Windows development) get reasonable confidence about patch
and the review done for the same, they commit the patch. It happens both
for the patches where I was Author and where I was Reviewer, although I
agree that it takes more time.Here is Windows change to properly detach the server process that never
got implemented as no Windows expert developed or tested a patch, but
test code was posted:/messages/by-id/53759381.4000205@cubiclesoft.com
Here is another unapplied Windows patch for preventing Control-C from
closing the server:/messages/by-id/20140410214426.GI6917@momjian.us
Again, I would love to say we handle Windows patches well, but we don't.
I unfortunately don't the issue is that specific to windows.
FWIW, I am putting those things in my never-ending TODO list. Thanks
for the reminder, Bruce.
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Mon, Mar 2, 2015 at 9:42 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:
On Thu, Jan 22, 2015 at 4:07 PM, Asif Naeem <anaeem.it@gmail.com> wrote:
Thank you. I have added it to next commitfest.
Sorry for the late reply.
So I have been looking at both patches, and I would definitely be
useful to authorize the use of restricted tokens for both pg_upgrade
and pg_resetxlog. The patch for pg_resetxlog should be rebased on
latest HEAD because there is a small diff with get_restricted_token()
though.The portion for initdb to make it use restricted tokens has been added
some time ago with fc9c20e, and the one of pg_ctl with a25cd81, both
of them in 2006, so instead of duplicating again the logic for
pg_upgrade and pg_resetxlog, shouldn't we create a common routine in
src/common and link to it all the frontend binaries that need it?
Hence, Asif, could you refactor first the existing code and make it
use a new API in libpqcommon? Let's say in
src/common/restricted_token.c or a better name than the one I just
gave out. Then, you could plug-in this new common routine with
pg_upgrade and pg_resetxlog in a second patch.
Sure. PFA patch, restricted token code is moved to
src/common/restricted_token.c, Now it uses common routine for initdb,
pg_upgrade and pg_resetxlog. Please do let me know if I missed something or
more information is required. Thanks.
Regards,
Muhammad Asif Naeem
Show quoted text
Regards,
--
Michael
Attachments:
restricted_token.v1.patchapplication/octet-stream; name=restricted_token.v1.patchDownload+232-171
On Wed, Mar 4, 2015 at 10:53 PM, Asif Naeem wrote:
On Mon, Mar 2, 2015 at 9:42 AM, Michael Paquier wrote:
The portion for initdb to make it use restricted tokens has been added
some time ago with fc9c20e, and the one of pg_ctl with a25cd81, both
of them in 2006, so instead of duplicating again the logic for
pg_upgrade and pg_resetxlog, shouldn't we create a common routine in
src/common and link to it all the frontend binaries that need it?
Hence, Asif, could you refactor first the existing code and make it
use a new API in libpqcommon?Sure. PFA patch, restricted token code is moved to
src/common/restricted_token.c, Now it uses common routine for initdb,
pg_upgrade and pg_resetxlog. Please do let me know if I missed something or
more information is required. Thanks.
Thanks. This looks fine to me (tested on MinGW-32 and MSVC). I just
have one comment: get_restricted_token should use progname as
argument. This way you can just keep progname as a static variable for
each frontend and not play with constant variables in both code paths,
aka src/common and src/bin stuff.
Regards,
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Thank you Michael. Good suggestion, PFA updated patch, it don't have
dependency on "progname" global variable any more and expecting this value
as related function argument.
Regards,
Muhammad Asif Naeem
On Thu, Mar 5, 2015 at 11:37 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:
Show quoted text
On Wed, Mar 4, 2015 at 10:53 PM, Asif Naeem wrote:
On Mon, Mar 2, 2015 at 9:42 AM, Michael Paquier wrote:
The portion for initdb to make it use restricted tokens has been added
some time ago with fc9c20e, and the one of pg_ctl with a25cd81, both
of them in 2006, so instead of duplicating again the logic for
pg_upgrade and pg_resetxlog, shouldn't we create a common routine in
src/common and link to it all the frontend binaries that need it?
Hence, Asif, could you refactor first the existing code and make it
use a new API in libpqcommon?Sure. PFA patch, restricted token code is moved to
src/common/restricted_token.c, Now it uses common routine for initdb,
pg_upgrade and pg_resetxlog. Please do let me know if I missed somethingor
more information is required. Thanks.
Thanks. This looks fine to me (tested on MinGW-32 and MSVC). I just
have one comment: get_restricted_token should use progname as
argument. This way you can just keep progname as a static variable for
each frontend and not play with constant variables in both code paths,
aka src/common and src/bin stuff.Regards,
--
Michael
Attachments:
restricted_token.v2.patchapplication/octet-stream; name=restricted_token.v2.patchDownload+222-170
Asif Naeem wrote:
Thank you Michael. Good suggestion, PFA updated patch, it don't have
dependency on "progname" global variable any more and expecting this value
as related function argument.
We have another copy of this in pg_ctl, which looks pretty much
identical except there's some additional stuff at the bottom. Would it
make sense to add those additional "Job"-related things as a separate
routine, so that we can remove the duplicate copy of the "restricted
token" code, and have pg_ctl use get_restricted_process from src/common
instead?
There is a further copy of very similar stuff in pg_regress.c, but what
it does is slightly different; not sure how difficult it would be to
refactor that one too.
Is this a backpatchable bug fix, or are we considering this only for the
master branch?
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Thu, Mar 12, 2015 at 6:50 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Asif Naeem wrote:
Thank you Michael. Good suggestion, PFA updated patch, it don't have
dependency on "progname" global variable any more and expecting this value
as related function argument.We have another copy of this in pg_ctl, which looks pretty much
identical except there's some additional stuff at the bottom. Would it
make sense to add those additional "Job"-related things as a separate
routine, so that we can remove the duplicate copy of the "restricted
token" code, and have pg_ctl use get_restricted_process from src/common
instead?
Yes, looking at the code it makes sense.
There is a further copy of very similar stuff in pg_regress.c, but what
it does is slightly different; not sure how difficult it would be to
refactor that one too.
In this part the only difference is a spawn of cmd /c, but I don't see
why it is useful to spawn a new command prompt here for this case, so
we could just drop this part. Looking at the log history, this has
been added since this code creation..
Is this a backpatchable bug fix, or are we considering this only for the
master branch?
It would be good to get that backpatched, that's something we really
miss now IMO. Now it modifies libpgcommon, so Windows packagers (me
being one) will certainly need to patch a bit stuff but that's a
one-line changer so it's not a big deal. And I imagine that this is
actually the reason why Asif reported that as a bug as well.
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Thu, Mar 12, 2015 at 11:23 PM, Michael Paquier wrote:
On Thu, Mar 12, 2015 at 6:50 AM, Alvaro Herrera wrote:
In this part the only difference is a spawn of cmd /c, but I don't see
why it is useful to spawn a new command prompt here for this case, so
we could just drop this part. Looking at the log history, this has
been added since this code creation..
Taking back my words here. We definitely want to spawn a new process
in the case of pg_regress, so I think that the best thing to do would
be to pass the to-be-launched command to get_restrict_token(), and be
careful with WaitForSingleObject and GetExitCodeProcess() as there are
cases where we cannot wait for a process, so we are going to need a
control flag, or to let the callers of get_restricted_token() do the
wait themselves. I would think that the latter is better, additional
opinions being welcome.
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Michael Paquier wrote:
On Thu, Mar 12, 2015 at 11:23 PM, Michael Paquier wrote:
On Thu, Mar 12, 2015 at 6:50 AM, Alvaro Herrera wrote:
In this part the only difference is a spawn of cmd /c, but I don't see
why it is useful to spawn a new command prompt here for this case, so
we could just drop this part. Looking at the log history, this has
been added since this code creation..Taking back my words here. We definitely want to spawn a new process
in the case of pg_regress, so I think that the best thing to do would
be to pass the to-be-launched command to get_restrict_token(), and be
careful with WaitForSingleObject and GetExitCodeProcess() as there are
cases where we cannot wait for a process, so we are going to need a
control flag, or to let the callers of get_restricted_token() do the
wait themselves. I would think that the latter is better, additional
opinions being welcome.
Maybe we want a specialized routine for pg_regress; maybe
get_restricted_token would call it with some args set to null or false
as appropriate. My point is not to make the other callers of
get_restricted_token more complex.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs