Merge branch 'develop' into binlog_server_wait_data

include/maxscale/config.h

@@ -32,6 +32,7 @@
 #include <limits.h>
 #include <sys/utsname.h>
 #include <openssl/sha.h>
+#include <maxscale/gw_ssl.h>
 
 MXS_BEGIN_DECLS
 
@@ -129,13 +130,91 @@ typedef struct
 } GATEWAY_CONF;
 
 
+/**
+ * @brief Creates an empty configuration context
+ *
+ * @param section Context name
+ * @return New context or NULL on memory allocation failure
+ */
+CONFIG_CONTEXT* config_context_create(const char *section);
+
+/**
+ * @brief Free a configuration context
+ *
+ * @param context The context to free
+ */
+void config_context_free(CONFIG_CONTEXT *context);
+
+/**
+ * @brief Get a configuration parameter
+ *
+ * @param params List of parameters
+ * @param name Name of parameter to get
+ * @return The parameter or NULL if the parameter was not found
+ */
+CONFIG_PARAMETER* config_get_param(CONFIG_PARAMETER* params, const char* name);
+
+/**
+ * @brief Add a parameter to a configuration context
+ *
+ * @param obj Context where the parameter should be added
+ * @param key Key to add
+ * @param value Value for the key
+ * @return True on success, false on memory allocation error
+ */
+bool config_add_param(CONFIG_CONTEXT* obj, const char* key, const char* value);
+
+/**
+ * @brief Append to an existing parameter
+ *
+ * @param obj Configuration context
+ * @param key Parameter name
+ * @param value Value to append to the parameter
+ * @return True on success, false on memory allocation error
+ */
+bool config_append_param(CONFIG_CONTEXT* obj, const char* key, const char* value);
+
+/**
+ * @brief Check if all SSL parameters are defined
+ *
+ * Helper function to check whether all of the required SSL parameters are defined
+ * in the configuration context. The checked parameters are 'ssl', 'ssl_key',
+ * 'ssl_cert' and 'ssl_ca_cert'. The 'ssl' parameter must also have a value of
+ * 'required'.
+ *
+ * @param obj Configuration context
+ * @return True if all required parameters are present
+ */
+bool config_have_required_ssl_params(CONFIG_CONTEXT *obj);
+
+/**
+ * @brief Helper function for checking SSL parameters
+ *
+ * @param key Parameter name
+ * @return True if the parameter is an SSL parameter
+ */
+bool config_is_ssl_parameter(const char *key);
+
+/**
+ * @brief Construct an SSL structure
+ *
+ * The SSL structure is used by both listeners and servers.
+ *
+ * TODO: Rename to something like @c config_construct_ssl
+ *
+ * @param obj Configuration context
+ * @param require_cert Whether certificates are required
+ * @param error_count Pointer to an int which is incremented for each error
+ * @return New SSL_LISTENER structure or NULL on error
+ */
+SSL_LISTENER *make_ssl_structure(CONFIG_CONTEXT *obj, bool require_cert, int *error_count);
+
 char*               config_clean_string_list(const char* str);
 CONFIG_PARAMETER*   config_clone_param(const CONFIG_PARAMETER* param);
 void                config_enable_feedback_task(void);
 void                config_disable_feedback_task(void);
 unsigned long       config_get_gateway_id(void);
 GATEWAY_CONF*       config_get_global_options();
-CONFIG_PARAMETER*   config_get_param(CONFIG_PARAMETER* params, const char* name);
 config_param_type_t config_get_paramtype(const CONFIG_PARAMETER* param);
 bool                config_get_valint(int* val,
                                       const CONFIG_PARAMETER* param,

include/maxscale/monitor.h

@@ -233,6 +233,22 @@ connect_result_t mon_connect_to_db(MONITOR* mon, MONITOR_SERVERS *database);
 void mon_log_connect_error(MONITOR_SERVERS* database, connect_result_t rval);
 void mon_log_state_change(MONITOR_SERVERS *ptr);
 
+/**
+ * @brief Serialize a monitor to a file
+ *
+ * This partially converts @c monitor into an INI format file. Only the servers
+ * of the monitor are serialized. This allows the monitor to keep monitoring
+ * the servers that were added at runtime even after a restart.
+ *
+ * NOTE: This does not persist the complete monitor configuration and requires
+ * that an existing monitor configuration is in the main configuration file.
+ * Changes to monitor parameters are not persisted.
+ *
+ * @param monitor Monitor to serialize
+ * @return False if the serialization of the monitor fails, true if it was successful
+ */
+bool monitor_serialize_servers(const MONITOR *monitor);
+
 /**
  * Check if a monitor uses @c servers
  * @param server Server that is queried

include/maxscale/server.h

@@ -245,6 +245,18 @@ extern bool server_create(const char *name, const char *address, const char *por
                           const char *protocol, const char *authenticator,
                           const char *options);
 
+/**
+ * @brief Serialize a server to a file
+ *
+ * This converts @c server into an INI format file. This allows created servers
+ * to be persisted to disk. This will replace any existing files with the same
+ * name.
+ *
+ * @param server Server to serialize
+ * @return False if the serialization of the server fails, true if it was successful
+ */
+bool server_serialize(const SERVER *server);
+
 /**
  * @brief Destroy a server
  *
@@ -280,7 +292,5 @@ extern void server_update_port(SERVER *,  unsigned short);
 extern RESULTSET *serverGetList();
 extern unsigned int server_map_status(char *str);
 extern bool server_set_version_string(SERVER* server, const char* string);
-extern bool server_is_ssl_parameter(const char *key);
-extern void server_update_ssl(SERVER *server, const char *key, const char *value);
 
 MXS_END_DECLS

include/maxscale/service.h

@@ -200,7 +200,7 @@ extern int serviceHasProtocol(SERVICE *service, const char *protocol,
                               const char* address, unsigned short port);
 extern void serviceAddBackend(SERVICE *, SERVER *);
 extern void serviceRemoveBackend(SERVICE *, const SERVER *);
-extern int serviceHasBackend(SERVICE *, SERVER *);
+extern bool serviceHasBackend(SERVICE *, SERVER *);
 extern void serviceAddRouterOption(SERVICE *, char *);
 extern void serviceClearRouterOptions(SERVICE *);
 extern int serviceStart(SERVICE *);
@@ -264,4 +264,20 @@ static inline uint64_t service_get_capabilities(const SERVICE *service)
  */
 bool service_server_in_use(const SERVER *server);
 
+/**
+ * @brief Serialize a service to a file
+ *
+ * This partially converts @c service into an INI format file. Only the servers
+ * of the service are serialized. This allows the service to keep using the servers
+ * added at runtime even after a restart.
+ *
+ * NOTE: This does not persist the complete service configuration and requires
+ * that an existing service configuration is in the main configuration file.
+ * Changes to service parameters are not persisted.
+ *
+ * @param service Service to serialize
+ * @return False if the serialization of the service fails, true if it was successful
+ */
+bool service_serialize_servers(const SERVICE *service);
+
 MXS_END_DECLS

server/core/config.c

@@ -80,7 +80,6 @@ static void duplicate_context_finish(DUPLICATE_CONTEXT* context);
 extern int setipaddress(struct in_addr *, char *);
 static bool process_config_context(CONFIG_CONTEXT *);
 static bool process_config_update(CONFIG_CONTEXT *);
-static void free_config_context(CONFIG_CONTEXT      *);
 static char *config_get_value(CONFIG_PARAMETER *, const char *);
 static char *config_get_password(CONFIG_PARAMETER *);
 static const char *config_get_value_string(CONFIG_PARAMETER *, const char *);
@@ -90,13 +89,11 @@ static void global_defaults();
 static void feedback_defaults();
 static bool check_config_objects(CONFIG_CONTEXT *context);
 static int maxscale_getline(char** dest, int* size, FILE* file);
-static SSL_LISTENER *make_ssl_structure(CONFIG_CONTEXT *obj, bool require_cert, int *error_count);
 
 int config_truth_value(char *str);
 int config_get_ifaddr(unsigned char *output);
 static int config_get_release_string(char* release);
 FEEDBACK_CONF *config_get_feedback_data();
-void config_add_param(CONFIG_CONTEXT*, char*, char*);
 bool config_has_duplicate_sections(const char* config, DUPLICATE_CONTEXT* context);
 int create_new_service(CONFIG_CONTEXT *obj);
 int create_new_server(CONFIG_CONTEXT *obj);
@@ -326,6 +323,20 @@ char* config_clean_string_list(const char* str)
     return dest;
 }
 
+CONFIG_CONTEXT* config_context_create(const char *section)
+{
+    CONFIG_CONTEXT* ctx = (CONFIG_CONTEXT *)MXS_MALLOC(sizeof(CONFIG_CONTEXT));
+    if (ctx)
+    {
+        ctx->object = MXS_STRDUP_A(section);
+        ctx->parameters = NULL;
+        ctx->next = NULL;
+        ctx->element = NULL;
+    }
+
+    return ctx;
+}
+
 /**
  * Config item handler for the ini file reader
  *
@@ -340,7 +351,6 @@ ini_handler(void *userdata, const char *section, const char *name, const char *v
 {
     CONFIG_CONTEXT   *cntxt = (CONFIG_CONTEXT *)userdata;
     CONFIG_CONTEXT   *ptr = cntxt;
-    CONFIG_PARAMETER *param, *p1;
 
     if (strcmp(section, "gateway") == 0 || strcasecmp(section, "MaxScale") == 0)
     {
@@ -368,55 +378,27 @@ ini_handler(void *userdata, const char *section, const char *name, const char *v
 
     if (!ptr)
     {
-        if ((ptr = (CONFIG_CONTEXT *)MXS_MALLOC(sizeof(CONFIG_CONTEXT))) == NULL)
+        if ((ptr = config_context_create(section)) == NULL)
         {
             return 0;
         }
 
-        ptr->object = MXS_STRDUP_A(section);
-        ptr->parameters = NULL;
         ptr->next = cntxt->next;
-        ptr->element = NULL;
         cntxt->next = ptr;
     }
-    /* Check to see if the parameter already exists for the section */
+
-    p1 = ptr->parameters;
+    if (config_get_param(ptr->parameters, name))
-    while (p1)
     {
-        if (!strcmp(p1->name, name))
+        if (!config_append_param(ptr, name, value))
         {
-            char *tmp;
+            return 0;
-            int paramlen = strlen(p1->value) + strlen(value) + 2;
-
-            if ((tmp = MXS_REALLOC(p1->value, sizeof(char) * (paramlen))) == NULL)
-            {
-                return 0;
-            }
-            strcat(tmp, ",");
-            strcat(tmp, value);
-            if ((p1->value = config_clean_string_list(tmp)) == NULL)
-            {
-                p1->value = tmp;
-                MXS_ERROR("[%s] Cleaning configuration parameter failed.", __FUNCTION__);
-                return 0;
-            }
-            MXS_FREE(tmp);
-            return 1;
         }
-        p1 = p1->next;
     }
-
+    else if (!config_add_param(ptr, name, value))
-    if ((param = (CONFIG_PARAMETER *)MXS_MALLOC(sizeof(CONFIG_PARAMETER))) == NULL)
     {
         return 0;
     }
 
-    param->name = MXS_STRDUP_A(name);
-    param->value = MXS_STRDUP_A(value);
-    param->next = ptr->parameters;
-    param->qfd_param_type = UNDEFINED_TYPE;
-    ptr->parameters = param;
-
     return 1;
 }
 
@@ -581,6 +563,47 @@ static bool is_directory(const char *dir)
     return rval;
 }
 
+/**
+ * @brief Check if a directory contains .cnf files
+ *
+ * @param path Path to a directory
+ * @return True if the directory contained one or more .cnf files
+ */
+static bool contains_cnf_files(const char *path)
+{
+    bool rval = false;
+    glob_t matches;
+    const char suffix[] = "/*.cnf";
+    char pattern[strlen(path) + sizeof(suffix)];
+
+    strcpy(pattern, path);
+    strcat(pattern, suffix);
+    int rc = glob(pattern, GLOB_NOSORT, NULL, &matches);
+
+    switch (rc)
+    {
+        case 0:
+            rval = true;
+            break;
+
+        case GLOB_NOSPACE:
+            MXS_OOM();
+            break;
+
+        case GLOB_ABORTED:
+            MXS_ERROR("Failed to read directory '%s'", path);
+            break;
+
+        default:
+            ss_dassert(rc == GLOB_NOMATCH);
+            break;
+    }
+
+    globfree(&matches);
+
+    return rval;
+}
+
 /**
  * @brief Load the specified configuration file for MaxScale
  *
@@ -625,7 +648,23 @@ config_load_and_process(const char* filename, bool (*process_config)(CONFIG_CONT
 
             if (is_directory(persist_cnf))
             {
-                rval = config_load_dir(persist_cnf, &dcontext, &ccontext);
+                DUPLICATE_CONTEXT p_dcontext;
+                /**
+                 * We need to initialize a second duplicate context for the
+                 * generated configuration files as the monitors and services will
+                 * have duplicate sections. The duplicate sections are used to
+                 * store changes to the list of servers the services and monitors
+                 * use, and thus should not be treated as errors.
+                 */
+                if (duplicate_context_init(&p_dcontext))
+                {
+                    rval = config_load_dir(persist_cnf, &p_dcontext, &ccontext);
+                    duplicate_context_finish(&p_dcontext);
+                }
+                else
+                {
+                    rval = false;
+                }
             }
 
             if (rval)
@@ -633,11 +672,18 @@ config_load_and_process(const char* filename, bool (*process_config)(CONFIG_CONT
                 if (!check_config_objects(ccontext.next) || !process_config(ccontext.next))
                 {
                     rval = false;
+                    if (contains_cnf_files(persist_cnf))
+                    {
+                        MXS_WARNING("One or more generated configurations were found at '%s'. "
+                                    "If the error relates to any of the files located there, "
+                                    "remove the offending configurations from this directory.",
+                                    persist_cnf);
+                    }
                 }
             }
         }
 
-        free_config_context(ccontext.next);
+        config_context_free(ccontext.next);
 
         duplicate_context_finish(&dcontext);
     }
@@ -876,10 +922,7 @@ config_get_value_string(CONFIG_PARAMETER *params, const char *name)
     return "";
 }
 
-
+CONFIG_PARAMETER* config_get_param(CONFIG_PARAMETER* params, const char* name)
-CONFIG_PARAMETER* config_get_param(
-    CONFIG_PARAMETER* params,
-    const char*       name)
 {
     while (params)
     {
@@ -1042,22 +1085,15 @@ void free_config_parameter(CONFIG_PARAMETER* p1)
     }
 }
 
-/**
+void config_context_free(CONFIG_CONTEXT *context)
- * Free a config tree
- *
- * @param context       The configuration data
- */
-static  void
-free_config_context(CONFIG_CONTEXT *context)
 {
     CONFIG_CONTEXT   *obj;
-    CONFIG_PARAMETER *p1, *p2;
 
     while (context)
     {
-        MXS_FREE(context->object);
-        free_config_parameter(context->parameters);
         obj = context->next;
+        free_config_parameter(context->parameters);
+        MXS_FREE(context->object);
         MXS_FREE(context);
         context = obj;
     }
@@ -1362,8 +1398,7 @@ free_ssl_structure(SSL_LISTENER *ssl)
  * @param *error_count  An error count which may be incremented
  * @return SSL_LISTENER structure or NULL
  */
-static SSL_LISTENER *
+SSL_LISTENER* make_ssl_structure (CONFIG_CONTEXT *obj, bool require_cert, int *error_count)
-make_ssl_structure (CONFIG_CONTEXT *obj, bool require_cert, int *error_count)
 {
     char *ssl, *ssl_version, *ssl_cert, *ssl_key, *ssl_ca_cert, *ssl_cert_verify_depth;
     int local_errors = 0;
@@ -2288,26 +2323,57 @@ unsigned long config_get_gateway_id()
     return gateway.id;
 }
 
-void config_add_param(CONFIG_CONTEXT* obj, char* key, char* value)
+bool config_add_param(CONFIG_CONTEXT* obj, const char* key, const char* value)
 {
-    key = MXS_STRDUP(key);
+    ss_dassert(config_get_param(obj->parameters, key) == NULL);
-    value = MXS_STRDUP(value);
+    bool rval = false;
+    char *my_key = MXS_STRDUP(key);
+    char *my_value = MXS_STRDUP(value);
+    CONFIG_PARAMETER* param = (CONFIG_PARAMETER *)MXS_MALLOC(sizeof(*param));
 
-    CONFIG_PARAMETER* param = (CONFIG_PARAMETER *)MXS_MALLOC(sizeof(CONFIG_PARAMETER));
+    if (my_key && my_value && param)
-
-    if (!key || !value || !param)
     {
-        MXS_FREE(key);
+        param->name = my_key;
-        MXS_FREE(value);
+        param->value = my_value;
+        param->qfd_param_type = UNDEFINED_TYPE;
+        param->next = obj->parameters;
+        obj->parameters = param;
+        rval = true;
+    }
+    else
+    {
+        MXS_FREE(my_key);
+        MXS_FREE(my_value);
         MXS_FREE(param);
-        return;
     }
 
-    param->name = key;
+    return rval;
-    param->value = value;
-    param->next = obj->parameters;
-    obj->parameters = param;
 }
+
+bool config_append_param(CONFIG_CONTEXT* obj, const char* key, const char* value)
+{
+    CONFIG_PARAMETER *param = config_get_param(obj->parameters, key);
+    ss_dassert(param);
+    int paramlen = strlen(param->value) + strlen(value) + 2;
+    char tmp[paramlen];
+    bool rval = false;
+
+    strcpy(tmp, param->value);
+    strcat(tmp, ",");
+    strcat(tmp, value);
+
+    char *new_value = config_clean_string_list(tmp);
+
+    if (new_value)
+    {
+        MXS_FREE(param->value);
+        param->value = new_value;
+        rval = true;
+    }
+
+    return rval;
+}
+
 /**
  * Return the pointer to the global options for MaxScale.
  * @return Pointer to the GATEWAY_CONF structure. This is a static structure and
@@ -3158,3 +3224,38 @@ int create_new_filter(CONFIG_CONTEXT *obj)
 
     return error_count;
 }
+
+bool config_have_required_ssl_params(CONFIG_CONTEXT *obj)
+{
+    CONFIG_PARAMETER *param = obj->parameters;
+
+    return config_get_param(param, "ssl") &&
+        config_get_param(param, "ssl_key") &&
+        config_get_param(param, "ssl_cert") &&
+        config_get_param(param, "ssl_ca_cert") &&
+        strcmp(config_get_value_string(param, "ssl"), "required") == 0;
+}
+
+bool config_is_ssl_parameter(const char *key)
+{
+    const char *ssl_params[] =
+    {
+        "ssl_cert",
+        "ssl_ca_cert",
+        "ssl",
+        "ssl_key",
+        "ssl_version",
+        "ssl_cert_verify_depth",
+        NULL
+    };
+
+    for (int i = 0; ssl_params[i]; i++)
+    {
+        if (strcmp(key, ssl_params[i]) == 0)
+        {
+            return true;
+        }
+    }
+
+    return false;
+}

server/core/monitor.c

@@ -40,6 +40,7 @@
 #include <mysqld_error.h>
 #include <maxscale/mysql_utils.h>
 #include <maxscale/alloc.h>
+#include <maxscale/gwdirs.h>
 
 /*
  *  Create declarations of the enum for monitor events and also the array of
@@ -244,46 +245,62 @@ monitorStopAll()
 void
 monitorAddServer(MONITOR *mon, SERVER *server)
 {
-    MONITOR_SERVERS *db = (MONITOR_SERVERS *)MXS_MALLOC(sizeof(MONITOR_SERVERS));
+    bool new_server = true;
-    MXS_ABORT_IF_NULL(db);
-
-    db->server = server;
-    db->con = NULL;
-    db->next = NULL;
-    db->mon_err_count = 0;
-    db->log_version_err = true;
-    /** Server status is uninitialized */
-    db->mon_prev_status = -1;
-    /* pending status is updated by get_replication_tree */
-    db->pending_status = 0;
-
-    monitor_state_t old_state = mon->state;
-
-    if (old_state == MONITOR_STATE_RUNNING)
-    {
-        monitorStop(mon);
-    }
-
     spinlock_acquire(&mon->lock);
 
-    if (mon->databases == NULL)
+    for (MONITOR_SERVERS *db = mon->databases; db; db = db->next)
     {
-        mon->databases = db;
+        if (db->server == server)
-    }
-    else
-    {
-        MONITOR_SERVERS *ptr = mon->databases;
-        while (ptr->next != NULL)
         {
-            ptr = ptr->next;
+            new_server = false;
         }
-        ptr->next = db;
     }
+
     spinlock_release(&mon->lock);
 
-    if (old_state == MONITOR_STATE_RUNNING)
+    if (new_server)
     {
-        monitorStart(mon, mon->parameters);
+        MONITOR_SERVERS *db = (MONITOR_SERVERS *)MXS_MALLOC(sizeof(MONITOR_SERVERS));
+        MXS_ABORT_IF_NULL(db);
+
+        db->server = server;
+        db->con = NULL;
+        db->next = NULL;
+        db->mon_err_count = 0;
+        db->log_version_err = true;
+        /** Server status is uninitialized */
+        db->mon_prev_status = -1;
+        /* pending status is updated by get_replication_tree */
+        db->pending_status = 0;
+
+        monitor_state_t old_state = mon->state;
+
+        if (old_state == MONITOR_STATE_RUNNING)
+        {
+            monitorStop(mon);
+        }
+
+        spinlock_acquire(&mon->lock);
+
+        if (mon->databases == NULL)
+        {
+            mon->databases = db;
+        }
+        else
+        {
+            MONITOR_SERVERS *ptr = mon->databases;
+            while (ptr->next != NULL)
+            {
+                ptr = ptr->next;
+            }
+            ptr->next = db;
+        }
+        spinlock_release(&mon->lock);
+
+        if (old_state == MONITOR_STATE_RUNNING)
+        {
+            monitorStart(mon, mon->parameters);
+        }
     }
 }
 
@@ -1152,3 +1169,86 @@ bool monitor_server_in_use(const SERVER *server)
 
     return rval;
 }
+
+/**
+ * Creates a monitor configuration at the location pointed by @c filename
+ *
+ * @param monitor Monitor to serialize into a configuration
+ * @param filename Filename where configuration is written
+ * @return True on success, false on error
+ */
+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)
+    {
+        char errbuf[MXS_STRERROR_BUFLEN];
+        MXS_ERROR("Failed to open file '%s' when serializing monitor '%s': %d, %s",
+                  filename, monitor->name, errno, strerror_r(errno, errbuf, sizeof(errbuf)));
+        return false;
+    }
+
+    /**
+     * Only additional parameters are added to the configuration. This prevents
+     * duplication or addition of parameters that don't support it.
+     *
+     * TODO: Check for return values on all of the dprintf calls
+     */
+    dprintf(file, "[%s]\n", monitor->name);
+
+    if (monitor->databases)
+    {
+        dprintf(file, "servers=");
+        for (MONITOR_SERVERS *db = monitor->databases; db; db = db->next)
+        {
+            if (db != monitor->databases)
+            {
+                dprintf(file, ",");
+            }
+            dprintf(file, "%s", db->server->unique_name);
+        }
+        dprintf(file, "\n");
+    }
+
+    close(file);
+
+    return true;
+}
+
+bool monitor_serialize_servers(const MONITOR *monitor)
+{
+    bool rval = false;
+    char filename[PATH_MAX];
+    snprintf(filename, sizeof(filename), "%s/%s.cnf.tmp", get_config_persistdir(),
+             monitor->name);
+
+    if (unlink(filename) == -1 && errno != ENOENT)
+    {
+        char err[MXS_STRERROR_BUFLEN];
+        MXS_ERROR("Failed to remove temporary monitor configuration at '%s': %d, %s",
+                  filename, errno, strerror_r(errno, err, sizeof(err)));
+    }
+    else if (create_monitor_config(monitor, filename))
+    {
+        char final_filename[PATH_MAX];
+        strcpy(final_filename, filename);
+
+        char *dot = strrchr(final_filename, '.');
+        ss_dassert(dot);
+        *dot = '\0';
+
+        if (rename(filename, final_filename) == 0)
+        {
+            rval = true;
+        }
+        else
+        {
+            char err[MXS_STRERROR_BUFLEN];
+            MXS_ERROR("Failed to rename temporary monitor configuration at '%s': %d, %s",
+                      filename, errno, strerror_r(errno, err, sizeof(err)));
+        }
+    }
+
+    return rval;
+}

server/core/server.c

@@ -987,6 +987,8 @@ server_update_address(SERVER *server, char *address)
     spinlock_acquire(&server_spin);
     if (server && address)
     {
+        MXS_NOTICE("Updated the address of server '%s' from '%s' to '%s'.",
+                   server->unique_name, server->name, address);
         strcpy(server->name, address);
     }
     spinlock_release(&server_spin);
@@ -1005,6 +1007,8 @@ server_update_port(SERVER *server, unsigned short port)
     spinlock_acquire(&server_spin);
     if (server && port > 0)
     {
+        MXS_NOTICE("Updated the port of server '%s' from %d to %d.",
+                   server->unique_name, server->port, port);
         server->port = port;
     }
     spinlock_release(&server_spin);
@@ -1082,7 +1086,7 @@ bool server_set_version_string(SERVER* server, const char* string)
  * @param filename Filename where configuration is written
  * @return True on success, false on error
  */
-static bool create_server_config(SERVER *server, const char *filename)
+static bool create_server_config(const SERVER *server, const char *filename)
 {
     int file = open(filename, O_EXCL | O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 
@@ -1182,17 +1186,7 @@ static bool create_server_config(SERVER *server, const char *filename)
     return true;
 }
 
-/**
+bool server_serialize(const SERVER *server)
- * @brief Serialize a server to a file
- *
- * This converts @c server into an INI format file. This allows created servers
- * to be persisted to disk. This will replace any existing files with the same
- * name.
- *
- * @param server Server to serialize
- * @return False if the serialization of the server fails, true if it was successful
- */
-static bool server_serialize(SERVER *server)
 {
     bool rval = false;
     char filename[PATH_MAX];
@@ -1293,15 +1287,19 @@ bool server_create(const char *name, const char *address, const char *port,
             server->is_active = true;
             rval = true;
         }
-        else if ((server = server_alloc(name, address, atoi(port), protocol, authenticator,
+        else
-                                        authenticator_options)))
         {
-            if (server_serialize(server))
+            /**
-            {
+             * server_alloc will add the server to the global list of
-                /** server_alloc will add the server to the global list of
+             * servers so we don't need to manually add it.
-                 * servers so we don't need to manually add it. */
+             */
-                rval = true;
+            server = server_alloc(name, address, atoi(port), protocol,
-            }
+                                  authenticator, authenticator_options);
+        }
+
+        if (server && server_serialize(server))
+        {
+            rval = true;
         }
     }
 
@@ -1354,14 +1352,3 @@ bool server_destroy(SERVER *server)
 
     return rval;
 }
-
-bool server_is_ssl_parameter(const char *key)
-{
-    // TODO: Implement this
-    return false;
-}
-
-void server_update_ssl(SERVER *server, const char *key, const char *value)
-{
-    // TODO: Implement this
-}

server/core/service.c

@@ -43,6 +43,10 @@
 #include <string.h>
 #include <ctype.h>
 #include <errno.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <math.h>
+#include <fcntl.h>
 #include <maxscale/session.h>
 #include <maxscale/service.h>
 #include <maxscale/gw_protocol.h>
@@ -56,12 +60,9 @@
 #include <maxscale/filter.h>
 #include <maxscale/poll.h>
 #include <maxscale/log_manager.h>
-#include <sys/stat.h>
-#include <sys/types.h>
 #include <maxscale/housekeeper.h>
 #include <maxscale/resultset.h>
 #include <maxscale/gwdirs.h>
-#include <math.h>
 #include <maxscale/version.h>
 #include <maxscale/queuemanager.h>
 #include <maxscale/alloc.h>
@@ -769,43 +770,46 @@ static SERVER_REF* server_ref_create(SERVER *server)
 void
 serviceAddBackend(SERVICE *service, SERVER *server)
 {
-    SERVER_REF *new_ref = server_ref_create(server);
+    if (!serviceHasBackend(service, server))
-
-    if (new_ref)
     {
-        spinlock_acquire(&service->spin);
+        SERVER_REF *new_ref = server_ref_create(server);
 
-        service->n_dbref++;
+        if (new_ref)
-
-        if (service->dbref)
         {
-            SERVER_REF *ref = service->dbref;
+            spinlock_acquire(&service->spin);
-            SERVER_REF *prev = ref;
 
-            while (ref)
+            service->n_dbref++;
+
+            if (service->dbref)
             {
-                if (ref->server == server)
+                SERVER_REF *ref = service->dbref;
+                SERVER_REF *prev = ref;
+
+                while (ref)
                 {
-                    ref->active = true;
+                    if (ref->server == server)
-                    break;
+                    {
+                        ref->active = true;
+                        break;
+                    }
+                    prev = ref;
+                    ref = ref->next;
                 }
-                prev = ref;
-                ref = ref->next;
-            }
 
-            if (ref == NULL)
+                if (ref == NULL)
-            {
+                {
-                /** A new server that hasn't been used by this service */
+                    /** A new server that hasn't been used by this service */
-                atomic_synchronize();
+                    atomic_synchronize();
-                prev->next = new_ref;
+                    prev->next = new_ref;
+                }
             }
+            else
+            {
+                atomic_synchronize();
+                service->dbref = new_ref;
+            }
+            spinlock_release(&service->spin);
         }
-        else
-        {
-            atomic_synchronize();
-            service->dbref = new_ref;
-        }
-        spinlock_release(&service->spin);
     }
 }
 
@@ -841,7 +845,7 @@ void serviceRemoveBackend(SERVICE *service, const SERVER *server)
  * @param server        The server to add
  * @return              Non-zero if the server is already part of the service
  */
-int
+bool
 serviceHasBackend(SERVICE *service, SERVER *server)
 {
     SERVER_REF *ptr;
@@ -2217,3 +2221,85 @@ bool service_server_in_use(const SERVER *server)
 
     return rval;
 }
+
+/**
+ * Creates a service configuration at the location pointed by @c filename
+ *
+ * @param service Service to serialize into a configuration
+ * @param filename Filename where configuration is written
+ * @return True on success, false on error
+ */
+static bool create_service_config(const SERVICE *service, const char *filename)
+{
+    int file = open(filename, O_EXCL | O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+
+    if (file == -1)
+    {
+        char errbuf[MXS_STRERROR_BUFLEN];
+        MXS_ERROR("Failed to open file '%s' when serializing service '%s': %d, %s",
+                  filename, service->name, errno, strerror_r(errno, errbuf, sizeof(errbuf)));
+        return false;
+    }
+
+    /**
+     * Only additional parameters are added to the configuration. This prevents
+     * duplication or addition of parameters that don't support it.
+     *
+     * TODO: Check for return values on all of the dprintf calls
+     */
+    dprintf(file, "[%s]\n", service->name);
+    if (service->dbref)
+    {
+        dprintf(file, "servers=");
+        for (SERVER_REF *db = service->dbref; db; db = db->next)
+        {
+            if (db != service->dbref)
+            {
+                dprintf(file, ",");
+            }
+            dprintf(file, "%s", db->server->unique_name);
+        }
+        dprintf(file, "\n");
+    }
+
+    close(file);
+
+    return true;
+}
+
+bool service_serialize_servers(const SERVICE *service)
+{
+    bool rval = false;
+    char filename[PATH_MAX];
+    snprintf(filename, sizeof(filename), "%s/%s.cnf.tmp", get_config_persistdir(),
+             service->name);
+
+    if (unlink(filename) == -1 && errno != ENOENT)
+    {
+        char err[MXS_STRERROR_BUFLEN];
+        MXS_ERROR("Failed to remove temporary service configuration at '%s': %d, %s",
+                  filename, errno, strerror_r(errno, err, sizeof(err)));
+    }
+    else if (create_service_config(service, filename))
+    {
+        char final_filename[PATH_MAX];
+        strcpy(final_filename, filename);
+
+        char *dot = strrchr(final_filename, '.');
+        ss_dassert(dot);
+        *dot = '\0';
+
+        if (rename(filename, final_filename) == 0)
+        {
+            rval = true;
+        }
+        else
+        {
+            char err[MXS_STRERROR_BUFLEN];
+            MXS_ERROR("Failed to rename temporary service configuration at '%s': %d, %s",
+                      filename, errno, strerror_r(errno, err, sizeof(err)));
+        }
+    }
+
+    return rval;
+}

server/modules/protocol/MySQL/MySQLBackend/mysql_backend.c

@@ -617,6 +617,11 @@ gw_read_backend_event(DCB *dcb)
                 gw_reply_on_error(dcb, proto->protocol_auth_state);
             }
         }
+        else if (proto->protocol_auth_state == MXS_AUTH_STATE_CONNECTED &&
+                 dcb->ssl_state == SSL_ESTABLISHED)
+        {
+            proto->protocol_auth_state = gw_send_backend_auth(dcb);
+        }
     }
 
     return rc;

server/modules/protocol/MySQL/mysql_common.c

@@ -1339,7 +1339,7 @@ mxs_auth_state_t gw_send_backend_auth(DCB *dcb)
         (dcb->session->state != SESSION_STATE_READY &&
          dcb->session->state != SESSION_STATE_ROUTER_READY) ||
         (dcb->server->server_ssl &&
-         dcb->ssl_state != SSL_HANDSHAKE_FAILED))
+         dcb->ssl_state == SSL_HANDSHAKE_FAILED))
     {
         return MXS_AUTH_STATE_FAILED;
     }

server/modules/routing/debugcli/debugcmd.c

@@ -749,10 +749,12 @@ static void cmd_AddServer(DCB *dcb, void *a, void *b)
         if (service)
         {
             serviceAddBackend(service, server);
+            service_serialize_servers(service);
         }
         else if (monitor)
         {
             monitorAddServer(monitor, server);
+            monitor_serialize_servers(monitor);
         }
 
         const char *target = service ? "service" : "monitor";
@@ -805,10 +807,12 @@ static void cmd_RemoveServer(DCB *dcb, void *a, void *b)
         if (service)
         {
             serviceRemoveBackend(service, server);
+            service_serialize_servers(service);
         }
         else if (monitor)
         {
             monitorRemoveServer(monitor, server);
+            monitor_serialize_servers(monitor);
         }
 
         const char *target = service ? "service" : "monitor";
@@ -980,9 +984,9 @@ static void createServer(DCB *dcb, char *name, char *address, char *port,
 struct subcommand createoptions[] =
 {
     {
-        "server", 3, 6, createServer,
+        "server", 2, 6, createServer,
         "Create a new server",
-        "Usage: create server NAME HOST PORT [PROTOCOL] [AUTHENTICATOR] [OPTIONS]\n"
+        "Usage: create server NAME HOST [PORT] [PROTOCOL] [AUTHENTICATOR] [OPTIONS]\n"
         "Create a new server from the following parameters.\n"
         "NAME          Server name\n"
         "HOST          Server host address\n"
@@ -1032,9 +1036,9 @@ struct subcommand destroyoptions[] =
     }
 };
 
-static void alterServer(DCB *dcb, SERVER *server, char *key, char *value)
+static bool handle_alter_server(SERVER *server, char *key, char *value)
 {
-    bool unknown = false;
+    bool valid = true;
 
     if (strcmp(key, "address") == 0)
     {
@@ -1052,18 +1056,108 @@ static void alterServer(DCB *dcb, SERVER *server, char *key, char *value)
     {
         server_update_credentials(server, server->monuser, value);
     }
-    else if (server_is_ssl_parameter(key))
+    else
     {
-        server_update_ssl(server, key, value);
+        valid = false;
+    }
+
+    return valid;
+}
+
+void handle_server_ssl(DCB *dcb, SERVER *server, CONFIG_CONTEXT *obj)
+{
+    if (config_have_required_ssl_params(obj))
+    {
+        int err = 0;
+        SSL_LISTENER *ssl = make_ssl_structure(obj, true, &err);
+
+        if (err == 0 && ssl && listener_init_SSL(ssl) == 0)
+        {
+            /** Sync to prevent reads on partially initialized server_ssl */
+            atomic_synchronize();
+
+            server->server_ssl = ssl;
+            if (server_serialize(server))
+            {
+                dcb_printf(dcb, "SSL enabled for server '%s'\n", server->unique_name);
+            }
+            else
+            {
+                dcb_printf(dcb, "SSL enabled for server '%s' but persisting "
+                           "it to disk failed, see log for more details.\n",
+                           server->unique_name);
+            }
+        }
+        else
+        {
+            dcb_printf(dcb, "Enabling SSL for server '%s' failed, see log "
+                       "for more details.\n", server->unique_name);
+        }
     }
     else
     {
-        unknown = true;
+        dcb_printf(dcb, "Error: SSL configuration requires the following parameters:\n"
+                   "ssl=required ssl_key=PATH ssl_cert=PATH ssl_ca_cert=PATH\n");
+    }
+}
+
+/**
+ * @brief Process multiple alter operations at once
+ *
+ * This is a somewhat ugly way to handle multiple key-value changes in one operation
+ * with one function. This could be handled with a variadic function but the
+ * required complexity would probably negate any benefits.
+ */
+static void alterServer(DCB *dcb, SERVER *server, char *v1, char *v2, char *v3,
+                        char *v4, char *v5, char *v6, char *v7, char *v8, char *v9,
+                        char *v10, char *v11)
+{
+    char *values[11] = {v1, v2, v3, v4, v5, v6, v7, v8, v9, v10, v11};
+    const int items = sizeof(values) / sizeof(values[0]);
+    CONFIG_CONTEXT *obj = NULL;
+
+    for (int i = 0; i < items; i++)
+    {
+        if (values[i])
+        {
+            char *key = values[i];
+            char *value = strchr(key, '=');
+
+            if (value)
+            {
+                *value++ = '\0';
+
+                if (config_is_ssl_parameter(key))
+                {
+                    /**
+                     * All the required SSL parameters must be defined at once to
+                     * enable SSL for created servers. This removes the problem
+                     * of partial configuration and allows a somewhat atomic
+                     * operation.
+                     */
+                    if ((obj == NULL && (obj = config_context_create(server->unique_name)) == NULL) ||
+                        (!config_add_param(obj, key, value)))
+                    {
+                        dcb_printf(dcb, "Internal error, see log for more details\n");
+                    }
+                }
+                else if (!handle_alter_server(server, key, value))
+                {
+                    dcb_printf(dcb, "Error: Bad key-value parameter: %s=%s\n", key, value);
+                }
+            }
+            else
+            {
+                dcb_printf(dcb, "Error: not a key-value parameter: %s\n", values[i]);
+            }
+        }
     }
 
-    if (unknown)
+    if (obj)
     {
-        dcb_printf(dcb, "Unknown parameter '%s'", key);
+        /** We have SSL parameters, try to process them */
+        handle_server_ssl(dcb, server, obj);
+        config_context_free(obj);
     }
 }
 
@@ -1072,11 +1166,10 @@ static void alterServer(DCB *dcb, SERVER *server, char *key, char *value)
  *
  * If the value is not a positive integer, an error is printed to @c dcb.
  *
- * @param dcb Client DCB
  * @param value String value
  * @return 0 on error, otherwise a positive integer
  */
-static long get_positive_int(DCB *dcb, const char *value)
+static long get_positive_int(const char *value)
 {
     char *endptr;
     long ival = strtol(value, &endptr, 10);
@@ -1086,82 +1179,119 @@ static long get_positive_int(DCB *dcb, const char *value)
         return ival;
     }
 
-    dcb_printf(dcb, "Invalid value: %s", value);
     return 0;
 }
 
-static void alterMonitor(DCB *dcb, MONITOR *monitor, char *key, char *value)
+static bool handle_alter_monitor(MONITOR *monitor, char *key, char *value)
 {
-    bool unknown = false;
+    bool valid = false;
+
     if (strcmp(key, "user") == 0)
     {
+        valid = true;
         monitorAddUser(monitor, value, monitor->password);
     }
     else if (strcmp(key, "password") == 0)
     {
+        valid = true;
         monitorAddUser(monitor, monitor->user, value);
     }
     else if (strcmp(key, "monitor_interval") == 0)
     {
-        long ival = get_positive_int(dcb, value);
+        long ival = get_positive_int(value);
         if (ival)
         {
+            valid = true;
             monitorSetInterval(monitor, ival);
         }
     }
     else if (strcmp(key, "backend_connect_timeout") == 0)
     {
-        long ival = get_positive_int(dcb, value);
+        long ival = get_positive_int(value);
         if (ival)
         {
+            valid = true;
             monitorSetNetworkTimeout(monitor, MONITOR_CONNECT_TIMEOUT, ival);
         }
     }
     else if (strcmp(key, "backend_write_timeout") == 0)
     {
-        long ival = get_positive_int(dcb, value);
+        long ival = get_positive_int(value);
         if (ival)
         {
-            monitorSetNetworkTimeout(monitor, MONITOR_READ_TIMEOUT, ival);
+            valid = true;
+            monitorSetNetworkTimeout(monitor, MONITOR_WRITE_TIMEOUT, ival);
         }
     }
     else if (strcmp(key, "backend_read_timeout") == 0)
     {
-        long ival = get_positive_int(dcb, value);
+        long ival = get_positive_int(value);
         if (ival)
         {
-            monitorSetNetworkTimeout(monitor, MONITOR_WRITE_TIMEOUT, ival);
+            valid = true;
+            monitorSetNetworkTimeout(monitor, MONITOR_READ_TIMEOUT, ival);
         }
     }
-    else
+
+    return valid;
+}
+
+static void alterMonitor(DCB *dcb, MONITOR *monitor, char *v1, char *v2, char *v3,
+                        char *v4, char *v5, char *v6, char *v7, char *v8, char *v9,
+                        char *v10, char *v11)
+{
+    char *values[11] = {v1, v2, v3, v4, v5, v6, v7, v8, v9, v10, v11};
+    const int items = sizeof(values) / sizeof(values[0]);
+
+    for (int i = 0; i < items; i++)
     {
-        unknown = true;
+        if (values[i])
+        {
+            char *key = values[i];
+            char *value = strchr(key, '=');
+
+            if (value)
+            {
+                *value++ = '\0';
+
+                if (!handle_alter_monitor(monitor, key, value))
+                {
+                    dcb_printf(dcb, "Error: Bad key-value parameter: %s=%s\n", key, value);
+                }
+            }
+            else
+            {
+                dcb_printf(dcb, "Error: not a key-value parameter: %s\n", values[i]);
+            }
+        }
     }
 
-    if (unknown)
-    {
-        dcb_printf(dcb, "Unknown parameter '%s'", key);
-    }
 }
 
 struct subcommand alteroptions[] =
 {
     {
-        "server", 3, 3, alterServer,
+        "server", 2, 12, alterServer,
         "Alter server parameters",
-        "Usage: alter server NAME KEY VALUE\n"
+        "Usage: alter server NAME KEY=VALUE ...\n"
         "This will alter an existing parameter of a server. The accepted values\n"
-        "for KEY are: 'address', 'port', 'monuser', 'monpw'",
+        "for KEY are: 'address', 'port', 'monuser', 'monpw'\n"
-        {ARG_TYPE_SERVER, ARG_TYPE_STRING, ARG_TYPE_STRING}
+        "A maximum of 11 parameters can be changed at one time",
+        { ARG_TYPE_SERVER, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING,
+            ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING,
+            ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING}
     },
     {
-        "monitor", 3, 3, alterMonitor,
+        "monitor", 2, 12, alterMonitor,
         "Alter monitor parameters",
-        "Usage: alter monitor NAME KEY VALUE\n"
+        "Usage: alter monitor NAME KEY=VALUE ...\n"
         "This will alter an existing parameter of a monitor. The accepted values\n"
         "for KEY are: 'user', 'password', 'monitor_interval',\n"
-        "'backend_connect_timeout', 'backend_write_timeout', 'backend_read_timeout'",
+        "'backend_connect_timeout', 'backend_write_timeout', 'backend_read_timeout'\n"
-        {ARG_TYPE_MONITOR, ARG_TYPE_STRING, ARG_TYPE_STRING}
+        "A maximum of 11 parameters can be changed at one time",
+        {ARG_TYPE_MONITOR, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING,
+            ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING,
+            ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING}
     },
     {
         EMPTY_OPTION
@@ -1456,12 +1586,29 @@ execute_cmd(CLI_SESSION *cli)
                     if (strcasecmp(args[1], cmds[i].options[j].arg1) == 0)
                     {
                         found = 1; /**< command and sub-command match */
-                        if (argc < cmds[i].options[j].argc_min)
+
+                        if (cmds[i].options[j].argc_min == cmds[i].options[j].argc_max &&
+                            argc != cmds[i].options[j].argc_min)
                         {
+                            /** Wrong number of arguments */
+                            dcb_printf(dcb, "Incorrect number of arguments: %s %s expects %d arguments\n",
+                                       cmds[i].cmd, cmds[i].options[j].arg1,
+                                       cmds[i].options[j].argc_min);
+                        }
+                        else if (argc < cmds[i].options[j].argc_min)
+                        {
+                            /** Not enough arguments */
                             dcb_printf(dcb, "Incorrect number of arguments: %s %s expects at least %d arguments\n",
                                        cmds[i].cmd, cmds[i].options[j].arg1,
                                        cmds[i].options[j].argc_min);
                         }
+                        else if (argc > cmds[i].options[j].argc_max)
+                        {
+                            /** Too many arguments */
+                            dcb_printf(dcb, "Incorrect number of arguments: %s %s expects at most %d arguments\n",
+                                       cmds[i].cmd, cmds[i].options[j].arg1,
+                                       cmds[i].options[j].argc_max);
+                        }
                         else
                         {
                             unsigned long arg_list[MAXARGS] = {};