MXS-2304 Clean up configuration serialization

The parameters are now written in the order they appear in the module
parameter definitions. Also enabled a previously disabled part in
server unit test.

server/core/config.cc

@@ -4869,24 +4869,28 @@ bool config_parse_disk_space_threshold(SERVER::DiskSpaceLimits* pDisk_space_thre
     return success;
 }
 
-void dump_param_list(int file,
+std::string generate_config_string(const std::string& instance_name, const MXS_CONFIG_PARAMETER& parameters,
-                     const MXS_CONFIG_PARAMETER* list,
+                                   const MXS_MODULE_PARAM* common_param_defs,
-                     const std::unordered_set<std::string>& ignored,
+                                   const MXS_MODULE_PARAM* module_param_defs)
-                     const MXS_MODULE_PARAM* common_params,
-                     const MXS_MODULE_PARAM* module_params)
 {
-    for (const auto& p : *list)
+    string output = "[" + instance_name + "]\n";;
+    // Common params and module params are null-terminated arrays. Loop over both and print parameter
+    // names and values.
+    for (auto param_set : {common_param_defs, module_param_defs})
     {
-        const string& name = p.first;
+        for (int i = 0; param_set[i].name; i++)
-        const string& value = p.second;
-        if (ignored.count(name) == 0 && !value.empty())
         {
-            if (dprintf(file, "%s=%s\n", name.c_str(), value.c_str()) == -1)
+            auto param_info = param_set + i;
+            string param_name = param_info->name;
+            if (parameters.contains(param_name))
             {
-                MXS_ERROR("Failed to serialize service value: %d, %s", errno, mxs_strerror(errno));
+                // Parameter value in the container can be an empty string and still be printed.
+                string param_value = parameters.get_string(param_name);
+                output += param_name + "=" + param_value + "\n";
             }
         }
     }
+    return output;
 }
 
 /**

server/core/filter.cc

@@ -514,18 +514,17 @@ static bool create_filter_config(const SFilterDef& filter, const char* filename)
     }
 
     Guard guard(filter->lock);
-
-    dprintf(file, "[%s]\n", filter->name.c_str());
-    dprintf(file, "%s=%s\n", CN_TYPE, CN_FILTER);
-    dprintf(file, "%s=%s\n", CN_MODULE, filter->module.c_str());
-
     const MXS_MODULE* mod = get_module(filter->module.c_str(), NULL);
     mxb_assert(mod);
 
-    MXS_MODULE_PARAM no_common_params = {};
+    string config_str = generate_config_string(filter->name, *filter->parameters,
-    dump_param_list(file, filter->parameters, {CN_TYPE, CN_MODULE}, &no_common_params, mod->parameters);
+                                               config_filter_params, mod->parameters);
+    if (dprintf(file, "%s", config_str.c_str()) == -1)
+    {
+        MXS_ERROR("Could not write serialized configuration to file '%s': %d, %s",
+                  filename, errno, mxs_strerror(errno));
+    }
     close(file);
-
     return true;
 }
 

server/core/internal/config.hh

@@ -233,12 +233,19 @@ bool get_suffixed_duration(const char* zValue,
                            std::chrono::milliseconds* pDuration,
                            mxs::config::DurationUnit* pUnit = nullptr);
 
-// Dump a parameter list into a file as `key=value` pairs
+/**
-void dump_param_list(int file,
+ * Generate configuration file contents out of module configuration parameters. Only parameters defined
-                     const MXS_CONFIG_PARAMETER* list,
+ * in the parameter definition arrays are printed. Printing is in the order the parameters are given in
-                     const std::unordered_set<std::string>& ignored,
+ * the definitions.
-                     const MXS_MODULE_PARAM* common_params,
+ *
-                     const MXS_MODULE_PARAM* module_params);
+ * @param instance_name The module instance name
+ * @param parameters Configuration parameter values
+ * @param common_param_defs Common module parameter definitions. These are printed first.
+ * @param module_param_defs Module-specific parameter definitions.
+ */
+std::string generate_config_string(const std::string& instance_name, const MXS_CONFIG_PARAMETER& parameters,
+                                   const MXS_MODULE_PARAM* common_param_defs,
+                                   const MXS_MODULE_PARAM* module_param_defs);
 
 /**
  * Check whether a parameter can be modified at runtime

server/core/monitor.cc

@@ -1325,7 +1325,6 @@ Monitor* monitor_server_in_use(const SERVER* server)
 static bool create_monitor_config(const Monitor* monitor, const char* filename)
 {
     int file = open(filename, O_EXCL | O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
-
     if (file == -1)
     {
         MXS_ERROR("Failed to open file '%s' when serializing monitor '%s': %d, %s",
@@ -1338,31 +1337,18 @@ static bool create_monitor_config(const Monitor* monitor, const char* filename)
 
     {
         Guard guard(monitor->m_lock);
-        dprintf(file, "[%s]\n", monitor->m_name);
-        dprintf(file, "%s=monitor\n", CN_TYPE);
-
-        if (!monitor->m_servers.empty())
-        {
-            dprintf(file, "%s=", CN_SERVERS);
-            for (MXS_MONITORED_SERVER* db : monitor->m_servers)
-            {
-                if (db != monitor->m_servers[0])
-                {
-                    dprintf(file, ",");
-                }
-                dprintf(file, "%s", db->server->name());
-            }
-            dprintf(file, "\n");
-        }
 
         const MXS_MODULE* mod = get_module(monitor->m_module.c_str(), NULL);
         mxb_assert(mod);
 
-        dump_param_list(file,
+        string config = generate_config_string(monitor->m_name, monitor->parameters,
-                        &monitor->parameters,
+                                               config_monitor_params, mod->parameters);
-                        {CN_TYPE, CN_SERVERS},
+
-                        config_monitor_params,
+        if (dprintf(file, "%s", config.c_str()) == -1)
-                        mod->parameters);
+        {
+            MXS_ERROR("Could not write serialized configuration to file '%s': %d, %s",
+                      filename, errno, mxs_strerror(errno));
+        }
     }
 
     close(file);

server/core/server.cc

@@ -880,22 +880,23 @@ bool Server::create_server_config(const Server* server, const char* filename)
     }
 
     // TODO: Check for return values on all of the dprintf calls
-    dprintf(file, "[%s]\n", server->name());
-    dprintf(file, "%s=server\n", CN_TYPE);
-
     const MXS_MODULE* mod = get_module(server->m_settings.protocol.c_str(), MODULE_PROTOCOL);
-    dump_param_list(file,
+    string config = generate_config_string(server->name(),
-                    ParamAdaptor(server->m_settings.all_parameters),
+                                           *ParamAdaptor(server->m_settings.all_parameters),
-                    {CN_TYPE},
+                                           config_server_params,
-                    config_server_params,
+                                           mod->parameters);
-                    mod->parameters);
 
     // Print custom parameters
     for (const auto& elem : server->m_settings.custom_parameters)
     {
-        dprintf(file, "%s=%s\n", elem.first.c_str(), elem.second.c_str());
+        config += elem.first + "=" + elem.second + "\n";
     }
 
+    if (dprintf(file, "%s", config.c_str()) == -1)
+    {
+        MXS_ERROR("Could not write serialized configuration to file '%s': %d, %s",
+                  filename, errno, mxs_strerror(errno));
+    }
     close(file);
     return true;
 }

server/core/service.cc

@@ -1417,12 +1417,23 @@ bool Service::dump_config(const char* filename) const
         return false;
     }
 
+    const MXS_MODULE* mod = get_module(router_name(), NULL);
+    mxb_assert(mod);
+
+    MXS_CONFIG_PARAMETER params_to_print = svc_config_param;
+    // The next text-mode parameter may not be up-to-date, print them manually. TODO: Fix
+    params_to_print.remove(CN_FILTERS);
+    params_to_print.remove(CN_SERVERS);
+
+    string config = generate_config_string(name(), params_to_print, config_service_params, mod->parameters);
+    if (dprintf(file, "%s", config.c_str()) == -1)
+    {
+        MXS_ERROR("Could not write serialized configuration to file '%s': %d, %s",
+                  filename, errno, mxs_strerror(errno));
+    }
     /**
      * TODO: Check for return values on all of the dprintf calls
      */
-    dprintf(file, "[%s]\n", name());
-    dprintf(file, "%s=service\n", CN_TYPE);
-
     if (!m_filters.empty())
     {
         dprintf(file, "%s=", CN_FILTERS);
@@ -1452,16 +1463,6 @@ bool Service::dump_config(const char* filename) const
         }
         dprintf(file, "\n");
     }
-
-    const MXS_MODULE* mod = get_module(router_name(), NULL);
-    mxb_assert(mod);
-
-    dump_param_list(file,
-                    &svc_config_param,
-                    {CN_TYPE, CN_FILTERS, CN_SERVERS},
-                    config_service_params,
-                    mod->parameters);
-
     close(file);
 
     return true;

server/core/test/test_server.cc

@@ -149,12 +149,9 @@ bool test_serialize()
     TEST(created->serialize(), "Failed to synchronize the copied server");
 
     /** Check that they serialize to identical files */
-    // TODO: Disabled for now. Enable once config parameters are kept in a well-defined order.
-    /*
     char cmd[1024];
     sprintf(cmd, "diff ./%s ./%s", config_name, old_config_name);
     TEST(system(cmd) == 0, "The files are not identical");
-     */
 
     return true;
 }