Commit 35884802 authored by unknown's avatar unknown

post-review fixes

server-tools/instance-manager/Makefile.am:
  Removed entry for deleted file
server-tools/instance-manager/buffer.cc:
  cleanup
server-tools/instance-manager/commands.cc:
  cleanup, added missing error handling
server-tools/instance-manager/instance.cc:
  added waitpid in instance_start, added few checks
server-tools/instance-manager/instance_map.cc:
  error handling for hash_init added
server-tools/instance-manager/instance_map.h:
  Extended constructor
server-tools/instance-manager/instance_options.cc:
  made add_option less bulky
server-tools/instance-manager/instance_options.h:
  -
server-tools/instance-manager/listener.cc:
  added missing close, fixed typo
server-tools/instance-manager/manager.cc:
  moved some Instance_map initialization to costructor
server-tools/instance-manager/protocol.cc:
  error handling added
server-tools/instance-manager/protocol.h:
  store_to_string fixed to return a value
server-tools/instance-manager/user_map.cc:
  error handling for hash_init added
server-tools/instance-manager/user_map.h:
  added init() for User map (becouse of the hash_init check)
parent c77560bf
Branches unavailable
Tags unavailable
No related merge requests found
...@@ -80,8 +80,7 @@ mysqlmanager_SOURCES= mysqlmanager.cc manager.h manager.cc log.h log.cc \ ...@@ -80,8 +80,7 @@ mysqlmanager_SOURCES= mysqlmanager.cc manager.h manager.cc log.h log.cc \
instance_map.h instance_map.cc\ instance_map.h instance_map.cc\
instance_options.h instance_options.cc \ instance_options.h instance_options.cc \
buffer.h buffer.cc parse.cc parse.h \ buffer.h buffer.cc parse.cc parse.h \
guardian.cc guardian.h common_structures.h \ guardian.cc guardian.h mysql_manager_error.h
mysql_manager_error.h
mysqlmanager_LDADD= liboptions.a \ mysqlmanager_LDADD= liboptions.a \
libnet.a \ libnet.a \
......
...@@ -77,7 +77,7 @@ int Buffer::append(uint position, const char *string, uint len_arg) ...@@ -77,7 +77,7 @@ int Buffer::append(uint position, const char *string, uint len_arg)
int Buffer::reserve(uint position, uint len_arg) int Buffer::reserve(uint position, uint len_arg)
{ {
if (position + len_arg >= MAX_BUFFER_SIZE) if (position + len_arg >= MAX_BUFFER_SIZE)
return 1; goto err;
if (position + len_arg>= buffer_size) if (position + len_arg>= buffer_size)
{ {
...@@ -85,8 +85,13 @@ int Buffer::reserve(uint position, uint len_arg) ...@@ -85,8 +85,13 @@ int Buffer::reserve(uint position, uint len_arg)
min(MAX_BUFFER_SIZE, min(MAX_BUFFER_SIZE,
max((uint) (buffer_size*1.5), max((uint) (buffer_size*1.5),
position + len_arg))); position + len_arg)));
if (buffer= NULL)
goto err;
buffer_size= (uint) (buffer_size*1.5); buffer_size= (uint) (buffer_size*1.5);
} }
return 0; return 0;
err:
return 1;
} }
...@@ -184,14 +184,17 @@ int Show_instance_status::do_command(struct st_net *net, ...@@ -184,14 +184,17 @@ int Show_instance_status::do_command(struct st_net *net,
} }
my_net_write(net, send_buff.buffer, (uint) position); if (my_net_write(net, send_buff.buffer, (uint) position))
goto err;
} }
send_eof(net); send_eof(net);
net_flush(net); net_flush(net);
err:
return 0; return 0;
err:
return 1;
} }
...@@ -258,7 +261,8 @@ int Show_instance_options::do_command(struct st_net *net, ...@@ -258,7 +261,8 @@ int Show_instance_options::do_command(struct st_net *net,
goto err; goto err;
store_to_string(&send_buff, (char *) "instance_name", &position); store_to_string(&send_buff, (char *) "instance_name", &position);
store_to_string(&send_buff, (char *) instance_name, &position); store_to_string(&send_buff, (char *) instance_name, &position);
my_net_write(net, send_buff.buffer, (uint) position); if (my_net_write(net, send_buff.buffer, (uint) position))
goto err;
if (instance->options.mysqld_path != NULL) if (instance->options.mysqld_path != NULL)
{ {
position= 0; position= 0;
...@@ -266,7 +270,17 @@ int Show_instance_options::do_command(struct st_net *net, ...@@ -266,7 +270,17 @@ int Show_instance_options::do_command(struct st_net *net,
store_to_string(&send_buff, store_to_string(&send_buff,
(char *) instance->options.mysqld_path, (char *) instance->options.mysqld_path,
&position); &position);
my_net_write(net, send_buff.buffer, (uint) position); if (my_net_write(net, send_buff.buffer, (uint) position))
goto err;
}
if (instance->options.is_guarded != NULL)
{
position= 0;
store_to_string(&send_buff, (char *) "guarded", &position);
store_to_string(&send_buff, "", &position);
if (my_net_write(net, send_buff.buffer, (uint) position))
goto err;
} }
if (instance->options.mysqld_user != NULL) if (instance->options.mysqld_user != NULL)
...@@ -276,7 +290,8 @@ int Show_instance_options::do_command(struct st_net *net, ...@@ -276,7 +290,8 @@ int Show_instance_options::do_command(struct st_net *net,
store_to_string(&send_buff, store_to_string(&send_buff,
(char *) instance->options.mysqld_user, (char *) instance->options.mysqld_user,
&position); &position);
my_net_write(net, send_buff.buffer, (uint) position); if (my_net_write(net, send_buff.buffer, (uint) position))
goto err;
} }
if (instance->options.mysqld_password != NULL) if (instance->options.mysqld_password != NULL)
...@@ -286,7 +301,8 @@ int Show_instance_options::do_command(struct st_net *net, ...@@ -286,7 +301,8 @@ int Show_instance_options::do_command(struct st_net *net,
store_to_string(&send_buff, store_to_string(&send_buff,
(char *) instance->options.mysqld_password, (char *) instance->options.mysqld_password,
&position); &position);
my_net_write(net, send_buff.buffer, (uint) position); if (my_net_write(net, send_buff.buffer, (uint) position))
goto err;
} }
/* loop through the options stored in DYNAMIC_ARRAY */ /* loop through the options stored in DYNAMIC_ARRAY */
...@@ -302,7 +318,8 @@ int Show_instance_options::do_command(struct st_net *net, ...@@ -302,7 +318,8 @@ int Show_instance_options::do_command(struct st_net *net,
store_to_string(&send_buff, option_value + 1, &position); store_to_string(&send_buff, option_value + 1, &position);
/* join name and the value into the same option again */ /* join name and the value into the same option again */
*option_value= '='; *option_value= '=';
my_net_write(net, send_buff.buffer, (uint) position); if (my_net_write(net, send_buff.buffer, (uint) position))
goto err;
} }
} }
......
...@@ -24,7 +24,7 @@ ...@@ -24,7 +24,7 @@
#include <my_sys.h> #include <my_sys.h>
#include <signal.h> #include <signal.h>
#include <m_string.h> #include <m_string.h>
#include <sys/wait.h>
/* /*
The method starts an instance. The method starts an instance.
...@@ -41,11 +41,12 @@ ...@@ -41,11 +41,12 @@
int Instance::start() int Instance::start()
{ {
pid_t pid;
if (!is_running()) if (!is_running())
{ {
log_info("trying to start instance %s", options.instance_name); log_info("trying to start instance %s", options.instance_name);
switch (fork()) { switch (pid= fork()) {
case 0: case 0:
if (fork()) /* zombie protection */ if (fork()) /* zombie protection */
exit(0); /* parent goes bye-bye */ exit(0); /* parent goes bye-bye */
...@@ -57,6 +58,7 @@ int Instance::start() ...@@ -57,6 +58,7 @@ int Instance::start()
case -1: case -1:
return ER_CANNOT_START_INSTANCE; return ER_CANNOT_START_INSTANCE;
default: default:
waitpid(pid, NULL, 0);
return 0; return 0;
} }
} }
...@@ -77,21 +79,32 @@ int Instance::cleanup() ...@@ -77,21 +79,32 @@ int Instance::cleanup()
return 0; return 0;
} }
Instance::~Instance() Instance::~Instance()
{ {
pthread_mutex_destroy(&LOCK_instance); pthread_mutex_destroy(&LOCK_instance);
} }
bool Instance::is_running() bool Instance::is_running()
{ {
uint port;
const char *socket;
if (options.mysqld_port)
port= atoi(strchr(options.mysqld_port, '=') + 1);
if (options.mysqld_socket)
socket= strchr(options.mysqld_socket, '=') + 1;
pthread_mutex_lock(&LOCK_instance); pthread_mutex_lock(&LOCK_instance);
if (!is_connected) if (!is_connected)
{ {
mysql_init(&mysql); mysql_init(&mysql);
if (mysql_real_connect(&mysql, LOCAL_HOST, options.mysqld_user, if (mysql_real_connect(&mysql, LOCAL_HOST, options.mysqld_user,
options.mysqld_password, options.mysqld_password,
NullS, atoi(strchr(options.mysqld_port, '=') + 1), NullS, port,
strchr(options.mysqld_socket, '=') + 1, 0)) socket, 0))
{ {
is_connected= TRUE; is_connected= TRUE;
pthread_mutex_unlock(&LOCK_instance); pthread_mutex_unlock(&LOCK_instance);
......
...@@ -110,14 +110,26 @@ static int process_option(void * ctx, const char *group, const char *option) ...@@ -110,14 +110,26 @@ static int process_option(void * ctx, const char *group, const char *option)
C_MODE_END C_MODE_END
Instance_map::Instance_map() Instance_map::Instance_map(const char *default_mysqld_path_arg,
const char *default_admin_user_arg,
const char *default_admin_password_arg)
{ {
hash_init(&hash, default_charset_info, START_HASH_SIZE, 0, 0, mysqld_path= default_mysqld_path_arg;
get_instance_key, delete_instance, 0); user= default_admin_user_arg;
password= default_admin_password_arg;
pthread_mutex_init(&LOCK_instance_map, 0); pthread_mutex_init(&LOCK_instance_map, 0);
} }
int Instance_map::init()
{
if (hash_init(&hash, default_charset_info, START_HASH_SIZE, 0, 0,
get_instance_key, delete_instance, 0))
return 1;
return 0;
}
Instance_map::~Instance_map() Instance_map::~Instance_map()
{ {
pthread_mutex_lock(&LOCK_instance_map); pthread_mutex_lock(&LOCK_instance_map);
......
...@@ -48,7 +48,7 @@ class Instance_map ...@@ -48,7 +48,7 @@ class Instance_map
Instance_map *instance_map; Instance_map *instance_map;
public: public:
Iterator(Instance_map *instance_map_arg) : Iterator(Instance_map *instance_map_arg) :
instance_map(instance_map_arg), current_instance(0) current_instance(0), instance_map(instance_map_arg)
{} {}
void go_to_first(); void go_to_first();
...@@ -63,8 +63,11 @@ class Instance_map ...@@ -63,8 +63,11 @@ class Instance_map
int cleanup(); int cleanup();
int lock(); int lock();
int unlock(); int unlock();
int init();
Instance_map(); Instance_map(const char *default_mysqld_path_arg,
const char *default_admin_user_arg,
const char *default_admin_password_arg);
~Instance_map(); ~Instance_map();
/* loads options from config files */ /* loads options from config files */
......
...@@ -86,90 +86,58 @@ int Instance_options::complete_initialization(const char *default_path, ...@@ -86,90 +86,58 @@ int Instance_options::complete_initialization(const char *default_path,
int Instance_options::add_option(const char* option) int Instance_options::add_option(const char* option)
{ {
uint elements_count=0;
static const char socket[]= "--socket=";
static const char port[]= "--port=";
static const char datadir[]= "--datadir=";
static const char language[]= "--bind-address=";
static const char pid_file[]= "--pid-file=";
static const char path[]= "--mysqld_path=";
static const char user[]= "--admin_user=";
static const char password[]= "--admin_password=";
static const char guarded[]= "--guarded";
char *tmp; char *tmp;
enum { SAVE_VALUE= 1, SAVE_WHOLE, SAVE_WHOLE_AND_ADD };
if (!(tmp= strdup_root(&alloc, option))) struct selected_options_st
goto err;
/* To get rid the final zero in a string we subtract 1 from sizeof value */
if (strncmp(tmp, socket, sizeof socket - 1) == 0)
{
mysqld_socket= tmp;
goto add_options;
}
if (strncmp(tmp, port, sizeof port - 1) == 0)
{
mysqld_port= tmp;
goto add_options;
}
if (strncmp(tmp, datadir, sizeof datadir - 1) == 0)
{
mysqld_datadir= tmp;
goto add_options;
}
if (strncmp(tmp, language, sizeof language - 1) == 0)
{ {
mysqld_bind_address= tmp; const char *name;
goto add_options; uint length;
} const char **value;
uint type;
if (strncmp(tmp, pid_file, sizeof pid_file - 1) == 0) } options[]=
{ {
mysqld_pid_file= tmp; {"--socket=", 9, &mysqld_socket, SAVE_WHOLE_AND_ADD},
goto add_options; {"--port=", 7, &mysqld_port, SAVE_WHOLE_AND_ADD},
} {"--datadir=", 10, &mysqld_datadir, SAVE_WHOLE_AND_ADD},
{"--bind-address=", 15, &mysqld_bind_address, SAVE_WHOLE_AND_ADD},
{"--pid-file=", 11, &mysqld_pid_file, SAVE_WHOLE_AND_ADD},
{"--mysqld_path=", 14, &mysqld_path, SAVE_VALUE},
{"--admin_user=", 13, &mysqld_user, SAVE_VALUE},
{"--admin_password=", 17, &mysqld_password, SAVE_VALUE},
{"--guarded", 9, &is_guarded, SAVE_WHOLE},
{NULL, 0, NULL, 0}
};
struct selected_options_st *selected_options;
/* if (!(tmp= strdup_root(&alloc, option)))
We don't need a prefix in the next three optios. goto err;
We also don't need to add them to argv array =>
return instead of goto.
*/
if (strncmp(tmp, path, sizeof path - 1) == 0) for (selected_options= options; selected_options->name; selected_options++)
{ {
mysqld_path= strchr(tmp, '=') + 1; if (!strncmp(tmp, selected_options->name, selected_options->length))
switch(selected_options->type){
case SAVE_WHOLE_AND_ADD:
*(selected_options->value)= tmp;
insert_dynamic(&options_array,(gptr) &tmp);
return 0; return 0;
} case SAVE_VALUE:
*(selected_options->value)= strchr(tmp, '=') + 1;
if (strncmp(tmp, user, sizeof user - 1) == 0)
{
mysqld_user= strchr(tmp, '=') + 1;
return 0; return 0;
} case SAVE_WHOLE:
*(selected_options->value)= tmp;
if (strncmp(tmp, password, sizeof password - 1) == 0)
{
mysqld_password= strchr(tmp, '=') + 1;
return 0; return 0;
defaut:
break;
} }
if (strncmp(tmp, guarded, sizeof guarded - 1) == 0)
{
is_guarded= tmp;
return 0;
} }
add_options:
insert_dynamic(&options_array,(gptr) &tmp);
return 0; return 0;
err: err:
return 1; return 1;
} }
int Instance_options::add_to_argv(const char* option) int Instance_options::add_to_argv(const char* option)
{ {
DBUG_ASSERT(filled_default_options < MAX_NUMBER_OF_DEFAULT_OPTIONS); DBUG_ASSERT(filled_default_options < MAX_NUMBER_OF_DEFAULT_OPTIONS);
...@@ -191,7 +159,8 @@ int Instance_options::init(const char *instance_name_arg) ...@@ -191,7 +159,8 @@ int Instance_options::init(const char *instance_name_arg)
init_alloc_root(&alloc, MEM_ROOT_BLOCK_SIZE, 0); init_alloc_root(&alloc, MEM_ROOT_BLOCK_SIZE, 0);
my_init_dynamic_array(&options_array, sizeof(char *), 0, 32); if (my_init_dynamic_array(&options_array, sizeof(char *), 0, 32))
goto err;
if (!(instance_name= strmake_root(&alloc, (char *) instance_name_arg, if (!(instance_name= strmake_root(&alloc, (char *) instance_name_arg,
instance_name_len))) instance_name_len)))
......
...@@ -132,7 +132,7 @@ void Listener_thread::run() ...@@ -132,7 +132,7 @@ void Listener_thread::run()
/* set the socket nonblocking */ /* set the socket nonblocking */
flags= fcntl(ip_socket, F_GETFL, 0); flags= fcntl(ip_socket, F_GETFL, 0);
fcntl(ip_socket, F_SETFL, flags | O_NONBLOCK); fcntl(ip_socket, F_SETFL, flags | O_NONBLOCK);
/* make shure that instances won't be listening our sockets */ /* make sure that instances won't be listening our sockets */
flags= fcntl(ip_socket, F_GETFD, 0); flags= fcntl(ip_socket, F_GETFD, 0);
fcntl(ip_socket, F_SETFD, flags | FD_CLOEXEC); fcntl(ip_socket, F_SETFD, flags | FD_CLOEXEC);
...@@ -184,7 +184,7 @@ void Listener_thread::run() ...@@ -184,7 +184,7 @@ void Listener_thread::run()
/* set the socket nonblocking */ /* set the socket nonblocking */
flags= fcntl(unix_socket, F_GETFL, 0); flags= fcntl(unix_socket, F_GETFL, 0);
fcntl(unix_socket, F_SETFL, flags | O_NONBLOCK); fcntl(unix_socket, F_SETFL, flags | O_NONBLOCK);
/* make shure that instances won't be listening our sockets */ /* make sure that instances won't be listening our sockets */
flags= fcntl(unix_socket, F_GETFD, 0); flags= fcntl(unix_socket, F_GETFD, 0);
fcntl(unix_socket, F_SETFD, flags | FD_CLOEXEC); fcntl(unix_socket, F_SETFD, flags | FD_CLOEXEC);
} }
...@@ -253,6 +253,7 @@ void Listener_thread::run() ...@@ -253,6 +253,7 @@ void Listener_thread::run()
log_info("Listener_thread::run(): shutdown requested, exiting..."); log_info("Listener_thread::run(): shutdown requested, exiting...");
close(unix_socket); close(unix_socket);
close(ip_socket);
unlink(unix_socket_address.sun_path); unlink(unix_socket_address.sun_path);
} }
......
...@@ -65,7 +65,9 @@ void manager(const Options &options) ...@@ -65,7 +65,9 @@ void manager(const Options &options)
*/ */
User_map user_map; User_map user_map;
Instance_map instance_map; Instance_map instance_map(options.default_mysqld_path,
options.default_admin_user,
options.default_admin_password);
Guardian_thread guardian_thread(thread_registry, Guardian_thread guardian_thread(thread_registry,
&instance_map, &instance_map,
options.monitoring_interval); options.monitoring_interval);
...@@ -73,16 +75,10 @@ void manager(const Options &options) ...@@ -73,16 +75,10 @@ void manager(const Options &options)
Listener_thread_args listener_args(thread_registry, options, user_map, Listener_thread_args listener_args(thread_registry, options, user_map,
instance_map); instance_map);
instance_map.mysqld_path= options.default_mysqld_path;
instance_map.user= options.default_admin_user;
instance_map.password= options.default_admin_password;
instance_map.guardian= &guardian_thread; instance_map.guardian= &guardian_thread;
if (instance_map.init() || user_map.init() || instance_map.load() ||
if (instance_map.load()) user_map.load(options.password_file_name))
return;
if (user_map.load(options.password_file_name))
return; return;
/* write pid file */ /* write pid file */
......
...@@ -99,16 +99,22 @@ char *net_store_length(char *pkg, uint length) ...@@ -99,16 +99,22 @@ char *net_store_length(char *pkg, uint length)
} }
void store_to_string(Buffer *buf, const char *string, uint *position) int store_to_string(Buffer *buf, const char *string, uint *position)
{ {
uint currpos; uint currpos;
uint string_len; uint string_len;
string_len= strlen(string); string_len= strlen(string);
buf->reserve(*position, 2); if (buf->reserve(*position, 2))
goto err;
currpos= (net_store_length(buf->buffer + *position, string_len) - buf->buffer); currpos= (net_store_length(buf->buffer + *position, string_len) - buf->buffer);
buf->append(currpos, string, string_len); if (buf->append(currpos, string, string_len))
goto err;
*position= *position + string_len + (currpos - *position); *position= *position + string_len + (currpos - *position);
return 0;
err:
return 1;
} }
...@@ -134,7 +140,8 @@ int send_fields(struct st_net *net, LIST *fields) ...@@ -134,7 +140,8 @@ int send_fields(struct st_net *net, LIST *fields)
/* send the number of fileds */ /* send the number of fileds */
net_store_length(small_buff, (uint) list_length(fields)); net_store_length(small_buff, (uint) list_length(fields));
my_net_write(net, small_buff, (uint) 1); if (my_net_write(net, small_buff, (uint) 1))
goto err;
while (tmp) while (tmp)
{ {
...@@ -147,7 +154,8 @@ int send_fields(struct st_net *net, LIST *fields) ...@@ -147,7 +154,8 @@ int send_fields(struct st_net *net, LIST *fields)
store_to_string(&send_buff, (char *) "", &position); /* table name alias */ store_to_string(&send_buff, (char *) "", &position); /* table name alias */
store_to_string(&send_buff, field->name, &position); /* column name */ store_to_string(&send_buff, field->name, &position); /* column name */
store_to_string(&send_buff, field->name, &position); /* column name alias */ store_to_string(&send_buff, field->name, &position); /* column name alias */
send_buff.reserve(position, 12); if (send_buff.reserve(position, 12))
goto err;
send_buff.buffer[position++]= 12; send_buff.buffer[position++]= 12;
int2store(send_buff.buffer + position, 1); /* charsetnr */ int2store(send_buff.buffer + position, 1); /* charsetnr */
int4store(send_buff.buffer + position + 2, field->length); /* field length */ int4store(send_buff.buffer + position + 2, field->length); /* field length */
...@@ -162,7 +170,7 @@ int send_fields(struct st_net *net, LIST *fields) ...@@ -162,7 +170,7 @@ int send_fields(struct st_net *net, LIST *fields)
tmp= rest(tmp); tmp= rest(tmp);
} }
if ( my_net_write(net, eof_buff, 1)) if (my_net_write(net, eof_buff, 1))
goto err; goto err;
return 0; return 0;
......
...@@ -36,7 +36,7 @@ int send_fields(struct st_net *net, LIST *fields); ...@@ -36,7 +36,7 @@ int send_fields(struct st_net *net, LIST *fields);
char *net_store_length(char *pkg, uint length); char *net_store_length(char *pkg, uint length);
void store_to_string(Buffer *buf, const char *string, uint *position); int store_to_string(Buffer *buf, const char *string, uint *position);
int send_eof(struct st_net *net); int send_eof(struct st_net *net);
......
...@@ -93,13 +93,16 @@ static void delete_user(void *u) ...@@ -93,13 +93,16 @@ static void delete_user(void *u)
C_MODE_END C_MODE_END
User_map::User_map() int User_map::init()
{ {
enum { START_HASH_SIZE = 16 }; enum { START_HASH_SIZE = 16 };
hash_init(&hash, default_charset_info, START_HASH_SIZE, 0, 0, if (hash_init(&hash, default_charset_info, START_HASH_SIZE, 0, 0,
get_user_key, delete_user, 0); get_user_key, delete_user, 0))
return 1;
return 0;
} }
User_map::~User_map() User_map::~User_map()
{ {
hash_free(&hash); hash_free(&hash);
...@@ -134,7 +137,8 @@ int User_map::load(const char *password_file_name) ...@@ -134,7 +137,8 @@ int User_map::load(const char *password_file_name)
while (fgets(line, sizeof(line), file)) while (fgets(line, sizeof(line), file))
{ {
/* skip comments and empty lines */ /* skip comments and empty lines */
if (line[0] == '#' || line[0] == '\n' && line[1] == '\0') if (line[0] == '#' || line[0] == '\n' &&
(line[1] == '\0' || line[1] == '\r'))
continue; continue;
if ((user= new User) == 0) if ((user= new User) == 0)
goto done; goto done;
......
...@@ -31,9 +31,9 @@ ...@@ -31,9 +31,9 @@
class User_map class User_map
{ {
public: public:
User_map();
~User_map(); ~User_map();
int init();
int load(const char *password_file_name); int load(const char *password_file_name);
int authenticate(const char *user_name, uint length, int authenticate(const char *user_name, uint length,
const char *scrambled_password, const char *scrambled_password,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment