Skip to content

add infrastructure for configuration warnings#492

Merged
arogge merged 1 commit intobareos:masterfrom
arogge:dev/arogge/deprecation-messages
Apr 27, 2020
Merged

add infrastructure for configuration warnings#492
arogge merged 1 commit intobareos:masterfrom
arogge:dev/arogge/deprecation-messages

Conversation

@arogge
Copy link
Member

@arogge arogge commented Apr 21, 2020

This PR will add a list of warnings to the configuration parser along with some utility functions. This list can then be filled during (and after) parsing the configuration.
It will also add a few ways to see these deprecation warnings.

@arogge arogge force-pushed the dev/arogge/deprecation-messages branch from 6c7184e to 2072ced Compare April 21, 2020 10:48
@arogge arogge marked this pull request as ready for review April 23, 2020 09:05
@arogge arogge requested review from franku and pstorz and removed request for franku April 23, 2020 09:07
Copy link
Contributor

@franku franku left a comment

Choose a reason for hiding this comment

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

Very nice!
Have some notes you may want to double-check.

if (my_config->HaveDeprecationWarnings()) {
// FIXME: cannot use Jmsg here as only M_FATAL and M_ERROR_TERM will
// work because daemon_msgs is not initialized
fprintf(stderr, _("Deprecated configuration settings detected:\n"));
Copy link
Contributor

Choose a reason for hiding this comment

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

When running as a daemon stderr could have been closed.

Copy link
Member Author

Choose a reason for hiding this comment

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

if (test_config) gurarantees that we haven't daemonized, because daemon_start() is called in a if (!test_config)-block. And even if daemon_start() had already run stderr would be still open and point to /dev/null (see SetupStdFileDescriptors() in core/src/lib/daemon.cc.

}
}

static void sendit(const std::string& msg, StatusPacket* sp)
Copy link
Contributor

Choose a reason for hiding this comment

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

This general function is same for all three daemon. Could move it somewhere into libbareos.

Copy link
Member Author

Choose a reason for hiding this comment

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

Point taken. I should probably refactor the whole sendit()-family somewhere into the library.


if (test_config) {
if (my_config->HasDeprecationWarnings()) {
// FIXME: cannot use Jmsg here as only M_FATAL and M_ERROR_TERM will
Copy link
Contributor

Choose a reason for hiding this comment

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

A comment would be helpful anyway to understand why none of the Dmg or Jms can be used.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

@arogge arogge force-pushed the dev/arogge/deprecation-messages branch from dd3d947 to a12729c Compare April 24, 2020 14:35
This patch adds a list of warnings to the configuration parser along
with some utility functions.
The director, storagedaemon and filedaemon in config_test mode will
show this list of warnings.
When using the status commands, the daemons will report if they have at
least one record in that list.
The director now has a new command "status configuration" that will
currently show only that list of warnings.
@arogge arogge force-pushed the dev/arogge/deprecation-messages branch from a12729c to c4e88eb Compare April 27, 2020 07:30
@arogge arogge changed the title add infrastructure for deprecation warnings add infrastructure for configuration warnings Apr 27, 2020
@arogge arogge merged commit 45aed68 into bareos:master Apr 27, 2020
@arogge arogge deleted the dev/arogge/deprecation-messages branch August 10, 2020 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants