Browse Source

[master]Merge branch 'master' of ssh://git.bind10.isc.org//var/bind10/git/bind10

typos in ChangeLog fixed
Jeremy C. Reed 12 years ago
parent
commit
f49e54056a
59 changed files with 1965 additions and 636 deletions
  1. 28 0
      ChangeLog
  2. 110 32
      doc/guide/bind10-guide.xml
  3. 6 0
      src/bin/auth/tests/query_unittest.cc
  4. 6 0
      src/bin/d2/.gitignore
  5. 5 0
      src/bin/d2/Makefile.am
  6. 29 9
      src/bin/d2/d2_messages.mes
  7. 89 0
      src/bin/d2/d2_process.cc
  8. 98 0
      src/bin/d2/d2_process.h
  9. 151 0
      src/bin/d2/d_process.h
  10. 3 5
      src/bin/d2/main.cc
  11. 1 0
      src/bin/d2/tests/.gitignore
  12. 6 0
      src/bin/d2/tests/Makefile.am
  13. 166 0
      src/bin/d2/tests/d2_process_unittests.cc
  14. 1 1
      src/bin/d2/tests/d2_test.py
  15. 0 1
      src/bin/loadzone/.gitignore
  16. 1 0
      src/bin/resolver/bench/.gitignore
  17. 1 1
      src/bin/xfrin/b10-xfrin.xml
  18. 179 77
      src/bin/xfrin/tests/xfrin_test.py
  19. 283 161
      src/bin/xfrin/xfrin.py.in
  20. 45 11
      src/bin/xfrin/xfrin.spec
  21. 32 3
      src/bin/xfrin/xfrin_messages.mes
  22. 42 1
      src/lib/datasrc/client_list.cc
  23. 51 4
      src/lib/datasrc/client_list.h
  24. 2 1
      src/lib/datasrc/memory/zone_table.cc
  25. 4 1
      src/lib/datasrc/memory/zone_table.h
  26. 7 10
      src/lib/datasrc/memory/zone_table_segment.h
  27. 0 5
      src/lib/datasrc/memory/zone_table_segment_local.cc
  28. 0 4
      src/lib/datasrc/memory/zone_table_segment_local.h
  29. 62 56
      src/lib/datasrc/memory/zone_table_segment_mapped.cc
  30. 4 10
      src/lib/datasrc/memory/zone_table_segment_mapped.h
  31. 87 35
      src/lib/datasrc/memory/zone_writer.cc
  32. 9 17
      src/lib/datasrc/memory/zone_writer.h
  33. 224 43
      src/lib/datasrc/tests/client_list_unittest.cc
  34. 8 32
      src/lib/datasrc/tests/memory/zone_table_segment_mapped_unittest.cc
  35. 0 5
      src/lib/datasrc/tests/memory/zone_table_segment_mock.h
  36. 0 13
      src/lib/datasrc/tests/memory/zone_table_segment_unittest.cc
  37. 2 28
      src/lib/datasrc/tests/memory/zone_writer_unittest.cc
  38. 8 3
      src/lib/dhcp/tests/iface_mgr_unittest.cc
  39. 16 2
      src/lib/dhcp/tests/pkt_filter_inet_unittest.cc
  40. 16 2
      src/lib/dhcp/tests/pkt_filter_lpf_unittest.cc
  41. 4 2
      src/lib/dhcp/tests/protocol_util_unittest.cc
  42. 1 0
      src/lib/log/interprocess/tests/.gitignore
  43. 19 4
      src/lib/log/logger_manager_impl.cc
  44. 1 1
      src/lib/log/tests/Makefile.am
  45. 5 1
      src/lib/log/tests/logger_manager_unittest.cc
  46. 72 0
      src/lib/python/isc/datasrc/configurableclientlist_python.cc
  47. 41 14
      src/lib/util/memory_segment.h
  48. 1 1
      src/lib/util/memory_segment_local.cc
  49. 1 1
      src/lib/util/memory_segment_local.h
  50. 16 7
      src/lib/util/memory_segment_mapped.cc
  51. 1 1
      src/lib/util/memory_segment_mapped.h
  52. 2 7
      src/lib/util/random/random_number_generator.h
  53. 12 0
      src/lib/util/tests/memory_segment_common_unittest.cc
  54. 0 18
      src/lib/util/tests/random_number_generator_unittest.cc
  55. 0 1
      tests/lettuce/configurations/ixfr-out/testset1-config.db
  56. 1 2
      tests/lettuce/configurations/xfrin/retransfer_slave_diffs.conf
  57. 2 1
      tests/lettuce/configurations/xfrin/retransfer_slave_notify.conf.orig
  58. 2 1
      tests/lettuce/configurations/xfrin/retransfer_slave_notify_v4.conf
  59. 2 1
      tests/lettuce/features/xfrin_bind10.feature

+ 28 - 0
ChangeLog

@@ -1,3 +1,31 @@
+624.	[bug]		jinmei
+	logging: prevented multiple BIND 10 processes from generating
+	multiple small log files when they dumped logs to files and try
+	to roll over them simultaneously.  This fix relies on a feature of
+	underling logging library (log4cplus) version 1.1.0 or higher,
+	so the problem can still happen if BIND 10 is built with an older
+	version of log4cplus. (But this is expected to happen rarely in
+	any case unless a verbose debug level is specified).
+	(Trac #1622, git 5da8f8131b1224c99603852e1574b2a1adace236)
+
+623.	[func]		tmark
+	Created the initial, bare-bones implementation of DHCP-DDNS service
+	process class, D2Process, and the abstract class from which it derives,
+	DProcessBase. D2Process will provide the DHCP-DDNS specific event loop
+	and business logic.
+	(Trac #2955, git dbe4772246039a1257b6492936fda2a8600cd245)
+
+622.	[func]*		jinmei
+	b10-xfrin now has tighter control on the choice of IXFR or AXFR
+	through zones/request_ixfr configuration item.  It includes
+	the new "IXFR only" behavior for some special cases.  b10-xfrin
+	now also uses AXFR whenever necessary, so it is now safe to try
+	IXFR by default and it's made the default.  The previous
+	use_ixfr configuration item was deprecated and triggers startup
+	failure if specified; configuration using use_ixfr should be
+	updated.
+	(Trac #2911, git 8118f8e4e9c0ad3e7b690bbce265a163e4f8767a)
+
 621.	[func]		team
 	libdns++: All Rdata classes now use the generic lexer in
 	constructors from text. This means that the name fields in such

+ 110 - 32
doc/guide/bind10-guide.xml

@@ -2725,11 +2725,8 @@ TODO
 
     <para>
       The <command>b10-xfrin</command> process supports both AXFR and
-      IXFR.  Due to some implementation limitations of the current
-      development release, however, it only tries AXFR by default,
-      and care should be taken to enable IXFR.
+      IXFR.
     </para>
-<!-- TODO: http://bind10.isc.org/ticket/1279 -->
 
     <section>
       <title>Configuration for Incoming Zone Transfers</title>
@@ -2763,34 +2760,82 @@ TODO
 &gt; <userinput>config set Xfrin/zones[0]/tsig_key "<option>example.key</option>"</userinput>
     </section>
 
-    <section>
-      <title>Enabling IXFR</title>
-      <para>
-        As noted above, <command>b10-xfrin</command> uses AXFR for
-        zone transfers by default.  To enable IXFR for zone transfers
-        for a particular zone, set the <varname>use_ixfr</varname>
-        configuration parameter to <quote>true</quote>.
-        In the above example of configuration sequence, you'll need
-        to add the following before performing <userinput>commit</userinput>:
-      <screen>&gt; <userinput>config set Xfrin/zones[0]/use_ixfr true</userinput></screen>
-      </para>
+    <section id="request_ixfr">
+      <title>Control the use of IXFR</title>
+      <para>
+	By default, <command>b10-xfrin</command> uses IXFR for
+	transferring zones specified in
+	the <varname>Xfrin/zones</varname> list of the configuration,
+	unless it doesn't know the current SOA serial of the zone
+	(including the case where the zone has never transferred or
+	locally loaded), in which case it automatically uses AXFR.
+	If the attempt of IXFR fails, <command>b10-xfrin</command>
+	automatically retries the transfer using AXFR.
+	In general, this works for any master server implementations
+	including those that don't support IXFR and in any local state
+	of the zone.  So there should normally be no need to configure
+	on whether to use IXFR.
+      </para>
+
+      <para>
+	In some cases, however, it may be desirable to specify how and
+	whether to use IXFR and AXFR.
+	The <varname>request_ixfr</varname> configuration item under
+	<varname>Xfrin/zones</varname> can be used to control such
+	policies.
+	It can take the following values.
+      </para>
+      <variablelist>
+	<varlistentry>
+	  <term>yes</term>
+	  <listitem>
+	    <simpara>
+	      This is the default behavior as described above.
+	    </simpara>
+	  </listitem>
+	</varlistentry>
+	<varlistentry>
+	  <term>no</term>
+	  <listitem>
+	    <simpara>
+	      Only use AXFR.  Note that this value normally shouldn't
+	      be needed thanks to the automatic fallback from IXFR to IXFR.
+	      A possible case where this value needs to be used is
+	      that the master server has a bug and crashes if it
+	      receives an IXFR request.
+	    </simpara>
+	  </listitem>
+	</varlistentry>
+	<varlistentry>
+	  <term>only</term>
+	  <listitem>
+	    <simpara>
+	      Only use IXFR except when the current SOA serial is not
+	      known.
+	      This value has a severe drawback, that is, if the master
+	      server does not support IXFR zone transfers never
+	      succeed (except for the very first one, which will use AXFR),
+	      and the zone will eventually expire.
+	      Therefore it should not be used in general.
+	      Still, in some special cases the use of this value may
+	      make sense.  For example, if the operator is sure that
+	      the master server supports IXFR and the zone is very
+	      large, they may want to avoid falling back to AXFR as
+	      it can be more expensive.
+	    </simpara>
+	  </listitem>
+	</varlistentry>
+      </variablelist>
+
+      <note>
+        <simpara>
+	  There used to be a boolean configuration item named
+	  <varname>use_ixfr</varname>.
+	  It was deprecated for the finer control described above.
+	  The <varname>request_ixfr</varname> item should be used instead.
+	</simpara>
+      </note>
 
-<!-- TODO: http://bind10.isc.org/ticket/1279 -->
-      <note><simpara>
-      One reason why IXFR is disabled by default in the current
-      release is because it does not support automatic fallback from IXFR to
-      AXFR when it encounters a primary server that doesn't support
-      outbound IXFR (and, not many existing implementations support
-      it).  Another, related reason is that it does not use AXFR even
-      if it has no knowledge about the zone (like at the very first
-      time the secondary server is set up).  IXFR requires the
-      "current version" of the zone, so obviously it doesn't work
-      in this situation and AXFR is the only workable choice.
-      The current release of <command>b10-xfrin</command> does not
-      make this selection automatically.
-      These features will be implemented in a near future
-      version, at which point we will enable IXFR by default.
-      </simpara></note>
     </section>
 
 <!-- TODO:
@@ -2854,6 +2899,23 @@ what if a NOTIFY is sent?
 
         <screen>&gt; <userinput>Xfrin retransfer zone_name="<option>foo.example.org</option>" master=<option>192.0.2.99</option></userinput></screen>
       </para>
+
+      <para>
+	The <command>retransfer</command> command always uses AXFR.
+	To use IXFR for a zone that has already been transferred once,
+	use the <command>refresh</command> command.
+	It honors the <varname>Xfrin/zones/request_ixfr</varname>
+	configuration item (see <xref linkend="request_ixfr"/>.), and
+	if it's configured to use IXFR, it will be used.
+      </para>
+
+      <para>
+	Both the <command>retransfer</command>
+	and <command>refresh</command> commands can be used for
+	an initial transfer before setting up secondary
+	configurations.
+	In this case AXFR will be used for the obvious reason.
+      </para>
     </section>
 
     <section>
@@ -2879,7 +2941,6 @@ http://bind10.isc.org/wiki/ScalableZoneLoadDesign#a7.2UpdatingaZone
       </para>
     </section>
 
-<!-- TODO: can that retransfer be used to identify a new zone? -->
 <!-- TODO: what if doesn't exist at that master IP? -->
 
   </chapter>
@@ -5506,6 +5567,23 @@ TODO; there's a ticket to determine these levels, see #1074
             If this is 0, no maximum file size is used.
           </para>
 
+          <note>
+            <simpara>
+	      Due to a limitation of the underlying logging library
+	      (log4cplus), rolling over the log files (from ".1" to
+	      ".2", etc) may show odd results: There can be
+	      multiple small files at the timing of roll over.  This
+	      can happen when multiple BIND 10 processes try to roll
+	      over the files simultaneously.
+	      Version 1.1.0 of log4cplus solved this problem, so if
+	      this or higher version of log4cplus is used to build
+	      BIND 10, it shouldn't happen.  Even for older versions
+	      it is normally expected to happen rarely unless the log
+	      messages are produced very frequently by multiple
+	      different processes.
+	    </simpara>
+	  </note>
+
         </section>
 
         <section>

+ 6 - 0
src/bin/auth/tests/query_unittest.cc

@@ -80,6 +80,12 @@ public:
                 return (FindResult());
         }
     }
+    virtual ConstZoneTableAccessorPtr
+    getZoneTableAccessor(const std::string&, bool) const {
+        isc_throw(isc::NotImplemented,
+                  "getZoneTableAccessor not implemented for SingletonList");
+    }
+
 private:
     DataSourceClient& client_;
 };

+ 6 - 0
src/bin/d2/.gitignore

@@ -0,0 +1,6 @@
+/b10-d2
+/b10-d2.8
+/d2_messages.cc
+/d2_messages.h
+/spec_config.h
+/spec_config.h.pre

+ 5 - 0
src/bin/d2/Makefile.am

@@ -48,12 +48,17 @@ pkglibexec_PROGRAMS = b10-d2
 
 b10_d2_SOURCES  = main.cc
 b10_d2_SOURCES += d2_log.cc d2_log.h
+b10_d2_SOURCES += d_process.h 
+b10_d2_SOURCES += d2_process.cc d2_process.h
 
 nodist_b10_d2_SOURCES = d2_messages.h d2_messages.cc
 EXTRA_DIST += d2_messages.mes
 
 b10_d2_LDADD = $(top_builddir)/src/lib/log/libb10-log.la
 b10_d2_LDADD += $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
+b10_d2_LDADD += $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
+b10_d2_LDADD += $(top_builddir)/src/lib/cc/libb10-cc.la
+b10_d2_LDADD += $(top_builddir)/src/lib/config/libb10-cfgclient.la
 
 b10_d2dir = $(pkgdatadir)
 b10_d2_DATA = d2.spec

+ 29 - 9
src/bin/d2/d2_messages.mes

@@ -14,15 +14,35 @@
 
 $NAMESPACE isc::d2
 
-% D2_STARTING : process starting
-This is a debug message issued during a D2 process startup.
+% D2CTL_STARTING DHCP-DDNS controller starting, pid: %1
+This is an informational message issued when controller for DHCP-DDNS 
+service first starts.
 
-% D2_START_INFO pid: %1, verbose: %2, standalone: %3
-This is a debug message issued during the D2 process startup.
-It lists some information about the parameters with which the
-process is running.
+% D2CTL_STOPPING DHCP-DDNS controller is exiting
+This is an informational message issued when the controller is exiting 
+following a shut down (normal or otherwise) of the DDHCP-DDNS process.
 
-% D2_SHUTDOWN : process is performing a normal shutting down
-This is a debug message issued when a D2 process shuts down
-normally in response to command to stop.
+% D2PRC_SHUTDOWN DHCP-DDNS process is performing a normal shut down
+This is a debug message issued when the service process has been instructed
+to shut down by the controller.
+
+% D2PRC_RUN_ENTER process has entered the event loop
+This is a debug message issued when the D2 process enters it's
+run method. 
+
+% D2PRC_RUN_EXIT process is exiting the event loop
+This is a debug message issued when the D2 process exits the
+in event loop. 
+
+% D2PRC_FAILED process experienced a fatal error: %1
+This is a debug message issued when the D2 process encounters an
+unrecoverable error from within the event loop.
+
+% D2PRC_CONFIGURE new configuration received: %1
+This is a debug message issued when the D2 process configure method
+has been invoked.
+
+% D2PRC_COMMAND command directive received, command: %1 - args: %2
+This is a debug message issued when the D2 process command method
+has been invoked.
 

+ 89 - 0
src/bin/d2/d2_process.cc

@@ -0,0 +1,89 @@
+// Copyright (C) 2013  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.
+
+#include <config/ccsession.h>
+#include <d2/d2_log.h>
+#include <d2/d2_process.h>
+
+using namespace asio;
+
+namespace isc {
+namespace d2 {
+
+D2Process::D2Process(const char* name, IOServicePtr io_service) 
+    : DProcessBase(name, io_service) {
+};
+
+void
+D2Process::init() {
+};
+
+int
+D2Process::run() {
+    // Until shut down or an fatal error occurs, wait for and
+    // execute a single callback. This is a preliminary implementation
+    // that is likely to evolve as development progresses.
+    // To use run(), the "managing" layer must issue an io_service::stop 
+    // or the call to run will continue to block, and shutdown will not
+    // occur.
+    LOG_DEBUG(d2_logger, DBGLVL_START_SHUT, D2PRC_RUN_ENTER);
+    IOServicePtr& io_service = getIoService();
+    while (!shouldShutdown()) {
+        try {
+            io_service->run_one();
+        } catch (const std::exception& ex) {
+            LOG_FATAL(d2_logger, D2PRC_FAILED).arg(ex.what());
+            return (EXIT_FAILURE); 
+        }
+    }
+
+    LOG_DEBUG(d2_logger, DBGLVL_START_SHUT, D2PRC_RUN_EXIT);
+    return (EXIT_SUCCESS);
+};
+
+int 
+D2Process::shutdown() {
+    LOG_DEBUG(d2_logger, DBGLVL_START_SHUT, D2PRC_SHUTDOWN);
+    setShutdownFlag(true);
+    return (0);
+}    
+
+isc::data::ConstElementPtr 
+D2Process::configure(isc::data::ConstElementPtr config_set) {
+    // @TODO This is the initial implementation which simply accepts
+    // any content in config_set as valid.  This is sufficient to 
+    // allow participation as a BIND10 module, while D2 configuration support
+    // is being developed.
+    LOG_DEBUG(d2_logger, DBGLVL_TRACE_BASIC, 
+              D2PRC_CONFIGURE).arg(config_set->str());
+
+    return (isc::config::createAnswer(0, "Configuration accepted."));
+}
+
+isc::data::ConstElementPtr 
+D2Process::command(const std::string& command, isc::data::ConstElementPtr args){
+    // @TODO This is the initial implementation.  If and when D2 is extended
+    // to support its own commands, this implementation must change. Otherwise
+    // it should reject all commands as it does now. 
+    LOG_DEBUG(d2_logger, DBGLVL_TRACE_BASIC, 
+              D2PRC_COMMAND).arg(command).arg(args->str());
+
+    return (isc::config::createAnswer(1, "Unrecognized command:" + command));
+}
+
+D2Process::~D2Process() {
+};
+
+}; // namespace isc::d2 
+}; // namespace isc

+ 98 - 0
src/bin/d2/d2_process.h

@@ -0,0 +1,98 @@
+// Copyright (C) 2013  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 D2_PROCESS_H
+#define D2_PROCESS_H
+
+#include <d2/d_process.h>
+
+namespace isc {
+namespace d2 {
+
+/// @brief @TODO DHCP-DDNS Application Process 
+///
+/// D2Process provides the top level application logic for DHCP-driven DDNS 
+/// update processing.  It provides the asynchronous event processing required 
+/// to receive DNS mapping change requests and carry them out.   
+/// It implements the DProcessBase interface, which structures it such that it
+/// is a managed "application", controlled by a management layer. 
+
+class D2Process : public DProcessBase {
+public:
+    /// @brief Constructor
+    ///
+    /// @param name name is a text label for the process. Generally used
+    /// in log statements, but otherwise arbitrary. 
+    /// @param io_service is the io_service used by the caller for
+    /// asynchronous event handling.
+    ///
+    /// @throw DProcessBaseError is io_service is NULL. 
+    D2Process(const char* name, IOServicePtr io_service);
+
+    /// @brief Will be used after instantiation to perform initialization 
+    /// unique to D2. This will likely include interactions with QueueMgr and 
+    /// UpdateMgr, to prepare for request receipt and processing.
+    virtual void init();
+
+    /// @brief Implements the process's event loop. 
+    /// The initial implementation is quite basic, surrounding calls to 
+    /// io_service->runOne() with a test of the shutdown flag.
+    /// Once invoked, the method will continue until the process itself is 
+    /// exiting due to a request to shutdown or some anomaly forces an exit.   
+    /// @return  returns 0 upon a successful, "normal" termination, non
+    /// zero to indicate an abnormal termination.    
+    virtual int run();
+
+    // @TODO need brief
+    virtual int shutdown();
+
+    // @TODO need brief
+    /// @brief Processes the given configuration. 
+    /// 
+    /// This method may be called multiple times during the process lifetime.
+    /// Certainly once during process startup, and possibly later if the user
+    /// alters configuration. This method must not throw, it should catch any
+    /// processing errors and return a success or failure answer as described
+    /// below. 
+    ///
+    /// @param config_set a new configuration (JSON) for the process
+    /// @return an Element that contains the results of configuration composed
+    /// of an integer status value (0 means successful, non-zero means failure),
+    /// and a string explanation of the outcome. 
+    virtual isc::data::ConstElementPtr configure(isc::data::ConstElementPtr
+                                                 config_set);
+
+    // @TODO need brief
+    /// @brief Processes the given command. 
+    /// 
+    /// This method is called to execute any custom commands supported by the 
+    /// process. This method must not throw, it should catch any processing 
+    /// errors and return a success or failure answer as described below.
+    ///
+    /// @param command is a string label representing the command to execute.
+    /// @param args is a set of arguments (if any) required for the given
+    /// command. 
+    /// @return an Element that contains the results of command composed
+    /// of an integer status value (0 means successful, non-zero means failure),
+    /// and a string explanation of the outcome.  
+    virtual isc::data::ConstElementPtr command(const std::string& command, 
+                                               isc::data::ConstElementPtr args);
+    // @TODO need brief
+    virtual ~D2Process();
+};
+
+}; // namespace isc::d2 
+}; // namespace isc
+
+#endif

+ 151 - 0
src/bin/d2/d_process.h

@@ -0,0 +1,151 @@
+// Copyright (C) 2013  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 D_PROCESS_H
+#define D_PROCESS_H
+
+#include <asiolink/asiolink.h>
+#include <cc/data.h>
+#include <boost/shared_ptr.hpp>
+
+#include <exceptions/exceptions.h>
+
+typedef boost::shared_ptr<isc::asiolink::IOService> IOServicePtr;
+
+namespace isc {
+namespace d2 {
+
+/// @brief Exception thrown if the process encountered an operational error.
+class DProcessBaseError : public isc::Exception {
+public:
+    DProcessBaseError(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) { };
+};
+
+/// @brief Application Process Interface
+///
+/// DProcessBase is an abstract class represents the primary "application" 
+/// level object in a "managed" asynchronous application. It provides a uniform 
+/// interface such that a managing layer can construct, intialize, and start
+/// the application's event loop.  The event processing is centered around the
+/// use of isc::asiolink::io_service. The io_service is shared between the 
+/// the managing layer and the DProcessBase.  This allows management layer IO 
+/// such as directives to be sensed and handled, as well as processing IO 
+/// activity specific to the application.  In terms of management layer IO,
+/// there are methods shutdown, configuration updates, and commands unique
+/// to the application.  
+class DProcessBase {
+public:
+    /// @brief Constructor
+    ///
+    /// @param name name is a text label for the process. Generally used
+    /// in log statements, but otherwise arbitrary. 
+    /// @param io_service is the io_service used by the caller for
+    /// asynchronous event handling.
+    ///
+    /// @throw DProcessBaseError is io_service is NULL. 
+    DProcessBase(const char* name, IOServicePtr io_service) : name_(name),
+        io_service_(io_service), shut_down_flag_(false) {
+
+        if (!io_service_) {
+            isc_throw (DProcessBaseError, "IO Service cannot be null");
+        }
+    };
+
+    /// @brief May be used after instantiation to perform initialization unique
+    /// to application. It must be invoked prior to invoking run. This would 
+    /// likely include the creation of additional IO sources and their 
+    /// integration into the io_service. 
+    virtual void init() = 0; 
+
+    /// @brief Implements the process's event loop. In its simplest form it 
+    /// would an invocation io_service_->run().  This method should not exit 
+    /// until the process itself is exiting due to a request to shutdown or 
+    /// some anomaly is forcing an exit.   
+    /// @return  returns EXIT_SUCCESS upon a successful, normal termination, 
+    /// and EXIT_FAILURE to indicate an abnormal termination.    
+    virtual int run() = 0; 
+
+    /// @brief Implements the process's shutdown processing. When invoked, it 
+    /// should ensure that the process gracefully exits the run method. 
+    virtual int shutdown() = 0;
+
+    /// @brief Processes the given configuration. 
+    /// 
+    /// This method may be called multiple times during the process lifetime.
+    /// Certainly once during process startup, and possibly later if the user
+    /// alters configuration. This method must not throw, it should catch any
+    /// processing errors and return a success or failure answer as described
+    /// below. 
+    ///
+    /// @param config_set a new configuration (JSON) for the process
+    /// @return an Element that contains the results of configuration composed
+    /// of an integer status value (0 means successful, non-zero means failure),
+    /// and a string explanation of the outcome. 
+    virtual isc::data::ConstElementPtr configure(isc::data::ConstElementPtr
+                                                 config_set) = 0; 
+
+    /// @brief Processes the given command. 
+    /// 
+    /// This method is called to execute any custom commands supported by the 
+    /// process. This method must not throw, it should catch any processing 
+    /// errors and return a success or failure answer as described below.
+    ///
+    /// @param command is a string label representing the command to execute.
+    /// @param args is a set of arguments (if any) required for the given
+    /// command. 
+    /// @return an Element that contains the results of command composed
+    /// of an integer status value (0 means successful, non-zero means failure),
+    /// and a string explanation of the outcome.  
+    virtual isc::data::ConstElementPtr command(
+            const std::string& command, isc::data::ConstElementPtr args) = 0; 
+
+    /// @brief Destructor 
+    virtual ~DProcessBase(){};
+
+    bool shouldShutdown() { 
+        return (shut_down_flag_); 
+    }
+
+    void setShutdownFlag(bool value) { 
+        shut_down_flag_ = value; 
+    }
+
+    const std::string& getName() const {
+        return (name_);
+    }
+
+    IOServicePtr& getIoService() {
+        return (io_service_);
+    }
+
+private:
+    /// @brief Text label for the process. Generally used in log statements, 
+    /// but otherwise can be arbitrary. 
+    std::string name_;
+
+    /// @brief The IOService to be used for asynchronous event handling. 
+    IOServicePtr io_service_;
+
+    /// @brief Boolean flag set when shutdown has been requested.
+    bool shut_down_flag_;
+};
+
+/// @brief Defines a shared pointer to DProcessBase.
+typedef boost::shared_ptr<DProcessBase> DProcessBasePtr;
+
+}; // namespace isc::d2 
+}; // namespace isc
+
+#endif

+ 3 - 5
src/bin/d2/main.cc

@@ -82,16 +82,14 @@ main(int argc, char* argv[]) {
                          ((verbose_mode && stand_alone)
                            ? isc::log::DEBUG : isc::log::INFO),
                          isc::log::MAX_DEBUG_LEVEL, NULL, !stand_alone);
-    LOG_INFO(d2_logger, D2_STARTING);
-    LOG_DEBUG(d2_logger, DBGLVL_START_SHUT, D2_START_INFO)
-              .arg(getpid()).arg(verbose_mode ? "yes" : "no")
-              .arg(stand_alone ? "yes" : "no" );
+
+    LOG_INFO(d2_logger, D2CTL_STARTING);
 
     // For now we will sleep awhile to simulate doing something.
     // Without at least a sleep, the process will start, exit and be
     // restarted by Bind10/Init endlessley in a rapid succession.
     sleep(1000);
-    LOG_INFO(d2_logger, D2_SHUTDOWN);
+    LOG_INFO(d2_logger, D2CTL_STOPPING);
     return (EXIT_SUCCESS);
 }
 

+ 1 - 0
src/bin/d2/tests/.gitignore

@@ -0,0 +1 @@
+/d2_unittests

+ 6 - 0
src/bin/d2/tests/Makefile.am

@@ -52,7 +52,10 @@ if HAVE_GTEST
 TESTS += d2_unittests
 
 d2_unittests_SOURCES = ../d2_log.h ../d2_log.cc
+d2_unittests_SOURCES += ../d_process.h 
+d2_unittests_SOURCES += ../d2_process.cc ../d2_process.h
 d2_unittests_SOURCES += d2_unittests.cc
+d2_unittests_SOURCES += d2_process_unittests.cc
 nodist_d2_unittests_SOURCES = ../d2_messages.h ../d2_messages.cc
 
 d2_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
@@ -60,6 +63,9 @@ d2_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
 d2_unittests_LDADD = $(GTEST_LDADD)
 d2_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
 d2_unittests_LDADD += $(top_builddir)/src/lib/log/libb10-log.la
+d2_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
+d2_unittests_LDADD += $(top_builddir)/src/lib/cc/libb10-cc.la
+d2_unittests_LDADD += $(top_builddir)/src/lib/config/libb10-cfgclient.la
 endif
 
 noinst_PROGRAMS = $(TESTS)

+ 166 - 0
src/bin/d2/tests/d2_process_unittests.cc

@@ -0,0 +1,166 @@
+// Copyright (C) 2013  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.
+
+
+#include <config/ccsession.h>
+#include <d2/d2_process.h>
+
+#include <boost/date_time/posix_time/posix_time.hpp>
+#include <gtest/gtest.h>
+
+#include <config.h>
+#include <sstream>
+
+using namespace std;
+using namespace isc;
+using namespace isc::config;
+using namespace isc::d2;
+using namespace boost::posix_time;
+
+namespace {
+
+/// @brief D2Process test fixture class
+class D2ProcessTest : public ::testing::Test {
+public:
+
+    /// @brief Static instance accessible via test callbacks.
+    static DProcessBasePtr process_;
+
+    /// @brief Constructor
+    D2ProcessTest() {
+        io_service_.reset(new isc::asiolink::IOService());
+        process_.reset(new D2Process("TestProcess", io_service_));
+    }
+
+    /// @brief Destructor 
+    ~D2ProcessTest() {
+        io_service_.reset();
+        process_.reset();
+    }
+
+    /// @brief Callback that will invoke shutdown method.
+    static void genShutdownCallback() {
+        process_->shutdown();
+    }
+
+    /// @brief Callback that throws an exception.
+    static void genFatalErrorCallback() {
+        isc_throw (DProcessBaseError, "simulated fatal error");
+    }
+
+    /// @brief IOService for event processing. Fills in for IOservice
+    /// supplied by management layer.
+    IOServicePtr io_service_;
+};
+
+// Define the static process instance
+DProcessBasePtr D2ProcessTest::process_;
+
+
+/// @brief Verifies D2Process constructor behavior.
+/// 1. Verifies that constructor fails with an invalid IOService
+/// 2. Verifies that constructor succeeds with a valid IOService
+TEST(D2Process, construction) {
+    // Verify that the constructor will fail if given an empty
+    // io service.
+    IOServicePtr lcl_io_service;
+    EXPECT_THROW (D2Process("TestProcess", lcl_io_service), DProcessBaseError);
+
+    // Verify that the constructor succeeds with a valid io_service
+    lcl_io_service.reset(new isc::asiolink::IOService());
+    ASSERT_NO_THROW (D2Process("TestProcess", lcl_io_service));
+}
+
+/// @brief Verifies basic configure method behavior.
+// @TODO This test is simplistic and will need to be augmented
+// as configuration ability is implemented.
+TEST_F(D2ProcessTest, configure) {
+    // Verify that given a configuration "set", configure returns
+    // a successful response.
+    int rcode = -1;
+    string config = "{ \"test-value\": 1000 } ";
+    isc::data::ElementPtr json = isc::data::Element::fromJSON(config);
+    isc::data::ConstElementPtr answer = process_->configure(json); 
+    isc::config::parseAnswer(rcode, answer);
+    EXPECT_EQ(0, rcode);
+}
+
+/// @brief Verifies basic command method behavior. 
+// @TODO IF the D2Process is extended to support extra commands
+// this testing will need to augmented accordingly.
+TEST_F(D2ProcessTest, command) {
+    // Verfiy that the process will process unsupported command and
+    // return a failure response.
+    int rcode = -1;
+    string args = "{ \"arg1\": 77 } ";
+    isc::data::ElementPtr json = isc::data::Element::fromJSON(args);
+    isc::data::ConstElementPtr answer = 
+                                    process_->command("bogus_command", json); 
+    parseAnswer(rcode, answer);
+    EXPECT_EQ(1, rcode);
+}
+
+/// @brief Verifies that an "external" call to shutdown causes 
+/// the run method to exit gracefully with a return value of EXIT_SUCCESS.
+TEST_F(D2ProcessTest, normalShutdown) {
+    // Use an asiolink IntervalTimer and callback to generate the
+    // shutdown invocation. (Note IntervalTimer setup is in milliseconds).
+    isc::asiolink::IntervalTimer timer(*io_service_);
+    timer.setup(genShutdownCallback, 2 * 1000);
+
+    // Record start time, and invoke run().
+    ptime start = microsec_clock::universal_time();
+    int rcode = process_->run();
+
+    // Record stop time.
+    ptime stop = microsec_clock::universal_time();
+
+    // Verify normal shutdown status.
+    EXPECT_EQ(EXIT_SUCCESS, rcode);
+
+    // Verify that duration of the run invocation is the same as the
+    // timer duration.  This demonstrates that the shutdown was driven
+    // by an io_service event and callback.
+    time_duration elapsed = stop - start;
+    EXPECT_TRUE(elapsed.total_milliseconds() >= 1900 && 
+                elapsed.total_milliseconds() <= 2100);
+}
+
+/// @brief Verifies that an "uncaught" exception thrown during event loop 
+/// processing is treated as a fatal error.
+TEST_F(D2ProcessTest, fatalErrorShutdown) {
+    // Use an asiolink IntervalTimer and callback to generate the
+    // the exception.  (Note IntervalTimer setup is in milliseconds).
+    isc::asiolink::IntervalTimer timer(*io_service_);
+    timer.setup(genFatalErrorCallback, 2 * 1000);
+
+    // Record start time, and invoke run().
+    ptime start = microsec_clock::universal_time();
+    int rcode = process_->run();
+
+    // Record stop time.
+    ptime stop = microsec_clock::universal_time();
+
+    // Verify failure status.
+    EXPECT_EQ(EXIT_FAILURE, rcode);
+
+    // Verify that duration of the run invocation is the same as the
+    // timer duration.  This demonstrates that the anomaly occurred
+    // during io callback processing. 
+    time_duration elapsed = stop - start;
+    EXPECT_TRUE(elapsed.total_milliseconds() >= 1900 && 
+                elapsed.total_milliseconds() <= 2100);
+}
+
+} // end of anonymous namespace

+ 1 - 1
src/bin/d2/tests/d2_test.py

@@ -161,7 +161,7 @@ class TestD2Daemon(unittest.TestCase):
         # soon enough to catch it.
         (returncode, output, error) = self.runCommand(["../b10-d2", "-s"])
         output_text = str(output) + str(error)
-        self.assertEqual(output_text.count("D2_STARTING"), 1)
+        self.assertEqual(output_text.count("D2CTL_STARTING"), 1)
 
 if __name__ == '__main__':
     unittest.main()

+ 0 - 1
src/bin/loadzone/.gitignore

@@ -1,5 +1,4 @@
 /b10-loadzone
-/b10-loadzone.py
 /loadzone.py
 /run_loadzone.sh
 /b10-loadzone.8

+ 1 - 0
src/bin/resolver/bench/.gitignore

@@ -0,0 +1 @@
+/resolver-bench

+ 1 - 1
src/bin/xfrin/b10-xfrin.xml

@@ -111,7 +111,7 @@ in separate zonemgr process.
       <varname>class</varname> (defaults to <quote>IN</quote>),
       <varname>master_addr</varname> (the zone master to transfer from),
       <varname>master_port</varname> (defaults to 53),
-      <varname>use_ixfr</varname> (defaults to false), and
+      <varname>request_ixfr</varname> (defaults to yes), and
       <varname>tsig_key</varname> (optional TSIG key name to use).
       The <varname>tsig_key</varname> is specified using a name that
       corresponds to one of the TSIG keys configured in the global

+ 179 - 77
src/bin/xfrin/tests/xfrin_test.py

@@ -129,17 +129,12 @@ class XfrinTestException(Exception):
 class XfrinTestTimeoutException(Exception):
     pass
 
-class MockCC(MockModuleCCSession):
-    def get_default_value(self, identifier):
-        # The returned values should be identical to the spec file
-        # XXX: these should be retrieved from the spec file
-        # (see MyCCSession of xfrout_test.py.in)
-        if identifier == "zones/master_port":
-            return TEST_MASTER_PORT
-        if identifier == "zones/class":
-            return TEST_RRCLASS_STR
-        if identifier == "zones/use_ixfr":
-            return False
+class MockCC(MockModuleCCSession, ConfigData):
+    def __init__(self):
+        super().__init__()
+        module_spec = isc.config.module_spec_from_file(
+            xfrin.SPECFILE_LOCATION)
+        ConfigData.__init__(self, module_spec)
 
     def add_remote_config_by_name(self, name, callback):
         pass
@@ -242,6 +237,10 @@ class MockDataSourceClient():
         self.committed_diffs.append(self.diffs)
         self.diffs = []
 
+    def create_zone(self, zone_name):
+        # pretend it just succeeds
+        pass
+
 class MockXfrin(Xfrin):
     # This is a class attribute of a callable object that specifies a non
     # default behavior triggered in _cc_check_command().  Specific test methods
@@ -265,21 +264,21 @@ class MockXfrin(Xfrin):
             MockXfrin.check_command_hook()
 
     def xfrin_start(self, zone_name, rrclass, db_file, master_addrinfo,
-                    tsig_key, request_type, check_soa=True):
+                    tsig_key, request_ixfr, check_soa=True):
         # store some of the arguments for verification, then call this
         # method in the superclass
         self.xfrin_started_master_addr = master_addrinfo[2][0]
         self.xfrin_started_master_port = master_addrinfo[2][1]
-        self.xfrin_started_request_type = request_type
+        self.xfrin_started_request_ixfr = request_ixfr
         return Xfrin.xfrin_start(self, zone_name, rrclass, None,
                                  master_addrinfo, tsig_key,
-                                 request_type, check_soa)
+                                 request_ixfr, check_soa)
 
 class MockXfrinConnection(XfrinConnection):
     def __init__(self, sock_map, zone_name, rrclass, datasrc_client,
                  shutdown_event, master_addr, tsig_key=None):
         super().__init__(sock_map, zone_name, rrclass, MockDataSourceClient(),
-                         shutdown_event, master_addr, TEST_DB_FILE)
+                         shutdown_event, master_addr, begin_soa_rrset)
         self.query_data = b''
         self.reply_data = b''
         self.force_time_out = False
@@ -846,7 +845,9 @@ class TestXfrinConnection(unittest.TestCase):
 
         '''
         self.conn._zone_name = zone_name
-        self.conn._zone_soa = self.conn._get_zone_soa()
+        self.conn._zone_soa = xfrin._get_zone_soa(self.conn._datasrc_client,
+                                                  zone_name,
+                                                  self.conn._rrclass)
 
 class TestAXFR(TestXfrinConnection):
     def setUp(self):
@@ -970,7 +971,9 @@ class TestAXFR(TestXfrinConnection):
                           RRType.IXFR)
 
         self._set_test_zone(Name('dup-soa.example'))
-        self.conn._zone_soa = self.conn._get_zone_soa()
+        self.conn._zone_soa = xfrin._get_zone_soa(self.conn._datasrc_client,
+                                                  self.conn._zone_name,
+                                                  self.conn._rrclass)
         self.assertRaises(XfrinException, self.conn._create_query,
                           RRType.IXFR)
 
@@ -2411,7 +2414,7 @@ class TestXfrinProcess(unittest.TestCase):
         # Normal, successful case.  We only check that things are cleaned up
         # at the tearDown time.
         process_xfrin(self, self, TEST_ZONE_NAME, TEST_RRCLASS, None, None,
-                      self.master,  False, None, RRType.AXFR,
+                      self.master,  False, None, ZoneInfo.REQUEST_IXFR_DISABLED,
                       self.create_xfrinconn)
 
     def test_process_xfrin_exception_on_connect(self):
@@ -2419,7 +2422,7 @@ class TestXfrinProcess(unittest.TestCase):
         # cleaned up.
         self.do_raise_on_connect = True
         process_xfrin(self, self, TEST_ZONE_NAME, TEST_RRCLASS, None, None,
-                      self.master,  False, None, RRType.AXFR,
+                      self.master,  False, None, ZoneInfo.REQUEST_IXFR_DISABLED,
                       self.create_xfrinconn)
 
     def test_process_xfrin_exception_on_close(self):
@@ -2429,7 +2432,7 @@ class TestXfrinProcess(unittest.TestCase):
         self.do_raise_on_connect = True
         self.do_raise_on_close = True
         process_xfrin(self, self, TEST_ZONE_NAME, TEST_RRCLASS, None, None,
-                      self.master,  False, None, RRType.AXFR,
+                      self.master,  False, None, ZoneInfo.REQUEST_IXFR_DISABLED,
                       self.create_xfrinconn)
 
     def test_process_xfrin_exception_on_publish(self):
@@ -2437,7 +2440,7 @@ class TestXfrinProcess(unittest.TestCase):
         # everything must still be cleaned up.
         self.do_raise_on_publish = True
         process_xfrin(self, self, TEST_ZONE_NAME, TEST_RRCLASS, None, None,
-                      self.master,  False, None, RRType.AXFR,
+                      self.master,  False, None, ZoneInfo.REQUEST_IXFR_DISABLED,
                       self.create_xfrinconn)
 
 class TestXfrin(unittest.TestCase):
@@ -2537,7 +2540,8 @@ class TestXfrin(unittest.TestCase):
         self.assertEqual(self.args['master'], self.xfr.xfrin_started_master_addr)
         self.assertEqual(int(self.args['port']), self.xfr.xfrin_started_master_port)
         # By default we use AXFR (for now)
-        self.assertEqual(RRType.AXFR, self.xfr.xfrin_started_request_type)
+        self.assertEqual(ZoneInfo.REQUEST_IXFR_DISABLED,
+                         self.xfr.xfrin_started_request_ixfr)
 
     def test_command_handler_retransfer_short_command1(self):
         # try it when only specifying the zone name (of unknown zone)
@@ -2650,8 +2654,9 @@ class TestXfrin(unittest.TestCase):
                          self.xfr.xfrin_started_master_addr)
         self.assertEqual(int(TEST_MASTER_PORT),
                          self.xfr.xfrin_started_master_port)
-        # By default we use AXFR (for now)
-        self.assertEqual(RRType.AXFR, self.xfr.xfrin_started_request_type)
+        # By default we use IXFR (with AXFR fallback)
+        self.assertEqual(ZoneInfo.REQUEST_IXFR_FIRST,
+                         self.xfr.xfrin_started_request_ixfr)
 
     def test_command_handler_notify(self):
         # at this level, refresh is no different than retransfer.
@@ -2734,12 +2739,19 @@ class TestXfrin(unittest.TestCase):
                                  Name(zone_config['tsig_key']).to_text())
             else:
                 self.assertIsNone(zone_info.tsig_key_name)
-            if 'use_ixfr' in zone_config and\
-               zone_config.get('use_ixfr'):
-                self.assertTrue(zone_info.use_ixfr)
-            else:
-                # if not set, should default to False
-                self.assertFalse(zone_info.use_ixfr)
+            if ('request_ixfr' in zone_config and
+                zone_config.get('request_ixfr')):
+                cfg_val = zone_config.get('request_ixfr')
+                val = zone_info.request_ixfr
+                if cfg_val == 'yes':
+                    self.assertEqual(ZoneInfo.REQUEST_IXFR_FIRST, val)
+                elif cfg_val == 'no':
+                    self.assertEqual(ZoneInfo.REQUEST_IXFR_DISABLED, val)
+                elif cfg_val == 'only':
+                    self.assertEqual(ZoneInfo.REQUEST_IXFR_ONLY, val)
+            else:               # check the default
+                self.assertEqual(ZoneInfo.REQUEST_IXFR_FIRST,
+                                 zone_info.request_ixfr)
 
     def test_config_handler_zones(self):
         # This test passes a number of good and bad configs, and checks whether
@@ -2751,7 +2763,7 @@ class TestXfrin(unittest.TestCase):
                    { 'name': 'test.example.',
                     'master_addr': '192.0.2.1',
                     'master_port': 53,
-                    'use_ixfr': False
+                    'request_ixfr': 'yes'
                    }
                  ]}
         self.assertEqual(self.xfr.config_handler(config1)['result'][0], 0)
@@ -2763,12 +2775,24 @@ class TestXfrin(unittest.TestCase):
                     'master_addr': '192.0.2.2',
                     'master_port': 53,
                     'tsig_key': "example.com:SFuWd/q99SzF8Yzd1QbB9g==",
-                    'use_ixfr': True
+                    'request_ixfr': 'no'
                    }
                  ]}
         self.assertEqual(self.xfr.config_handler(config2)['result'][0], 0)
         self._check_zones_config(config2)
 
+        config3 = {'transfers_in': 4,
+                   'zones': [
+                   { 'name': 'test.example.',
+                    'master_addr': '192.0.2.2',
+                    'master_port': 53,
+                    'tsig_key': "example.com:SFuWd/q99SzF8Yzd1QbB9g==",
+                    'request_ixfr': 'only'
+                   }
+                 ]}
+        self.assertEqual(self.xfr.config_handler(config3)['result'][0], 0)
+        self._check_zones_config(config3)
+
         # test that configuring the zone multiple times fails
         zones = { 'transfers_in': 5,
                   'zones': [
@@ -2783,7 +2807,7 @@ class TestXfrin(unittest.TestCase):
                 ]}
         self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
         # since this has failed, we should still have the previous config
-        self._check_zones_config(config2)
+        self._check_zones_config(config3)
 
         zones = { 'zones': [
                   { 'name': 'test.example.',
@@ -2793,7 +2817,7 @@ class TestXfrin(unittest.TestCase):
                   }
                 ]}
         self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
-        self._check_zones_config(config2)
+        self._check_zones_config(config3)
 
         zones = { 'zones': [
                   { 'master_addr': '192.0.2.4',
@@ -2802,7 +2826,7 @@ class TestXfrin(unittest.TestCase):
                 ]}
         self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
         # since this has failed, we should still have the previous config
-        self._check_zones_config(config2)
+        self._check_zones_config(config3)
 
         zones = { 'zones': [
                   { 'name': 'bad..zone.',
@@ -2812,7 +2836,7 @@ class TestXfrin(unittest.TestCase):
                 ]}
         self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
         # since this has failed, we should still have the previous config
-        self._check_zones_config(config2)
+        self._check_zones_config(config3)
 
         zones = { 'zones': [
                   { 'name': '',
@@ -2822,7 +2846,7 @@ class TestXfrin(unittest.TestCase):
                 ]}
         self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
         # since this has failed, we should still have the previous config
-        self._check_zones_config(config2)
+        self._check_zones_config(config3)
 
         zones = { 'zones': [
                   { 'name': 'test.example',
@@ -2832,7 +2856,7 @@ class TestXfrin(unittest.TestCase):
                 ]}
         self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
         # since this has failed, we should still have the previous config
-        self._check_zones_config(config2)
+        self._check_zones_config(config3)
 
         zones = { 'zones': [
                   { 'name': 'test.example',
@@ -2842,7 +2866,7 @@ class TestXfrin(unittest.TestCase):
                 ]}
         self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
         # since this has failed, we should still have the previous config
-        self._check_zones_config(config2)
+        self._check_zones_config(config3)
 
         zones = { 'zones': [
                   { 'name': 'test.example',
@@ -2854,7 +2878,7 @@ class TestXfrin(unittest.TestCase):
                 ]}
         self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
         # since this has failed, we should still have the previous config
-        self._check_zones_config(config2)
+        self._check_zones_config(config3)
 
         # let's also add a zone that is correct too, and make sure
         # that the new config is not partially taken
@@ -2871,81 +2895,111 @@ class TestXfrin(unittest.TestCase):
                 ]}
         self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
         # since this has failed, we should still have the previous config
-        self._check_zones_config(config2)
+        self._check_zones_config(config3)
+
+        # invalid request_ixfr value
+        zones = { 'zones': [
+                  { 'name': 'test.example',
+                    'master_addr': '192.0.2.7',
+                    'request_ixfr': 'bad value'
+                  }
+                ]}
+        self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
+        # since this has failed, we should still have the previous config
+        self._check_zones_config(config3)
 
     def test_config_handler_zones_default(self):
         # Checking it some default config values apply.  Using a separate
         # test case for a fresh xfr object.
         config = { 'zones': [
                    { 'name': 'test.example.',
-                    'master_addr': '192.0.2.1',
-                    'master_port': 53,
+                     'master_addr': '192.0.2.1',
+                     'master_port': 53,
                    }
                  ]}
         self.assertEqual(self.xfr.config_handler(config)['result'][0], 0)
         self._check_zones_config(config)
 
-    def common_ixfr_setup(self, xfr_mode, use_ixfr, tsig_key_str = None):
+    def test_config_handler_use_ixfr(self):
+        # use_ixfr was deprecated and explicitly rejected for now.
+        config = { 'zones': [
+                   { 'name': 'test.example.',
+                     'master_addr': '192.0.2.1',
+                     'master_port': 53,
+                     'use_ixfr': True
+                   }
+                 ]}
+        self.assertEqual(self.xfr.config_handler(config)['result'][0], 1)
+
+    def common_ixfr_setup(self, xfr_mode, request_ixfr, tsig_key_str=None):
         # This helper method explicitly sets up a zone configuration with
-        # use_ixfr, and invokes either retransfer or refresh.
+        # request_ixfr, and invokes either retransfer or refresh.
         # Shared by some of the following test cases.
         config = {'zones': [
                 {'name': 'example.com.',
                  'master_addr': '192.0.2.1',
                  'tsig_key': tsig_key_str,
-                 'use_ixfr': use_ixfr}]}
+                 'request_ixfr': request_ixfr}]}
         self.assertEqual(self.xfr.config_handler(config)['result'][0], 0)
         self.assertEqual(self.xfr.command_handler(xfr_mode,
                                                   self.args)['result'][0], 0)
 
     def test_command_handler_retransfer_ixfr_enabled(self):
-        # If IXFR is explicitly enabled in config, IXFR will be used
-        self.common_ixfr_setup('retransfer', True)
-        self.assertEqual(RRType.IXFR, self.xfr.xfrin_started_request_type)
+        # retransfer always uses AXFR (disabling IXFR), regardless of
+        # request_ixfr value
+        self.common_ixfr_setup('retransfer', 'yes')
+        self.assertEqual(ZoneInfo.REQUEST_IXFR_DISABLED,
+                         self.xfr.xfrin_started_request_ixfr)
 
     def test_command_handler_refresh_ixfr_enabled(self):
-        # Same for refresh
-        self.common_ixfr_setup('refresh', True)
-        self.assertEqual(RRType.IXFR, self.xfr.xfrin_started_request_type)
+        # for refresh, it honors zone configuration if defined (the default
+        # case is covered in test_command_handler_refresh
+        self.common_ixfr_setup('refresh', 'no')
+        self.assertEqual(ZoneInfo.REQUEST_IXFR_DISABLED,
+                         self.xfr.xfrin_started_request_ixfr)
 
     def test_command_handler_retransfer_with_tsig(self):
-        self.common_ixfr_setup('retransfer', False, 'example.com.key')
-        self.assertEqual(RRType.AXFR, self.xfr.xfrin_started_request_type)
+        self.common_ixfr_setup('retransfer', 'no', 'example.com.key')
+        self.assertEqual(ZoneInfo.REQUEST_IXFR_DISABLED,
+                         self.xfr.xfrin_started_request_ixfr)
 
     def test_command_handler_retransfer_with_tsig_bad_key(self):
         # bad keys should not reach xfrin, but should they somehow,
         # they are ignored (and result in 'key not found' + error log).
         self.assertRaises(XfrinZoneInfoException, self.common_ixfr_setup,
-                          'retransfer', False, 'bad.key')
+                          'retransfer', 'no', 'bad.key')
 
     def test_command_handler_retransfer_with_tsig_unknown_key(self):
         self.assertRaises(XfrinZoneInfoException, self.common_ixfr_setup,
-                          'retransfer', False, 'no.such.key')
+                          'retransfer', 'no', 'no.such.key')
 
     def test_command_handler_refresh_with_tsig(self):
-        self.common_ixfr_setup('refresh', False, 'example.com.key')
-        self.assertEqual(RRType.AXFR, self.xfr.xfrin_started_request_type)
+        self.common_ixfr_setup('refresh', 'no', 'example.com.key')
+        self.assertEqual(ZoneInfo.REQUEST_IXFR_DISABLED,
+                         self.xfr.xfrin_started_request_ixfr)
 
     def test_command_handler_refresh_with_tsig_bad_key(self):
         # bad keys should not reach xfrin, but should they somehow,
         # they are ignored (and result in 'key not found' + error log).
         self.assertRaises(XfrinZoneInfoException, self.common_ixfr_setup,
-                          'refresh', False, 'bad.key')
+                          'refresh', 'no', 'bad.key')
 
     def test_command_handler_refresh_with_tsig_unknown_key(self):
         self.assertRaises(XfrinZoneInfoException, self.common_ixfr_setup,
-                          'refresh', False, 'no.such.key')
+                          'refresh', 'no', 'no.such.key')
 
     def test_command_handler_retransfer_ixfr_disabled(self):
         # Similar to the previous case, but explicitly disabled.  AXFR should
         # be used.
-        self.common_ixfr_setup('retransfer', False)
-        self.assertEqual(RRType.AXFR, self.xfr.xfrin_started_request_type)
+        self.common_ixfr_setup('retransfer', 'no')
+        self.assertEqual(ZoneInfo.REQUEST_IXFR_DISABLED,
+                         self.xfr.xfrin_started_request_ixfr)
 
     def test_command_handler_refresh_ixfr_disabled(self):
         # Same for refresh
-        self.common_ixfr_setup('refresh', False)
-        self.assertEqual(RRType.AXFR, self.xfr.xfrin_started_request_type)
+        self.common_ixfr_setup('refresh', 'no')
+        self.assertEqual(ZoneInfo.REQUEST_IXFR_DISABLED,
+                         self.xfr.xfrin_started_request_ixfr)
 
 class TestXfrinMemoryZones(unittest.TestCase):
     def setUp(self):
@@ -3157,6 +3211,13 @@ class TestXfrinProcess(unittest.TestCase):
         self.__published = []
         # How many connections were created.
         self.__created_connections = 0
+        # prepare for possible replacement
+        self.__orig_get_zone_soa = xfrin._get_zone_soa
+        xfrin._get_zone_soa = lambda x, y, z: begin_soa_rdata
+
+    def tearDown(self):
+        # restore original value
+        xfrin._get_zone_soa = self.__orig_get_zone_soa
 
     def __get_connection(self, *args):
         """
@@ -3212,7 +3273,7 @@ class TestXfrinProcess(unittest.TestCase):
         """
         pass
 
-    def __do_test(self, rets, transfers, request_type):
+    def __do_test(self, rets, transfers, request_ixfr):
         """
         Do the actual test. The request type, prepared sucesses/failures
         and expected sequence of transfers is passed to specify what test
@@ -3221,8 +3282,8 @@ class TestXfrinProcess(unittest.TestCase):
         self.__rets = rets
         published = rets[-1]
         xfrin.process_xfrin(self, XfrinRecorder(), Name("example.org."),
-                            RRClass.IN, None, None, None, True, None,
-                            request_type, self.__get_connection)
+                            RRClass.IN, None, None, TEST_MASTER_IPV4_ADDRINFO,
+                            True, None, request_ixfr, self.__get_connection)
         self.assertEqual([], self.__rets)
         self.assertEqual(transfers, self.__transfers)
         # Create a connection for each attempt
@@ -3233,7 +3294,7 @@ class TestXfrinProcess(unittest.TestCase):
         """
         Everything OK the first time, over IXFR.
         """
-        self.__do_test([XFRIN_OK], [RRType.IXFR], RRType.IXFR)
+        self.__do_test([XFRIN_OK], [RRType.IXFR], ZoneInfo.REQUEST_IXFR_FIRST)
         # Check there was loadzone command
         self.assertTrue(self._send_cc_session.send_called)
         self.assertTrue(self._send_cc_session.send_called_correctly)
@@ -3244,23 +3305,27 @@ class TestXfrinProcess(unittest.TestCase):
         """
         Everything OK the first time, over AXFR.
         """
-        self.__do_test([XFRIN_OK], [RRType.AXFR], RRType.AXFR)
+        self.__do_test([XFRIN_OK], [RRType.AXFR],
+                       ZoneInfo.REQUEST_IXFR_DISABLED)
 
     def test_axfr_fail(self):
         """
         The transfer failed over AXFR. Should not be retried (we don't expect
-        to fail on AXFR, but succeed on IXFR and we didn't use IXFR in the first
-        place for some reason.
+        to fail on AXFR, but succeed on IXFR and we didn't use IXFR in the
+        first place for some reason.
+
         """
-        self.__do_test([XFRIN_FAIL], [RRType.AXFR], RRType.AXFR)
+        self.__do_test([XFRIN_FAIL], [RRType.AXFR],
+                       ZoneInfo.REQUEST_IXFR_DISABLED)
 
     def test_ixfr_fallback(self):
         """
-        The transfer fails over IXFR, but suceeds over AXFR. It should fall back
-        to it and say everything is OK.
+        The transfer fails over IXFR, but suceeds over AXFR. It should fall
+        back to it and say everything is OK.
+
         """
         self.__do_test([XFRIN_FAIL, XFRIN_OK], [RRType.IXFR, RRType.AXFR],
-                       RRType.IXFR)
+                       ZoneInfo.REQUEST_IXFR_FIRST)
 
     def test_ixfr_fail(self):
         """
@@ -3268,18 +3333,55 @@ class TestXfrinProcess(unittest.TestCase):
         (only once) and should try both before giving up.
         """
         self.__do_test([XFRIN_FAIL, XFRIN_FAIL],
-                       [RRType.IXFR, RRType.AXFR], RRType.IXFR)
+                       [RRType.IXFR, RRType.AXFR], ZoneInfo.REQUEST_IXFR_FIRST)
+
+    def test_ixfr_only(self):
+        """
+        The transfer fails and IXFR_ONLY is specified.  It shouldn't fall
+        back to AXFR and should report failure.
+        """
+        self.__do_test([XFRIN_FAIL], [RRType.IXFR], ZoneInfo.REQUEST_IXFR_ONLY)
 
     def test_send_loadzone(self):
         """
         Check the loadzone command is sent after successful transfer.
         """
-        self.__do_test([XFRIN_OK], [RRType.IXFR], RRType.IXFR)
+        self.__do_test([XFRIN_OK], [RRType.IXFR],
+                       ZoneInfo.REQUEST_IXFR_FIRST)
         self.assertTrue(self._send_cc_session.send_called)
         self.assertTrue(self._send_cc_session.send_called_correctly)
         self.assertTrue(self._send_cc_session.recv_called)
         self.assertTrue(self._send_cc_session.recv_called_correctly)
 
+    def test_initial_request_type(self):
+        """Check initial xfr reuqest type (AXFR or IXFR).
+
+        Varying the policy of use of IXFR and availability of current
+        zone SOA.  We are only interested in the initial request type,
+        so won't check the xfr results.
+
+        """
+        for soa in [begin_soa_rdata, None]:
+            for request_ixfr in [ZoneInfo.REQUEST_IXFR_FIRST,
+                                 ZoneInfo.REQUEST_IXFR_ONLY,
+                                 ZoneInfo.REQUEST_IXFR_DISABLED]:
+                # set up our dummy _get_zone_soa()
+                xfrin._get_zone_soa = lambda x, y, z: soa
+
+                # Clear all counters
+                self.__transfers = []
+                self.__published = []
+                self.__created_connections = 0
+
+                # Determine the expected type
+                expected_type = RRType.IXFR
+                if (soa is None or
+                    request_ixfr == ZoneInfo.REQUEST_IXFR_DISABLED):
+                    expected_type = RRType.AXFR
+
+                # perform the test
+                self.__do_test([XFRIN_OK], [expected_type], request_ixfr)
+
 class TestFormatting(unittest.TestCase):
     # If the formatting functions are moved to a more general library
     # (ticket #1379), these tests should be moved with them.

+ 283 - 161
src/bin/xfrin/xfrin.py.in

@@ -31,6 +31,7 @@ from isc.config.ccsession import *
 from isc.statistics import Counters
 from isc.notify import notify_out
 import isc.util.process
+from isc.util.address_formatter import AddressFormatter
 from isc.datasrc import DataSourceClient, ZoneFinder
 import isc.net.parse
 from isc.xfrin.diff import Diff
@@ -565,18 +566,25 @@ class XfrinConnection(asyncore.dispatcher):
 
     def __init__(self,
                  sock_map, zone_name, rrclass, datasrc_client,
-                 shutdown_event, master_addrinfo, db_file, tsig_key=None,
+                 shutdown_event, master_addrinfo, zone_soa, tsig_key=None,
                  idle_timeout=60):
-        '''Constructor of the XfirnConnection class.
+        """Constructor of the XfirnConnection class.
+
+        Parameters:
+          sock_map: empty dict, used with asyncore.
+          zone_name (dns.Name): Zone name.
+          rrclass (dns.RRClass): Zone RR class.
+          datasrc_client (DataSourceClient): the data source client object
+            used for the XFR session.
+          shutdown_event (threading.Event): used for synchronization with
+            parent thread.
+          master_addrinfo (tuple: (sock family, sock type, sockaddr)):
+            address and port of the master server.
+          zone_soa (RRset or None): SOA RRset of zone's current SOA or None
+            if it's not available.
+          idle_timeout (int): max idle time for read data from socket.
 
-        db_file: SQLite3 DB file.  Unforutnately we still need this for
-                 temporary workaround in _get_zone_soa().  This should be
-                 removed when we eliminate the need for the workaround.
-        idle_timeout: max idle time for read data from socket.
-        datasrc_client: the data source client object used for the XFR session.
-                        This will eventually replace db_file completely.
-
-        '''
+        """
 
         asyncore.dispatcher.__init__(self, map=sock_map)
 
@@ -595,9 +603,8 @@ class XfrinConnection(asyncore.dispatcher):
         self._rrclass = rrclass
 
         # Data source handler
-        self._db_file = db_file
         self._datasrc_client = datasrc_client
-        self._zone_soa = self._get_zone_soa()
+        self._zone_soa = zone_soa
 
         self._sock_map = sock_map
         self._soa_rr_count = 0
@@ -626,54 +633,6 @@ class XfrinConnection(asyncore.dispatcher):
         self.create_socket(self._master_addrinfo[0], self._master_addrinfo[1])
         self.socket.setblocking(1)
 
-    def _get_zone_soa(self):
-        '''Retrieve the current SOA RR of the zone to be transferred.
-
-        It will be used for various purposes in subsequent xfr protocol
-        processing.   It is validly possible that the zone is currently
-        empty and therefore doesn't have an SOA, so this method doesn't
-        consider it an error and returns None in such a case.  It may or
-        may not result in failure in the actual processing depending on
-        how the SOA is used.
-
-        When the zone has an SOA RR, this method makes sure that it's
-        valid, i.e., it has exactly one RDATA; if it is not the case
-        this method returns None.
-
-        If the underlying data source doesn't even know the zone, this method
-        tries to provide backward compatible behavior where xfrin is
-        responsible for creating zone in the corresponding DB table.
-        For a longer term we should deprecate this behavior by introducing
-        more generic zone management framework, but at the moment we try
-        to not surprise existing users.  (Note also that the part of
-        providing the compatible behavior uses the old data source API.
-        We'll deprecate this API in a near future, too).
-
-        '''
-        # get the zone finder.  this must be SUCCESS (not even
-        # PARTIALMATCH) because we are specifying the zone origin name.
-        result, finder = self._datasrc_client.find_zone(self._zone_name)
-        if result != DataSourceClient.SUCCESS:
-            # The data source doesn't know the zone.  For now, we provide
-            # backward compatibility and creates a new one ourselves.
-            isc.datasrc.sqlite3_ds.load(self._db_file,
-                                        self._zone_name.to_text(),
-                                        lambda : [])
-            logger.warn(XFRIN_ZONE_CREATED, self.zone_str())
-            # try again
-            result, finder = self._datasrc_client.find_zone(self._zone_name)
-        if result != DataSourceClient.SUCCESS:
-            return None
-        result, soa_rrset, _ = finder.find(self._zone_name, RRType.SOA)
-        if result != ZoneFinder.SUCCESS:
-            logger.info(XFRIN_ZONE_NO_SOA, self.zone_str())
-            return None
-        if soa_rrset.get_rdata_count() != 1:
-            logger.warn(XFRIN_ZONE_MULTIPLE_SOA, self.zone_str(),
-                        soa_rrset.get_rdata_count())
-            return None
-        return soa_rrset
-
     def __set_xfrstate(self, new_state):
         self.__state = new_state
 
@@ -746,8 +705,9 @@ class XfrinConnection(asyncore.dispatcher):
 
         msg = self._create_query(query_type)
         render = MessageRenderer()
-        # XXX Currently, python wrapper doesn't accept 'None' parameter in this case,
-        # we should remove the if statement and use a universal interface later.
+        # XXX Currently, python wrapper doesn't accept 'None' parameter in this
+        # case, we should remove the if statement and use a universal
+        # interface later.
         if self._tsig_key is not None:
             self._tsig_ctx = self._tsig_ctx_creator(self._tsig_key)
             msg.to_wire(render, self._tsig_ctx)
@@ -1114,16 +1074,95 @@ class XfrinConnection(asyncore.dispatcher):
 
         return False
 
+def _get_zone_soa(datasrc_client, zone_name, zone_class):
+    """Retrieve the current SOA RR of the zone to be transferred.
+
+    This function is essentially private to the module, but will also
+    be called (or tweaked) from tests; no one else should use this
+    function directly.
+
+    It will be used for various purposes in subsequent xfr protocol
+    processing.   It is validly possible that the zone is currently
+    empty and therefore doesn't have an SOA, so this method doesn't
+    consider it an error and returns None in such a case.  It may or
+    may not result in failure in the actual processing depending on
+    how the SOA is used.
+
+    When the zone has an SOA RR, this method makes sure that it's
+    valid, i.e., it has exactly one RDATA; if it is not the case
+    this method returns None.
+
+    If the underlying data source doesn't even know the zone, this method
+    tries to provide backward compatible behavior where xfrin is
+    responsible for creating zone in the corresponding DB table.
+    For a longer term we should deprecate this behavior by introducing
+    more generic zone management framework, but at the moment we try
+    to not surprise existing users.
+
+    """
+    # datasrc_client should never be None in production case (only tests could
+    # specify None)
+    if datasrc_client is None:
+        return None
+
+    # get the zone finder.  this must be SUCCESS (not even
+    # PARTIALMATCH) because we are specifying the zone origin name.
+    result, finder = datasrc_client.find_zone(zone_name)
+    if result != DataSourceClient.SUCCESS:
+        # The data source doesn't know the zone.  For now, we provide
+        # backward compatibility and creates a new one ourselves.
+        # For longer term, we should probably separate this level of zone
+        # management outside of xfrin.
+        datasrc_client.create_zone(zone_name)
+        logger.warn(XFRIN_ZONE_CREATED, format_zone_str(zone_name, zone_class))
+        # try again
+        result, finder = datasrc_client.find_zone(zone_name)
+    if result != DataSourceClient.SUCCESS:
+        return None
+    result, soa_rrset, _ = finder.find(zone_name, RRType.SOA)
+    if result != ZoneFinder.SUCCESS:
+        logger.info(XFRIN_ZONE_NO_SOA, format_zone_str(zone_name, zone_class))
+        return None
+    if soa_rrset.get_rdata_count() != 1:
+        logger.warn(XFRIN_ZONE_MULTIPLE_SOA,
+                    format_zone_str(zone_name, zone_class),
+                    soa_rrset.get_rdata_count())
+        return None
+    return soa_rrset
+
+def __get_initial_xfr_type(zone_soa, request_ixfr, zname, zclass, master_addr):
+    """Determine the initial xfr request type.
+
+    This is a dedicated subroutine of __process_xfrin.
+    """
+    if zone_soa is None:
+        # This is a kind of special case, so we log it at info level.
+        logger.info(XFRIN_INITIAL_AXFR, format_zone_str(zname, zclass),
+                    AddressFormatter(master_addr))
+        return RRType.AXFR
+    if request_ixfr == ZoneInfo.REQUEST_IXFR_DISABLED:
+        logger.debug(DBG_XFRIN_TRACE, XFRIN_INITIAL_IXFR_DISABLED,
+                     format_zone_str(zname, zclass),
+                     AddressFormatter(master_addr))
+        return RRType.AXFR
+
+    assert(request_ixfr == ZoneInfo.REQUEST_IXFR_FIRST or
+           request_ixfr == ZoneInfo.REQUEST_IXFR_ONLY)
+    logger.debug(DBG_XFRIN_TRACE, XFRIN_INITIAL_IXFR,
+                     format_zone_str(zname, zclass),
+                     AddressFormatter(master_addr))
+    return RRType.IXFR
+
 def __process_xfrin(server, zone_name, rrclass, db_file,
                     shutdown_event, master_addrinfo, check_soa, tsig_key,
-                    request_type, conn_class):
+                    request_ixfr, conn_class):
     conn = None
     exception = None
     ret = XFRIN_FAIL
     try:
         # Create a data source client used in this XFR session.  Right now we
-        # still assume an sqlite3-based data source, and use both the old and new
-        # data source APIs.  We also need to use a mock client for tests.
+        # still assume an sqlite3-based data source, and use both the old and
+        # new data source APIs.  We also need to use a mock client for tests.
         # For a temporary workaround to deal with these situations, we skip the
         # creation when the given file is none (the test case).  Eventually
         # this code will be much cleaner.
@@ -1131,38 +1170,55 @@ def __process_xfrin(server, zone_name, rrclass, db_file,
         if db_file is not None:
             # temporary hardcoded sqlite initialization. Once we decide on
             # the config specification, we need to update this (TODO)
-            # this may depend on #1207, or any follow-up ticket created for #1207
+            # this may depend on #1207, or any follow-up ticket created for
+            # #1207
             datasrc_type = "sqlite3"
             datasrc_config = "{ \"database_file\": \"" + db_file + "\"}"
             datasrc_client = DataSourceClient(datasrc_type, datasrc_config)
 
-        # Create a TCP connection for the XFR session and perform the operation.
+        # Get the current zone SOA (if available) and determine the initial
+        # reuqest type: AXFR or IXFR.
+        zone_soa = _get_zone_soa(datasrc_client, zone_name, rrclass)
+        request_type = __get_initial_xfr_type(zone_soa, request_ixfr,
+                                              zone_name, rrclass,
+                                              master_addrinfo[2])
+
+        # Create a TCP connection for the XFR session and perform the
+        # operation.
         sock_map = {}
-        # In case we were asked to do IXFR and that one fails, we try again with
-        # AXFR. But only if we could actually connect to the server.
+        # In case we were asked to do IXFR and that one fails, we try again
+        # with AXFR. But only if we could actually connect to the server.
         #
-        # So we start with retry as True, which is set to false on each attempt.
-        # In the case of connected but failed IXFR, we set it to true once again.
+        # So we start with retry as True, which is set to false on each
+        # attempt. In the case of connected but failed IXFR, we set it to true
+        # once again.
         retry = True
         while retry:
             retry = False
             conn = conn_class(sock_map, zone_name, rrclass, datasrc_client,
-                              shutdown_event, master_addrinfo, db_file,
+                              shutdown_event, master_addrinfo, zone_soa,
                               tsig_key)
             conn.init_socket()
             ret = XFRIN_FAIL
             if conn.connect_to_master():
                 ret = conn.do_xfrin(check_soa, request_type)
                 if ret == XFRIN_FAIL and request_type == RRType.IXFR:
-                    # IXFR failed for some reason. It might mean the server can't
-                    # handle it, or we don't have the zone or we are out of sync or
-                    # whatever else. So we retry with with AXFR, as it may succeed
-                    # in many such cases.
-                    retry = True
-                    request_type = RRType.AXFR
-                    logger.warn(XFRIN_XFR_TRANSFER_FALLBACK, conn.zone_str())
-                    conn.close()
-                    conn = None
+                    # IXFR failed for some reason. It might mean the server
+                    # can't handle it, or we don't have the zone or we are out
+                    # of sync or whatever else. So we retry with with AXFR, as
+                    # it may succeed in many such cases; if "IXFR only" is
+                    # specified in request_ixfr, however, we suppress the
+                    # fallback.
+                    if request_ixfr == ZoneInfo.REQUEST_IXFR_ONLY:
+                        logger.warn(XFRIN_XFR_TRANSFER_FALLBACK_DISABLED,
+                                    conn.zone_str())
+                    else:
+                        retry = True
+                        request_type = RRType.AXFR
+                        logger.warn(XFRIN_XFR_TRANSFER_FALLBACK,
+                                    conn.zone_str())
+                        conn.close()
+                        conn = None
 
     except Exception as ex:
         # If exception happens, just remember it here so that we can re-raise
@@ -1188,7 +1244,7 @@ def __process_xfrin(server, zone_name, rrclass, db_file,
 
 def process_xfrin(server, xfrin_recorder, zone_name, rrclass, db_file,
                   shutdown_event, master_addrinfo, check_soa, tsig_key,
-                  request_type, conn_class=XfrinConnection):
+                  request_ixfr, conn_class=XfrinConnection):
     # Even if it should be rare, the main process of xfrin session can
     # raise an exception.  In order to make sure the lock in xfrin_recorder
     # is released in any cases, we delegate the main part to the helper
@@ -1198,14 +1254,17 @@ def process_xfrin(server, xfrin_recorder, zone_name, rrclass, db_file,
     try:
         __process_xfrin(server, zone_name, rrclass, db_file,
                         shutdown_event, master_addrinfo, check_soa, tsig_key,
-                        request_type, conn_class)
+                        request_ixfr, conn_class)
     except Exception as ex:
         # don't log it until we complete decrement().
         exception = ex
     xfrin_recorder.decrement(zone_name)
 
     if exception is not None:
-        typestr = "AXFR" if request_type == RRType.AXFR else "IXFR"
+        if request_ixfr == ZoneInfo.REQUEST_IXFR_DISABLED:
+            typestr = "AXFR"
+        else:
+            typestr = "IXFR"
         logger.error(XFRIN_XFR_PROCESS_FAILURE, typestr, zone_name.to_text(),
                      str(rrclass), str(exception))
 
@@ -1238,10 +1297,26 @@ class XfrinRecorder:
         return ret
 
 class ZoneInfo:
+    # Internal values corresponding to request_ixfr
+    REQUEST_IXFR_FIRST = 0      # request_ixfr=yes, use IXFR 1st then AXFR
+    REQUEST_IXFR_ONLY = 1       # request_ixfr=only, use IXFR only
+    REQUEST_IXFR_DISABLED = 2   # request_ixfr=no, AXFR-only
+
+    # Map from configuration values for request_ixfr to internal values
+    # This is a constant; don't modify.
+    REQUEST_IXFR_CFG_TO_VAL = { 'yes': REQUEST_IXFR_FIRST,
+                                'only': REQUEST_IXFR_ONLY,
+                                'no': REQUEST_IXFR_DISABLED }
+
     def __init__(self, config_data, module_cc):
         """Creates a zone_info with the config data element as
            specified by the 'zones' list in xfrin.spec. Module_cc is
            needed to get the defaults from the specification"""
+        # Handle deprecated config parameter explicitly for the moment.
+        if config_data.get('use_ixfr') is not None:
+            raise XfrinZoneInfoException('use_ixfr was deprecated.' +
+                                         'use rquest_ixfr')
+
         self._module_cc = module_cc
         self.set_name(config_data.get('name'))
         self.set_master_addr(config_data.get('master_addr'))
@@ -1249,7 +1324,17 @@ class ZoneInfo:
         self.set_master_port(config_data.get('master_port'))
         self.set_zone_class(config_data.get('class'))
         self.set_tsig_key_name(config_data.get('tsig_key'))
-        self.set_use_ixfr(config_data.get('use_ixfr'))
+        self.set_request_ixfr(config_data.get('request_ixfr'))
+
+    @property
+    def request_ixfr(self):
+        """Policy on the use of IXFR.
+
+        Possible values are REQUEST_IXFR_xxx, internally stored in
+        __request_ixfr, read-only outside of the class.
+
+        """
+        return self.__request_ixfr
 
     def set_name(self, name_str):
         """Set the name for this zone given a name string.
@@ -1336,16 +1421,15 @@ class ZoneInfo:
         else:
             return key
 
-    def set_use_ixfr(self, use_ixfr):
-        """Set use_ixfr. If set to True, it will use
-           IXFR for incoming transfers. If set to False, it will use AXFR.
-           At this moment there is no automatic fallback"""
-        # TODO: http://bind10.isc.org/ticket/1279
-        if use_ixfr is None:
-            self.use_ixfr = \
-                self._module_cc.get_default_value("zones/use_ixfr")
-        else:
-            self.use_ixfr = use_ixfr
+    def set_request_ixfr(self, request_ixfr):
+        if request_ixfr is None:
+            request_ixfr = \
+                self._module_cc.get_default_value("zones/request_ixfr")
+        try:
+            self.__request_ixfr = self.REQUEST_IXFR_CFG_TO_VAL[request_ixfr]
+        except KeyError:
+            raise XfrinZoneInfoException('invalid value for request_ixfr: ' +
+                                         request_ixfr)
 
     def get_master_addr_info(self):
         return (self.master_addr.family, socket.SOCK_STREAM,
@@ -1518,6 +1602,86 @@ class Xfrin:
                 continue
             th.join()
 
+    def __validate_notify_addr(self, notify_addr, zone_str, zone_info):
+        """Validate notify source as a destination for xfr source.
+
+        This is called from __handle_xfr_command in case xfr is triggered
+        by ZoneMgr either due to incoming Notify or periodic refresh event.
+
+        """
+        if zone_info is None:
+            # TODO what to do? no info known about zone. defaults?
+            errmsg = "Got notification to retransfer unknown zone " + zone_str
+            logger.info(XFRIN_RETRANSFER_UNKNOWN_ZONE, zone_str)
+            return create_answer(1, errmsg)
+        else:
+            master_addr = zone_info.get_master_addr_info()
+            if (notify_addr[0] != master_addr[0] or
+                notify_addr[2] != master_addr[2]):
+                notify_addr_str = format_addrinfo(notify_addr)
+                master_addr_str = format_addrinfo(master_addr)
+                errmsg = "Got notification for " + zone_str\
+                    + "from unknown address: " + notify_addr_str;
+                logger.info(XFRIN_NOTIFY_UNKNOWN_MASTER, zone_str,
+                            notify_addr_str, master_addr_str)
+                return create_answer(1, errmsg)
+
+        # Notified address is okay
+        return None
+
+    def __get_running_request_ixfr(self, arg_request_ixfr, zone_info):
+        """Determine the request_ixfr policy for a specific transfer.
+
+        This is a dedicated subroutine of __handle_xfr_command.
+
+        """
+        # If explicitly specified, use it.
+        if arg_request_ixfr is not None:
+            return arg_request_ixfr
+        # Otherwise, if zone info is known, use its value.
+        if zone_info is not None:
+            return zone_info.request_ixfr
+        # Otherwise, use the default value for ZoneInfo
+        request_ixfr_def = \
+            self._module_cc.get_default_value("zones/request_ixfr")
+        return ZoneInfo.REQUEST_IXFR_CFG_TO_VAL[request_ixfr_def]
+
+    def __handle_xfr_command(self, args, arg_db, check_soa, addr_validator,
+                             request_ixfr):
+        """Common subroutine for handling transfer commands.
+
+        This helper method unifies both cases of transfer command from
+        ZoneMgr or from a user.  Depending on who invokes the transfer,
+        details of validation and parameter selection slightly vary.
+        These conditions are passed through parameters and handled in the
+        unified code of this method accordingly.
+
+        If this is from the ZoneMgr due to incoming notify, zone transfer
+        should start from the notify's source address as long as it's
+        configured as a master address, according to RFC1996.  The current
+        implementation conforms to it in a limited way: we can only set one
+        master address. Once we add the ability to have multiple master
+        addresses, we should check if it matches one of them, and then use it.
+
+        In case of transfer command from the user, if the command specifies
+        the master address, use that one; otherwise try to use a configured
+        master address for the zone.
+
+        """
+        (zone_name, rrclass) = self._parse_zone_name_and_class(args)
+        master_addr = self._parse_master_and_port(args, zone_name, rrclass)
+        zone_info = self._get_zone_info(zone_name, rrclass)
+        tsig_key = None if zone_info is None else zone_info.get_tsig_key()
+        db_file = arg_db or self._get_db_file()
+        zone_str = format_zone_str(zone_name, rrclass) # for logging
+        answer = addr_validator(master_addr, zone_str, zone_info)
+        if answer is not None:
+            return answer
+        request_ixfr = self.__get_running_request_ixfr(request_ixfr, zone_info)
+        ret = self.xfrin_start(zone_name, rrclass, db_file, master_addr,
+                               tsig_key, request_ixfr, check_soa)
+        return create_answer(ret[0], ret[1])
+
     def command_handler(self, command, args):
         logger.debug(DBG_XFRIN_TRACE, XFRIN_RECEIVED_COMMAND, command)
         answer = create_answer(0)
@@ -1525,69 +1689,26 @@ class Xfrin:
             if command == 'shutdown':
                 self._shutdown_event.set()
             elif command == 'notify' or command == REFRESH_FROM_ZONEMGR:
-                # Xfrin receives the refresh/notify command from zone manager.
-                # notify command maybe has the parameters which
-                # specify the notifyfrom address and port, according the RFC1996, zone
-                # transfer should starts first from the notifyfrom, but now, let 'TODO' it.
-                # (using the value now, while we can only set one master address, would be
-                # a security hole. Once we add the ability to have multiple master addresses,
-                # we should check if it matches one of them, and then use it.)
-                (zone_name, rrclass) = self._parse_zone_name_and_class(args)
-                zone_str = format_zone_str(zone_name, rrclass)
-                zone_info = self._get_zone_info(zone_name, rrclass)
-                notify_addr = self._parse_master_and_port(args, zone_name,
-                                                          rrclass)
-                if zone_info is None:
-                    # TODO what to do? no info known about zone. defaults?
-                    errmsg = "Got notification to retransfer unknown zone " + zone_str
-                    logger.info(XFRIN_RETRANSFER_UNKNOWN_ZONE, zone_str)
-                    answer = create_answer(1, errmsg)
-                else:
-                    request_type = RRType.AXFR
-                    if zone_info.use_ixfr:
-                        request_type = RRType.IXFR
-                    master_addr = zone_info.get_master_addr_info()
-                    if notify_addr[0] == master_addr[0] and\
-                       notify_addr[2] == master_addr[2]:
-                        ret = self.xfrin_start(zone_name,
-                                               rrclass,
-                                               self._get_db_file(),
-                                               master_addr,
-                                               zone_info.get_tsig_key(), request_type,
-                                               True)
-                        answer = create_answer(ret[0], ret[1])
-                    else:
-                        notify_addr_str = format_addrinfo(notify_addr)
-                        master_addr_str = format_addrinfo(master_addr)
-                        errmsg = "Got notification for " + zone_str\
-                               + "from unknown address: " + notify_addr_str;
-                        logger.info(XFRIN_NOTIFY_UNKNOWN_MASTER, zone_str,
-                                    notify_addr_str, master_addr_str)
-                        answer = create_answer(1, errmsg)
-
-            elif command == 'retransfer' or command == 'refresh':
-                # Xfrin receives the retransfer/refresh from cmdctl(sent by bindctl).
-                # If the command has specified master address, do transfer from the
-                # master address, or else do transfer from the configured masters.
-                (zone_name, rrclass) = self._parse_zone_name_and_class(args)
-                master_addr = self._parse_master_and_port(args, zone_name,
-                                                          rrclass)
-                zone_info = self._get_zone_info(zone_name, rrclass)
-                tsig_key = None
-                request_type = RRType.AXFR
-                if zone_info:
-                    tsig_key = zone_info.get_tsig_key()
-                    if zone_info.use_ixfr:
-                        request_type = RRType.IXFR
-                db_file = args.get('db_file') or self._get_db_file()
-                ret = self.xfrin_start(zone_name,
-                                       rrclass,
-                                       db_file,
-                                       master_addr,
-                                       tsig_key, request_type,
-                                       (False if command == 'retransfer' else True))
-                answer = create_answer(ret[0], ret[1])
-
+                # refresh/notify command from zone manager.
+                # The address has to be validated, db_file is local only,
+                # and always perform SOA check.
+                addr_validator = \
+                    lambda x, y, z: self.__validate_notify_addr(x, y, z)
+                answer = self.__handle_xfr_command(args, None, True,
+                                                   addr_validator, None)
+            elif command == 'retransfer':
+                # retransfer from cmdctl (sent by bindctl).
+                # No need for address validation, db_file may be specified
+                # with the command, and skip SOA check, always use AXFR.
+                answer = self.__handle_xfr_command(
+                    args, args.get('db_file'), False, lambda x, y, z: None,
+                    ZoneInfo.REQUEST_IXFR_DISABLED)
+            elif command == 'refresh':
+                # retransfer from cmdctl (sent by bindctl).  similar to
+                # retransfer, but do SOA check, and honor request_ixfr config.
+                answer = self.__handle_xfr_command(
+                    args, args.get('db_file'), True, lambda x, y, z: None,
+                    None)
             # return statistics data to the stats daemon
             elif command == "getstats":
                 # The log level is here set to debug in order to avoid
@@ -1608,7 +1729,8 @@ class Xfrin:
         if zone_name_str is None:
             raise XfrinException('zone name should be provided')
 
-        return (_check_zone_name(zone_name_str), _check_zone_class(args.get('zone_class')))
+        return (_check_zone_name(zone_name_str),
+                _check_zone_class(args.get('zone_class')))
 
     def _parse_master_and_port(self, args, zone_name, zone_class):
         """
@@ -1725,7 +1847,7 @@ class Xfrin:
             self._cc_check_command()
 
     def xfrin_start(self, zone_name, rrclass, db_file, master_addrinfo,
-                    tsig_key, request_type, check_soa=True):
+                    tsig_key, request_ixfr, check_soa=True):
         if "pydnspp" not in sys.modules:
             return (1, "xfrin failed, can't load dns message python library: 'pydnspp'")
 
@@ -1744,7 +1866,7 @@ class Xfrin:
                                                 db_file,
                                                 self._shutdown_event,
                                                 master_addrinfo, check_soa,
-                                                tsig_key, request_type))
+                                                tsig_key, request_ixfr))
 
         xfrin_thread.start()
         return (0, 'zone xfrin is started')

+ 45 - 11
src/bin/xfrin/xfrin.spec

@@ -48,6 +48,11 @@
             "item_type": "boolean",
             "item_optional": false,
             "item_default": false
+          },
+          { "item_name": "request_ixfr",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": "yes"
           }
           ]
         }
@@ -56,7 +61,36 @@
     "commands": [
      {
         "command_name": "retransfer",
-        "command_description": "retransfer a single zone without checking zone serial number",
+        "command_description": "retransfer a single zone without checking zone serial number, always using AXFR",
+        "command_args": [ {
+            "item_name": "zone_name",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": ""
+          },
+          {
+            "item_name": "zone_class",
+            "item_type": "string",
+            "item_optional": true,
+            "item_default": "IN"
+          },
+          {
+            "item_name": "master",
+            "item_type": "string",
+            "item_optional": true,
+            "item_default": ""
+          },
+          {
+            "item_name": "port",
+            "item_type": "integer",
+            "item_optional": true,
+            "item_default": 53
+          }
+        ]
+      },
+     {
+        "command_name": "refresh",
+        "command_description": "transfer a single zone with checking zone serial number and honoring the request_ixfr policy",
         "command_args": [ {
             "item_name": "zone_name",
             "item_type": "string",
@@ -102,16 +136,16 @@
         "item_optional": false,
         "item_default": {
           "_SERVER_" : {
-	    "soaoutv4": 0,
-	    "soaoutv6": 0,
-	    "axfrreqv4": 0,
-	    "axfrreqv6": 0,
-	    "ixfrreqv4": 0,
-	    "ixfrreqv6": 0,
-	    "xfrsuccess": 0,
-	    "xfrfail": 0,
-	    "last_ixfr_duration": 0.0,
-	    "last_axfr_duration": 0.0
+            "soaoutv4": 0,
+            "soaoutv6": 0,
+            "axfrreqv4": 0,
+            "axfrreqv6": 0,
+            "ixfrreqv4": 0,
+            "ixfrreqv6": 0,
+            "xfrsuccess": 0,
+            "xfrfail": 0,
+            "last_ixfr_duration": 0.0,
+            "last_axfr_duration": 0.0
           }
         },
         "item_title": "Zone names",

+ 32 - 3
src/bin/xfrin/xfrin_messages.mes

@@ -80,6 +80,24 @@ is not equal to the requested SOA serial.
 There was an error importing the python DNS module pydnspp. The most
 likely cause is a PYTHONPATH problem.
 
+% XFRIN_INITIAL_AXFR no SOA available for %1 yet, requesting AXFR of initial version from %2
+On starting the zone transfer, it's detected that there is no SOA
+record available for the zone.  This is always the case for the very
+first transfer or if the administrator has removed the locally copied
+data by hand for some reason.  In this case trying IXFR does not make
+sense for the obvious reason, so AXFR will be used from the beginning,
+regardless of the request_ixfr configuration (even if "only" is
+specified).
+
+% XFRIN_INITIAL_IXFR requesting IXFR for %1 from %2
+IXFR will be used for the initial request type for the specified zone
+transfer.  It will fall back to AXFR if the initial request fails
+(and unless specified not to do so by configuration).
+
+% XFRIN_INITIAL_IXFR_DISABLED IXFR disabled for %1, requesting AXFR from %2
+The use of IXFR is disabled by configuration for the specified zone,
+so only AXFR will be tried.
+
 % XFRIN_INVALID_ZONE_DATA zone %1 received from %2 is broken and unusable
 The zone was received, but it failed sanity validation. The previous version
 of zone (if any is available) will be used. Look for previous
@@ -212,6 +230,17 @@ such that the remote server doesn't support IXFR, we don't have the SOA record
 (or the zone at all), we are out of sync, etc. In many of these situations,
 AXFR could still work. Therefore we try that one in case it helps.
 
+% XFRIN_XFR_TRANSFER_FALLBACK_DISABLED suppressing fallback from IXFR to AXFR for %1
+An IXFR transfer of the given zone failed.  By default AXFR will be
+tried next, but this fallback is disabled by configuration, so the
+whole transfer attempt failed at that point.  If the reason for the
+failure (which should be logged separately) is temporary, this is
+probably harmless or even desired as another IXFR will take place some
+time later (without falling back to the possibly expensive AXFR).  If
+this is a permanent error (e.g., some change at the master server
+completely disables IXFR), the secondary zone will eventually expire,
+so the configuration should be changed to allow AXFR.
+
 % XFRIN_XFR_TRANSFER_PROTOCOL_VIOLATION %1 transfer of zone %2 with %3 failed: %4
 The XFR transfer for the given zone has failed due to a protocol
 error, such as an unexpected response from the primary server.  The
@@ -250,9 +279,9 @@ On starting an xfrin session, it is identified that the zone to be
 transferred has multiple SOA RRs.  Such a zone is broken, but could be
 accidentally configured especially in a data source using "non
 captive" backend database.  The implementation ignores entire SOA RRs
-and tries to continue processing as if the zone were empty.  This
-means subsequent AXFR can succeed and possibly replace the zone with
-valid content, but an IXFR attempt will fail.
+and tries to continue processing as if the zone were empty.  This also
+means AXFR will be used unconditionally, regardless of the configured value
+for request_ixfr of the zone.
 
 % XFRIN_ZONE_NO_SOA Zone %1 does not have SOA
 On starting an xfrin session, it is identified that the zone to be

+ 42 - 1
src/lib/datasrc/client_list.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013  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
@@ -24,6 +24,7 @@
 #include <datasrc/memory/zone_data_loader.h>
 #include <datasrc/memory/zone_data_updater.h>
 #include <datasrc/logger.h>
+#include <datasrc/zone_table_accessor_cache.h>
 #include <dns/masterload.h>
 #include <util/memory_segment_local.h>
 
@@ -321,6 +322,21 @@ ConfigurableClientList::findInternal(MutableResult& candidate,
     // and the need_updater parameter is true, get the zone there.
 }
 
+void
+ConfigurableClientList::resetMemorySegment
+    (const std::string& datasrc_name,
+     ZoneTableSegment::MemorySegmentOpenMode mode,
+     ConstElementPtr config_params)
+{
+    BOOST_FOREACH(DataSourceInfo& info, data_sources_) {
+        if (info.name_ == datasrc_name) {
+            ZoneTableSegment& segment = *info.ztable_segment_;
+            segment.reset(mode, config_params);
+            break;
+        }
+    }
+}
+
 ConfigurableClientList::ZoneWriterPair
 ConfigurableClientList::getCachedZoneWriter(const Name& name,
                                             const std::string& datasrc_name)
@@ -403,5 +419,30 @@ ConfigurableClientList::getStatus() const {
     return (result);
 }
 
+ConstZoneTableAccessorPtr
+ConfigurableClientList::getZoneTableAccessor(const std::string& datasrc_name,
+                                             bool use_cache) const
+{
+    if (!use_cache) {
+        isc_throw(isc::NotImplemented,
+              "getZoneTableAccessor only implemented for cache");
+    }
+
+    // Find the matching data source
+    BOOST_FOREACH(const DataSourceInfo& info, data_sources_) {
+        if (!datasrc_name.empty() && datasrc_name != info.name_) {
+            continue;
+        }
+
+        const internal::CacheConfig* config(info.getCacheConfig());
+        // If caching is disabled for the named data source, this will
+        // return an accessor to an effectivley empty table.
+        return (ConstZoneTableAccessorPtr
+                (new internal::ZoneTableAccessorCache(*config)));
+    }
+
+    return (ConstZoneTableAccessorPtr());
+}
+
 }
 }

+ 51 - 4
src/lib/datasrc/client_list.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013  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
@@ -21,7 +21,8 @@
 #include <dns/rrclass.h>
 #include <cc/data.h>
 #include <exceptions/exceptions.h>
-#include "memory/zone_table_segment.h"
+#include <datasrc/memory/zone_table_segment.h>
+#include <datasrc/zone_table_accessor.h>
 
 #include <vector>
 #include <boost/shared_ptr.hpp>
@@ -120,6 +121,8 @@ private:
     MemorySegmentState state_;
 };
 
+typedef boost::shared_ptr<const ZoneTableAccessor> ConstZoneTableAccessorPtr;
+
 /// \brief The list of data source clients.
 ///
 /// The purpose of this class is to hold several data source clients and search
@@ -281,6 +284,19 @@ public:
     virtual FindResult find(const dns::Name& zone,
                             bool want_exact_match = false,
                             bool want_finder = true) const = 0;
+
+    /// \brief Creates a ZoneTableAccessor object for the specified data
+    /// source.
+    ///
+    /// \param datasrc_name If not empty, the name of the data source.
+    /// \param use_cache If true, create a zone table for in-memory cache.
+    /// \throw NotImplemented if this method is not implemented.
+    /// \return A pointer to the accessor, or NULL if the requested data
+    /// source is not found.
+    virtual ConstZoneTableAccessorPtr
+    getZoneTableAccessor(const std::string& datasrc_name,
+                         bool use_cache) const = 0;
+
 };
 
 /// \brief Shared pointer to the list.
@@ -288,8 +304,8 @@ typedef boost::shared_ptr<ClientList> ClientListPtr;
 /// \brief Shared const pointer to the list.
 typedef boost::shared_ptr<const ClientList> ConstClientListPtr;
 
-/// \Concrete implementation of the ClientList, which is constructed based on
-///     configuration.
+/// \brief Concrete implementation of the ClientList, which is constructed
+/// based on configuration.
 ///
 /// This is the implementation which is expected to be used in the servers.
 /// However, it is expected most of the code will use it as the ClientList,
@@ -348,6 +364,21 @@ public:
         return (configuration_);
     }
 
+    /// \brief Resets the zone table segment for a datasource with a new
+    /// memory segment.
+    ///
+    /// See documentation of \c ZoneTableSegment interface
+    /// implementations (such as \c ZoneTableSegmentMapped) for the
+    /// syntax of \c config_params.
+    ///
+    /// \param datasrc_name The name of the data source whose segment to reset
+    /// \param mode The open mode for the new memory segment
+    /// \param config_params The configuration for the new memory segment.
+    void resetMemorySegment
+        (const std::string& datasrc_name,
+         memory::ZoneTableSegment::MemorySegmentOpenMode mode,
+         isc::data::ConstElementPtr config_params);
+
 private:
     /// \brief Convenience type shortcut
     typedef boost::shared_ptr<memory::ZoneWriter> ZoneWriterPtr;
@@ -490,6 +521,22 @@ public:
     /// it might be, so it is just made public (there's no real reason to
     /// hide it).
     const DataSources& getDataSources() const { return (data_sources_); }
+
+    /// \brief Creates a ZoneTableAccessor object for the specified data
+    /// source.
+    ///
+    /// \param datasrc_name If not empty, the name of the data source
+    /// \param use_cache If true, create a zone table for in-memory cache.
+    /// \note At present, the only concrete implementation of
+    /// ZoneTableAccessor is ZoneTableAccessorCache, so \c use_cache must be
+    /// true.
+    /// \throw NotImplemented if \c use_cache is false.
+    /// \return A pointer to the accessor, or NULL if the requested data
+    /// source is not found.
+    ConstZoneTableAccessorPtr
+    getZoneTableAccessor(const std::string& datasrc_name,
+                         bool use_cache) const;
+
 private:
     struct MutableResult;
     /// \brief Internal implementation of find.

+ 2 - 1
src/lib/datasrc/memory/zone_table.cc

@@ -72,7 +72,8 @@ ZoneTable::create(util::MemorySegment& mem_sgmt, const RRClass& zone_class) {
 }
 
 void
-ZoneTable::destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable) {
+ZoneTable::destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable, int)
+{
     ZoneTableTree::destroy(mem_sgmt, ztable->zones_.get(),
                            boost::bind(deleteZoneData, &mem_sgmt, _1,
                                        ztable->rrclass_));

+ 4 - 1
src/lib/datasrc/memory/zone_table.h

@@ -151,7 +151,10 @@ public:
     /// \param ztable A non NULL pointer to a valid \c ZoneTable object
     /// that was originally created by the \c create() method (the behavior
     /// is undefined if this condition isn't met).
-    static void destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable);
+    /// \param unused Ununsed parameter, provided so it's compatible to
+    /// SegmentObjectHolder.
+    static void destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable,
+                        int unused = 0);
 
     /// \brief Return the number of zones contained in the zone table.
     ///

+ 7 - 10
src/lib/datasrc/memory/zone_table_segment.h

@@ -108,10 +108,9 @@ private:
 /// for the specific memory-implementation behavior.
 ///
 /// Note: At some point in the future, methods such as \c reset(),
-/// \c clear(), \c resetHeader(), \c getHeader(), \c isWritable(),
-/// \c isUsable() may become non-virtual methods. Such a change should
-/// not affect any code that uses this class, but please be aware of
-/// such plans.
+/// \c clear(), \c getHeader(), \c isWritable(), \c isUsable() may
+/// become non-virtual methods. Such a change should not affect any code
+/// that uses this class, but please be aware of such plans.
 class ZoneTableSegment {
 protected:
     /// \brief Protected constructor
@@ -128,6 +127,10 @@ public:
     /// \brief Return a string name for the \c ZoneTableSegment
     /// implementation.
     ///
+    /// Implementations of this method should ensure that the returned
+    /// string is identical to the corresponding string passed to
+    /// \c ZoneTableSegment::create() for that implementation.
+    ///
     /// \throw None This method's implementations must be
     /// exception-free.
     virtual const std::string& getImplType() const = 0;
@@ -334,12 +337,6 @@ public:
     /// Note that after calling \c clear(), this method will return
     /// false until the segment is reset successfully again.
     virtual bool isUsable() const = 0;
-
-    /// \brief Reset the table header address.
-    ///
-    /// This method must recalculate the \c ZoneTableHeader address, so
-    /// that it is valid when requested using the \c getHeader() method.
-    virtual void resetHeader() = 0;
 };
 
 } // namespace memory

+ 0 - 5
src/lib/datasrc/memory/zone_table_segment_local.cc

@@ -60,11 +60,6 @@ ZoneTableSegmentLocal::clear()
               "should not be used.");
 }
 
-void
-ZoneTableSegmentLocal::resetHeader() {
-    // This method does not have to do anything in this implementation.
-}
-
 // After more methods' definitions are added here, it would be a good
 // idea to move getHeader() and getMemorySegment() definitions to the
 // header file.

+ 0 - 4
src/lib/datasrc/memory/zone_table_segment_local.h

@@ -49,10 +49,6 @@ public:
     /// \brief Returns "local" as the implementation type.
     virtual const std::string& getImplType() const;
 
-    /// \brief This method does not have to do anything in this
-    /// implementation. It has an empty definition.
-    virtual void resetHeader();
-
     /// \brief Return the \c ZoneTableHeader for this local zone table
     /// segment.
     virtual ZoneTableHeader& getHeader();

+ 62 - 56
src/lib/datasrc/memory/zone_table_segment_mapped.cc

@@ -13,12 +13,15 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <datasrc/memory/zone_table_segment_mapped.h>
+#include <datasrc/memory/zone_table.h>
+#include <datasrc/memory/segment_object_holder.h>
 
 #include <memory>
 
 using namespace isc::data;
 using namespace isc::dns;
 using namespace isc::util;
+using isc::datasrc::memory::detail::SegmentObjectHolder;
 
 namespace isc {
 namespace datasrc {
@@ -37,8 +40,7 @@ const char* const ZONE_TABLE_HEADER_NAME = "zone_table_header";
 ZoneTableSegmentMapped::ZoneTableSegmentMapped(const RRClass& rrclass) :
     ZoneTableSegment(rrclass),
     impl_type_("mapped"),
-    rrclass_(rrclass),
-    cached_header_(NULL)
+    rrclass_(rrclass)
 {
 }
 
@@ -53,7 +55,7 @@ ZoneTableSegmentMapped::getImplType() const {
 
 bool
 ZoneTableSegmentMapped::processChecksum(MemorySegmentMapped& segment,
-                                        bool create,
+                                        bool create, bool has_allocations,
                                         std::string& error_msg)
 {
     const MemorySegment::NamedAddressResult result =
@@ -80,6 +82,14 @@ ZoneTableSegmentMapped::processChecksum(MemorySegmentMapped& segment,
             }
         }
     } else {
+        if ((!create) && has_allocations) {
+            // If we are resetting in READ_WRITE mode, and some memory
+            // was already allocated but there is no checksum name, that
+            // indicates that the segment is corrupted.
+            error_msg = "Existing segment is missing a checksum name";
+            return (false);
+        }
+
         // Allocate space for a checksum (which is saved during close).
         void* checksum = NULL;
         while (!checksum) {
@@ -90,9 +100,7 @@ ZoneTableSegmentMapped::processChecksum(MemorySegmentMapped& segment,
             }
         }
         *static_cast<size_t*>(checksum) = 0;
-        const bool grew = segment.setNamedAddress(ZONE_TABLE_CHECKSUM_NAME,
-                                                  checksum);
-        assert(!grew);
+        segment.setNamedAddress(ZONE_TABLE_CHECKSUM_NAME, checksum);
     }
 
     return (true);
@@ -100,14 +108,14 @@ ZoneTableSegmentMapped::processChecksum(MemorySegmentMapped& segment,
 
 bool
 ZoneTableSegmentMapped::processHeader(MemorySegmentMapped& segment,
-                                      bool create,
+                                      bool create, bool has_allocations,
                                       std::string& error_msg)
 {
     const MemorySegment::NamedAddressResult result =
         segment.getNamedAddress(ZONE_TABLE_HEADER_NAME);
     if (result.first) {
         if (create) {
-            // There must be no previously saved checksum.
+            // There must be no previously saved header.
             error_msg = "There is already a saved ZoneTableHeader in the "
                  "segment opened in create mode";
             return (false);
@@ -115,25 +123,24 @@ ZoneTableSegmentMapped::processHeader(MemorySegmentMapped& segment,
             assert(result.second);
         }
     } else {
-        void* ptr = NULL;
-        while (!ptr) {
-            try {
-                ptr = segment.allocate(sizeof(ZoneTableHeader));
-            } catch (const MemorySegmentGrown&) {
-                // Do nothing and try again.
-            }
+        if ((!create) && has_allocations) {
+            // If we are resetting in READ_WRITE mode, and some memory
+            // was already allocated but there is no header name, that
+            // indicates that the segment is corrupted.
+            error_msg = "Existing segment is missing a ZoneTableHeader name";
+            return (false);
         }
-        try {
-            ZoneTableHeader* new_header = new(ptr)
-                ZoneTableHeader(ZoneTable::create(segment, rrclass_));
-            const bool grew = segment.setNamedAddress(ZONE_TABLE_HEADER_NAME,
-                                                      new_header);
-            assert(!grew);
-        } catch (const MemorySegmentGrown&) {
-            // This is extremely unlikely and we just throw a fatal
-            // exception here without attempting to recover.
-
-            throw std::bad_alloc();
+
+        while (true) {
+            try {
+                SegmentObjectHolder<ZoneTable, int> zt_holder(segment, 0);
+                zt_holder.set(ZoneTable::create(segment, rrclass_));
+                void* ptr = segment.allocate(sizeof(ZoneTableHeader));
+                ZoneTableHeader* new_header = new(ptr)
+                    ZoneTableHeader(zt_holder.release());
+                segment.setNamedAddress(ZONE_TABLE_HEADER_NAME, new_header);
+                break;
+            } catch (const MemorySegmentGrown&) {}
         }
     }
 
@@ -152,9 +159,13 @@ ZoneTableSegmentMapped::openReadWrite(const std::string& filename,
     std::auto_ptr<MemorySegmentMapped> segment
         (new MemorySegmentMapped(filename, mode));
 
+    // This flag is used inside processCheckSum() and processHeader(),
+    // and must be initialized before we make any further allocations.
+    const bool has_allocations = !segment->allMemoryDeallocated();
+
     std::string error_msg;
-    if ((!processChecksum(*segment, create, error_msg)) ||
-        (!processHeader(*segment, create, error_msg))) {
+    if ((!processChecksum(*segment, create, has_allocations, error_msg)) ||
+        (!processHeader(*segment, create, has_allocations, error_msg))) {
          if (mem_sgmt_) {
               isc_throw(ResetFailed,
                         "Error in resetting zone table segment to use "
@@ -268,7 +279,7 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode,
         break;
 
     default:
-        isc_throw(isc::InvalidOperation,
+        isc_throw(isc::InvalidParameter,
                   "Invalid MemorySegmentOpenMode passed to reset()");
     }
 
@@ -276,9 +287,11 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode,
     current_mode_ = mode;
     mem_sgmt_.reset(segment.release());
 
-    // Given what we setup above, resetHeader() must not throw at this
-    // point. If it does, all bets are off.
-    resetHeader();
+    if (!isWritable()) {
+        // Given what we setup above, the following must not throw at
+        // this point. If it does, all bets are off.
+        cached_ro_header_ = getHeaderHelper<ZoneTableHeader>(true);
+    }
 }
 
 void
@@ -310,15 +323,18 @@ ZoneTableSegmentMapped::clear() {
     }
 }
 
-void
-ZoneTableSegmentMapped::resetHeader() {
-    // We cannot perform resetHeader() lazily during getHeader() as
-    // getHeader() has to work on const objects too. So we do it here
-    // now.
-
+template<typename T>
+T*
+ZoneTableSegmentMapped::getHeaderHelper(bool initial) const {
     if (!isUsable()) {
         isc_throw(isc::InvalidOperation,
-                  "resetHeader() called without calling reset() first");
+                  "getHeader() called without calling reset() first");
+    }
+
+    if (!isWritable() && !initial) {
+        // The header address would not have changed since reset() for
+        // READ_ONLY segments.
+        return (cached_ro_header_);
     }
 
     const MemorySegment::NamedAddressResult result =
@@ -329,29 +345,19 @@ ZoneTableSegmentMapped::resetHeader() {
                   "getHeader()");
     }
 
-    cached_header_ = static_cast<ZoneTableHeader*>(result.second);
-}
-
-template<typename T>
-T*
-ZoneTableSegmentMapped::getHeaderHelper() const {
-    if (!isUsable()) {
-        isc_throw(isc::InvalidOperation,
-                  "getHeader() called without calling reset() first");
-    }
-
-    assert(cached_header_);
-    return (cached_header_);
+    T* header = static_cast<ZoneTableHeader*>(result.second);
+    assert(header);
+    return (header);
 }
 
 ZoneTableHeader&
 ZoneTableSegmentMapped::getHeader() {
-    return (*getHeaderHelper<ZoneTableHeader>());
+    return (*getHeaderHelper<ZoneTableHeader>(false));
 }
 
 const ZoneTableHeader&
 ZoneTableSegmentMapped::getHeader() const {
-    return (*getHeaderHelper<const ZoneTableHeader>());
+    return (*getHeaderHelper<const ZoneTableHeader>(false));
 }
 
 MemorySegment&
@@ -373,8 +379,8 @@ bool
 ZoneTableSegmentMapped::isWritable() const {
     if (!isUsable()) {
         // If reset() was never performed for this segment, or if the
-        // most recent reset() had failed, then the segment is not
-        // writable.
+        // most recent reset() had failed, or if the segment had been
+        // cleared, then the segment is not writable.
         return (false);
     }
 

+ 4 - 10
src/lib/datasrc/memory/zone_table_segment_mapped.h

@@ -50,12 +50,6 @@ public:
     /// \brief Returns "mapped" as the implementation type.
     virtual const std::string& getImplType() const;
 
-    /// \brief Resets the \c ZoneTableHeader address from the named
-    /// address in the mapped file. This method should be called once
-    /// before calls to \c getHeader() if the mapped \c MemorySegment
-    /// has grown.
-    virtual void resetHeader();
-
     /// \brief Return the \c ZoneTableHeader for this mapped zone table
     /// segment.
     ///
@@ -122,15 +116,15 @@ private:
     void sync();
 
     bool processChecksum(isc::util::MemorySegmentMapped& segment, bool create,
-                         std::string& error_msg);
+                         bool has_allocations, std::string& error_msg);
     bool processHeader(isc::util::MemorySegmentMapped& segment, bool create,
-                       std::string& error_msg);
+                       bool has_allocations, std::string& error_msg);
 
     isc::util::MemorySegmentMapped* openReadWrite(const std::string& filename,
                                                   bool create);
     isc::util::MemorySegmentMapped* openReadOnly(const std::string& filename);
 
-    template<typename T> T* getHeaderHelper() const;
+    template<typename T> T* getHeaderHelper(bool initial) const;
 
 private:
     std::string impl_type_;
@@ -140,7 +134,7 @@ private:
     // Internally holds a MemorySegmentMapped. This is NULL on
     // construction, and is set by the \c reset() method.
     boost::scoped_ptr<isc::util::MemorySegmentMapped> mem_sgmt_;
-    ZoneTableHeader* cached_header_;
+    ZoneTableHeader* cached_ro_header_;
 };
 
 } // namespace memory

+ 87 - 35
src/lib/datasrc/memory/zone_writer.cc

@@ -15,6 +15,11 @@
 #include <datasrc/memory/zone_writer.h>
 #include <datasrc/memory/zone_data.h>
 #include <datasrc/memory/zone_table_segment.h>
+#include <datasrc/memory/segment_object_holder.h>
+
+#include <boost/scoped_ptr.hpp>
+
+#include <dns/rrclass.h>
 
 #include <datasrc/exceptions.h>
 
@@ -26,87 +31,133 @@ namespace isc {
 namespace datasrc {
 namespace memory {
 
+ZoneTableSegment&
+checkZoneTableSegment(ZoneTableSegment& segment) {
+    if (!segment.isWritable()) {
+        isc_throw(isc::InvalidOperation,
+                  "Attempt to construct ZoneWriter for a read-only segment");
+    }
+    return (segment);
+}
+
+struct ZoneWriter::Impl {
+    Impl(ZoneTableSegment& segment, const LoadAction& load_action,
+         const dns::Name& origin, const dns::RRClass& rrclass,
+         bool throw_on_load_error) :
+        // We validate segment first so we can use it to initialize
+        // data_holder_ safely.
+        segment_(checkZoneTableSegment(segment)),
+        load_action_(load_action),
+        origin_(origin),
+        rrclass_(rrclass),
+        state_(ZW_UNUSED),
+        catch_load_error_(throw_on_load_error)
+    {
+        while (true) {
+            try {
+                data_holder_.reset(
+                    new ZoneDataHolder(segment.getMemorySegment(), rrclass_));
+                break;
+            } catch (const isc::util::MemorySegmentGrown&) {}
+        }
+    }
+
+    ZoneTableSegment& segment_;
+    const LoadAction load_action_;
+    const dns::Name origin_;
+    const dns::RRClass rrclass_;
+    enum State {
+        ZW_UNUSED,
+        ZW_LOADED,
+        ZW_INSTALLED,
+        ZW_CLEANED
+    };
+    State state_;
+    const bool catch_load_error_;
+    typedef detail::SegmentObjectHolder<ZoneData, dns::RRClass> ZoneDataHolder;
+    boost::scoped_ptr<ZoneDataHolder> data_holder_;
+};
+
 ZoneWriter::ZoneWriter(ZoneTableSegment& segment,
                        const LoadAction& load_action,
                        const dns::Name& origin,
                        const dns::RRClass& rrclass,
                        bool throw_on_load_error) :
-    segment_(segment),
-    load_action_(load_action),
-    origin_(origin),
-    rrclass_(rrclass),
-    zone_data_(NULL),
-    state_(ZW_UNUSED),
-    catch_load_error_(throw_on_load_error)
+    impl_(new Impl(segment, load_action, origin, rrclass, throw_on_load_error))
 {
-    if (!segment.isWritable()) {
-        isc_throw(isc::InvalidOperation,
-                  "Attempt to construct ZoneWriter for a read-only segment");
-    }
 }
 
 ZoneWriter::~ZoneWriter() {
     // Clean up everything there might be left if someone forgot, just
     // in case.
     cleanup();
+    delete impl_;
 }
 
 void
 ZoneWriter::load(std::string* error_msg) {
-    if (state_ != ZW_UNUSED) {
+    if (impl_->state_ != Impl::ZW_UNUSED) {
         isc_throw(isc::InvalidOperation, "Trying to load twice");
     }
 
     try {
-        zone_data_ = load_action_(segment_.getMemorySegment());
-        segment_.resetHeader();
+        ZoneData* zone_data =
+            impl_->load_action_(impl_->segment_.getMemorySegment());
 
-        if (!zone_data_) {
-            // Bug inside load_action_.
+        if (!zone_data) {
+            // Bug inside impl_->load_action_.
             isc_throw(isc::InvalidOperation,
                       "No data returned from load action");
         }
+
+        impl_->data_holder_->set(zone_data);
+
     } catch (const ZoneLoaderException& ex) {
-        if (!catch_load_error_) {
+        if (!impl_->catch_load_error_) {
             throw;
         }
         if (error_msg) {
             *error_msg = ex.what();
         }
-        segment_.resetHeader();
     }
 
-    state_ = ZW_LOADED;
+    impl_->state_ = Impl::ZW_LOADED;
 }
 
 void
 ZoneWriter::install() {
-    if (state_ != ZW_LOADED) {
+    if (impl_->state_ != Impl::ZW_LOADED) {
         isc_throw(isc::InvalidOperation, "No data to install");
     }
 
     // Check the internal integrity assumption: we should have non NULL
     // zone data or we've allowed load error to create an empty zone.
-    assert(zone_data_ || catch_load_error_);
+    assert(impl_->data_holder_.get() || impl_->catch_load_error_);
 
     // FIXME: This retry is currently untested, as there seems to be no
     // reasonable way to create a zone table segment with non-local memory
     // segment. Once there is, we should provide the test.
-    while (state_ != ZW_INSTALLED) {
+    while (impl_->state_ != Impl::ZW_INSTALLED) {
         try {
-            ZoneTable* table(segment_.getHeader().getTable());
-            if (table == NULL) {
+            ZoneTable* table(impl_->segment_.getHeader().getTable());
+            if (!table) {
                 isc_throw(isc::Unexpected, "No zone table present");
             }
+            // We still need to hold the zone data until we return from
+            // addZone in case it throws, but we then need to immediately
+            // release it as the ownership is transferred to the zone table.
+            // we release this by (re)set it to the old data; that way we can
+            // use the holder for the final cleanup.
             const ZoneTable::AddResult result(
-                zone_data_ ? table->addZone(segment_.getMemorySegment(),
-                                            rrclass_, origin_, zone_data_) :
-                table->addEmptyZone(segment_.getMemorySegment(), origin_));
-            state_ = ZW_INSTALLED;
-            zone_data_ = result.zone_data;
+                impl_->data_holder_->get() ?
+                table->addZone(impl_->segment_.getMemorySegment(),
+                               impl_->rrclass_, impl_->origin_,
+                               impl_->data_holder_->get()) :
+                table->addEmptyZone(impl_->segment_.getMemorySegment(),
+                                    impl_->origin_));
+            impl_->data_holder_->set(result.zone_data);
+            impl_->state_ = Impl::ZW_INSTALLED;
         } catch (const isc::util::MemorySegmentGrown&) {}
-
-        segment_.resetHeader();
     }
 }
 
@@ -114,10 +165,11 @@ void
 ZoneWriter::cleanup() {
     // We eat the data (if any) now.
 
-    if (zone_data_ != NULL) {
-        ZoneData::destroy(segment_.getMemorySegment(), zone_data_, rrclass_);
-        zone_data_ = NULL;
-        state_ = ZW_CLEANED;
+    ZoneData* zone_data = impl_->data_holder_->release();
+    if (zone_data) {
+        ZoneData::destroy(impl_->segment_.getMemorySegment(), zone_data,
+                          impl_->rrclass_);
+        impl_->state_ = Impl::ZW_CLEANED;
     }
 }
 

+ 9 - 17
src/lib/datasrc/memory/zone_writer.h

@@ -15,15 +15,16 @@
 #ifndef MEM_ZONE_WRITER_H
 #define MEM_ZONE_WRITER_H
 
-#include <datasrc/memory/zone_table_segment.h>
 #include <datasrc/memory/load_action.h>
 
-#include <dns/rrclass.h>
-#include <dns/name.h>
+#include <boost/noncopyable.hpp>
+
+#include <dns/dns_fwd.h>
 
 namespace isc {
 namespace datasrc {
 namespace memory {
+class ZoneTableSegment;
 
 /// \brief Does an update to a zone.
 ///
@@ -38,7 +39,7 @@ namespace memory {
 /// This class provides strong exception guarantee for each public
 /// method. That is, when any of the methods throws, the entire state
 /// stays the same as before the call.
-class ZoneWriter {
+class ZoneWriter : boost::noncopyable {
 public:
     /// \brief Constructor
     ///
@@ -127,19 +128,10 @@ public:
     void cleanup();
 
 private:
-    ZoneTableSegment& segment_;
-    const LoadAction load_action_;
-    const dns::Name origin_;
-    const dns::RRClass rrclass_;
-    ZoneData* zone_data_;
-    enum State {
-        ZW_UNUSED,
-        ZW_LOADED,
-        ZW_INSTALLED,
-        ZW_CLEANED
-    };
-    State state_;
-    const bool catch_load_error_;
+    // We hide details as this class will be used by various applications
+    // and we use some internal data structures in the implementation.
+    struct Impl;
+    Impl* impl_;
 };
 
 }

+ 224 - 43
src/lib/datasrc/tests/client_list_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013  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
@@ -32,7 +32,9 @@
 
 #include <gtest/gtest.h>
 
+#include <boost/format.hpp>
 #include <boost/shared_ptr.hpp>
+#include <boost/interprocess/file_mapping.hpp>
 
 #include <set>
 #include <fstream>
@@ -105,7 +107,18 @@ const char* ds_zones[][3] = {
 
 const size_t ds_count = (sizeof(ds_zones) / sizeof(*ds_zones));
 
-class ListTest : public ::testing::Test {
+class SegmentType {
+public:
+    virtual ~SegmentType() {}
+    virtual ConstElementPtr getCacheConfig(bool enabled, const Name& zone)
+        const = 0;
+    virtual void reset(ConfigurableClientList& list,
+                       const std::string& datasrc_name,
+                       ZoneTableSegment::MemorySegmentOpenMode mode,
+                       ConstElementPtr config_params) = 0;
+};
+
+class ListTest : public ::testing::TestWithParam<SegmentType*> {
 public:
     ListTest() :
         rrclass_(RRClass::IN()),
@@ -121,8 +134,7 @@ public:
             "   \"type\": \"test_type\","
             "   \"params\": [\"example.org\", \"example.com\", "
             "                \"noiter.org\", \"null.org\"]"
-            "}]")),
-        ztable_segment_(ZoneTableSegment::create(rrclass_, "local"))
+            "}]"))
     {
         for (size_t i(0); i < ds_count; ++ i) {
             shared_ptr<MockDataSourceClient>
@@ -135,6 +147,24 @@ public:
         }
     }
 
+    ~ListTest() {
+        ds_info_.clear();
+        for (size_t i(0); i < ds_count; ++ i) {
+            ds_[i].reset();
+        }
+        ds_.clear();
+
+        for (size_t i(0); i < ds_count; ++ i) {
+            boost::interprocess::file_mapping::remove(
+                getMappedFilename(i).c_str());
+        }
+    }
+
+    static std::string getMappedFilename(size_t index) {
+         return (boost::str(boost::format(TEST_DATA_BUILDDIR "/test%d.mapped")
+                            % index));
+    }
+
     // Install a "fake" cached zone using a temporary underlying data source
     // client.  If 'enabled' is set to false, emulate a disabled cache, in
     // which case there will be no data in memory.
@@ -152,11 +182,7 @@ public:
         // Build new cache config to load the specified zone, and replace
         // the data source info with the new config.
         ConstElementPtr cache_conf_elem =
-            Element::fromJSON("{\"type\": \"mock\","
-                              " \"cache-enable\": " +
-                              string(enabled ? "true," : "false,") +
-                              " \"cache-zones\": "
-                              "   [\"" + zone.toText() + "\"]}");
+            GetParam()->getCacheConfig(enabled, zone);
         boost::shared_ptr<internal::CacheConfig> cache_conf(
             new internal::CacheConfig("mock", mock_client, *cache_conf_elem,
                                       true));
@@ -167,6 +193,15 @@ public:
 
         // Load the data into the zone table.
         if (enabled) {
+            const ConstElementPtr config_ztable_segment(
+                Element::fromJSON("{\"mapped-file\": \"" +
+                                  getMappedFilename(index) +
+                                  "\"}"));
+
+            GetParam()->reset(*list_, dsrc_info.name_,
+                              memory::ZoneTableSegment::CREATE,
+                              config_ztable_segment);
+
             boost::scoped_ptr<memory::ZoneWriter> writer(
                 new memory::ZoneWriter(
                     *dsrc_info.ztable_segment_,
@@ -175,6 +210,10 @@ public:
             writer->load();
             writer->install();
             writer->cleanup(); // not absolutely necessary, but just in case
+
+            GetParam()->reset(*list_, dsrc_info.name_,
+                              memory::ZoneTableSegment::READ_WRITE,
+                              config_ztable_segment);
         }
 
         // On completion of load revert to the previous state of underlying
@@ -266,6 +305,8 @@ public:
     }
     ConfigurableClientList::CacheStatus doReload(
         const Name& origin, const string& datasrc_name = "");
+    void accessorIterate(const ConstZoneTableAccessorPtr& accessor,
+        int numZones, const string& zoneName);
 
     const RRClass rrclass_;
     shared_ptr<TestedList> list_;
@@ -273,11 +314,64 @@ public:
     vector<shared_ptr<MockDataSourceClient> > ds_;
     vector<ConfigurableClientList::DataSourceInfo> ds_info_;
     const ConstElementPtr config_elem_, config_elem_zones_;
-    shared_ptr<ZoneTableSegment> ztable_segment_;
 };
 
+class LocalSegmentType : public SegmentType {
+public:
+    virtual ConstElementPtr getCacheConfig(bool enabled, const Name& zone)
+        const
+    {
+        return (Element::fromJSON("{\"type\": \"mock\","
+                                  " \"cache-enable\": " +
+                                  string(enabled ? "true," : "false,") +
+                                  " \"cache-zones\": "
+                                  "   [\"" + zone.toText() + "\"]}"));
+    }
+    virtual void reset(ConfigurableClientList&, const std::string&,
+                       ZoneTableSegment::MemorySegmentOpenMode,
+                       ConstElementPtr) {
+        // We must not call reset on local ZoneTableSegments.
+    }
+};
+
+LocalSegmentType local_segment_type;
+
+INSTANTIATE_TEST_CASE_P(ListTestLocal, ListTest,
+                        ::testing::Values(static_cast<SegmentType*>(
+                                              &local_segment_type)));
+
+#ifdef USE_SHARED_MEMORY
+
+class MappedSegmentType : public SegmentType {
+public:
+    virtual ConstElementPtr getCacheConfig(bool enabled, const Name& zone)
+        const
+    {
+        return (Element::fromJSON("{\"type\": \"mock\","
+                                  " \"cache-enable\": " +
+                                  string(enabled ? "true," : "false,") +
+                                  " \"cache-type\": \"mapped\"," +
+                                  " \"cache-zones\": "
+                                  "   [\"" + zone.toText() + "\"]}"));
+    }
+    virtual void reset(ConfigurableClientList& list,
+                       const std::string& datasrc_name,
+                       ZoneTableSegment::MemorySegmentOpenMode mode,
+                       ConstElementPtr config_params) {
+        list.resetMemorySegment(datasrc_name, mode, config_params);
+    }
+};
+
+MappedSegmentType mapped_segment_type;
+
+INSTANTIATE_TEST_CASE_P(ListTestMapped, ListTest,
+                        ::testing::Values(static_cast<SegmentType*>(
+                                              &mapped_segment_type)));
+
+#endif
+
 // Test the test itself
-TEST_F(ListTest, selfTest) {
+TEST_P(ListTest, selfTest) {
     EXPECT_EQ(result::SUCCESS, ds_[0]->findZone(Name("example.org")).code);
     EXPECT_EQ(result::PARTIALMATCH,
               ds_[0]->findZone(Name("sub.example.org")).code);
@@ -291,14 +385,14 @@ TEST_F(ListTest, selfTest) {
 }
 
 // Test the list we create with empty configuration is, in fact, empty
-TEST_F(ListTest, emptyList) {
+TEST_P(ListTest, emptyList) {
     EXPECT_TRUE(list_->getDataSources().empty());
 }
 
 // Check the values returned by a find on an empty list. It should be
 // a negative answer (nothing found) no matter if we want an exact or inexact
 // match.
-TEST_F(ListTest, emptySearch) {
+TEST_P(ListTest, emptySearch) {
     // No matter what we try, we don't get an answer.
 
     // Note: we don't have operator<< for the result class, so we cannot use
@@ -315,7 +409,7 @@ TEST_F(ListTest, emptySearch) {
 
 // Put a single data source inside the list and check it can find an
 // exact match if there's one.
-TEST_F(ListTest, singleDSExactMatch) {
+TEST_P(ListTest, singleDSExactMatch) {
     list_->getDataSources().push_back(ds_info_[0]);
     // This zone is not there
     EXPECT_TRUE(negative_result_ == list_->find(Name("org."), true));
@@ -329,7 +423,7 @@ TEST_F(ListTest, singleDSExactMatch) {
 }
 
 // When asking for a partial match, we get all that the exact one, but more.
-TEST_F(ListTest, singleDSBestMatch) {
+TEST_P(ListTest, singleDSBestMatch) {
     list_->getDataSources().push_back(ds_info_[0]);
     // This zone is not there
     EXPECT_TRUE(negative_result_ == list_->find(Name("org.")));
@@ -349,7 +443,7 @@ const char* const test_names[] = {
     "With a duplicity"
 };
 
-TEST_F(ListTest, multiExactMatch) {
+TEST_P(ListTest, multiExactMatch) {
     // Run through all the multi-configurations
     for (size_t i(0); i < sizeof(test_names) / sizeof(*test_names); ++i) {
         SCOPED_TRACE(test_names[i]);
@@ -368,7 +462,7 @@ TEST_F(ListTest, multiExactMatch) {
     }
 }
 
-TEST_F(ListTest, multiBestMatch) {
+TEST_P(ListTest, multiBestMatch) {
     // Run through all the multi-configurations
     for (size_t i(0); i < 4; ++ i) {
         SCOPED_TRACE(test_names[i]);
@@ -389,7 +483,7 @@ TEST_F(ListTest, multiBestMatch) {
 }
 
 // Check the configuration is empty when the list is empty
-TEST_F(ListTest, configureEmpty) {
+TEST_P(ListTest, configureEmpty) {
     const ConstElementPtr elem(new ListElement);
     list_->configure(elem, true);
     EXPECT_TRUE(list_->getDataSources().empty());
@@ -398,7 +492,7 @@ TEST_F(ListTest, configureEmpty) {
 }
 
 // Check we can get multiple data sources and they are in the right order.
-TEST_F(ListTest, configureMulti) {
+TEST_P(ListTest, configureMulti) {
     const ConstElementPtr elem(Element::fromJSON("["
         "{"
         "   \"type\": \"type1\","
@@ -420,7 +514,7 @@ TEST_F(ListTest, configureMulti) {
 }
 
 // Check we can pass whatever we want to the params
-TEST_F(ListTest, configureParams) {
+TEST_P(ListTest, configureParams) {
     const char* params[] = {
         "true",
         "false",
@@ -445,7 +539,7 @@ TEST_F(ListTest, configureParams) {
     }
 }
 
-TEST_F(ListTest, status) {
+TEST_P(ListTest, status) {
     EXPECT_TRUE(list_->getStatus().empty());
     const ConstElementPtr elem(Element::fromJSON("["
         "{"
@@ -472,7 +566,7 @@ TEST_F(ListTest, status) {
     EXPECT_EQ("local", statuses[1].getSegmentType());
 }
 
-TEST_F(ListTest, wrongConfig) {
+TEST_P(ListTest, wrongConfig) {
     const char* configs[] = {
         // A lot of stuff missing from there
         "[{\"type\": \"test_type\", \"params\": 13}, {}]",
@@ -570,7 +664,7 @@ TEST_F(ListTest, wrongConfig) {
 }
 
 // The param thing defaults to null. Cache is not used yet.
-TEST_F(ListTest, defaults) {
+TEST_P(ListTest, defaults) {
     const ConstElementPtr elem(Element::fromJSON("["
         "{"
         "   \"type\": \"type1\""
@@ -581,7 +675,7 @@ TEST_F(ListTest, defaults) {
 }
 
 // Check we can call the configure multiple times, to change the configuration
-TEST_F(ListTest, reconfigure) {
+TEST_P(ListTest, reconfigure) {
     const ConstElementPtr empty(new ListElement);
     list_->configure(config_elem_, true);
     checkDS(0, "test_type", "{}", false);
@@ -592,7 +686,7 @@ TEST_F(ListTest, reconfigure) {
 }
 
 // Make sure the data source error exception from the factory is propagated
-TEST_F(ListTest, dataSrcError) {
+TEST_P(ListTest, dataSrcError) {
     const ConstElementPtr elem(Element::fromJSON("["
         "{"
         "   \"type\": \"error\""
@@ -604,7 +698,7 @@ TEST_F(ListTest, dataSrcError) {
 }
 
 // Check we can get the cache
-TEST_F(ListTest, configureCacheEmpty) {
+TEST_P(ListTest, configureCacheEmpty) {
     const ConstElementPtr elem(Element::fromJSON("["
         "{"
         "   \"type\": \"type1\","
@@ -626,7 +720,7 @@ TEST_F(ListTest, configureCacheEmpty) {
 }
 
 // But no cache if we disallow it globally
-TEST_F(ListTest, configureCacheDisabled) {
+TEST_P(ListTest, configureCacheDisabled) {
     const ConstElementPtr elem(Element::fromJSON("["
         "{"
         "   \"type\": \"type1\","
@@ -648,7 +742,7 @@ TEST_F(ListTest, configureCacheDisabled) {
 }
 
 // Put some zones into the cache
-TEST_F(ListTest, cacheZones) {
+TEST_P(ListTest, cacheZones) {
     const ConstElementPtr elem(Element::fromJSON("["
         "{"
         "   \"type\": \"type1\","
@@ -682,7 +776,7 @@ TEST_F(ListTest, cacheZones) {
 
 // Check the caching handles misbehaviour from the data source and
 // misconfiguration gracefully
-TEST_F(ListTest, badCache) {
+TEST_P(ListTest, badCache) {
     list_->configure(config_elem_, true);
     checkDS(0, "test_type", "{}", false);
     // First, the zone is not in the data source. configure() should still
@@ -731,7 +825,7 @@ TEST_F(ListTest, badCache) {
 }
 
 // This test relies on the property of mapped type of cache.
-TEST_F(ListTest,
+TEST_P(ListTest,
 #ifdef USE_SHARED_MEMORY
        cacheInNonWritableSegment
 #else
@@ -758,7 +852,7 @@ TEST_F(ListTest,
               doReload(Name("example.org")));
 }
 
-TEST_F(ListTest, masterFiles) {
+TEST_P(ListTest, masterFiles) {
     const ConstElementPtr elem(Element::fromJSON("["
         "{"
         "   \"type\": \"MasterFiles\","
@@ -783,7 +877,7 @@ TEST_F(ListTest, masterFiles) {
 }
 
 // Test the names are set correctly and collission is detected.
-TEST_F(ListTest, names) {
+TEST_P(ListTest, names) {
     // Explicit name
     const ConstElementPtr elem1(Element::fromJSON("["
         "{"
@@ -830,7 +924,7 @@ TEST_F(ListTest, names) {
                  ConfigurableClientList::ConfigurationError);
 }
 
-TEST_F(ListTest, BadMasterFile) {
+TEST_P(ListTest, BadMasterFile) {
     // Configuration should succeed, and the good zones in the list
     // below should be loaded.  Bad zones won't be "loaded" in its usual sense,
     // but are still recognized with conceptual "empty" data.
@@ -906,7 +1000,7 @@ ListTest::doReload(const Name& origin, const string& datasrc_name) {
 }
 
 // Test we can reload a zone
-TEST_F(ListTest, reloadSuccess) {
+TEST_P(ListTest, reloadSuccess) {
     list_->configure(config_elem_zones_, true);
     const Name name("example.org");
     prepareCache(0, name);
@@ -926,7 +1020,7 @@ TEST_F(ListTest, reloadSuccess) {
 }
 
 // The cache is not enabled. The load should be rejected.
-TEST_F(ListTest, reloadNotAllowed) {
+TEST_P(ListTest, reloadNotAllowed) {
     list_->configure(config_elem_zones_, false);
     const Name name("example.org");
     // We put the cache in even when not enabled. This won't confuse the thing.
@@ -946,7 +1040,7 @@ TEST_F(ListTest, reloadNotAllowed) {
 }
 
 // Similar to the previous case, but the cache is disabled in config.
-TEST_F(ListTest, reloadNotEnabled) {
+TEST_P(ListTest, reloadNotEnabled) {
     list_->configure(config_elem_zones_, true);
     const Name name("example.org");
     // We put the cache, actually disabling it.
@@ -957,7 +1051,7 @@ TEST_F(ListTest, reloadNotEnabled) {
 }
 
 // Test several cases when the zone does not exist
-TEST_F(ListTest, reloadNoSuchZone) {
+TEST_P(ListTest, reloadNoSuchZone) {
     list_->configure(config_elem_zones_, true);
     const Name name("example.org");
     // We put the cache in even when not enabled. This won't confuse the
@@ -988,7 +1082,7 @@ TEST_F(ListTest, reloadNoSuchZone) {
 
 // Check we gracefully reject reloading (i.e. no exception) when a zone
 // disappeared in the underlying data source when we want to reload it
-TEST_F(ListTest, reloadZoneGone) {
+TEST_P(ListTest, reloadZoneGone) {
     list_->configure(config_elem_zones_, true);
     const Name name("example.org");
     // We put in a cache for non-existent zone. This emulates being loaded
@@ -1009,7 +1103,7 @@ TEST_F(ListTest, reloadZoneGone) {
               list_->find(name).finder_->find(name, RRType::SOA())->code);
 }
 
-TEST_F(ListTest, reloadNewZone) {
+TEST_P(ListTest, reloadNewZone) {
     // Test the case where a zone to be cached originally doesn't exist
     // in the underlying data source and is added later.  reload() will
     // succeed once it's available in the data source.
@@ -1036,7 +1130,7 @@ TEST_F(ListTest, reloadNewZone) {
 }
 
 // The underlying data source throws. Check we don't modify the state.
-TEST_F(ListTest, reloadZoneThrow) {
+TEST_P(ListTest, reloadZoneThrow) {
     list_->configure(config_elem_zones_, true);
     const Name name("noiter.org");
     prepareCache(0, name);
@@ -1050,7 +1144,7 @@ TEST_F(ListTest, reloadZoneThrow) {
               list_->find(name).finder_->find(name, RRType::SOA())->code);
 }
 
-TEST_F(ListTest, reloadNullIterator) {
+TEST_P(ListTest, reloadNullIterator) {
     list_->configure(config_elem_zones_, true);
     const Name name("null.org");
     prepareCache(0, name);
@@ -1065,7 +1159,7 @@ TEST_F(ListTest, reloadNullIterator) {
 }
 
 // Test we can reload the master files too (special-cased)
-TEST_F(ListTest, reloadMasterFile) {
+TEST_P(ListTest, reloadMasterFile) {
     const char* const install_cmd = INSTALL_PROG " -c " TEST_DATA_DIR
         "/root.zone " TEST_DATA_BUILDDIR "/root.zone.copied";
     if (system(install_cmd) != 0) {
@@ -1100,7 +1194,7 @@ TEST_F(ListTest, reloadMasterFile) {
                                                    RRType::TXT())->code);
 }
 
-TEST_F(ListTest, reloadByDataSourceName) {
+TEST_P(ListTest, reloadByDataSourceName) {
     // We use three data sources (and their clients).  2nd and 3rd have
     // the same name of the zones.
     const ConstElementPtr config_elem = Element::fromJSON(
@@ -1147,6 +1241,93 @@ TEST_F(ListTest, reloadByDataSourceName) {
               doReload(Name("example.org"), "test_type4"));
 }
 
+// This takes the accessor provided by getZoneTableAccessor(), iterates
+// through the table, and verifies that the expected number of zones are
+// present, as well as the named zone.
+void
+ListTest::accessorIterate(const ConstZoneTableAccessorPtr& accessor,
+                          int numZones, const string& zoneName="")
+{
+    // Confirm basic iterator behavior.
+    ASSERT_TRUE(accessor);
+    ZoneTableAccessor::IteratorPtr it = accessor->getIterator();
+    ASSERT_TRUE(it);
+    // Iterator does not guarantee ordering, so we look for the target
+    // name anywhere in the table.
+    bool found = false;
+    int i;
+    for (i = 0; !it->isLast(); ++i, it->next()) {
+	if (Name(zoneName) == it->getCurrent().origin) {
+	    found = true;
+	}
+    }
+    EXPECT_EQ(i, numZones);
+    if (numZones > 0) {
+        EXPECT_TRUE(found);
+    }
+}
+
+TEST_F(ListTest, zoneTableAccessor) {
+    // empty configuration
+    const ConstElementPtr elem(new ListElement);
+    list_->configure(elem, true);
+    // null pointer treated as false
+    EXPECT_FALSE(list_->getZoneTableAccessor("", true));
+
+    // empty list; expect it to return an empty list
+    list_->configure(config_elem_, true);
+    ConstZoneTableAccessorPtr z(list_->getZoneTableAccessor("", true));
+    accessorIterate(z, 0);
+
+    const ConstElementPtr elem2(Element::fromJSON("["
+        "{"
+        "   \"type\": \"type1\","
+        "   \"cache-enable\": true,"
+        "   \"cache-zones\": [\"example.com\"],"
+        "   \"params\": [\"example.com\"]"
+        "},"
+        "{"
+        "   \"type\": \"type2\","
+        "   \"cache-enable\": false,"
+        "   \"params\": [\"example.org\"]"
+        "},"
+        "{"
+        "   \"type\": \"type3\","
+        "   \"cache-enable\": true,"
+        "   \"cache-zones\": [\"example.net\", \"example.info\"],"
+        "   \"params\": [\"example.net\", \"example.info\"]"
+        "}]"));
+
+    // allow_cache = false
+    // ask for a non-existent zone table, expect null
+    list_->configure(elem2, false);
+    EXPECT_FALSE(list_->getZoneTableAccessor("bogus", true));
+    // ask for any zone table, expect an empty list
+    z = list_->getZoneTableAccessor("", true);
+    accessorIterate(z, 0);
+
+    // allow_cache = true, use_cache = false
+    list_->configure(elem2, true);
+    EXPECT_THROW(list_->getZoneTableAccessor("", false), isc::NotImplemented);
+    EXPECT_THROW(list_->getZoneTableAccessor("type1", false),
+                 isc::NotImplemented);
+
+    // datasrc not found, returns NULL pointer
+    EXPECT_FALSE(list_->getZoneTableAccessor("bogus", true));
+
+    // return first datasrc
+    z = list_->getZoneTableAccessor("", true);
+    accessorIterate(z, 1, "example.com");
+
+    // datasrc has cache disabled, returns accessor to empty list
+    z = list_->getZoneTableAccessor("type2", true);
+    accessorIterate(z, 0);
+
+    // search by name
+    z = list_->getZoneTableAccessor("type3", true);
+    accessorIterate(z, 2, "example.net");
+}
+
 // Check the status holds data
 TEST(DataSourceStatus, status) {
     const DataSourceStatus status("Test", SEGMENT_INUSE, "local");

+ 8 - 32
src/lib/datasrc/tests/memory/zone_table_segment_mapped_unittest.cc

@@ -267,6 +267,14 @@ TEST_F(ZoneTableSegmentMappedTest, reset) {
     EXPECT_FALSE(ztable_segment_->isUsable());
     EXPECT_FALSE(ztable_segment_->isWritable());
 
+    // If a Python binding passes an invalid integer as the mode,
+    // reset() should reject it.
+    EXPECT_THROW({
+        ztable_segment_->reset
+            (static_cast<ZoneTableSegment::MemorySegmentOpenMode>(1234),
+             config_params_);
+    }, isc::InvalidParameter);
+
     // READ_WRITE mode must create the mapped file if it doesn't exist
     // (and must not result in an exception).
     ztable_segment_->reset(ZoneTableSegment::READ_WRITE, config_params_);
@@ -555,36 +563,4 @@ TEST_F(ZoneTableSegmentMappedTest, resetCreateOverCorruptedFile) {
     EXPECT_FALSE(verifyData(ztable_segment_->getMemorySegment()));
 }
 
-TEST_F(ZoneTableSegmentMappedTest, resetHeaderUninitialized) {
-    // This should throw as we haven't called reset() yet.
-    EXPECT_THROW(ztable_segment_->resetHeader(), isc::InvalidOperation);
-}
-
-TEST_F(ZoneTableSegmentMappedTest, resetHeader) {
-    // First, open an underlying mapped file in read+write mode (doesn't
-    // exist yet)
-    ztable_segment_->reset(ZoneTableSegment::READ_WRITE, config_params_);
-
-    // Check if a valid ZoneTable is found.
-    {
-        const ZoneTableHeader& header = ztable_segment_->getHeader();
-        const ZoneTable* table = header.getTable();
-        EXPECT_EQ(0, table->getZoneCount());
-    }
-
-    // Grow the segment by allocating something large.
-    EXPECT_THROW(ztable_segment_->getMemorySegment().allocate(1<<16),
-                 MemorySegmentGrown);
-
-    // Reset the header address. This should not throw now.
-    EXPECT_NO_THROW(ztable_segment_->resetHeader());
-
-    // Check if a valid ZoneTable is found.
-    {
-        const ZoneTableHeader& header = ztable_segment_->getHeader();
-        const ZoneTable* table = header.getTable();
-        EXPECT_EQ(0, table->getZoneCount());
-    }
-}
-
 } // anonymous namespace

+ 0 - 5
src/lib/datasrc/tests/memory/zone_table_segment_mock.h

@@ -56,11 +56,6 @@ public:
         isc_throw(isc::NotImplemented, "clear() is not implemented");
     }
 
-    virtual void resetHeader() {
-        // This method does not have to do anything in this
-        // implementation.
-    }
-
     virtual ZoneTableHeader& getHeader() {
         return (header_);
     }

+ 0 - 13
src/lib/datasrc/tests/memory/zone_table_segment_unittest.cc

@@ -88,19 +88,6 @@ TEST_F(ZoneTableSegmentTest, getHeader) {
     // const version.
     testGetHeader<const ZoneTableSegment, const ZoneTableHeader,
                   const ZoneTable>(ztable_segment_);
-
-    // This is a nop for local segments.
-    ztable_segment_->resetHeader();
-
-    // The following still behave as before after resetHeader().
-
-    // non-const version.
-    testGetHeader<ZoneTableSegment, ZoneTableHeader, ZoneTable>
-        (ztable_segment_);
-
-    // const version.
-    testGetHeader<const ZoneTableSegment, const ZoneTableHeader,
-                  const ZoneTable>(ztable_segment_);
 }
 
 TEST_F(ZoneTableSegmentTest, getMemorySegment) {

+ 2 - 28
src/lib/datasrc/tests/memory/zone_writer_unittest.cc

@@ -42,25 +42,10 @@ namespace {
 
 class TestException {};
 
-class ZoneTableSegmentHelper : public ZoneTableSegmentMock {
-public:
-    ZoneTableSegmentHelper(const isc::dns::RRClass& rrclass,
-                           isc::util::MemorySegment& mem_sgmt) :
-        ZoneTableSegmentMock(rrclass, mem_sgmt),
-        reset_header_called_(false)
-    {}
-
-    virtual void resetHeader() {
-        reset_header_called_ = true;
-    }
-
-    bool reset_header_called_;
-};
-
 class ZoneWriterTest : public ::testing::Test {
 protected:
     ZoneWriterTest() :
-        segment_(new ZoneTableSegmentHelper(RRClass::IN(), mem_sgmt_)),
+        segment_(new ZoneTableSegmentMock(RRClass::IN(), mem_sgmt_)),
         writer_(new
             ZoneWriter(*segment_,
                        bind(&ZoneWriterTest::loadAction, this, _1),
@@ -76,7 +61,7 @@ protected:
         writer_.reset();
     }
     MemorySegmentMock mem_sgmt_;
-    scoped_ptr<ZoneTableSegmentHelper> segment_;
+    scoped_ptr<ZoneTableSegmentMock> segment_;
     scoped_ptr<ZoneWriter> writer_;
     bool load_called_;
     bool load_throw_;
@@ -148,18 +133,14 @@ TEST_F(ZoneWriterTest, constructForReadOnlySegment) {
 TEST_F(ZoneWriterTest, correctCall) {
     // Nothing called before we call it
     EXPECT_FALSE(load_called_);
-    EXPECT_FALSE(segment_->reset_header_called_);
 
     // Just the load gets called now
     EXPECT_NO_THROW(writer_->load());
     EXPECT_TRUE(load_called_);
-    EXPECT_TRUE(segment_->reset_header_called_);
     load_called_ = false;
-    segment_->reset_header_called_ = false;
 
     EXPECT_NO_THROW(writer_->install());
     EXPECT_FALSE(load_called_);
-    EXPECT_TRUE(segment_->reset_header_called_);
 
     // We don't check explicitly how this works, but call it to free memory. If
     // everything is freed should be checked inside the TearDown.
@@ -170,19 +151,15 @@ TEST_F(ZoneWriterTest, loadTwice) {
     // Load it the first time
     EXPECT_NO_THROW(writer_->load());
     EXPECT_TRUE(load_called_);
-    EXPECT_TRUE(segment_->reset_header_called_);
     load_called_ = false;
-    segment_->reset_header_called_ = false;
 
     // The second time, it should not be possible
     EXPECT_THROW(writer_->load(), isc::InvalidOperation);
     EXPECT_FALSE(load_called_);
-    EXPECT_FALSE(segment_->reset_header_called_);
 
     // The object should not be damaged, try installing and clearing now
     EXPECT_NO_THROW(writer_->install());
     EXPECT_FALSE(load_called_);
-    EXPECT_TRUE(segment_->reset_header_called_);
 
     // We don't check explicitly how this works, but call it to free memory. If
     // everything is freed should be checked inside the TearDown.
@@ -197,18 +174,15 @@ TEST_F(ZoneWriterTest, loadLater) {
     EXPECT_NO_THROW(writer_->install());
     // Reset so we see nothing is called now
     load_called_ = false;
-    segment_->reset_header_called_ = false;
 
     EXPECT_THROW(writer_->load(), isc::InvalidOperation);
     EXPECT_FALSE(load_called_);
-    EXPECT_FALSE(segment_->reset_header_called_);
 
     // Cleanup and try loading again. Still shouldn't work.
     EXPECT_NO_THROW(writer_->cleanup());
 
     EXPECT_THROW(writer_->load(), isc::InvalidOperation);
     EXPECT_FALSE(load_called_);
-    EXPECT_FALSE(segment_->reset_header_called_);
 }
 
 // Try calling install at various bad times

+ 8 - 3
src/lib/dhcp/tests/iface_mgr_unittest.cc

@@ -79,13 +79,18 @@ public:
     }
 
     /// Pretends to open socket. Only records a call to this function.
+    /// This function returns fake socket descriptor (always the same).
+    /// Note that the returned value has been selected to be unique
+    /// (because real values are rather less than 255). Values greater
+    /// than 255 are not recommended because they cause warnings to be
+    /// reported by Valgrind when invoking close() on them.
     virtual int openSocket(const Iface&,
                            const isc::asiolink::IOAddress&,
                            const uint16_t,
                            const bool,
                            const bool) {
         open_socket_called_ = true;
-        return (1024);
+        return (255);
     }
 
     /// Does nothing
@@ -917,8 +922,8 @@ TEST_F(IfaceMgrTest, setPacketFilter) {
 
     // Check that openSocket function was called.
     EXPECT_TRUE(custom_packet_filter->open_socket_called_);
-    // This function always returns fake socket descriptor equal to 1024.
-    EXPECT_EQ(1024, socket1);
+    // This function always returns fake socket descriptor equal to 255.
+    EXPECT_EQ(255, socket1);
 
     // Replacing current packet filter object while there are IPv4
     // sockets open is not allowed!

+ 16 - 2
src/lib/dhcp/tests/pkt_filter_inet_unittest.cc

@@ -37,15 +37,29 @@ const size_t RECV_BUF_SIZE = 2048;
 /// its index.
 class PktFilterInetTest : public ::testing::Test {
 public:
-    PktFilterInetTest() {
+
+    /// @brief Constructor
+    ///
+    /// This constructor initializes socket_ member to a negative value.
+    /// Explcit initialization is performed here because some of the
+    /// tests do not initialize this value. In such cases, destructor
+    /// could invoke close() on uninitialized socket descriptor which
+    /// would result in errors being reported by Valgrind.
+    PktFilterInetTest()
+        : socket_(-1) {
         // Initialize ifname_ and ifindex_.
         loInit();
     }
 
+    /// @brief Destructor
+    ///
+    /// Closes open socket (if any).
     ~PktFilterInetTest() {
         // Cleanup after each test. This guarantees
         // that the socket does not hang after a test.
-        close(socket_);
+        if (socket_ >= 0) {
+            close(socket_);
+        }
     }
 
     /// @brief Detect loopback interface.

+ 16 - 2
src/lib/dhcp/tests/pkt_filter_lpf_unittest.cc

@@ -41,15 +41,29 @@ const size_t RECV_BUF_SIZE = 2048;
 /// its index.
 class PktFilterLPFTest : public ::testing::Test {
 public:
-    PktFilterLPFTest() {
+
+    /// @brief Constructor
+    ///
+    /// This constructor initializes socket_ member to a negative value.
+    /// Explcit initialization is performed here because some of the
+    /// tests do not initialize this value. In such cases, destructor
+    /// could invoke close() on uninitialized socket descriptor which
+    /// would result in errors being reported by Valgrind.
+    PktFilterLPFTest()
+        : socket_(-1) {
         // Initialize ifname_ and ifindex_.
         loInit();
     }
 
+    /// @brief Destructor
+    ///
+    /// Closes open socket (if any).
     ~PktFilterLPFTest() {
         // Cleanup after each test. This guarantees
         // that the socket does not hang after a test.
-        close(socket_);
+        if (socket_ >= 0) {
+            close(socket_);
+        }
     }
 
     /// @brief Detect loopback interface.

+ 4 - 2
src/lib/dhcp/tests/protocol_util_unittest.cc

@@ -220,8 +220,10 @@ TEST(ProtocolUtilTest, writeEthernetHeader) {
     HWAddrPtr local_hw_addr(new HWAddr(src_hw_addr, 6, 1));
     ASSERT_NO_THROW(pkt->setLocalHWAddr(local_hw_addr));
 
-    // Set invalid length (7) of the hw address.
-    HWAddrPtr remote_hw_addr(new HWAddr(&std::vector<uint8_t>(1, 7)[0], 7, 1));
+    // Set invalid length (7) of the hw address. Fill it with
+    // values of 1.
+    std::vector<uint8_t> invalid_length_addr(7, 1);
+    HWAddrPtr remote_hw_addr(new HWAddr(invalid_length_addr, 1));
     ASSERT_NO_THROW(pkt->setRemoteHWAddr(remote_hw_addr));
     // HW address is too long, so it should fail.
     EXPECT_THROW(writeEthernetHeader(pkt, buf), BadValue);

+ 1 - 0
src/lib/log/interprocess/tests/.gitignore

@@ -0,0 +1 @@
+/run_unittests

+ 19 - 4
src/lib/log/logger_manager_impl.cc

@@ -35,7 +35,10 @@
 #include <log/logger_specification.h>
 #include <log/buffer_appender_impl.h>
 
+#include <boost/lexical_cast.hpp>
+
 using namespace std;
+using boost::lexical_cast;
 
 namespace isc {
 namespace log {
@@ -121,21 +124,33 @@ LoggerManagerImpl::createConsoleAppender(log4cplus::Logger& logger,
 
 // File appender.  Depending on whether a maximum size is given, either
 // a standard file appender or a rolling file appender will be created.
+// In the case of the latter, we set "UseLockFile" to true so that
+// log4cplus internally avoids race in rolling over the files by multiple
+// processes.  This feature isn't supported in log4cplus 1.0.x, but setting
+// the property unconditionally is okay as unknown properties are simply
+// ignored.
 void
 LoggerManagerImpl::createFileAppender(log4cplus::Logger& logger,
-                                         const OutputOption& opt)
+                                      const OutputOption& opt)
 {
     // Append to existing file
-    std::ios::openmode mode = std::ios::app;
+    const std::ios::openmode mode = std::ios::app;
 
     log4cplus::SharedAppenderPtr fileapp;
     if (opt.maxsize == 0) {
         fileapp = log4cplus::SharedAppenderPtr(new log4cplus::FileAppender(
             opt.filename, mode, opt.flush));
     } else {
+        log4cplus::helpers::Properties properties;
+        properties.setProperty("File", opt.filename);
+        properties.setProperty("MaxFileSize",
+                               lexical_cast<string>(opt.maxsize));
+        properties.setProperty("MaxBackupIndex",
+                               lexical_cast<string>(opt.maxver));
+        properties.setProperty("ImmediateFlush", opt.flush ? "true" : "false");
+        properties.setProperty("UseLockFile", "true");
         fileapp = log4cplus::SharedAppenderPtr(
-            new log4cplus::RollingFileAppender(opt.filename, opt.maxsize,
-                                               opt.maxver, opt.flush));
+            new log4cplus::RollingFileAppender(properties));
     }
 
     // use the same console layout for the files.

+ 1 - 1
src/lib/log/tests/Makefile.am

@@ -10,7 +10,7 @@ if USE_STATIC_LINK
 AM_LDFLAGS += -static
 endif
 
-CLEANFILES = *.gcno *.gcda
+CLEANFILES = *.gcno *.gcda *.lock
 
 EXTRA_DIST = log_test_messages.mes
 BUILT_SOURCES = log_test_messages.h log_test_messages.cc

+ 5 - 1
src/lib/log/tests/logger_manager_unittest.cc

@@ -77,7 +77,11 @@ public:
     // Destructor, remove the file.  This is only a test, so ignore failures
     ~SpecificationForFileLogger() {
         if (! name_.empty()) {
-            (void) unlink(name_.c_str());
+            static_cast<void>(unlink(name_.c_str()));
+
+            // Depending on the log4cplus version, a lock file may also be
+            // created.
+            static_cast<void>(unlink((name_ + ".lock").c_str()));
         }
     }
 

+ 72 - 0
src/lib/python/isc/datasrc/configurableclientlist_python.cc

@@ -27,6 +27,7 @@
 
 #include <dns/python/rrclass_python.h>
 #include <dns/python/name_python.h>
+#include <dns/python/pydnspp_common.h>
 
 #include <datasrc/client_list.h>
 
@@ -38,7 +39,9 @@
 using namespace std;
 using namespace isc::util::python;
 using namespace isc::datasrc;
+using namespace isc::datasrc::memory;
 using namespace isc::datasrc::python;
+using namespace isc::dns::python;
 
 //
 // ConfigurableClientList
@@ -116,6 +119,39 @@ ConfigurableClientList_configure(PyObject* po_self, PyObject* args) {
 }
 
 PyObject*
+ConfigurableClientList_resetMemorySegment(PyObject* po_self, PyObject* args) {
+    s_ConfigurableClientList* self =
+        static_cast<s_ConfigurableClientList*>(po_self);
+    try {
+        const char* datasrc_name_p;
+        int mode_int;
+        const char* config_p;
+        if (PyArg_ParseTuple(args, "sis", &datasrc_name_p, &mode_int,
+                             &config_p)) {
+            const std::string datasrc_name(datasrc_name_p);
+            const isc::data::ConstElementPtr
+                config(isc::data::Element::fromJSON(std::string(config_p)));
+            ZoneTableSegment::MemorySegmentOpenMode mode =
+                static_cast<ZoneTableSegment::MemorySegmentOpenMode>
+                    (mode_int);
+            self->cppobj->resetMemorySegment(datasrc_name, mode, config);
+            Py_RETURN_NONE;
+        }
+    } catch (const isc::data::JSONError& jse) {
+        const string ex_what(std::string("JSON parse error in memory segment"
+                               " configuration: ") + jse.what());
+        PyErr_SetString(getDataSourceException("Error"), ex_what.c_str());
+    } catch (const std::exception& exc) {
+        PyErr_SetString(getDataSourceException("Error"), exc.what());
+    } 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);
@@ -191,6 +227,19 @@ configuration preserved.\n\
 Parameters:\n\
   configuration     The configuration, as a JSON encoded string.\
   allow_cache       If caching is allowed." },
+    { "reset_memory_segment", ConfigurableClientList_resetMemorySegment,
+      METH_VARARGS,
+        "reset_memory_segment(datasrc_name, mode, config_params) -> None\n\
+\n\
+Wrapper around C++ ConfigurableClientList::resetMemorySegment\n\
+\n\
+This resets the zone table segment for a datasource with a new\n\
+memory segment.\n\
+\n\
+Parameters:\n\
+  datasrc_name      The name of the data source whose segment to reset.\
+  mode              The open mode for the new memory segment.\
+  config_params     The configuration for the new memory segment, as a JSON encoded string." },
     { "find", ConfigurableClientList_find, METH_VARARGS,
 "find(zone, want_exact_match=False, want_finder=True) -> datasrc_client,\
 zone_finder, exact_match\n\
@@ -300,6 +349,29 @@ initModulePart_ConfigurableClientList(PyObject* mod) {
     }
     Py_INCREF(&configurableclientlist_type);
 
+    // FIXME: These should eventually be moved to the ZoneTableSegment
+    // class when we add Python bindings for the memory data source
+    // specific bits. But for now, we add these enums here to support
+    // reloading a zone table segment.
+    try {
+        installClassVariable(configurableclientlist_type, "CREATE",
+                             Py_BuildValue("I", ZoneTableSegment::CREATE));
+        installClassVariable(configurableclientlist_type, "READ_WRITE",
+                             Py_BuildValue("I", ZoneTableSegment::READ_WRITE));
+        installClassVariable(configurableclientlist_type, "READ_ONLY",
+                             Py_BuildValue("I", ZoneTableSegment::READ_ONLY));
+    } catch (const std::exception& ex) {
+        const std::string ex_what =
+            "Unexpected failure in ConfigurableClientList initialization: " +
+            std::string(ex.what());
+        PyErr_SetString(po_IscException, ex_what.c_str());
+        return (false);
+    } catch (...) {
+        PyErr_SetString(PyExc_SystemError,
+            "Unexpected failure in ConfigurableClientList initialization");
+        return (false);
+    }
+
     return (true);
 }
 

+ 41 - 14
src/lib/util/memory_segment.h

@@ -157,7 +157,7 @@ public:
     /// \return Returns <code>true</code> if all allocated memory (including
     /// names associated by memory addresses by \c setNamedAddress()) was
     /// deallocated, <code>false</code> otherwise.
-    virtual bool allMemoryDeallocated() = 0;
+    virtual bool allMemoryDeallocated() const = 0;
 
     /// \brief Associate specified address in the segment with a given name.
     ///
@@ -171,6 +171,10 @@ public:
     /// corresponding address by that name (in such cases the real address
     /// may be different between these two processes).
     ///
+    /// Some names are reserved for internal use by this class. If such
+    /// a name is passed to this method, an \c isc::InvalidParameter
+    /// exception will be thrown. See \c validateName() method for details.
+    ///
     /// \c addr must be 0 (NULL) or an address that belongs to this segment.
     /// The latter case means it must be the return value of a previous call
     /// to \c allocate().  The actual implementation is encouraged to detect
@@ -214,7 +218,8 @@ public:
     ///
     /// \throw std::bad_alloc Allocation of a segment space for the given name
     /// failed.
-    /// \throw InvalidParameter name is NULL.
+    /// \throw InvalidParameter name is NULL, empty ("") or begins with
+    /// an underscore ('_').
     /// \throw MemorySegmentError Failure of implementation specific
     /// validation.
     ///
@@ -226,10 +231,7 @@ public:
         // This public method implements common validation.  The actual
         // work specific to the derived segment is delegated to the
         // corresponding protected method.
-        if (!name) {
-            isc_throw(InvalidParameter,
-                      "NULL name is given to setNamedAddress");
-        }
+        validateName(name);
         return (setNamedAddressImpl(name, addr));
     }
 
@@ -243,13 +245,18 @@ public:
     /// associated by a prior call to \c setNameAddress().  If no address
     /// associated with the given name is found, it returns NULL.
     ///
+    /// Some names are reserved for internal use by this class. If such
+    /// a name is passed to this method, an \c isc::InvalidParameter
+    /// exception will be thrown. See \c validateName() method for details.
+    ///
     /// This method should generally be considered exception free, but there
     /// can be a small chance it throws, depending on the internal
     /// implementation (e.g., if it converts the name to std::string), so the
     /// API doesn't guarantee that property.  In general, if this method
     /// throws it should be considered a fatal condition.
     ///
-    /// \throw InvalidParameter name is NULL.
+    /// \throw InvalidParameter name is NULL, empty ("") or begins with
+    /// an underscore ('_').
     ///
     /// \param name A C string of which the segment memory address is to be
     /// returned.  Must not be NULL.
@@ -260,10 +267,7 @@ public:
         // This public method implements common validation.  The actual
         // work specific to the derived segment is delegated to the
         // corresponding protected method.
-        if (!name) {
-            isc_throw(InvalidParameter,
-                      "NULL name is given to getNamedAddress");
-        }
+        validateName(name);
         return (getNamedAddressImpl(name));
     }
 
@@ -274,9 +278,14 @@ public:
     /// \c setNamedAddress().  If there is no association for the given name
     /// this method returns false; otherwise it returns true.
     ///
+    /// Some names are reserved for internal use by this class. If such
+    /// a name is passed to this method, an \c isc::InvalidParameter
+    /// exception will be thrown. See \c validateName() method for details.
+    ///
     /// See \c getNamedAddress() about exception consideration.
     ///
-    /// \throw InvalidParameter name is NULL.
+    /// \throw InvalidParameter name is NULL, empty ("") or begins with
+    /// an underscore ('_').
     /// \throw MemorySegmentError Failure of implementation specific
     /// validation.
     ///
@@ -286,11 +295,29 @@ public:
         // This public method implements common validation.  The actual
         // work specific to the derived segment is delegated to the
         // corresponding protected method.
+        validateName(name);
+        return (clearNamedAddressImpl(name));
+    }
+
+private:
+    /// \brief Validate the passed name.
+    ///
+    /// This method validates the passed name (for name/address pairs)
+    /// and throws \c InvalidParameter if the name fails
+    /// validation. Otherwise, it does nothing.
+    ///
+    /// \throw InvalidParameter name is NULL, empty ("") or begins with
+    /// an underscore ('_').
+    static void validateName(const char* name) {
         if (!name) {
+            isc_throw(InvalidParameter, "NULL is invalid for a name.");
+        } else if (*name == '\0') {
+            isc_throw(InvalidParameter, "Empty names are invalid.");
+        } else if (*name == '_') {
             isc_throw(InvalidParameter,
-                      "NULL name is given to clearNamedAddress");
+                      "Names beginning with '_' are reserved for "
+                      "internal use only.");
         }
-        return (clearNamedAddressImpl(name));
     }
 
 protected:

+ 1 - 1
src/lib/util/memory_segment_local.cc

@@ -47,7 +47,7 @@ MemorySegmentLocal::deallocate(void* ptr, size_t size) {
 }
 
 bool
-MemorySegmentLocal::allMemoryDeallocated() {
+MemorySegmentLocal::allMemoryDeallocated() const {
     return (allocated_size_ == 0 && named_addrs_.empty());
 }
 

+ 1 - 1
src/lib/util/memory_segment_local.h

@@ -64,7 +64,7 @@ public:
     ///
     /// \return Returns <code>true</code> if all allocated memory was
     /// deallocated, <code>false</code> otherwise.
-    virtual bool allMemoryDeallocated();
+    virtual bool allMemoryDeallocated() const;
 
     /// \brief Local segment version of getNamedAddress.
     ///

+ 16 - 7
src/lib/util/memory_segment_mapped.cc

@@ -137,7 +137,7 @@ struct MemorySegmentMapped::Impl {
         reserveMemory();
     }
 
-    void reserveMemory() {
+    void reserveMemory(bool no_grow = false) {
         if (!read_only_) {
             // Reserve a named address for use during
             // setNamedAddress(). Though this will almost always succeed
@@ -153,6 +153,7 @@ struct MemorySegmentMapped::Impl {
                 if (reserved_storage) {
                     break;
                 }
+                assert(!no_grow);
 
                 growSegment();
             }
@@ -324,12 +325,20 @@ MemorySegmentMapped::deallocate(void* ptr, size_t) {
 }
 
 bool
-MemorySegmentMapped::allMemoryDeallocated() {
-    impl_->freeReservedMemory();
-    const bool result = impl_->base_sgmt_->all_memory_deallocated();
-    impl_->reserveMemory();
-
-    return (result);
+MemorySegmentMapped::allMemoryDeallocated() const {
+    // This method is not technically const, but it reserves the
+    // const-ness property. In case of exceptions, we abort here. (See
+    // ticket #2850 for additional commentary.)
+    try {
+        impl_->freeReservedMemory();
+        const bool result = impl_->base_sgmt_->all_memory_deallocated();
+        // reserveMemory() should succeed now as the memory was already
+        // allocated, so we set no_grow to true.
+        impl_->reserveMemory(true);
+        return (result);
+    } catch (...) {
+        abort();
+    }
 }
 
 MemorySegment::NamedAddressResult

+ 1 - 1
src/lib/util/memory_segment_mapped.h

@@ -182,7 +182,7 @@ public:
     /// read-only mode; in that case MemorySegmentError will be thrown.
     virtual void deallocate(void* ptr, size_t size);
 
-    virtual bool allMemoryDeallocated();
+    virtual bool allMemoryDeallocated() const;
 
     /// \brief Mapped segment version of setNamedAddress.
     ///

+ 2 - 7
src/lib/util/random/random_number_generator.h

@@ -60,9 +60,7 @@ public:
     ///
     /// \param min The minimum number in the range
     /// \param max The maximum number in the range
-    /// \param seed A seed for the RNG. If 0 is passed, the current time
-    /// is used.
-    UniformRandomIntegerGenerator(int min, int max, unsigned int seed = 0):
+    UniformRandomIntegerGenerator(int min, int max):
         min_(std::min(min, max)), max_(std::max(min, max)),
         dist_(min_, max_), generator_(rng_, dist_)
     {
@@ -75,10 +73,7 @@ public:
         }
 
         // Init with the current time
-        if (seed == 0) {
-            seed = time(NULL);
-        }
-        rng_.seed(seed);
+        rng_.seed(time(NULL));
     }
 
     /// \brief Generate uniformly distributed integer

+ 12 - 0
src/lib/util/tests/memory_segment_common_unittest.cc

@@ -41,6 +41,18 @@ checkSegmentNamedAddress(MemorySegment& segment, bool out_of_segment_ok) {
 
     // NULL name isn't allowed.
     EXPECT_THROW(segment.setNamedAddress(NULL, ptr32), InvalidParameter);
+    EXPECT_THROW(segment.getNamedAddress(NULL), InvalidParameter);
+    EXPECT_THROW(segment.clearNamedAddress(NULL), InvalidParameter);
+
+    // Empty names are not allowed.
+    EXPECT_THROW(segment.setNamedAddress("", ptr32), InvalidParameter);
+    EXPECT_THROW(segment.getNamedAddress(""), InvalidParameter);
+    EXPECT_THROW(segment.clearNamedAddress(""), InvalidParameter);
+
+    // Names beginning with _ are not allowed.
+    EXPECT_THROW(segment.setNamedAddress("_foo", ptr32), InvalidParameter);
+    EXPECT_THROW(segment.getNamedAddress("_foo"), InvalidParameter);
+    EXPECT_THROW(segment.clearNamedAddress("_foo"), InvalidParameter);
 
     // we can now get it; the stored value should be intact.
     MemorySegment::NamedAddressResult result =

+ 0 - 18
src/lib/util/tests/random_number_generator_unittest.cc

@@ -20,10 +20,7 @@
 #include <boost/shared_ptr.hpp>
 
 #include <iostream>
-#include <climits>
 
-#include <sys/types.h>
-#include <unistd.h>
 
 namespace isc {
 namespace util {
@@ -87,21 +84,6 @@ TEST_F(UniformRandomIntegerGeneratorTest, IntegerRange) {
     ASSERT_EQ(it - numbers.begin(), max() - min() + 1);
 }
 
-TEST_F(UniformRandomIntegerGeneratorTest, withSeed) {
-    // Test that two generators with the same seed return the same
-    // sequence.
-    UniformRandomIntegerGenerator gen1(0, INT_MAX, getpid());
-    vector<int> numbers;
-    for (int i = 0; i < 1024; ++i) {
-        numbers.push_back(gen1());
-    }
-
-    UniformRandomIntegerGenerator gen2(0, INT_MAX, getpid());
-    for (int i = 0; i < 1024; ++i) {
-        EXPECT_EQ(numbers[i], gen2());
-    }
-}
-
 /// \brief Test Fixture Class for weighted random number generator
 class WeightedRandomIntegerGeneratorTest : public ::testing::Test {
 public:

+ 0 - 1
tests/lettuce/configurations/ixfr-out/testset1-config.db

@@ -2,7 +2,6 @@
     "Xfrin": {
         "zones": [
             {
-                "use_ixfr": true,
                 "class": "IN",
                 "name": "example.com.",
                 "master_addr": "178.18.82.80"

+ 1 - 2
tests/lettuce/configurations/xfrin/retransfer_slave_diffs.conf

@@ -18,8 +18,7 @@
         "zones": [ {
             "name": "example",
             "master_addr": "::1",
-            "master_port": 47807,
-            "use_ixfr": true
+            "master_port": 47807
         } ]
     },
     "data_sources": {

+ 2 - 1
tests/lettuce/configurations/xfrin/retransfer_slave_notify.conf.orig

@@ -28,7 +28,8 @@
         "zones": [ {
             "name": "example.org",
             "master_addr": "::1",
-            "master_port": 47807
+            "master_port": 47807,
+            "request_ixfr": "no"
         } ]
     },
     "Zonemgr": {

+ 2 - 1
tests/lettuce/configurations/xfrin/retransfer_slave_notify_v4.conf

@@ -28,7 +28,8 @@
         "zones": [ {
             "name": "example.org",
             "master_addr": "127.0.0.1",
-            "master_port": 47809
+            "master_port": 47809,
+            "request_ixfr": "no"
         } ]
     },
     "Zonemgr": {

+ 2 - 1
tests/lettuce/features/xfrin_bind10.feature

@@ -182,7 +182,8 @@ Feature: Xfrin
     example.    3600    IN      SOA     ns1.example. hostmaster.example. 94 3600 900 7200 300
     """
 
-    When I send bind10 the command Xfrin retransfer example. IN ::1 47807
+    # To invoke IXFR we need to use refresh command
+    When I send bind10 the command Xfrin refresh example. IN ::1 47807
     Then wait for new bind10 stderr message XFRIN_GOT_INCREMENTAL_RESP
     Then wait for new bind10 stderr message XFRIN_IXFR_TRANSFER_SUCCESS not XFRIN_XFR_PROCESS_FAILURE
     # This can't be 'wait for new'