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

dyndns: collect nsupdate debug output #7713

Closed
wants to merge 1 commit into from

Conversation

sumit-bose
Copy link
Contributor

It looks like in current code the assumption is that the nsupdate
command can just send its debug output into the backend log by
duplicating the file descriptor. This won't work since the logs file is
opened with O_CLOEXEC so that it is closed when nsupdate is started.

Additionally it is questionable if this approach is a good idea because
it would lead to a random intermixing of debug information. This patch
collects the output on strderr of nsupdate separately and adds it into
the backend log similar to the input send to nsupdate.

It looks like in current code the assumption is that the nsupdate
command can just send its debug output into the backend log by
duplicating the file descriptor. This won't work since the logs file is
opened with O_CLOEXEC so that it is closed when nsupdate is started.

Additionally it is questionable if this approach is a good idea because
it would lead to a random intermixing of debug information. This patch
collects the output on strderr of nsupdate separately and adds it into
the backend log similar to the input send to nsupdate.
Copy link
Contributor

@thalman thalman left a comment

Choose a reason for hiding this comment

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

I tested it locally, works as expected.

Thanks

@pbrezina
Copy link
Member

Pushed PR: #7713

  • master
    • e4b2604 - dyndns: collect nsupdate debug output
  • sssd-2-10
    • d2d229d - dyndns: collect nsupdate debug output
  • sssd-2-9
    • c6792b7 - dyndns: collect nsupdate debug output

@pbrezina pbrezina added Pushed and removed Accepted Ready to push Ready to push labels Nov 27, 2024
@pbrezina pbrezina closed this Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants