Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PP-410: Improvement to server logging ... #139

Merged
merged 1 commit into from
Aug 16, 2016

Conversation

agrawalravi90
Copy link

Issue-ID

Problem description

When pbs_server would fail because of a failed fork() inside go_to_background(),
there was no error/log message printed which would let the admin know of what
happened.

Solution description

Added a call to log_err() inside go_to_background() when fork fails.

Checklist:

****For further information please visit the Developer Guide Home.****

Ticket Title:
Improvement to server logging on a failed call to go_to_background()

Problem description:
When pbs_server would fail because of a failed fork() inside go_to_background(),
there was no error/log message printed which would let the admin know of what
happened.

Solution description:
Added a call to log_err() inside go_to_background() when fork fails.
@@ -412,8 +412,10 @@ go_to_background()

lock_out(lockfds, F_UNLCK);
rc = fork();
if (rc == -1) /* fork failed */
if (rc == -1) { /* fork failed */
log_err(errno, msg_daemonname, "fork failed");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to supply the errno?

@bhroam
Copy link
Contributor

bhroam commented Aug 15, 2016

I think the errno provides the error message of the failure to allocate memory.

I like the change. I sign off.

@agrawalravi90
Copy link
Author

@bhroam thanks for the sign-off.

@mike0042 Yes, we do need to provide an errno for log_err to print the correct message via strerror() as fork() can fail due to any of the following three reasons:

[EAGAIN] The system-imposed limit on the total number of pro-
cesses under execution would be exceeded. This limit
is configuration-dependent.

[EAGAIN] The system-imposed limit MAXUPRC (<sys/param.h>) on the
total number of processes under execution by a single
user would be exceeded.

[ENOMEM] There is insufficient swap space for the new process.

@subhasisb subhasisb added the bug Something isn't working label Aug 16, 2016
@subhasisb subhasisb self-assigned this Aug 16, 2016
@mike0042
Copy link
Contributor

Bhroam is correct. Sorry about that. I'll merge the pull request.

@mike0042 mike0042 merged commit f103d64 into openpbs:master Aug 16, 2016
@agrawalravi90
Copy link
Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants