BIND 10 trac736, updated. 25cb92eaa2253745fd27c957e8d576cd90b7b244 [trac736] make one setting 'output' for facility/filename/stream
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Jun 7 14:09:53 UTC 2011
The branch, trac736 has been updated
via 25cb92eaa2253745fd27c957e8d576cd90b7b244 (commit)
via 73487153601d7ad71f04e6762cabd3b3712c860a (commit)
from 542a15d604018ea73a5a28f26b30b92fb668e399 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 25cb92eaa2253745fd27c957e8d576cd90b7b244
Author: Jelte Jansen <jelte at isc.org>
Date: Tue Jun 7 15:50:04 2011 +0200
[trac736] make one setting 'output' for facility/filename/stream
commit 73487153601d7ad71f04e6762cabd3b3712c860a
Author: Jelte Jansen <jelte at isc.org>
Date: Tue Jun 7 15:07:23 2011 +0200
[trac736] address review comments (the trivial ones)
-----------------------------------------------------------------------
Summary of changes:
src/bin/cfgmgr/plugins/b10logging.py | 43 +++++++++++++++++++++++++++------
src/bin/cfgmgr/plugins/logging.spec | 12 +--------
src/lib/config/ccsession.cc | 20 ++++++++--------
src/lib/config/ccsession.h | 8 +++---
src/lib/config/config_data.cc | 9 ++++++-
5 files changed, 58 insertions(+), 34 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/cfgmgr/plugins/b10logging.py b/src/bin/cfgmgr/plugins/b10logging.py
index c513a31..6af3f66 100644
--- a/src/bin/cfgmgr/plugins/b10logging.py
+++ b/src/bin/cfgmgr/plugins/b10logging.py
@@ -46,19 +46,46 @@ def check(config):
# The 'layout' is ok, now check for specific values
if 'loggers' in config:
for logger in config['loggers']:
+ # name should always be present
+ name = logger['name']
+
if 'severity' in logger and\
logger['severity'].lower() not in ALLOWED_SEVERITIES:
- errors.append("bad severity value " + logger['severity'])
+ errors.append("bad severity value for logger " + name +
+ ": " + logger['severity'])
if 'output_options' in logger:
for output_option in logger['output_options']:
if 'destination' in output_option:
- if output_option['destination'].lower() not in ALLOWED_DESTINATIONS:
- errors.append("bad destination: " + output_option['destination'])
- if 'filename' not in output_option:
- errors.append("destination set to file but no filename specified")
- if 'stream' in output_option and\
- output_option['stream'].lower() not in ALLOWED_STREAMS:
- errors.append("bad stream: " + output_option['stream'])
+ destination = output_option['destination'].lower()
+ if destination not in ALLOWED_DESTINATIONS:
+ errors.append("bad destination for logger " +
+ name + ": " + output_option['destination'])
+ else:
+ # if left to default, output is stdout, and
+ # it will not show in the updated config,
+ # so 1. we only need to check it if present,
+ # and 2. if destination is changed, so should
+ # output. So first check checks 'in', and the
+ # others 'not in' for 'output'
+ if destination == "console" and\
+ 'output' in output_option and\
+ output_option['output'] not in ALLOWED_STREAMS:
+ errors.append("bad output for logger " + name +
+ ": " + output_option['stream'] +
+ ", must be stdout or stderr")
+ elif destination == "file" and\
+ 'output' not in output_option or\
+ output_option['output'] == "":
+ errors.append("destination set to file but "
+ "output not set to any "
+ "filename for logger "
+ + name)
+ elif destination == "syslog" and\
+ 'output' not in output_option or\
+ output_option['output'] == "":
+ errors.append("destination set to syslog but "
+ "output not set to any facility"
+ " for logger " + name)
if errors:
return ', '.join(errors)
diff --git a/src/bin/cfgmgr/plugins/logging.spec b/src/bin/cfgmgr/plugins/logging.spec
index d686684..e377b0e 100644
--- a/src/bin/cfgmgr/plugins/logging.spec
+++ b/src/bin/cfgmgr/plugins/logging.spec
@@ -49,7 +49,7 @@
"item_optional": false,
"item_default": "console"
},
- { "item_name": "stream",
+ { "item_name": "output",
"item_type": "string",
"item_optional": false,
"item_default": "stdout"
@@ -59,16 +59,6 @@
"item_optional": false,
"item_default": false
},
- { "item_name": "facility",
- "item_type": "string",
- "item_optional": false,
- "item_default": ""
- },
- { "item_name": "filename",
- "item_type": "string",
- "item_optional": false,
- "item_default": ""
- },
{ "item_name": "maxsize",
"item_type": "integer",
"item_optional": false,
diff --git a/src/lib/config/ccsession.cc b/src/lib/config/ccsession.cc
index 035735e..857de63 100644
--- a/src/lib/config/ccsession.cc
+++ b/src/lib/config/ccsession.cc
@@ -185,19 +185,19 @@ readOutputOptionConf(isc::log::OutputOption& output_option,
"destination", config_data,
"loggers/output_options/destination");
output_option.destination = isc::log::getDestination(destination_el->stringValue());
- ConstElementPtr stream_el = getValueOrDefault(output_option_el,
- "stream", config_data,
- "loggers/output_options/stream");
- output_option.stream = isc::log::getStream(stream_el->stringValue());
+ ConstElementPtr output_el = getValueOrDefault(output_option_el,
+ "output", config_data,
+ "loggers/output_options/output");
+ if (output_option.destination == isc::log::OutputOption::DEST_CONSOLE) {
+ output_option.stream = isc::log::getStream(output_el->stringValue());
+ } else if (output_option.destination == isc::log::OutputOption::DEST_FILE) {
+ output_option.filename = output_el->stringValue();
+ } else if (output_option.destination == isc::log::OutputOption::DEST_SYSLOG) {
+ output_option.facility = output_el->stringValue();
+ }
output_option.flush = getValueOrDefault(output_option_el,
"flush", config_data,
"loggers/output_options/flush")->boolValue();
- output_option.facility = getValueOrDefault(output_option_el,
- "facility", config_data,
- "loggers/output_options/facility")->stringValue();
- output_option.filename = getValueOrDefault(output_option_el,
- "filename", config_data,
- "loggers/output_options/filename")->stringValue();
output_option.maxsize = getValueOrDefault(output_option_el,
"maxsize", config_data,
"loggers/output_options/maxsize")->intValue();
diff --git a/src/lib/config/ccsession.h b/src/lib/config/ccsession.h
index bc4d853..53aab78 100644
--- a/src/lib/config/ccsession.h
+++ b/src/lib/config/ccsession.h
@@ -161,9 +161,6 @@ public:
* configuration of the local module needs to be updated.
* This must refer to a valid object of a concrete derived class of
* AbstractSession without establishing the session.
- * @param handle_logging If true, the ModuleCCSession will automatically
- * take care of logging configuration through the virtual Logging config
- * module.
*
* Note: the design decision on who is responsible for establishing the
* session is in flux, and may change in near future.
@@ -175,11 +172,14 @@ public:
* @param command_handler A callback function pointer to be called when
* a control command from a remote agent needs to be performed on the
* local module.
- * @start_immediately If true (default), start listening to new commands
+ * @param start_immediately If true (default), start listening to new commands
* and configuration changes asynchronously at the end of the constructor;
* if false, it will be delayed until the start() method is explicitly
* called. (This is a short term workaround for an initialization trouble.
* We'll need to develop a cleaner solution, and then remove this knob)
+ * @param handle_logging If true, the ModuleCCSession will automatically
+ * take care of logging configuration through the virtual Logging config
+ * module.
*/
ModuleCCSession(const std::string& spec_file_name,
isc::cc::AbstractSession& session,
diff --git a/src/lib/config/config_data.cc b/src/lib/config/config_data.cc
index efa913c..ebe51cc 100644
--- a/src/lib/config/config_data.cc
+++ b/src/lib/config/config_data.cc
@@ -23,7 +23,9 @@ using namespace isc::data;
namespace {
-// Returns the '_spec' part of a list or map specification
+// Returns the '_spec' part of a list or map specification (recursively,
+// i.e. if it is a list of lists or maps, will return the spec of the
+// inner-most list or map).
//
// \param spec_part the list or map specification (part)
// \return the value of spec_part's "list_item_spec" or "map_item_spec",
@@ -44,6 +46,11 @@ ConstElementPtr findListOrMapSubSpec(ConstElementPtr spec_part) {
// Returns a specific Element in a given specification ListElement
//
+// \exception DataNotFoundError if the given identifier does not
+// point to an existing element. Since we are dealing with the
+// specification here, and not the config data itself, this should
+// not happen, and is a code bug.
+//
// \param spec_part ListElement to find the element in
// \param id_part the name of the element to find (must match the value
// "item_name" in the list item
More information about the bind10-changes
mailing list