[trac736] make one setting 'output' for facility/filename/stream

src/bin/cfgmgr/plugins/b10logging.py

@@ -56,19 +56,36 @@ def check(config):
             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:
+                        destination = output_option['destination'].lower()
+                        if destination not in ALLOWED_DESTINATIONS:
                             errors.append("bad destination for logger " +
                             name + ": " + output_option['destination'])
-                        if output_option['destination'].lower() == "file" and\
-                           ('filename' not in output_option or
-                            output_option('filename') == ""):
-                            errors.append("destination set to file but "
-                                          "no filename specified for"
-                                          "logger " + name)
-                    if 'stream' in output_option and\
-                       output_option['stream'].lower() not in ALLOWED_STREAMS:
-                        errors.append("bad stream for logger " + name +
-                                      ": " + output_option['stream'])
+                        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)

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,

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();

src/lib/config/config_data.cc

@@ -46,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