Merge branch 'master' into trac2120

ChangeLog

@@ -1,3 +1,17 @@
+454.	[bug]		jelte
+	b10-cfgmgr now loads its configuration check plugins directly from
+	the plugin search path, as opposed to importing them from the
+	general python system module path list; this prevents naming
+	conflicts with real python modules.
+	(Trac #2119, git 2f68d7ac5c3c7cc88a3663191113eece32d46a3d)
+
+453.	[bug]		jelte
+	b10-auth no longer tries to send DDNS UPDATE messages to b10-ddns if
+	b10-ddns is not running. Sending an UPDATE to BIND 10 that is not
+	configured to run DDNS will now result in a response with rcode
+	NOTIMP instead of SERVFAIL.
+	(Trac #1986, git bd6b0a5ed3481f78fb4e5cb0b18c7b6e5920f9f8)
+
 452.	[func]*		muks
 	b10-showtech: An initial implementation of the b10-showtech tool
 	is now available. It gathers and outputs system information which
@@ -291,8 +305,8 @@ bind10-devel-20120329 released on March 29, 2012
 
 407.	[build]		haikuo
 	Remove "--enable-boost-threads" switch in configure command. This
-	thread lock mechanism is useless for bind10 and causes performance 
-	hits. 
+	thread lock mechanism is useless for bind10 and causes performance
+	hits.
 	(Trac #1680, git 9c4d0cadf4adc802cc41a2610dc2c30b25aad728)
 
 406.	[bug]		muks
@@ -788,26 +802,26 @@ bind10-devel-20120119 released on January 19, 2012
 	(Trac #1350, git cc20ff993da1ddb1c6e8a98370438b45a2be9e0a)
 
 336.	[func]		jelte
-	libdns++ (and its python wrapper) now includes a class Serial, for 
-	SOA SERIAL comparison and addition. Operations on instances of this 
-	class follow the specification from RFC 1982. 
-	Rdata::SOA::getSerial() now returns values of this type (and not 
+	libdns++ (and its python wrapper) now includes a class Serial, for
+	SOA SERIAL comparison and addition. Operations on instances of this
+	class follow the specification from RFC 1982.
+	Rdata::SOA::getSerial() now returns values of this type (and not
 	uint32_t).
 	(Trac #1278, git 2ae72d76c74f61a67590722c73ebbf631388acbd)
 
 335.	[bug]*		jelte
-	The DataSourceClientContainer class that dynamically loads 
-	datasource backend libraries no longer provides just a .so file name 
-	to its call to dlopen(), but passes it an absolute path. This means 
-	that it is no longer an system implementation detail that depends on 
-	[DY]LD_LIBRARY_PATH which file is chosen, should there be multiple 
-	options (for instance, when test-running a new build while a 
+	The DataSourceClientContainer class that dynamically loads
+	datasource backend libraries no longer provides just a .so file name
+	to its call to dlopen(), but passes it an absolute path. This means
+	that it is no longer an system implementation detail that depends on
+	[DY]LD_LIBRARY_PATH which file is chosen, should there be multiple
+	options (for instance, when test-running a new build while a
 	different version is installed).
-	These loadable libraries are also no longer installed in the default 
-	library path, but in a subdirectory of the libexec directory of the 
+	These loadable libraries are also no longer installed in the default
+	library path, but in a subdirectory of the libexec directory of the
 	target ($prefix/libexec/[version]/backends).
-	This also removes the need to handle b10-xfin and b10-xfrout as 
-	'special' hardcoded components, and they are now started as regular 
+	This also removes the need to handle b10-xfin and b10-xfrout as
+	'special' hardcoded components, and they are now started as regular
 	components as dictated by the configuration of the boss process.
 	(Trac #1292, git 83ce13c2d85068a1bec015361e4ef8c35590a5d0)
 
@@ -952,12 +966,12 @@ bind10-devel-20111128 released on November 28, 2011
 	(Trac #1228, git 31d5a4f66b18cca838ca1182b9f13034066427a7)
 
 314.	[bug]		jelte
-	b10-xfrin would previously initiate incoming transfers upon 
-	receiving NOTIFY messages from any address (if the zone was 
-	known to b10-xfrin, and using the configured address). It now 
-	only starts a transfer if the source address from the NOTIFY 
-	packet matches the configured master address and port. This was 
-	really already fixed in release bind10-devel-20111014, but there 
+	b10-xfrin would previously initiate incoming transfers upon
+	receiving NOTIFY messages from any address (if the zone was
+	known to b10-xfrin, and using the configured address). It now
+	only starts a transfer if the source address from the NOTIFY
+	packet matches the configured master address and port. This was
+	really already fixed in release bind10-devel-20111014, but there
 	were some deferred cleanups to add.
 	(Trac #1298, git 1177bfe30e17a76bea6b6447e14ae9be9e1ca8c2)
 
@@ -971,7 +985,7 @@ bind10-devel-20111128 released on November 28, 2011
 	(Trac #1329, git 1aa233fab1d74dc776899df61181806679d14013)
 
 312.	[func]		jelte
-	Added an initial framework for doing system tests using the 
+	Added an initial framework for doing system tests using the
 	cucumber-based BDD tool Lettuce. A number of general steps are
 	included,  for instance running bind10 with specific
 	configurations, sending queries, and inspecting query answers. A
@@ -1349,7 +1363,7 @@ bind10-devel-20110705 released on July 05, 2011
 	(Trac #710, git dae1d2e24f993e1eef9ab429326652f40a006dfb)
 
 257.	[bug]		y-aharen
-	Fixed a bug an instance of IntervalTimerImpl may be destructed 
+	Fixed a bug an instance of IntervalTimerImpl may be destructed
 	while deadline_timer is holding the handler. This fix addresses
 	occasional failure of IntervalTimerTest.destructIntervalTimer.
 	(Trac #957, git e59c215e14b5718f62699ec32514453b983ff603)
@@ -2019,7 +2033,7 @@ bind10-devel-20110120 released on January 20, 2011
 	(Trac #513, git 285c5ee3d5582ed6df02d1aa00387f92a74e3695)
 
 151.	[bug]		smann
-	lib/log/dummylog.h: 
+	lib/log/dummylog.h:
 	lib/log/dummylog.cc: Modify dlog so that it takes an optional
 	2nd argument of type bool (true or false). This flag, if
 	set, will cause the message to be printed whether or not
@@ -2327,11 +2341,11 @@ bind10-devel-20101201 released on December 01, 2010
 104.	[bug]		jerry
 	bin/zonemgr: zonemgr should be attempting to refresh expired zones.
 	(Trac #336, r3139)
- 
+
 103.	[bug]		jerry
 	lib/python/isc/log: Fixed an issue with python logging,
 	python log shouldn't die with OSError. (Trac #267, r3137)
- 
+
 102.	[build]		jinmei
 	Disable threads in ASIO to minimize build time dependency.
 	(Trac #345, r3100)
@@ -2366,7 +2380,7 @@ bind10-devel-20101201 released on December 01, 2010
 	it can be customized; Make sure --disable-static works.
 	(Trac #325, r2976)
 
-bind10-devel-20100917 released on September 17, 2010 
+bind10-devel-20100917 released on September 17, 2010
 
 95.	[doc]		jreed
 	Add b10-zonemgr manual page. Update other docs to introduce
@@ -2419,14 +2433,14 @@ bind10-devel-20100917 released on September 17, 2010
 	reason. (Trac #296, r2761)
 
 87.	[func]		zhanglikun
-	lib/python/isc/notifyout: Add the feature of notify-out, when 
+	lib/python/isc/notifyout: Add the feature of notify-out, when
 	zone axfr/ixfr finishing, the server will notify its slaves.
 	(Trac #289, svn r2737)
 
 86.	[func]		jerry
-	bin/zonemgr: Added zone manager module. The zone manager is one 
-	of the co-operating processes of BIND10, which keeps track of 
-	timers and other information necessary for BIND10 to act as a 
+	bin/zonemgr: Added zone manager module. The zone manager is one
+	of the co-operating processes of BIND10, which keeps track of
+	timers and other information necessary for BIND10 to act as a
 	slave. (Trac #215, svn r2737)
 
 85.	[build]*		jinmei
@@ -2439,7 +2453,7 @@ bind10-devel-20100917 released on September 17, 2010
 bind10-devel-20100812 released on August 12, 2010
 
 84.	[bug]		jinmei, jerry
-	This is a quick fix patch for the issue: AXFR fails half the 
+	This is a quick fix patch for the issue: AXFR fails half the
 	time because of connection problems. xfrout client will make
 	a new connection every time. (Trac #299, svn r2697)
 
@@ -2489,7 +2503,7 @@ bind10-devel-20100812 released on August 12, 2010
 	(Trac #256, r2549)
 
 77.	[func]		zhanglikun
-	Make error message be more friendly when running cmdctl and it's 
+	Make error message be more friendly when running cmdctl and it's
 	already running (listening on same port)(Trac #277, r2540)
 
 76.	[bug]		jelte
@@ -2545,8 +2559,8 @@ bind10-devel-20100701 released on July 1, 2010
 68.	[func]		zhanglikun
 	Add options -c (--certificate-chain) to bindctl. Override class
 	HTTPSConnection to support server certificate validation.
-	Add support to cmdctl.spec file, now there are three configurable 
-	items for cmdctl: 'key_file', 'cert_file' and 'accounts_file', 
+	Add support to cmdctl.spec file, now there are three configurable
+	items for cmdctl: 'key_file', 'cert_file' and 'accounts_file',
 	all of them can be changed in runtime.
 	(Trac #127, svn r2357)
 
@@ -2566,7 +2580,7 @@ bind10-devel-20100701 released on July 1, 2010
 	section; this, among other things, will prevent multiple copies
 	of the same CNAME from showing up when there's a loop. (Trac #69,
 	svn r2350)
-    
+
 65.	[func]		shentingting
 	Various loadzone improvements: allow optional comment for
 	$TTL, allow optional origin and comment for $INCLUDE, allow
@@ -2587,7 +2601,7 @@ bind10-devel-20100701 released on July 1, 2010
 
 63.	[func]		shane
 	Added initial support for setuid(), using the "-u" flag. This will
-	be replaced in the future, but for now provides a reasonable 
+	be replaced in the future, but for now provides a reasonable
 	starting point.
 	(Trac #180, svn r2330)
 
@@ -2636,17 +2650,17 @@ bind10-devel-20100701 released on July 1, 2010
 	(Trac #224, svn r2103)
 
 53.	[bug]		zhanglikun
-	bin/bindctl: Generate a unique session ID by using 
-	socket.gethostname() instead of socket.gethostbyname(), 
-	since the latter one could make bindctl	stall if its own 
+	bin/bindctl: Generate a unique session ID by using
+	socket.gethostname() instead of socket.gethostbyname(),
+	since the latter one could make bindctl	stall if its own
 	host name can't be resolved.
 	(Trac #228, svn r2096)
 
 52.	[func]		zhanglikun
 	bin/xfrout: When xfrout is launched, check whether the
-	socket file is being used by one running xfrout process, 
-	if it is, exit from python.	If the file isn't a socket file 
-	or nobody is listening, it will be removed. If it can't 
+	socket file is being used by one running xfrout process,
+	if it is, exit from python.	If the file isn't a socket file
+	or nobody is listening, it will be removed. If it can't
 	be removed, exit from python.
 	(Trac #151, svn r2091)
 
@@ -2659,7 +2673,7 @@ bind10-devel-20100602 released on June 2, 2010
 	(Trac #223)
 
 50.	[bug]		zhanglikun
-	bin/xfrin: a regression in xfrin: it can't communicate with 
+	bin/xfrin: a regression in xfrin: it can't communicate with
 	a remote server. (Trac #218, svn r2038)
 
 49.	[func]*		jelte

src/bin/auth/auth.spec.pre.in

@@ -131,6 +131,16 @@
             "item_optional": true, "item_default": "memory"
           }
         ]
+      },
+      {
+        "command_name": "start_ddns_forwarder",
+        "command_description": "(Re)start internal forwarding of DDNS Update messages. This is automatically called if b10-ddns is started, and is not expected to be called by administrators; it will be removed as a public command in the future.",
+        "command_args": []
+      },
+      {
+        "command_name": "stop_ddns_forwarder",
+        "command_description": "Stop internal forwarding of DDNS Update messages. This is automatically called if b10-ddns is stopped, and is not expected to be called by administrators; it will be removed as a public command in the future.",
+        "command_args": []
       }
     ],
     "statistics": [

src/bin/auth/auth_messages.mes

@@ -110,9 +110,6 @@ look into the cause and address the issue.  The log message includes
 the client's address (and port), and the error message sent from the
 lower layer that detects the failure.
 
-% AUTH_RECEIVED_NOTIFY received incoming NOTIFY for zone name %1, zone class %2
-This is a debug message reporting that an incoming NOTIFY was received.
-
 % AUTH_NOTIFY_QUESTIONS invalid number of questions (%1) in incoming NOTIFY
 This debug message is logged by the authoritative server when it receives
 a NOTIFY packet that contains zero or more than one question. (A valid
@@ -169,6 +166,9 @@ bug ticket for this issue.
 This is a debug message issued when the authoritative server has received
 a command on the command channel.
 
+% AUTH_RECEIVED_NOTIFY received incoming NOTIFY for zone name %1, zone class %2
+This is a debug message reporting that an incoming NOTIFY was received.
+
 % AUTH_RECEIVED_SENDSTATS command 'sendstats' received
 This is a debug message issued when the authoritative server has received
 a command from the statistics module to send it data. The 'sendstats'
@@ -235,6 +235,13 @@ This is a debug message indicating that the authoritative server has
 found that the data source it is loading is an SQLite3 data source,
 so no further validation is needed.
 
+% AUTH_START_DDNS_FORWARDER DDNS UPDATE handling started
+This is a debug message indicating that b10-auth has received a message
+that it should internally forward UPDATE message to b10-ddns. When b10-ddns
+is not running, b10-auth will respond to UPDATE requests with rcode NOTIMP.
+When b10-ddns is running, b10-ddns will handle and respond to the UPDATE
+message.
+
 % AUTH_STATS_CHANNEL_CREATED STATS session channel created
 This is a debug message indicating that the authoritative server has
 created a channel to the statistics process.  It is issued during server
@@ -266,6 +273,15 @@ This is a debug message indicating that the statistics timer has been
 enabled and that the authoritative server will produce statistics data
 at the specified interval.
 
+% AUTH_STOP_DDNS_FORWARDER DDNS UPDATE handling stopped
+This is a debug message indicating that b10-auth has received a message
+that it should stop internally forwarding UPDATE message to b10-ddns.
+b10-auth will no longer forward UPDATE messages to b10-ddns, but will
+respond itself with error code NOTIMP.
+This message is also logged when the forwarding is restarted (for instance
+if b10-ddns is restarted and the internal connection needs to be created
+again), in which case it should be followed by AUTH_START_DDNS_FORWARDER.
+
 % AUTH_UNSUPPORTED_OPCODE unsupported opcode: %1
 This is a debug message, produced when a received DNS packet being
 processed by the authoritative server has been found to contain an

src/bin/auth/auth_srv.cc

@@ -58,6 +58,7 @@
 
 #include <boost/bind.hpp>
 #include <boost/lexical_cast.hpp>
+#include <boost/scoped_ptr.hpp>
 
 #include <algorithm>
 #include <cassert>
@@ -270,6 +271,14 @@ public:
     /// isc:config::ModuleSpec::validateStatistics.
     void registerStatisticsValidator();
 
+    /// Socket session forwarder for dynamic update requests
+    BaseSocketSessionForwarder& ddns_base_forwarder_;
+
+    /// Holder for the DDNS Forwarder, which is used to send
+    /// DDNS messages to b10-ddns, but can be set to empty if
+    /// b10-ddns is not running
+    boost::scoped_ptr<SocketSessionForwarderHolder> ddns_forwarder_;
+
     /// \brief Resume the server
     ///
     /// This is a wrapper call for DNSServer::resume(done), if 'done' is true,
@@ -285,6 +294,7 @@ public:
     void resumeServer(isc::asiodns::DNSServer* server,
                       isc::dns::Message& message,
                       bool done);
+
 private:
     std::string db_file_;
 
@@ -297,9 +307,6 @@ private:
     bool xfrout_connected_;
     AbstractXfroutClient& xfrout_client_;
 
-    // Socket session forwarder for dynamic update requests
-    SocketSessionForwarderHolder ddns_forwarder_;
-
     /// Increment query counter
     void incCounter(const int protocol);
 
@@ -318,9 +325,10 @@ AuthSrvImpl::AuthSrvImpl(const bool use_cache,
     statistics_timer_(io_service_),
     counters_(),
     keyring_(NULL),
+    ddns_base_forwarder_(ddns_forwarder),
+    ddns_forwarder_(NULL),
     xfrout_connected_(false),
-    xfrout_client_(xfrout_client),
-    ddns_forwarder_("update", ddns_forwarder)
+    xfrout_client_(xfrout_client)
 {
     // cur_datasrc_ is automatically initialized by the default constructor,
     // effectively being an empty (sqlite) data source.  once ccsession is up
@@ -456,7 +464,7 @@ makeErrorMessage(MessageRenderer& renderer, Message& message,
     for_each(questions.begin(), questions.end(), QuestionInserter(message));
 
     message.setRcode(rcode);
-    
+
     RendererHolder holder(renderer, &buffer);
     if (tsig_context.get() != NULL) {
         message.toWire(renderer, *tsig_context);
@@ -653,7 +661,12 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message,
             send_answer = impl_->processNotify(io_message, message, buffer,
                                                tsig_context);
         } else if (opcode == Opcode::UPDATE()) {
-            send_answer = impl_->processUpdate(io_message);
+            if (impl_->ddns_forwarder_) {
+                send_answer = impl_->processUpdate(io_message);
+            } else {
+                makeErrorMessage(impl_->renderer_, message, buffer,
+                                 Rcode::NOTIMP(), tsig_context);
+            }
         } else if (opcode != Opcode::QUERY()) {
             LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_UNSUPPORTED_OPCODE)
                       .arg(message.getOpcode().toText());
@@ -880,7 +893,7 @@ AuthSrvImpl::processUpdate(const IOMessage& io_message) {
     // Push the update request to a separate process via the forwarder.
     // On successful push, the request shouldn't be responded from b10-auth,
     // so we return false.
-    ddns_forwarder_.push(io_message);
+    ddns_forwarder_->push(io_message);
     return (false);
 }
 
@@ -1027,3 +1040,20 @@ void
 AuthSrv::setTSIGKeyRing(const boost::shared_ptr<TSIGKeyRing>* keyring) {
     impl_->keyring_ = keyring;
 }
+
+void
+AuthSrv::createDDNSForwarder() {
+    LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_START_DDNS_FORWARDER);
+    impl_->ddns_forwarder_.reset(
+        new SocketSessionForwarderHolder("update", impl_->ddns_base_forwarder_));
+}
+
+void
+AuthSrv::destroyDDNSForwarder() {
+    if (impl_->ddns_forwarder_) {
+        LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_STOP_DDNS_FORWARDER);
+        impl_->ddns_forwarder_.reset();
+    }
+}
+
+

src/bin/auth/auth_srv.h

@@ -111,7 +111,7 @@ public:
     /// This method should never throw an exception.
     void stop();
 
-    /// \brief Process an incoming DNS message, then signal 'server' to resume 
+    /// \brief Process an incoming DNS message, then signal 'server' to resume
     ///
     /// A DNS query (or other message) has been received by a \c DNSServer
     /// object.  Find an answer, then post the \c DNSServer object on the
@@ -355,13 +355,13 @@ public:
     bool submitStatistics() const;
 
     /// \brief Get the value of counter in the AuthCounters.
-    /// 
+    ///
     /// This function calls AuthCounters::getCounter() and
     /// returns its return value.
     ///
     /// This function never throws an exception as far as
     /// AuthCounters::getCounter() doesn't throw.
-    /// 
+    ///
     /// Note: Currently this function is for testing purpose only.
     ///
     /// \param type Type of a counter to get the value of
@@ -418,6 +418,25 @@ public:
     void setTSIGKeyRing(const boost::shared_ptr<isc::dns::TSIGKeyRing>*
                         keyring);
 
+    /// \brief Create the internal forwarder for DDNS update messages
+    ///
+    /// Until this method is called (it is called when the
+    /// start_ddns_forwarder command is sent to b10-auth), b10-auth will
+    /// respond to UPDATE messages with a NOTIMP rcode.
+    /// If the internal forwarder was already created, it is destroyed and
+    /// created again. This is useful for instance when b10-ddns is shut
+    /// down and restarted.
+    void createDDNSForwarder();
+
+    /// \brief Destroy the internal forwarder for DDNS update messages
+    ///
+    /// After this method has been called (it is called when the
+    /// stop_ddns_forwarder command is sent to b10-auth), DDNS Update
+    /// messages are no longer forwarded internally, but b10-auth will
+    /// immediately respond with a NOTIMP rcode.
+    /// If there was no forwarder yet, this method does nothing.
+    void destroyDDNSForwarder();
+
 private:
     AuthSrvImpl* impl_;
     isc::asiolink::SimpleCallback* checkin_;
@@ -428,6 +447,6 @@ private:
 
 #endif // __AUTH_SRV_H
 
-// Local Variables: 
+// Local Variables:
 // mode: c++
-// End: 
+// End:

src/bin/auth/command.cc

@@ -141,6 +141,20 @@ public:
     }
 };
 
+class StartDDNSForwarderCommand : public AuthCommand {
+public:
+    virtual void exec(AuthSrv& server, isc::data::ConstElementPtr) {
+        server.createDDNSForwarder();
+    }
+};
+
+class StopDDNSForwarderCommand : public AuthCommand {
+public:
+    virtual void exec(AuthSrv& server, isc::data::ConstElementPtr) {
+        server.destroyDDNSForwarder();
+    }
+};
+
 // Handle the "loadzone" command.
 class LoadZoneCommand : public AuthCommand {
 public:
@@ -309,6 +323,10 @@ createAuthCommand(const string& command_id) {
         return (new SendStatsCommand());
     } else if (command_id == "loadzone") {
         return (new LoadZoneCommand());
+    } else if (command_id == "start_ddns_forwarder") {
+        return (new StartDDNSForwarderCommand());
+    } else if (command_id == "stop_ddns_forwarder") {
+        return (new StopDDNSForwarderCommand());
     } else if (false && command_id == "_throw_exception") {
         // This is for testing purpose only and should not appear in the
         // actual configuration syntax.

src/bin/auth/common.cc

@@ -57,3 +57,4 @@ getDDNSSocketPath() {
 }
 
 const char* const AUTH_NAME = "b10-auth";
+const char* const AUTH_STARTED_NOTIFICATION = "auth_started";

src/bin/auth/common.h

@@ -57,6 +57,11 @@ std::string getDDNSSocketPath();
 /// This is currently b10-auth, but it can be changed easily in one place.
 extern const char* const AUTH_NAME;
 
+/// \brief Notification string that is used to inform auth is starting
+///
+/// This is sent to interested modules (currently only b10-ddns)
+extern const char* const AUTH_STARTED_NOTIFICATION;
+
 #endif // __COMMON_H
 
 // Local Variables:

src/bin/auth/main.cc

@@ -210,6 +210,12 @@ main(int argc, char* argv[]) {
 
         // Successfully initialized.
         LOG_INFO(auth_logger, AUTH_SERVER_STARTED);
+
+        // Ping any interested module that (a new) auth is up
+        // Currently, only the DDNS module is notified, but we could consider
+        // make an announcement channel for these (one-way) messages
+        cc_session->group_sendmsg(
+            isc::config::createCommand(AUTH_STARTED_NOTIFICATION), "DDNS");
         io_service.run();
 
     } catch (const std::exception& ex) {

src/bin/auth/tests/auth_srv_unittest.cc

@@ -31,6 +31,7 @@
 
 #include <datasrc/memory_datasrc.h>
 #include <auth/auth_srv.h>
+#include <auth/command.h>
 #include <auth/common.h>
 #include <auth/statistics.h>
 
@@ -99,6 +100,7 @@ protected:
         server.setDNSService(dnss_);
         server.setXfrinSession(&notify_session);
         server.setStatisticsSession(&statistics_session);
+        server.createDDNSForwarder();
     }
 
     ~AuthSrvTest() {
@@ -106,6 +108,7 @@ protected:
         // type information may be lost if the message is cleared
         // automatically later, so as a precaution we do it now.
         parse_message->clear(Message::PARSE);
+        server.destroyDDNSForwarder();
     }
 
     virtual void processMessage() {
@@ -1625,6 +1628,7 @@ TEST_F(AuthSrvTest, DDNSForwardPushFail) {
 
 TEST_F(AuthSrvTest, DDNSForwardClose) {
     scoped_ptr<AuthSrv> tmp_server(new AuthSrv(true, xfrout, ddns_forwarder));
+    tmp_server->createDDNSForwarder();
     UnitTestUtil::createRequestMessage(request_message, Opcode::UPDATE(),
                                        default_qid, Name("example.com"),
                                        RRClass::IN(), RRType::SOA());
@@ -1639,4 +1643,85 @@ TEST_F(AuthSrvTest, DDNSForwardClose) {
     EXPECT_FALSE(ddns_forwarder.isConnected());
 }
 
+namespace {
+    // Send a basic command without arguments, and check the response has
+    // result code 0
+    void sendSimpleCommand(AuthSrv& server, const std::string& command) {
+        ConstElementPtr response = execAuthServerCommand(server, command,
+                                                         ConstElementPtr());
+        int command_result = -1;
+        isc::config::parseAnswer(command_result, response);
+        EXPECT_EQ(0, command_result);
+    }
+} // end anonymous namespace
+
+TEST_F(AuthSrvTest, DDNSForwardCreateDestroy) {
+    // Test that AuthSrv returns NOTIMP before ddns forwarder is created,
+    // that the ddns_forwarder is connected when the 'start_ddns_forwarder'
+    // command has been sent, and that it is no longer connected and auth
+    // returns NOTIMP after the stop_ddns_forwarding command is sent.
+    scoped_ptr<AuthSrv> tmp_server(new AuthSrv(true, xfrout, ddns_forwarder));
+
+    // Prepare update message to send
+    UnitTestUtil::createRequestMessage(request_message, Opcode::UPDATE(),
+                                       default_qid, Name("example.com"),
+                                       RRClass::IN(), RRType::SOA());
+    createRequestPacket(request_message, IPPROTO_UDP);
+
+    // before creating forwarder. isConnected() should be false and
+    // rcode to UPDATE should be NOTIMP
+    parse_message->clear(Message::PARSE);
+    tmp_server->processMessage(*io_message, *parse_message, *response_obuffer,
+                               &dnsserv);
+    EXPECT_FALSE(ddns_forwarder.isConnected());
+    EXPECT_TRUE(dnsserv.hasAnswer());
+    headerCheck(*parse_message, default_qid, Rcode::NOTIMP(),
+                Opcode::UPDATE().getCode(), QR_FLAG, 0, 0, 0, 0);
+
+    // now create forwarder
+    sendSimpleCommand(*tmp_server, "start_ddns_forwarder");
+
+    // our mock does not respond, and since auth is supposed to send it on,
+    // there should now be no result when an UPDATE is sent
+    parse_message->clear(Message::PARSE);
+    tmp_server->processMessage(*io_message, *parse_message, *response_obuffer,
+                               &dnsserv);
+    EXPECT_FALSE(dnsserv.hasAnswer());
+    EXPECT_TRUE(ddns_forwarder.isConnected());
+
+    // If we send a start again, the connection should be recreated,
+    // visible because isConnected() reports false until an actual message
+    // has been forwarded
+    sendSimpleCommand(*tmp_server, "start_ddns_forwarder");
+
+    EXPECT_FALSE(ddns_forwarder.isConnected());
+    parse_message->clear(Message::PARSE);
+    tmp_server->processMessage(*io_message, *parse_message, *response_obuffer,
+                               &dnsserv);
+    EXPECT_FALSE(dnsserv.hasAnswer());
+    EXPECT_TRUE(ddns_forwarder.isConnected());
+
+    // Now tell it to stop forwarder, should respond with NOTIMP again
+    sendSimpleCommand(*tmp_server, "stop_ddns_forwarder");
+
+    parse_message->clear(Message::PARSE);
+    tmp_server->processMessage(*io_message, *parse_message, *response_obuffer,
+                               &dnsserv);
+    EXPECT_FALSE(ddns_forwarder.isConnected());
+    EXPECT_TRUE(dnsserv.hasAnswer());
+    headerCheck(*parse_message, default_qid, Rcode::NOTIMP(),
+                Opcode::UPDATE().getCode(), QR_FLAG, 0, 0, 0, 0);
+
+    // Sending stop again should make no difference
+    sendSimpleCommand(*tmp_server, "stop_ddns_forwarder");
+
+    parse_message->clear(Message::PARSE);
+    tmp_server->processMessage(*io_message, *parse_message, *response_obuffer,
+                               &dnsserv);
+    EXPECT_FALSE(ddns_forwarder.isConnected());
+    EXPECT_TRUE(dnsserv.hasAnswer());
+    headerCheck(*parse_message, default_qid, Rcode::NOTIMP(),
+                Opcode::UPDATE().getCode(), QR_FLAG, 0, 0, 0, 0);
+}
+
 }

src/bin/cfgmgr/b10-cfgmgr.py.in

@@ -25,6 +25,7 @@ import os
 from optparse import OptionParser
 import glob
 import os.path
+import imp
 import isc.log
 isc.log.init("b10-cfgmgr")
 from isc.config.cfgmgr import ConfigManager, ConfigManagerDataReadError, logger
@@ -67,23 +68,16 @@ def load_plugins(path, cm):
     """Load all python files in the given path and treat them as plugins."""
     # Find the python files
     plugins = glob.glob(path + os.sep + '*.py')
-    # Search this directory first, but leave the others there for the imports
-    # of the modules
-    sys.path.insert(0, path)
-    try:
-        for plugin in plugins:
-            # Generate the name of the plugin
-            filename = os.path.basename(plugin)
-            name = filename[:-3]
-            # Load it
-            module = __import__(name)
-            # Ask it to provide the spec and checking function
-            (spec, check_func) = module.load()
-            # And insert it into the manager
-            cm.set_virtual_module(spec, check_func)
-    finally:
-        # Restore the search path
-        sys.path = sys.path[1:]
+    for plugin in plugins:
+        # Generate the name of the plugin
+        filename = os.path.basename(plugin)
+        name = filename[:-3]
+        # Load it
+        module = imp.load_source(name, plugin)
+        # Ask it to provide the spec and checking function
+        (spec, check_func) = module.load()
+        # And insert it into the manager
+        cm.set_virtual_module(spec, check_func)
 
 
 def determine_path_and_file(data_path_option, config_file_option):

src/bin/ddns/ddns.py.in

@@ -228,6 +228,9 @@ class DDNSServer:
         # Outstanding TCP context: fileno=>(context_obj, dst)
         self._tcp_ctxs = {}
 
+        # Notify Auth server that DDNS update messages can now be forwarded
+        self.__notify_start_forwarder()
+
     class InternalError(Exception):
         '''Exception for internal errors in an update session.
 
@@ -274,6 +277,9 @@ class DDNSServer:
             logger.info(DDNS_RECEIVED_SHUTDOWN_COMMAND)
             self.trigger_shutdown()
             answer = create_answer(0)
+        elif cmd == "auth_started":
+            self.__notify_start_forwarder()
+            answer = None
         else:
             answer = create_answer(1, "Unknown command: " + str(cmd))
         return answer
@@ -378,6 +384,8 @@ class DDNSServer:
         Do NOT call this to initialize shutdown, use trigger_shutdown().
 
         '''
+        # tell Auth not to forward UPDATE messages anymore
+        self.__notify_stop_forwarder()
         # tell the ModuleCCSession to send a message that this module is
         # stopping.
         self._cc.send_stopping()
@@ -533,6 +541,32 @@ class DDNSServer:
 
         return True
 
+    def __notify_start_forwarder(self):
+        '''Notify auth that DDNS Update messages can now be forwarded'''
+        try:
+            seq = self._cc._session.group_sendmsg(create_command(
+                    "start_ddns_forwarder"), AUTH_MODULE_NAME)
+            answer, _ = self._cc._session.group_recvmsg(False, seq)
+            rcode, error_msg = parse_answer(answer)
+            if rcode != 0:
+                logger.error(DDNS_START_FORWARDER_ERROR, error_msg)
+        except (SessionTimeout, SessionError, ProtocolError) as ex:
+            logger.error(DDNS_START_FORWARDER_FAIL, ex)
+
+    def __notify_stop_forwarder(self):
+        '''Notify auth that DDNS Update messages should no longer be forwarded.
+
+        '''
+        try:
+            seq = self._cc._session.group_sendmsg(create_command(
+                    "stop_ddns_forwarder"), AUTH_MODULE_NAME)
+            answer, _ = self._cc._session.group_recvmsg(False, seq)
+            rcode, error_msg = parse_answer(answer)
+            if rcode != 0:
+                logger.error(DDNS_STOP_FORWARDER_ERROR, error_msg)
+        except (SessionTimeout, SessionError, ProtocolError) as ex:
+            logger.error(DDNS_STOP_FORWARDER_FAIL, ex)
+
     def __notify_auth(self, zname, zclass):
         '''Notify auth of the update, if necessary.'''
         msg = auth_loadzone_command(self._cc, zname, zclass)

src/bin/ddns/ddns_messages.mes

@@ -192,6 +192,25 @@ be completed, after which the process will exit.
 The ddns process has successfully started and is now ready to receive commands
 and updates.
 
+% DDNS_START_FORWARDER_ERROR Error from b10-auth when requesting DDNS UPDATE forwarding: %1
+There was an error response from b10-auth to the command to start
+forwarding DDNS UPDATE messages to b10-ddns.
+It is almost certain that DDNS UPDATE messages are not handled now, and that
+they will be responded to with a NOTIMP error code, even though b10-ddns is
+running.
+The error message is printed, and additional information may be found in
+the b10-auth log output. Since this is an error that is sent by the
+b10-auth module, it should have more information as to what the actual
+problem was.
+
+% DDNS_START_FORWARDER_FAIL Error sending request for DDNS UPDATE forwarding to b10-auth: %1
+There was an error when attempting to send b10-auth the request to forward
+DDNS UPDATE messages to the b10-ddns module. This points to an internal
+problem using the inter-module messaging system.
+This needs to be inspected, as it is almost certain that DDNS UPDATE messages
+are not handled now.
+The specific error is printed in the log message.
+
 % DDNS_STOPPED ddns server has stopped
 The ddns process has successfully stopped and is no longer listening for or
 handling commands or updates, and will now exit.
@@ -200,6 +219,24 @@ handling commands or updates, and will now exit.
 There was a keyboard interrupt signal to stop the ddns process. The
 process will now shut down.
 
+% DDNS_STOP_FORWARDER_ERROR Error from b10-auth when requesting to stop DDNS UPDATE forwarding: %1
+There was an error response from b10-auth to the command to stop
+forwarding DDNS UPDATE messages to b10-ddns.
+This specific error may not be fatal; instead of returning NOTIMP for future
+DDNS UPDATE messages, it will return SERVFAIL. However, this does point to
+an underlying problem in the messaging system, and should be inspected.
+The error message is printed, and additional information may be found in
+the b10-auth log output.
+
+% DDNS_STOP_FORWARDER_FAIL Error sending request to stop DDNS UPDATE forwarding to b10-auth: %1
+There was an error when attempting to send b10-auth the request to stop
+forwarding DDNS UPDATE messages to the b10-ddns module. This points to an
+internal problem using the inter-module messaging system.
+This specific error may not be fatal; instead of returning NOTIMP for future
+DDNS UPDATE messages, it will return SERVFAIL. However, this does point to
+an underlying problem in the messaging system, and should be inspected.
+The specific error is printed in the log message.
+
 % DDNS_UNCAUGHT_EXCEPTION uncaught exception of type %1: %2
 The b10-ddns process encountered an uncaught exception and will now shut
 down. This is indicative of a programming error and should not happen under

src/bin/ddns/tests/ddns_test.py

@@ -918,6 +918,10 @@ class TestDDNSSession(unittest.TestCase):
         self.orig_tsig_keyring = isc.server_common.tsig_keyring
         isc.server_common.tsig_keyring = FakeKeyringModule()
         self.server = ddns.DDNSServer(self.__cc_session)
+        # Check that start_ddns_forwarder has been called upon
+        # initialization (before we do anything else that might
+        # cause messages to be sent)
+        self.check_session_start_forwarder_called()
         self.server._UpdateSessionClass = self.__fake_session_creator
         self.__faked_result = UPDATE_SUCCESS # will be returned by fake session
         self.__sock = FakeSocket(-1)
@@ -1179,8 +1183,33 @@ class TestDDNSSession(unittest.TestCase):
             self.assertEqual([], self.__cc_session._sent_msg)
             self.assertEqual(0, self.__cc_session._recvmsg_called)
 
+    def check_session_start_forwarder_called(self):
+        '''Check that the command 'start_ddns_forwarder' has been called
+           This test removes said message from the sent message queue.
+        '''
+        sent_msg, sent_group = self.__cc_session._sent_msg.pop(0)
+        sent_cmd = sent_msg['command']
+        self.assertEqual('Auth', sent_group)
+        self.assertEqual('start_ddns_forwarder', sent_cmd[0])
+        self.assertEqual(1, len(sent_cmd))
+        self.assertEqual(1, self.__cc_session._recvmsg_called)
+        # reset it for other tests
+        self.__cc_session._recvmsg_called = 0
+
+    def check_session_stop_forwarder_called(self):
+        '''Check that the command 'stop_ddns_forwarder' has been called
+           This test removes said message from the sent message queue.
+        '''
+        # check the last message sent
+        sent_msg, sent_group = self.__cc_session._sent_msg.pop()
+        sent_cmd = sent_msg['command']
+        self.assertEqual('Auth', sent_group)
+        self.assertEqual('stop_ddns_forwarder', sent_cmd[0])
+        self.assertEqual(1, len(sent_cmd))
+
     def test_session_msg(self):
         '''Test post update communication with other modules.'''
+
         # Normal cases, confirming communication takes place iff update
         # succeeds
         for r in [UPDATE_SUCCESS, UPDATE_ERROR, UPDATE_DROP]:
@@ -1224,8 +1253,14 @@ class TestDDNSSession(unittest.TestCase):
         self.__cc_session._sendmsg_exception = RuntimeError('unexpected')
         self.assertRaises(RuntimeError, self.check_session)
 
+    def test_session_shutdown_cleanup(self):
+        '''Test that the stop forwarding message is sent'''
+        self.server.shutdown_cleanup()
+        self.check_session_stop_forwarder_called()
+
     def test_session_msg_for_auth(self):
         '''Test post update communication with other modules including Auth.'''
+
         # Let the CC session return in-memory config with sqlite3 backend.
         # (The default case was covered by other tests.)
         self.__cc_session.auth_datasources = \
@@ -1270,6 +1305,34 @@ class TestDDNSSession(unittest.TestCase):
         # check the result
         self.check_session(UPDATE_DROP)
 
+    def test_session_start_stop_forwarder_failures(self):
+        '''Check that we don't crash if the server reports an error
+           setting up or closing down the DDNS UPDATE message forwarder,
+           or if there is an exception from the message queue.'''
+        self.__cc_session._answer_code = 1
+        self.server._DDNSServer__notify_start_forwarder()
+        self.server._DDNSServer__notify_stop_forwarder()
+
+        for exc in [ SessionError("sessionerror"),
+                     SessionTimeout("sessiontimeout"),
+                     ProtocolError("protocolerror") ]:
+            self.__cc_session._recvmsg_exception = exc
+            self.server._DDNSServer__notify_start_forwarder()
+            self.server._DDNSServer__notify_stop_forwarder()
+            self.__cc_session._recvmsg_exception = None
+
+            self.__cc_session._sendmsg_exception = exc
+            self.server._DDNSServer__notify_start_forwarder()
+            self.server._DDNSServer__notify_stop_forwarder()
+            self.__cc_session._recvmsg_exception = None
+
+    def test_session_auth_started(self):
+        '''Check that 'start_ddns_forwarder' is sent (again) when the
+           notification 'auth_started' is received'''
+        # auth_started message should trigger it again
+        answer = self.server.command_handler('auth_started', None)
+        self.check_session_start_forwarder_called()
+
 class TestMain(unittest.TestCase):
     def setUp(self):
         self._server = MyDDNSServer()

src/lib/config/module_spec.cc

@@ -476,9 +476,9 @@ ModuleSpec::validateSpecList(ConstElementPtr spec, ConstElementPtr data,
     typedef std::pair<std::string, ConstElementPtr> maptype;
     
     BOOST_FOREACH(maptype m, data->mapValue()) {
-        bool found = false;
         // Ignore 'version' as a config element
         if (m.first.compare("version") != 0) {
+            bool found = false;
             BOOST_FOREACH(ConstElementPtr cur_spec_el, spec->listValue()) {
                 if (cur_spec_el->get("item_name")->stringValue().compare(m.first) == 0) {
                     found = true;

src/lib/datasrc/client_list.cc

@@ -167,6 +167,39 @@ ConfigurableClientList::configure(const Element& config, bool allow_cache) {
     }
 }
 
+namespace {
+
+class CacheKeeper : public ClientList::FindResult::LifeKeeper {
+public:
+    CacheKeeper(const boost::shared_ptr<InMemoryClient>& cache) :
+        cache_(cache)
+    {}
+private:
+    const boost::shared_ptr<InMemoryClient> cache_;
+};
+
+class ContainerKeeper : public ClientList::FindResult::LifeKeeper {
+public:
+    ContainerKeeper(const DataSourceClientContainerPtr& container) :
+        container_(container)
+    {}
+private:
+    const DataSourceClientContainerPtr container_;
+};
+
+boost::shared_ptr<ClientList::FindResult::LifeKeeper>
+genKeeper(const ConfigurableClientList::DataSourceInfo& info) {
+    if (info.cache_) {
+        return (boost::shared_ptr<ClientList::FindResult::LifeKeeper>(
+            new CacheKeeper(info.cache_)));
+    } else {
+        return (boost::shared_ptr<ClientList::FindResult::LifeKeeper>(
+            new ContainerKeeper(info.container_)));
+    }
+}
+
+}
+
 ClientList::FindResult
 ConfigurableClientList::find(const dns::Name& name, bool want_exact_match,
                             bool) const
@@ -185,10 +218,11 @@ ConfigurableClientList::find(const dns::Name& name, bool want_exact_match,
         ZoneFinderPtr finder;
         uint8_t matched_labels;
         bool matched;
+        boost::shared_ptr<FindResult::LifeKeeper> keeper;
         operator FindResult() const {
             // Conversion to the right result. If we return this, there was
             // a partial match at best.
-            return (FindResult(datasrc_client, finder, false));
+            return (FindResult(datasrc_client, finder, false, keeper));
         }
     } candidate;
 
@@ -206,7 +240,7 @@ ConfigurableClientList::find(const dns::Name& name, bool want_exact_match,
                 // TODO: In case we have only the datasource and not the finder
                 // and the need_updater parameter is true, get the zone there.
                 return (FindResult(client, result.zone_finder,
-                                   true));
+                                   true, genKeeper(info)));
             case result::PARTIALMATCH:
                 if (!want_exact_match) {
                     // In case we have a partial match, check if it is better
@@ -230,6 +264,7 @@ ConfigurableClientList::find(const dns::Name& name, bool want_exact_match,
                         candidate.finder = result.zone_finder;
                         candidate.matched_labels = labels;
                         candidate.matched = true;
+                        candidate.keeper = genKeeper(info);
                     }
                 }
                 break;

src/lib/datasrc/client_list.h

@@ -65,15 +65,27 @@ public:
     /// Instead, all the member variables are defined as const and can be
     /// accessed directly.
     struct FindResult {
+        /// \brief Internal class for holding a reference.
+        ///
+        /// This is used to make sure the data source client isn't released
+        /// too soon.
+        ///
+        /// \see life_keeper_;
+        class LifeKeeper {
+        public:
+            virtual ~LifeKeeper() {};
+        };
         /// \brief Constructor.
         ///
         /// It simply fills in the member variables according to the
         /// parameters. See the member descriptions for their meaning.
         FindResult(DataSourceClient* dsrc_client, const ZoneFinderPtr& finder,
-                   bool exact_match) :
+                   bool exact_match,
+                   const boost::shared_ptr<LifeKeeper>& life_keeper) :
             dsrc_client_(dsrc_client),
             finder_(finder),
-            exact_match_(exact_match)
+            exact_match_(exact_match),
+            life_keeper_(life_keeper)
         {}
 
         /// \brief Negative answer constructor.
@@ -101,8 +113,9 @@ public:
         /// If no such data source exists, this is NULL pointer.
         ///
         /// Note that the pointer is valid only as long the ClientList which
-        /// returned the pointer is alive and was not reconfigured. The
-        /// ownership is preserved within the ClientList.
+        /// returned the pointer is alive and was not reconfigured or you hold
+        /// a reference to life_keeper_. The ownership is preserved within the
+        /// ClientList.
         DataSourceClient* const dsrc_client_;
 
         /// \brief The finder for the requested zone.
@@ -116,6 +129,12 @@ public:
 
         /// \brief If the result is an exact match.
         const bool exact_match_;
+
+        /// \brief Something that holds the dsrc_client_ valid.
+        ///
+        /// As long as you hold the life_keeper_, the dsrc_client_ is
+        /// guaranteed to be valid.
+        const boost::shared_ptr<LifeKeeper> life_keeper_;
     };
 
     /// \brief Search for a zone through the data sources.

src/lib/datasrc/rbtree.h

@@ -123,6 +123,8 @@ public:
     /// set to on by the \c setFlag() method.
     enum Flags {
         FLAG_CALLBACK = 1, ///< Callback enabled. See \ref callback
+        FLAG_RED = 2, ///< Node color; 1 if node is red, 0 if node is black.
+        FLAG_SUBTREE_ROOT = 4, ///< Set if the node is the root of a subtree
         FLAG_USER1 = 0x80000000U, ///< Application specific flag
         FLAG_USER2 = 0x40000000U, ///< Application specific flag
         FLAG_USER3 = 0x20000000U  ///< Application specific flag
@@ -246,6 +248,36 @@ private:
         return (&null_node);
     }
 
+    /// \brief Returns the color of this node
+    RBNodeColor getColor() const {
+        if ((flags_ & FLAG_RED) != 0) {
+            return (RED);
+        } else {
+            return (BLACK);
+        }
+    }
+
+    /// \brief Sets the color of this node
+    void setColor(const RBNodeColor color) {
+        if (color == RED) {
+            flags_ |= FLAG_RED;
+        } else {
+            flags_ &= ~FLAG_RED;
+        }
+    }
+
+    void setSubTreeRoot(bool root) {
+        if (root) {
+            flags_ |= FLAG_SUBTREE_ROOT;
+        } else {
+            flags_ &= ~FLAG_SUBTREE_ROOT;
+        }
+    }
+
+    bool isSubTreeRoot() const {
+        return ((flags_ & FLAG_SUBTREE_ROOT) != 0);
+    }
+
     /// \brief return the next node which is bigger than current node
     /// in the same subtree
     ///
@@ -299,7 +331,6 @@ private:
     RBNode<T>*  parent_;
     RBNode<T>*  left_;
     RBNode<T>*  right_;
-    RBNodeColor color_;
     //@}
 
     /// \brief Relative name of the node.
@@ -332,11 +363,10 @@ RBNode<T>::RBNode() :
     parent_(NULL),
     left_(NULL),
     right_(NULL),
-    color_(BLACK),
     // dummy name, the value doesn't matter:
     name_(isc::dns::Name::ROOT_NAME()),
     down_(NULL),
-    flags_(0)
+    flags_(FLAG_SUBTREE_ROOT)
 {
     // Some compilers object to use of "this" in initializer lists.
     parent_ = this;
@@ -350,10 +380,9 @@ RBNode<T>::RBNode(const isc::dns::Name& name) :
     parent_(NULL_NODE()),
     left_(NULL_NODE()),
     right_(NULL_NODE()),
-    color_(RED),
     name_(name),
     down_(NULL_NODE()),
-    flags_(0)
+    flags_(FLAG_RED | FLAG_SUBTREE_ROOT)
 {
 }
 
@@ -1374,10 +1403,13 @@ RBTree<T>::insert(const isc::dns::Name& target_name, RBNode<T>** new_node) {
         *current_root = node.get();
         //node is the new root of sub tree, so its init color
         // is BLACK
-        node->color_ = RBNode<T>::BLACK;
+        node->setColor(RBNode<T>::BLACK);
+        node->setSubTreeRoot(true);
     } else if (order < 0) {
+        node->setSubTreeRoot(false);
         parent->left_ = node.get();
     } else {
+        node->setSubTreeRoot(false);
         parent->right_ = node.get();
     }
     insertRebalance(current_root, node.get());
@@ -1409,10 +1441,19 @@ RBTree<T>::nodeFission(RBNode<T>& node, const isc::dns::Name& base_name) {
     // even if code after the call to this function throws an exception.
     std::swap(node.data_, down_node->data_);
     std::swap(node.flags_, down_node->flags_);
+
     down_node->down_ = node.down_;
     node.down_ = down_node.get();
+
+    // Restore the color of the node (may have gotten changed by the flags swap)
+    node.setColor(down_node->getColor());
+
     // root node of sub tree, the initial color is BLACK
-    down_node->color_ = RBNode<T>::BLACK;
+    down_node->setColor(RBNode<T>::BLACK);
+
+    // mark it as the root of a subtree
+    down_node->setSubTreeRoot(true);
+
     ++node_count_;
     down_node.release();
 }
@@ -1423,44 +1464,44 @@ void
 RBTree<T>::insertRebalance(RBNode<T>** root, RBNode<T>* node) {
 
     RBNode<T>* uncle;
-    while (node != *root && node->parent_->color_ == RBNode<T>::RED) {
+    while (node != *root && node->parent_->getColor() == RBNode<T>::RED) {
         if (node->parent_ == node->parent_->parent_->left_) {
             uncle = node->parent_->parent_->right_;
 
-            if (uncle->color_ == RBNode<T>::RED) {
-                node->parent_->color_ = RBNode<T>::BLACK;
-                uncle->color_ = RBNode<T>::BLACK;
-                node->parent_->parent_->color_ = RBNode<T>::RED;
+            if (uncle->getColor() == RBNode<T>::RED) {
+                node->parent_->setColor(RBNode<T>::BLACK);
+                uncle->setColor(RBNode<T>::BLACK);
+                node->parent_->parent_->setColor(RBNode<T>::RED);
                 node = node->parent_->parent_;
             } else {
                 if (node == node->parent_->right_) {
                     node = node->parent_;
                     leftRotate(root, node);
                 }
-                node->parent_->color_ = RBNode<T>::BLACK;
-                node->parent_->parent_->color_ = RBNode<T>::RED;
+                node->parent_->setColor(RBNode<T>::BLACK);
+                node->parent_->parent_->setColor(RBNode<T>::RED);
                 rightRotate(root, node->parent_->parent_);
             }
         } else {
             uncle = node->parent_->parent_->left_;
-            if (uncle->color_ == RBNode<T>::RED) {
-                node->parent_->color_ = RBNode<T>::BLACK;
-                uncle->color_ = RBNode<T>::BLACK;
-                node->parent_->parent_->color_ = RBNode<T>::RED;
+            if (uncle->getColor() == RBNode<T>::RED) {
+                node->parent_->setColor(RBNode<T>::BLACK);
+                uncle->setColor(RBNode<T>::BLACK);
+                node->parent_->parent_->setColor(RBNode<T>::RED);
                 node = node->parent_->parent_;
             } else {
                 if (node == node->parent_->left_) {
                     node = node->parent_;
                     rightRotate(root, node);
                 }
-                node->parent_->color_ = RBNode<T>::BLACK;
-                node->parent_->parent_->color_ = RBNode<T>::RED;
+                node->parent_->setColor(RBNode<T>::BLACK);
+                node->parent_->parent_->setColor(RBNode<T>::RED);
                 leftRotate(root, node->parent_->parent_);
             }
         }
     }
 
-    (*root)->color_ = RBNode<T>::BLACK;
+    (*root)->setColor(RBNode<T>::BLACK);
 }
 
 
@@ -1469,23 +1510,30 @@ RBNode<T>*
 RBTree<T>::leftRotate(RBNode<T>** root, RBNode<T>* node) {
     RBNode<T>* right = node->right_;
     node->right_ = right->left_;
-    if (right->left_ != NULLNODE)
+    if (right->left_ != NULLNODE) {
         right->left_->parent_ = node;
+        right->left_->setSubTreeRoot(false);
+    } else {
+        right->left_->setSubTreeRoot(true);
+    }
 
     right->parent_ = node->parent_;
 
     if (node->parent_ != NULLNODE) {
+        right->setSubTreeRoot(false);
         if (node == node->parent_->left_) {
             node->parent_->left_ = right;
         } else  {
             node->parent_->right_ = right;
         }
     } else {
+        right->setSubTreeRoot(true);
         *root = right;
     }
 
     right->left_ = node;
     node->parent_ = right;
+    node->setSubTreeRoot(false);
     return (node);
 }
 
@@ -1494,22 +1542,30 @@ RBNode<T>*
 RBTree<T>::rightRotate(RBNode<T>** root, RBNode<T>* node) {
     RBNode<T>* left = node->left_;
     node->left_ = left->right_;
-    if (left->right_ != NULLNODE)
+    if (left->right_ != NULLNODE) {
         left->right_->parent_ = node;
+        left->right_->setSubTreeRoot(false);
+    } else {
+        left->right_->setSubTreeRoot(true);
+    }
 
     left->parent_ = node->parent_;
 
     if (node->parent_ != NULLNODE) {
+        left->setSubTreeRoot(false);
         if (node == node->parent_->right_) {
             node->parent_->right_ = left;
         } else  {
             node->parent_->left_ = left;
         }
     } else {
+        left->setSubTreeRoot(true);
         *root = left;
     }
     left->right_ = node;
     node->parent_ = left;
+    node->setSubTreeRoot(false);
+
     return (node);
 }
 
@@ -1535,8 +1591,14 @@ RBTree<T>::dumpTreeHelper(std::ostream& os, const RBNode<T>* node,
 
     indent(os, depth);
     os << node->name_.toText() << " ("
-              << ((node->color_ == RBNode<T>::BLACK) ? "black" : "red") << ")";
-    os << ((node->isEmpty()) ? "[invisible] \n" : "\n");
+              << ((node->getColor() == RBNode<T>::BLACK) ? "black" : "red") << ")";
+    if (node->isEmpty()) {
+        os << " [invisible]";
+    }
+    if (node->isSubTreeRoot()) {
+        os << " [subtreeroot]";
+    }
+    os << "\n";
 
     if (node->down_ != NULLNODE) {
         indent(os, depth + 1);

src/lib/datasrc/tests/client_list_unittest.cc

@@ -245,6 +245,12 @@ public:
         ASSERT_NE(ZoneFinderPtr(), result.finder_);
         EXPECT_EQ(name, result.finder_->getOrigin());
         EXPECT_EQ(exact, result.exact_match_);
+        // If it is a positive result, there's something to keep
+        // alive, even when we don't know what it is.
+        // Any better idea how to test it actually keeps the thing
+        // alive?
+        EXPECT_NE(shared_ptr<ClientList::FindResult::LifeKeeper>(),
+                  result.life_keeper_);
         if (from_cache) {
             EXPECT_NE(shared_ptr<InMemoryZoneFinder>(),
                       dynamic_pointer_cast<InMemoryZoneFinder>(
@@ -315,6 +321,9 @@ TEST_F(ListTest, selfTest) {
     EXPECT_EQ(result::NOTFOUND, ds_[1]->findZone(Name("example.org")).code);
     EXPECT_EQ(result::NOTFOUND, ds_[0]->findZone(Name("aaa")).code);
     EXPECT_EQ(result::NOTFOUND, ds_[0]->findZone(Name("zzz")).code);
+    // Nothing to keep alive here.
+    EXPECT_EQ(shared_ptr<ClientList::FindResult::LifeKeeper>(),
+                  negativeResult_.life_keeper_);
 }
 
 // Test the list we create with empty configuration is, in fact, empty

src/lib/log/tests/logger_name_unittest.cc

@@ -31,9 +31,9 @@ using namespace isc::log;
 
 class LoggerNameTest : public ::testing::Test {
 public:
-    LoggerNameTest() {
-        name_ = getRootLoggerName();
-    }
+    LoggerNameTest() :
+        name_(getRootLoggerName())
+    {}
     ~LoggerNameTest() {
         setRootLoggerName(name_);
     }

src/lib/python/isc/datasrc/Makefile.am

@@ -18,6 +18,8 @@ datasrc_la_SOURCES += iterator_python.cc iterator_python.h
 datasrc_la_SOURCES += finder_python.cc finder_python.h
 datasrc_la_SOURCES += updater_python.cc updater_python.h
 datasrc_la_SOURCES += journal_reader_python.cc journal_reader_python.h
+datasrc_la_SOURCES += configurableclientlist_python.cc
+datasrc_la_SOURCES += configurableclientlist_python.h
 
 datasrc_la_CPPFLAGS = $(AM_CPPFLAGS) $(PYTHON_INCLUDES)
 datasrc_la_CXXFLAGS = $(AM_CXXFLAGS) $(PYTHON_CXXFLAGS)

src/lib/python/isc/datasrc/client_python.cc

@@ -28,6 +28,7 @@
 #include <datasrc/data_source.h>
 #include <datasrc/sqlite3_accessor.h>
 #include <datasrc/iterator.h>
+#include <datasrc/client_list.h>
 
 #include <dns/python/name_python.h>
 #include <dns/python/rrset_python.h>
@@ -51,8 +52,17 @@ namespace {
 // The s_* Class simply covers one instantiation of the object
 class s_DataSourceClient : public PyObject {
 public:
-    s_DataSourceClient() : cppobj(NULL) {};
+    s_DataSourceClient() :
+        cppobj(NULL),
+        client(NULL),
+        keeper(NULL)
+    {};
     DataSourceClientContainer* cppobj;
+    DataSourceClient* client;
+    // We can't rely on the constructor or destructor being
+    // called, so this is a pointer to shared pointer, so we
+    // can call the new and delete explicitly.
+    boost::shared_ptr<ClientList::FindResult::LifeKeeper>* keeper;
 };
 
 PyObject*
@@ -62,7 +72,7 @@ DataSourceClient_findZone(PyObject* po_self, PyObject* args) {
     if (PyArg_ParseTuple(args, "O!", &name_type, &name)) {
         try {
             DataSourceClient::FindResult find_result(
-                self->cppobj->getInstance().findZone(PyName_ToName(name)));
+                self->client->findZone(PyName_ToName(name)));
 
             result::Result r = find_result.code;
             ZoneFinderPtr zfp = find_result.zone_finder;
@@ -103,7 +113,7 @@ DataSourceClient_getIterator(PyObject* po_self, PyObject* args) {
                 }
             }
             return (createZoneIteratorObject(
-                self->cppobj->getInstance().getIterator(PyName_ToName(name_obj),
+                self->client->getIterator(PyName_ToName(name_obj),
                                                         separate_rrs),
                 po_self));
         } catch (const isc::NotImplemented& ne) {
@@ -139,7 +149,7 @@ DataSourceClient_getUpdater(PyObject* po_self, PyObject* args) {
         const bool journaling = (journaling_obj == Py_True);
         try {
             ZoneUpdaterPtr updater =
-                self->cppobj->getInstance().getUpdater(PyName_ToName(name_obj),
+                self->client->getUpdater(PyName_ToName(name_obj),
                                                        replace, journaling);
             if (!updater) {
                 return (Py_None);
@@ -184,7 +194,7 @@ DataSourceClient_getJournalReader(PyObject* po_self, PyObject* args) {
                          &begin_obj, &end_obj)) {
         try {
             pair<ZoneJournalReader::Result, ZoneJournalReaderPtr> result =
-                self->cppobj->getInstance().getJournalReader(
+                self->client->getJournalReader(
                     PyName_ToName(name_obj), static_cast<uint32_t>(begin_obj),
                     static_cast<uint32_t>(end_obj));
             PyObject* po_reader;
@@ -245,6 +255,8 @@ DataSourceClient_init(PyObject* po_self, PyObject* args, PyObject*) {
                 isc::data::Element::fromJSON(ds_config_str);
             self->cppobj = new DataSourceClientContainer(ds_type_str,
                                                          ds_config);
+            self->client = &self->cppobj->getInstance();
+            self->keeper = NULL;
             return (0);
         } else {
             return (-1);
@@ -280,7 +292,10 @@ void
 DataSourceClient_destroy(PyObject* po_self) {
     s_DataSourceClient* const self = static_cast<s_DataSourceClient*>(po_self);
     delete self->cppobj;
+    delete self->keeper;
     self->cppobj = NULL;
+    self->client = NULL;
+    self->keeper = NULL;
     Py_TYPE(self)->tp_free(self);
 }
 
@@ -342,6 +357,23 @@ PyTypeObject datasourceclient_type = {
     0                                   // tp_version_tag
 };
 
+PyObject*
+wrapDataSourceClient(DataSourceClient* client,
+                     const boost::shared_ptr<ClientList::FindResult::
+                     LifeKeeper>& life_keeper)
+{
+    s_DataSourceClient *result =
+        static_cast<s_DataSourceClient*>(PyObject_New(s_DataSourceClient,
+                                                      &datasourceclient_type));
+    CPPPyObjectContainer<s_DataSourceClient, DataSourceClientContainer>
+        container(result);
+    result->cppobj = NULL;
+    result->keeper =
+        new boost::shared_ptr<ClientList::FindResult::LifeKeeper>(life_keeper);
+    result->client = client;
+    return (container.release());
+}
+
 } // namespace python
 } // namespace datasrc
 } // namespace isc

src/lib/python/isc/datasrc/client_python.h

@@ -15,6 +15,8 @@
 #ifndef __PYTHON_DATASRC_CLIENT_H
 #define __PYTHON_DATASRC_CLIENT_H 1
 
+#include <datasrc/client_list.h>
+
 #include <Python.h>
 
 namespace isc {
@@ -25,6 +27,23 @@ namespace python {
 
 extern PyTypeObject datasourceclient_type;
 
+/// \brief Create a DataSourceClient python object
+///
+/// Unlike many similar functions, this one does not create a copied instance
+/// of the passed object. It wraps the given one. This is why the name is
+/// different than the usual.
+///
+/// \param client The client to wrap.
+/// \param life_keeper An optional object which keeps the client pointer valid.
+///     The object will be kept inside the wrapper too, making sure that the
+///     client is not destroyed sooner than the python object. The keeper thing
+///     is designed to acommodate the interface of the ClientList.
+PyObject*
+wrapDataSourceClient(DataSourceClient* client,
+                     const boost::shared_ptr<ClientList::FindResult::
+                     LifeKeeper>& life_keeper = boost::shared_ptr<ClientList::
+                     FindResult::LifeKeeper>());
+
 } // namespace python
 } // namespace datasrc
 } // namespace isc

src/lib/python/isc/datasrc/configurableclientlist_python.cc

@@ -0,0 +1,308 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// Enable this if you use s# variants with PyArg_ParseTuple(), see
+// http://docs.python.org/py3k/c-api/arg.html#strings-and-buffers
+//#define PY_SSIZE_T_CLEAN
+
+// Python.h needs to be placed at the head of the program file, see:
+// http://docs.python.org/py3k/extending/extending.html#a-simple-example
+#include <Python.h>
+
+#include <string>
+#include <stdexcept>
+
+#include <util/python/pycppwrapper_util.h>
+
+#include <dns/python/rrclass_python.h>
+#include <dns/python/name_python.h>
+
+#include <datasrc/client_list.h>
+
+#include "configurableclientlist_python.h"
+#include "datasrc.h"
+#include "finder_python.h"
+#include "client_python.h"
+
+using namespace std;
+using namespace isc::util::python;
+using namespace isc::datasrc;
+using namespace isc::datasrc::python;
+
+//
+// ConfigurableClientList
+//
+
+// Trivial constructor.
+s_ConfigurableClientList::s_ConfigurableClientList() : cppobj(NULL) {
+}
+
+namespace {
+
+int
+ConfigurableClientList_init(PyObject* po_self, PyObject* args, PyObject*) {
+    s_ConfigurableClientList* self =
+        static_cast<s_ConfigurableClientList*>(po_self);
+    try {
+        const PyObject* rrclass;
+        if (PyArg_ParseTuple(args, "O!", &isc::dns::python::rrclass_type,
+                             &rrclass)) {
+            self->cppobj =
+                new ConfigurableClientList(isc::dns::python::
+                                           PyRRClass_ToRRClass(rrclass));
+            return (0);
+        }
+    } catch (const exception& ex) {
+        const string ex_what = "Failed to construct ConfigurableClientList object: " +
+            string(ex.what());
+        PyErr_SetString(getDataSourceException("Error"), ex_what.c_str());
+        return (-1);
+    } catch (...) {
+        PyErr_SetString(PyExc_SystemError, "Unexpected C++ exception");
+        return (-1);
+    }
+
+    return (-1);
+}
+
+void
+ConfigurableClientList_destroy(PyObject* po_self) {
+    s_ConfigurableClientList* self =
+        static_cast<s_ConfigurableClientList*>(po_self);
+    delete self->cppobj;
+    self->cppobj = NULL;
+    Py_TYPE(self)->tp_free(self);
+}
+
+PyObject*
+ConfigurableClientList_configure(PyObject* po_self, PyObject* args) {
+    s_ConfigurableClientList* self =
+        static_cast<s_ConfigurableClientList*>(po_self);
+    try {
+        const char* configuration;
+        int allow_cache;
+        if (PyArg_ParseTuple(args, "si", &configuration, &allow_cache)) {
+            const isc::data::ConstElementPtr
+                element(isc::data::Element::fromJSON(string(configuration)));
+            self->cppobj->configure(*element, allow_cache);
+            Py_RETURN_NONE;
+        } else {
+            return (NULL);
+        }
+    } catch (const isc::data::JSONError& jse) {
+        const string ex_what(std::string("JSON parse error in data source"
+                               " configuration: ") + jse.what());
+        PyErr_SetString(getDataSourceException("Error"), ex_what.c_str());
+        return (NULL);
+    } catch (const std::exception& exc) {
+        PyErr_SetString(getDataSourceException("Error"), exc.what());
+        return (NULL);
+    } catch (...) {
+        PyErr_SetString(getDataSourceException("Error"),
+                        "Unknown C++ exception");
+        return (NULL);
+    }
+}
+
+PyObject*
+ConfigurableClientList_find(PyObject* po_self, PyObject* args) {
+    s_ConfigurableClientList* self =
+        static_cast<s_ConfigurableClientList*>(po_self);
+    try {
+        PyObject* name_obj;
+        int want_exact_match = 0;
+        int want_finder = 1;
+        if (PyArg_ParseTuple(args, "O!|ii", &isc::dns::python::name_type,
+                             &name_obj, &want_exact_match, &want_finder)) {
+            const isc::dns::Name
+                name(isc::dns::python::PyName_ToName(name_obj));
+            const ClientList::FindResult
+                result(self->cppobj->find(name, want_exact_match,
+                                          want_finder));
+            PyObjectContainer dsrc;
+            if (result.dsrc_client_ == NULL) {
+                // Use the Py_BuildValue, as it takes care of the
+                // reference counts correctly.
+                dsrc.reset(Py_BuildValue(""));
+            } else {
+                // Make sure we have a keeper there too, so it doesn't
+                // die when the underlying client list dies or is
+                // reconfigured.
+                //
+                // However, as it is inside the C++ part, is there a
+                // reasonable way to test it?
+                dsrc.reset(wrapDataSourceClient(result.dsrc_client_,
+                                                result.life_keeper_));
+            }
+            PyObjectContainer finder;
+            if (result.finder_ == NULL) {
+                finder.reset(Py_BuildValue(""));
+            } else {
+                // Make sure it keeps the data source client alive.
+                finder.reset(createZoneFinderObject(result.finder_,
+                                                    dsrc.get()));
+            }
+            PyObjectContainer exact(PyBool_FromLong(result.exact_match_));
+
+            return (Py_BuildValue("OOO", dsrc.get(), finder.get(),
+                                  exact.get()));
+        } else {
+            return (NULL);
+        }
+    } catch (const std::exception& exc) {
+        PyErr_SetString(getDataSourceException("Error"), exc.what());
+        return (NULL);
+    } catch (...) {
+        PyErr_SetString(getDataSourceException("Error"),
+                        "Unknown C++ exception");
+        return (NULL);
+    }
+}
+
+// This list contains the actual set of functions we have in
+// python. Each entry has
+// 1. Python method name
+// 2. Our static function here
+// 3. Argument type
+// 4. Documentation
+PyMethodDef ConfigurableClientList_methods[] = {
+    { "configure", ConfigurableClientList_configure, METH_VARARGS,
+        "configure(configuration, allow_cache) -> None\n\
+\n\
+Wrapper around C++ ConfigurableClientList::configure\n\
+\n\
+This sets the active configuration. It fills the ConfigurableClientList with\
+corresponding data source clients.\n\
+\n\
+If any error is detected, an exception is raised and the previous\
+configuration preserved.\n\
+\n\
+Parameters:\n\
+  configuration     The configuration, as a JSON encoded string.\
+  allow_cache       If caching is allowed." },
+    { "find", ConfigurableClientList_find, METH_VARARGS,
+"find(zone, want_exact_match=False, want_finder=True) -> datasrc_client,\
+zone_finder, exact_match\n\
+\n\
+Look for a data source containing the given zone.\n\
+\n\
+It searches through the contained data sources and returns a data source\
+containing the zone, the zone finder of the zone and a boolean if the answer\
+is an exact match.\n\
+\n\
+The first parameter is isc.dns.Name object of a name in the zone. If the\
+want_exact_match is True, only zone with this exact origin is returned.\
+If it is False, the best matching zone is returned.\n\
+\n\
+If the want_finder is False, the returned zone_finder might be None even\
+if the data source is identified (in such case, the datasrc_client is not\
+None). Setting it to false allows the client list some optimisations, if\
+you don't need it, but if you do need it, it is better to set it to True\
+instead of getting it from the datasrc_client later.\n\
+\n\
+If no answer is found, the datasrc_client and zone_finder are None." },
+    { NULL, NULL, 0, NULL }
+};
+
+const char* const ConfigurableClientList_doc = "\
+The list of data source clients\n\
+\n\
+The purpose is to have several data source clients of the same class\
+and then be able to search through them to identify the one containing\
+a given zone.\n\
+\n\
+Unlike the C++ version, we don't have the abstract base class. Abstract\
+classes are not needed due to the duck typing nature of python.\
+";
+
+} // end of unnamed namespace
+
+namespace isc {
+namespace datasrc {
+namespace python {
+// This defines the complete type for reflection in python and
+// parsing of PyObject* to s_ConfigurableClientList
+// Most of the functions are not actually implemented and NULL here.
+PyTypeObject configurableclientlist_type = {
+    PyVarObject_HEAD_INIT(NULL, 0)
+    "datasrc.ConfigurableClientList",
+    sizeof(s_ConfigurableClientList),                 // tp_basicsize
+    0,                                  // tp_itemsize
+    ConfigurableClientList_destroy,                 // tp_dealloc
+    NULL,                               // tp_print
+    NULL,                               // tp_getattr
+    NULL,                               // tp_setattr
+    NULL,                               // tp_reserved
+    NULL,                               // tp_repr
+    NULL,                               // tp_as_number
+    NULL,                               // tp_as_sequence
+    NULL,                               // tp_as_mapping
+    NULL,                               // tp_hash
+    NULL,                               // tp_call
+    NULL,                               // tp_str
+    NULL,                               // tp_getattro
+    NULL,                               // tp_setattro
+    NULL,                               // tp_as_buffer
+    Py_TPFLAGS_DEFAULT,                 // tp_flags
+    ConfigurableClientList_doc,
+    NULL,                               // tp_traverse
+    NULL,                               // tp_clear
+    NULL,                               // tp_richcompare
+    0,                                  // tp_weaklistoffset
+    NULL,                               // tp_iter
+    NULL,                               // tp_iternext
+    ConfigurableClientList_methods,                   // tp_methods
+    NULL,                               // tp_members
+    NULL,                               // tp_getset
+    NULL,                               // tp_base
+    NULL,                               // tp_dict
+    NULL,                               // tp_descr_get
+    NULL,                               // tp_descr_set
+    0,                                  // tp_dictoffset
+    ConfigurableClientList_init,                    // tp_init
+    NULL,                               // tp_alloc
+    PyType_GenericNew,                  // tp_new
+    NULL,                               // tp_free
+    NULL,                               // tp_is_gc
+    NULL,                               // tp_bases
+    NULL,                               // tp_mro
+    NULL,                               // tp_cache
+    NULL,                               // tp_subclasses
+    NULL,                               // tp_weaklist
+    NULL,                               // tp_del
+    0                                   // tp_version_tag
+};
+
+// Module Initialization, all statics are initialized here
+bool
+initModulePart_ConfigurableClientList(PyObject* mod) {
+    // We initialize the static description object with PyType_Ready(),
+    // then add it to the module. This is not just a check! (leaving
+    // this out results in segmentation faults)
+    if (PyType_Ready(&configurableclientlist_type) < 0) {
+        return (false);
+    }
+    void* p = &configurableclientlist_type;
+    if (PyModule_AddObject(mod, "ConfigurableClientList",
+                           static_cast<PyObject*>(p)) < 0) {
+        return (false);
+    }
+    Py_INCREF(&configurableclientlist_type);
+
+    return (true);
+}
+
+} // namespace python
+} // namespace datasrc
+} // namespace isc

src/lib/python/isc/datasrc/configurableclientlist_python.h

@@ -0,0 +1,44 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __PYTHON_CONFIGURABLECLIENTLIST_H
+#define __PYTHON_CONFIGURABLECLIENTLIST_H 1
+
+#include <Python.h>
+
+namespace isc {
+namespace datasrc {
+class ConfigurableClientList;
+
+namespace python {
+
+// The s_* Class simply covers one instantiation of the object
+class s_ConfigurableClientList : public PyObject {
+public:
+    s_ConfigurableClientList();
+    ConfigurableClientList* cppobj;
+};
+
+extern PyTypeObject configurableclientlist_type;
+
+bool initModulePart_ConfigurableClientList(PyObject* mod);
+
+} // namespace python
+} // namespace datasrc
+} // namespace isc
+#endif // __PYTHON_CONFIGURABLECLIENTLIST_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/python/isc/datasrc/datasrc.cc

@@ -28,6 +28,7 @@
 #include "iterator_python.h"
 #include "updater_python.h"
 #include "journal_reader_python.h"
+#include "configurableclientlist_python.h"
 
 #include <util/python/pycppwrapper_util.h>
 #include <dns/python/pydnspp_common.h>
@@ -284,6 +285,11 @@ PyInit_datasrc(void) {
         return (NULL);
     }
 
+    if (!initModulePart_ConfigurableClientList(mod)) {
+        Py_DECREF(mod);
+        return (NULL);
+    }
+
     try {
         po_DataSourceError = PyErr_NewException("isc.datasrc.Error", NULL,
                                                 NULL);

src/lib/python/isc/datasrc/tests/Makefile.am

@@ -1,7 +1,7 @@
 PYCOVERAGE_RUN = @PYCOVERAGE_RUN@
 # old tests, TODO remove or change to use new API?
 #PYTESTS = master_test.py
-PYTESTS =  datasrc_test.py sqlite3_ds_test.py
+PYTESTS =  datasrc_test.py sqlite3_ds_test.py clientlist_test.py
 EXTRA_DIST = $(PYTESTS)
 
 EXTRA_DIST += testdata/brokendb.sqlite3
@@ -36,6 +36,7 @@ endif
 	PYTHONPATH=:$(COMMON_PYTHON_PATH):$(abs_top_builddir)/src/lib/python/isc/log:$(abs_top_builddir)/src/lib/python/isc/datasrc/.libs:$(abs_top_builddir)/src/lib/dns/python/.libs \
 	TESTDATA_PATH=$(abs_srcdir)/testdata \
 	TESTDATA_WRITE_PATH=$(abs_builddir) \
+	GLOBAL_TESTDATA_PATH=$(abs_top_srcdir)/src/lib/testutils/testdata \
 	B10_FROM_BUILD=$(abs_top_builddir) \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/lib/python/isc/datasrc/tests/clientlist_test.py

@@ -0,0 +1,146 @@
+# Copyright (C) 2012  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+import isc.log
+import isc.datasrc
+import isc.dns
+import unittest
+import os
+import sys
+
+TESTDATA_PATH = os.environ['GLOBAL_TESTDATA_PATH'] + os.sep
+
+class ClientListTest(unittest.TestCase):
+    """
+    Test cases for the client lists. Currently, the python wrappers
+    contain the ConfigurableClientList only.
+    """
+
+    def test_constructors(self):
+        """
+        Test the constructor. It should accept an RRClass. Check it
+        reject invalid inputs.
+        """
+        isc.datasrc.ConfigurableClientList(isc.dns.RRClass.IN())
+        isc.datasrc.ConfigurableClientList(isc.dns.RRClass.CH())
+        # Not enough arguments
+        self.assertRaises(TypeError, isc.datasrc.ConfigurableClientList)
+        # Bad types of arguments
+        self.assertRaises(TypeError, isc.datasrc.ConfigurableClientList, 0)
+        self.assertRaises(TypeError, isc.datasrc.ConfigurableClientList, "IN")
+        # Too many arguments
+        self.assertRaises(TypeError, isc.datasrc.ConfigurableClientList,
+                         isc.dns.RRClass.IN(), isc.dns.RRClass.IN())
+
+    def test_configure(self):
+        """
+        Test we can configure the client list. This tests if the valid
+        ones are acceptend and invalid rejected. We check the changes
+        have effect.
+        """
+        clist = isc.datasrc.ConfigurableClientList(isc.dns.RRClass.IN())
+        # This should be NOP now
+        clist.configure("[]", True)
+        # Check the zone is not there yet
+        dsrc, finder, exact = clist.find(isc.dns.Name("example.org"))
+        self.assertIsNone(dsrc)
+        self.assertIsNone(finder)
+        self.assertFalse(exact)
+        # We can use this type, as it is not loaded dynamically.
+        clist.configure('''[{
+            "type": "MasterFiles",
+            "params": {
+                "example.org": "''' + TESTDATA_PATH + '''example.org.zone"
+            },
+            "cache-enable": true
+        }]''', True)
+        # Check the zone is there now. Proper tests of find are in other
+        # test methods.
+        dsrc, finder, exact = clist.find(isc.dns.Name("example.org"))
+        self.assertIsNotNone(dsrc)
+        self.assertTrue(isinstance(dsrc, isc.datasrc.DataSourceClient))
+        self.assertIsNotNone(finder)
+        self.assertTrue(isinstance(finder, isc.datasrc.ZoneFinder))
+        self.assertTrue(exact)
+        self.assertRaises(isc.datasrc.Error, clist.configure, '"bad type"',
+                          True)
+        self.assertRaises(isc.datasrc.Error, clist.configure, '''[{
+            "type": "bad type"
+        }]''', True)
+        self.assertRaises(isc.datasrc.Error, clist.configure, '''[{
+            bad JSON,
+        }]''', True)
+        self.assertRaises(TypeError, clist.configure, [], True)
+        self.assertRaises(TypeError, clist.configure, "[]")
+        self.assertRaises(TypeError, clist.configure, "[]", "true")
+
+    def test_find(self):
+        """
+        Test the find accepts the right arguments, some of them can be omitted,
+        etc.
+        """
+        clist = isc.datasrc.ConfigurableClientList(isc.dns.RRClass.IN())
+        clist.configure('''[{
+            "type": "MasterFiles",
+            "params": {
+                "example.org": "''' + TESTDATA_PATH + '''example.org.zone"
+            },
+            "cache-enable": true
+        }]''', True)
+        dsrc, finder, exact = clist.find(isc.dns.Name("sub.example.org"))
+        self.assertIsNotNone(dsrc)
+        self.assertTrue(isinstance(dsrc, isc.datasrc.DataSourceClient))
+        self.assertIsNotNone(finder)
+        self.assertTrue(isinstance(finder, isc.datasrc.ZoneFinder))
+        # Check the finder holds a reference to the data source
+        # Note that one reference is kept in the parameter list
+        # of getrefcount
+        self.assertEqual(3, sys.getrefcount(dsrc))
+        finder = None
+        self.assertEqual(2, sys.getrefcount(dsrc))
+        # We check an exact match in test_configure already
+        self.assertFalse(exact)
+        dsrc, finder, exact = clist.find(isc.dns.Name("sub.example.org"),
+                                         False)
+        self.assertIsNotNone(dsrc)
+        self.assertTrue(isinstance(dsrc, isc.datasrc.DataSourceClient))
+        self.assertIsNotNone(finder)
+        self.assertTrue(isinstance(finder, isc.datasrc.ZoneFinder))
+        self.assertFalse(exact)
+        dsrc, finder, exact = clist.find(isc.dns.Name("sub.example.org"),
+                                         True)
+        self.assertIsNone(dsrc)
+        self.assertIsNone(finder)
+        self.assertFalse(exact)
+        dsrc, finder, exact = clist.find(isc.dns.Name("sub.example.org"),
+                                         False, False)
+        self.assertIsNotNone(dsrc)
+        self.assertTrue(isinstance(dsrc, isc.datasrc.DataSourceClient))
+        self.assertIsNotNone(finder)
+        self.assertTrue(isinstance(finder, isc.datasrc.ZoneFinder))
+        self.assertFalse(exact)
+        dsrc, finder, exact = clist.find(isc.dns.Name("sub.example.org"),
+                                         True, False)
+        self.assertIsNone(dsrc)
+        self.assertIsNone(finder)
+        self.assertFalse(exact)
+        # Some invalid inputs
+        self.assertRaises(TypeError, clist.find, "example.org")
+        self.assertRaises(TypeError, clist.find)
+
+if __name__ == "__main__":
+    isc.log.init("bind10")
+    isc.log.resetUnitTestRootLogger()
+    unittest.main()

src/lib/server_common/tests/socket_requestor_test.cc

@@ -450,7 +450,7 @@ private:
     // NOTE: client_fd could leak on exception.  This should be cleaned up.
     // See the note about SocketSessionReceiver in socket_request.cc.
     void
-    serve(const std::vector<std::pair<std::string, int> > data) {
+    serve(const std::vector<std::pair<std::string, int> >& data) {
         const int client_fd = accept(fd_, NULL, NULL);
         if (client_fd == -1) {
             isc_throw(Unexpected, "Error in accept(): " << strerror(errno));

src/lib/util/filename.cc

@@ -34,9 +34,9 @@ Filename::split(const string& full_name, string& directory,
     string& name, string& extension) const
 {
     directory = name = extension = "";
-    bool dir_present = false;
     if (!full_name.empty()) {
 
+        bool dir_present = false;
         // Find the directory.
         size_t last_slash = full_name.find_last_of('/');
         if (last_slash != string::npos) {

src/lib/util/strutil.cc

@@ -39,10 +39,9 @@ normalizeSlash(std::string& name) {
 
 string
 trim(const string& instring) {
-    static const char* blanks = " \t\n";
-
     string retstring = "";
     if (!instring.empty()) {
+        static const char* blanks = " \t\n";
 
         // Search for first non-blank character in the string
         size_t first = instring.find_first_not_of(blanks);

tests/lettuce/features/ddns_system.feature

@@ -13,9 +13,8 @@ Feature: DDNS System
 
         # Test 1
         When I use DDNS to set the SOA serial to 1235
-        # Note: test spec says refused here, system returns SERVFAIL
-        #The DDNS response should be REFUSED
-        The DDNS response should be SERVFAIL
+        # Note: test spec says refused here, system returns NOTIMP
+        The DDNS response should be NOTIMP
         And the SOA serial for example.org should be 1234
 
         # Test 2
@@ -53,15 +52,14 @@ Feature: DDNS System
         And wait for new bind10 stderr message DDNS_STARTED
 
         # Test 8
-        # Known issue: after shutdown, first new attempt results in SERVFAIL
         When I use DDNS to set the SOA serial to 1238
-        The DDNS response should be SERVFAIL
-        And the SOA serial for example.org should be 1237
+        The DDNS response should be SUCCESS
+        And the SOA serial for example.org should be 1238
 
-        When I use DDNS to set the SOA serial to 1238
+        When I use DDNS to set the SOA serial to 1239
         And wait for new bind10 stderr message AUTH_LOAD_ZONE
         The DDNS response should be SUCCESS
-        And the SOA serial for example.org should be 1238
+        And the SOA serial for example.org should be 1239
 
         # Test 9
         When I send bind10 the command Auth shutdown
@@ -70,10 +68,10 @@ Feature: DDNS System
         And wait for new bind10 stderr message AUTH_SERVER_STARTED
 
         # Test 10
-        When I use DDNS to set the SOA serial to 1239
+        When I use DDNS to set the SOA serial to 1240
         And wait for new bind10 stderr message AUTH_LOAD_ZONE
         The DDNS response should be SUCCESS
-        And the SOA serial for example.org should be 1239
+        And the SOA serial for example.org should be 1240
 
         # Test 11
         When I configure BIND10 to stop running DDNS
@@ -82,10 +80,10 @@ Feature: DDNS System
         bind10 module DDNS should not be running
 
         # Test 12
-        When I use DDNS to set the SOA serial to 1240
+        When I use DDNS to set the SOA serial to 1241
         # should this be REFUSED again?
-        The DDNS response should be SERVFAIL
-        And the SOA serial for example.org should be 1239
+        The DDNS response should be NOTIMP
+        And the SOA serial for example.org should be 1240
 
     Scenario: ACL
         Given I have bind10 running with configuration ddns/ddns.config