#At file:///home/thek/Development/cpp/mysqlbzr/51-bug19027/ based on revid:holyfoot@stripped
2877 Kristofer Pettersson 2009-04-30
Bug#19027 MySQL 5.0 starts even with Fatal InnoDB errors
It is not possible to prevent the server from starting if a mandatory
built-in plugin fails to start. This can in some cases lead to data
corruption when the old table name space suddenly is used by a different
storage engine.
A boolean command line option in the form of --foobar is automatically
created for every existing plugin "foobar". By changing this command line
option from a boolean to a tristate { OFF, ON, FORCE } it is possible to
specify the plugin loading policy for each plugin.
The behavior is specified as follows:
OFF = Disable the plugin and start the server
ON = Enable the plugin and start the server even if an error occurrs
during plugin initialization.
FORCE = Enable the plugin but don't start the server if an error occurrs
during plugin initialization.
@ mysql-test/lib/mtr_cases.pm
* Changed --<pluginname> from a boolean to a tristate. construct_help_options() changes the value of mysqld_variables{'innodb'}
@ mysys/my_getopt.c
* Changed --<pluginname> from boolean to tristate. Optional arguments
must still work for tristates. It is praxis that disable means value 0
and enable is value 1. Since plugin name is the only tristate with
optional arguments this principle will still hold.
@ sql/sql_plugin.cc
* Changed --<pluginname> option from a boolean type to a tristate.
- FORCE will now terminate the server if the plugin fails to
initialize properly.
* Refactored prototypes for test_plugin_options() and construct_options()
to get rid of the 'enable' value pointer.
* Added documentation
@ sql/sql_plugin.h
* Introduced new member to st_plugin_int structure.
modified:
mysql-test/lib/mtr_cases.pm
mysys/my_getopt.c
sql/sql_plugin.cc
sql/sql_plugin.h
=== modified file 'mysql-test/lib/mtr_cases.pm'
--- a/mysql-test/lib/mtr_cases.pm 2009-01-21 10:17:16 +0000
+++ b/mysql-test/lib/mtr_cases.pm 2009-04-30 13:29:27 +0000
@@ -887,7 +887,7 @@ sub collect_one_test_case {
if ( $tinfo->{'innodb_test'} )
{
# This is a test that need innodb
- if ( $::mysqld_variables{'innodb'} ne "TRUE" )
+ if ( $::mysqld_variables{'innodb'} eq "OFF" )
{
# innodb is not supported, skip it
$tinfo->{'skip'}= 1;
=== modified file 'mysys/my_getopt.c'
--- a/mysys/my_getopt.c 2009-03-17 17:24:35 +0000
+++ b/mysys/my_getopt.c 2009-04-30 13:29:27 +0000
@@ -410,7 +410,8 @@ invalid value '%s'",
argument= optend;
}
else if (optp->arg_type == OPT_ARG &&
- (optp->var_type & GET_TYPE_MASK) == GET_BOOL)
+ (((optp->var_type & GET_TYPE_MASK) == GET_BOOL) ||
+ (optp->var_type & GET_TYPE_MASK) == GET_ENUM))
{
if (optend == disabled_my_option)
*((my_bool*) value)= (my_bool) 0;
=== modified file 'sql/sql_plugin.cc'
--- a/sql/sql_plugin.cc 2009-03-16 10:37:13 +0000
+++ b/sql/sql_plugin.cc 2009-04-30 13:29:27 +0000
@@ -29,6 +29,17 @@
extern struct st_mysql_plugin *mysqld_builtins[];
+/**
+ @note The order of the enumeration is critical.
+ @see construct_options
+*/
+static const char *global_plugin_typelib_names[]=
+ { "OFF", "ON", "FORCE", NULL };
+TYPELIB global_plugin_typelib= { array_elements(global_plugin_typelib_names)-1,
+ "", global_plugin_typelib_names,
+ NULL };
+
+
char *opt_plugin_load= NULL;
char *opt_plugin_dir_ptr;
char opt_plugin_dir[FN_REFLEN];
@@ -192,7 +203,7 @@ static void plugin_load(MEM_ROOT *tmp_ro
static bool plugin_load_list(MEM_ROOT *tmp_root, int *argc, char **argv,
const char *list);
static int test_plugin_options(MEM_ROOT *, struct st_plugin_int *,
- int *, char **, my_bool);
+ int *, char **);
static bool register_builtin(struct st_mysql_plugin *, struct st_plugin_int *,
struct st_plugin_int **);
static void unlock_variables(THD *thd, struct system_variables *vars);
@@ -751,7 +762,7 @@ static bool plugin_add(MEM_ROOT *tmp_roo
tmp.name.length= name_len;
tmp.ref_count= 0;
tmp.state= PLUGIN_IS_UNINITIALIZED;
- if (test_plugin_options(tmp_root, &tmp, argc, argv, true))
+ if (test_plugin_options(tmp_root, &tmp, argc, argv))
tmp.state= PLUGIN_IS_DISABLED;
if ((tmp_plugin_ptr= plugin_insert_or_reuse(&tmp)))
@@ -997,7 +1008,6 @@ static int plugin_initialize(struct st_p
DBUG_ENTER("plugin_initialize");
safe_mutex_assert_owner(&LOCK_plugin);
-
if (plugin_type_initialize[plugin->plugin->type])
{
if ((*plugin_type_initialize[plugin->plugin->type])(plugin))
@@ -1094,11 +1104,12 @@ uchar *get_bookmark_hash_key(const uchar
int plugin_init(int *argc, char **argv, int flags)
{
uint i;
- bool def_enabled, is_myisam;
+ bool is_myisam;
struct st_mysql_plugin **builtins;
struct st_mysql_plugin *plugin;
struct st_plugin_int tmp, *plugin_ptr, **reap;
MEM_ROOT tmp_root;
+ bool reaped_mandatory_plugin= FALSE;
DBUG_ENTER("plugin_init");
if (initialized)
@@ -1142,17 +1153,13 @@ int plugin_init(int *argc, char **argv,
!my_strnncoll(&my_charset_latin1, (const uchar*) plugin->name,
6, (const uchar*) "InnoDB", 6))
continue;
- /* by default, ndbcluster and federated are disabled */
- def_enabled=
- my_strcasecmp(&my_charset_latin1, plugin->name, "NDBCLUSTER") != 0 &&
- my_strcasecmp(&my_charset_latin1, plugin->name, "FEDERATED") != 0;
bzero(&tmp, sizeof(tmp));
tmp.plugin= plugin;
tmp.name.str= (char *)plugin->name;
tmp.name.length= strlen(plugin->name);
tmp.state= 0;
free_root(&tmp_root, MYF(MY_MARK_BLOCKS_FREE));
- if (test_plugin_options(&tmp_root, &tmp, argc, argv, def_enabled))
+ if (test_plugin_options(&tmp_root, &tmp, argc, argv))
tmp.state= PLUGIN_IS_DISABLED;
else
tmp.state= PLUGIN_IS_UNINITIALIZED;
@@ -1227,6 +1234,8 @@ int plugin_init(int *argc, char **argv,
while ((plugin_ptr= *(--reap)))
{
pthread_mutex_unlock(&LOCK_plugin);
+ if (plugin_ptr->is_mandatory)
+ reaped_mandatory_plugin= TRUE;
plugin_deinitialize(plugin_ptr, true);
pthread_mutex_lock(&LOCK_plugin);
plugin_del(plugin_ptr);
@@ -1234,6 +1243,8 @@ int plugin_init(int *argc, char **argv,
pthread_mutex_unlock(&LOCK_plugin);
my_afree(reap);
+ if (reaped_mandatory_plugin)
+ goto err;
end:
free_root(&tmp_root, MYF(0));
@@ -1299,7 +1310,7 @@ bool plugin_register_builtin(THD *thd, s
pthread_mutex_lock(&LOCK_plugin);
rw_wrlock(&LOCK_system_variables_hash);
- if (test_plugin_options(thd->mem_root, &tmp, &dummy_argc, NULL, true))
+ if (test_plugin_options(thd->mem_root, &tmp, &dummy_argc, NULL))
goto end;
tmp.state= PLUGIN_IS_UNINITIALIZED;
if ((result= register_builtin(plugin, &tmp, &ptr)))
@@ -2889,21 +2900,39 @@ my_bool get_one_plugin_option(int optid
}
+/**
+ Creates a set of my_option objects associated with a specified plugin-
+ handle.
+
+ @param mem_root Memory allocator to be used.
+ @param tmp A pointer to a plugin handle
+ @param[out] options A pointer to a pre-allocated static array
+
+ The set is stored in the pre-allocated static array supplied to the function.
+ The size of the array is calculated as (number_of_plugin_varaibles*2+3). The
+ reason is that each option can have a prefix '--plugin-' in addtion to the
+ shorter form '--<plugin-name>'. There is also space allocated for
+ terminating NULL pointers.
+
+ @return
+ @retval -1 An error occurred
+ @retval 0 Success
+*/
+
static int construct_options(MEM_ROOT *mem_root, struct st_plugin_int *tmp,
- my_option *options, my_bool **enabled,
- bool can_disable)
+ my_option *options)
{
const char *plugin_name= tmp->plugin->name;
uint namelen= strlen(plugin_name), optnamelen;
- uint buffer_length= namelen * 4 + (can_disable ? 75 : 10);
- char *name= (char*) alloc_root(mem_root, buffer_length) + 1;
+ char *name= (char*) alloc_root(mem_root, namelen*2 + 10);
+ const int comment_length= 80+(namelen*2+10)+strlen(plugin_name);
+ char *comment= (char *) alloc_root(mem_root, comment_length);
+
char *optname, *p;
int index= 0, offset= 0;
st_mysql_sys_var *opt, **plugin_option;
st_bookmark *v;
DBUG_ENTER("construct_options");
- DBUG_PRINT("plugin", ("plugin: '%s' enabled: %d can_disable: %d",
- plugin_name, **enabled, can_disable));
/* support --skip-plugin-foo syntax */
memcpy(name, plugin_name, namelen + 1);
@@ -2915,33 +2944,30 @@ static int construct_options(MEM_ROOT *m
if (*p == '_')
*p= '-';
- if (can_disable)
- {
- strxmov(name + namelen*2 + 10, "Enable ", plugin_name, " plugin. "
- "Disable with --skip-", name," (will save memory).", NullS);
- /*
- Now we have namelen * 2 + 10 (one char unused) + 7 + namelen + 9 +
- 20 + namelen + 20 + 1 == namelen * 4 + 67.
- */
-
- options[0].comment= name + namelen*2 + 10;
- }
+ options[1].name= (options[0].name= name) + namelen + 1;
+ options[0].id= options[1].id= 256; /* must be >255. dup id ok */
+ options[0].var_type= options[1].var_type= GET_ENUM;
+ options[0].arg_type= options[1].arg_type= OPT_ARG;
+ options[0].def_value= options[1].def_value= 1; /* ON */
+ options[0].typelib= options[1].typelib= &global_plugin_typelib;
/*
- NOTE: 'name' is one char above the allocated buffer!
- NOTE: This code assumes that 'my_bool' and 'char' are of same size.
+ NOTE: Make sure that comment doesn't become longer than comment_length
*/
- *((my_bool *)(name -1))= **enabled;
- *enabled= (my_bool *)(name - 1);
+ strxmov(comment, "Enable or disable ", plugin_name, " plugin. "
+ "Disable with --skip-", name," (will save memory).", NullS);
+ options[0].comment= comment;
+ /*
+ Allocate temporary space for the value of the tristate.
+ This option will have a limited lifetime and is not used beyond
+ server initialization.
+ */
+ options[0].value= options[1].value= (uchar **)alloc_root(mem_root,
+ sizeof(my_option **));
+ *((uint*) options[0].value)= *((uint*) options[1].value)=
+ (uint) options[0].def_value;
- options[1].name= (options[0].name= name) + namelen + 1;
- options[0].id= options[1].id= 256; /* must be >255. dup id ok */
- options[0].var_type= options[1].var_type= GET_BOOL;
- options[0].arg_type= options[1].arg_type= NO_ARG;
- options[0].def_value= options[1].def_value= **enabled;
- options[0].value= options[0].u_max_value=
- options[1].value= options[1].u_max_value= (uchar**) (name - 1);
options+= 2;
/*
@@ -3020,9 +3046,9 @@ static int construct_options(MEM_ROOT *m
if (!opt->update)
{
opt->update= update_func_str;
- if (!(opt->flags & PLUGIN_VAR_MEMALLOC | PLUGIN_VAR_READONLY))
+ if (!(opt->flags & (PLUGIN_VAR_MEMALLOC | PLUGIN_VAR_READONLY)))
{
- opt->flags|= PLUGIN_VAR_READONLY;
+ opt->flags|= (PLUGIN_VAR_READONLY);
sql_print_warning("Server variable %s of plugin %s was forced "
"to be read-only: string variable without "
"update_func and PLUGIN_VAR_MEMALLOC flag",
@@ -3105,7 +3131,7 @@ static int construct_options(MEM_ROOT *m
options[1]= options[0];
options[1].name= p= (char*) alloc_root(mem_root, optnamelen + 8);
- options[1].comment= 0; // hidden
+ options[1].comment= 0; /* Hidden from the help text */
strxmov(p, "plugin-", optname, NullS);
options+= 2;
@@ -3120,50 +3146,52 @@ static my_option *construct_help_options
{
st_mysql_sys_var **opt;
my_option *opts;
- my_bool dummy, can_disable;
- my_bool *dummy2= &dummy;
uint count= EXTRA_OPTIONS;
DBUG_ENTER("construct_help_options");
- for (opt= p->plugin->system_vars; opt && *opt; opt++, count+= 2);
+ for (opt= p->plugin->system_vars; opt && *opt; opt++, count+= 2)
+ ;
if (!(opts= (my_option*) alloc_root(mem_root, sizeof(my_option) * count)))
DBUG_RETURN(NULL);
bzero(opts, sizeof(my_option) * count);
- dummy= TRUE; /* plugin is enabled. */
-
- can_disable=
- my_strcasecmp(&my_charset_latin1, p->name.str, "MyISAM") &&
- my_strcasecmp(&my_charset_latin1, p->name.str, "MEMORY");
-
- if (construct_options(mem_root, p, opts, &dummy2, can_disable))
+ if (construct_options(mem_root, p, opts))
DBUG_RETURN(NULL);
DBUG_RETURN(opts);
}
-/*
- SYNOPSIS
- test_plugin_options()
- tmp_root temporary scratch space
- plugin internal plugin structure
- argc user supplied arguments
- argv user supplied arguments
- default_enabled default plugin enable status
- RETURNS:
- 0 SUCCESS - plugin should be enabled/loaded
- NOTE:
- Requires that a write-lock is held on LOCK_system_variables_hash
+/**
+ Create and register system variables supplied from the plugin and
+ assigns initial values from corresponding command line arguments.
+
+ @param tmp_root Temporary scratch space
+ @param[out] plugin Internal plugin structure
+ @param argc Number of command line arguments
+ @param argv Command line argument vector
+
+ The plugin will be updated with a policy on how to handle errors during
+ initialization.
+
+ @note Requires that a write-lock is held on LOCK_system_variables_hash
+
+ @return How initialization of the plugin should be handled.
+ @retval 0 Initialization should proceed.
+ @retval 1 Plugin is disabled.
+
*/
+
static int test_plugin_options(MEM_ROOT *tmp_root, struct st_plugin_int *tmp,
- int *argc, char **argv, my_bool default_enabled)
+ int *argc, char **argv)
{
struct sys_var_chain chain= { NULL, NULL };
- my_bool enabled_saved= default_enabled, can_disable;
- my_bool *enabled= &default_enabled;
+ my_bool can_disable;
+ bool disable_plugin= FALSE;
+ uint plugin_load_policy= 0; /* 0 = OFF, 1 = ON, 2 = FORCE */
+
MEM_ROOT *mem_root= alloc_root_inited(&tmp->mem_root) ?
&tmp->mem_root : &plugin_mem_root;
st_mysql_sys_var **opt;
@@ -3177,13 +3205,17 @@ static int test_plugin_options(MEM_ROOT
DBUG_ENTER("test_plugin_options");
DBUG_ASSERT(tmp->plugin && tmp->name.str);
+ /*
+ The 'federated' and 'ndbcluster' storage engines are always disabled by
+ default.
+ */
+ if (!(my_strcasecmp(&my_charset_latin1, tmp->name.str, "federated") &&
+ my_strcasecmp(&my_charset_latin1, tmp->name.str, "ndbcluster")))
+ disable_plugin= TRUE;
+
for (opt= tmp->plugin->system_vars; opt && *opt; opt++)
count+= 2; /* --{plugin}-{optname} and --plugin-{plugin}-{optname} */
- can_disable=
- my_strcasecmp(&my_charset_latin1, tmp->name.str, "MyISAM") &&
- my_strcasecmp(&my_charset_latin1, tmp->name.str, "MEMORY");
-
if (count > EXTRA_OPTIONS || (*argc > 1))
{
if (!(opts= (my_option*) alloc_root(tmp_root, sizeof(my_option) * count)))
@@ -3193,7 +3225,7 @@ static int test_plugin_options(MEM_ROOT
}
bzero(opts, sizeof(my_option) * count);
- if (construct_options(tmp_root, tmp, opts, &enabled, can_disable))
+ if (construct_options(tmp_root, tmp, opts))
{
sql_print_error("Bad options for plugin '%s'.", tmp->name.str);
DBUG_RETURN(-1);
@@ -3208,64 +3240,83 @@ static int test_plugin_options(MEM_ROOT
tmp->name.str);
goto err;
}
+ /*
+ Set plugin loading policy from option value. First element in the option
+ list is always the <plugin name> option value.
+ */
+ plugin_load_policy= (uint)*opts[0].value;
+ if (plugin_load_policy == 0)
+ disable_plugin= TRUE;
+ else
+ disable_plugin= FALSE;
}
- if (!*enabled && !can_disable)
+ /*
+ The 'MyISAM' and 'Memory' storage engines currently can't be disabled.
+ TODO It really isn't possible to select --myisam=OFF as this option name
+ isn't registered.
+ */
+ can_disable=
+ my_strcasecmp(&my_charset_latin1, tmp->name.str, "MyISAM") &&
+ my_strcasecmp(&my_charset_latin1, tmp->name.str, "MEMORY");
+
+ tmp->is_mandatory= (plugin_load_policy == 2 ? TRUE:FALSE);
+
+ if (disable_plugin && !can_disable)
{
sql_print_warning("Plugin '%s' cannot be disabled", tmp->name.str);
- *enabled= TRUE;
+ disable_plugin= FALSE;
}
- error= 1;
+ /*
+ If the plugin is disabled it should not be initialized.
+ */
+ if (disable_plugin)
+ {
+ sql_print_information("Plugin '%s' is disabled.",
+ tmp->name.str);
+ DBUG_RETURN(1);
+ }
- if (*enabled)
+ error= 1;
+ for (opt= tmp->plugin->system_vars; opt && *opt; opt++)
{
- for (opt= tmp->plugin->system_vars; opt && *opt; opt++)
+ if (((o= *opt)->flags & PLUGIN_VAR_NOSYSVAR))
+ continue;
+ if ((var= find_bookmark(tmp->name.str, o->name, o->flags)))
+ v= new (mem_root) sys_var_pluginvar(var->key + 1, o);
+ else
{
- if (((o= *opt)->flags & PLUGIN_VAR_NOSYSVAR))
- continue;
-
- if ((var= find_bookmark(tmp->name.str, o->name, o->flags)))
- v= new (mem_root) sys_var_pluginvar(var->key + 1, o);
- else
- {
- len= tmp->name.length + strlen(o->name) + 2;
- varname= (char*) alloc_root(mem_root, len);
- strxmov(varname, tmp->name.str, "-", o->name, NullS);
- my_casedn_str(&my_charset_latin1, varname);
-
- for (p= varname; *p; p++)
- if (*p == '-')
- *p= '_';
-
- v= new (mem_root) sys_var_pluginvar(varname, o);
- }
- DBUG_ASSERT(v); /* check that an object was actually constructed */
-
- /*
- Add to the chain of variables.
- Done like this for easier debugging so that the
- pointer to v is not lost on optimized builds.
- */
- v->chain_sys_var(&chain);
- }
- if (chain.first)
+ len= tmp->name.length + strlen(o->name) + 2;
+ varname= (char*) alloc_root(mem_root, len);
+ strxmov(varname, tmp->name.str, "-", o->name, NullS);
+ my_casedn_str(&my_charset_latin1, varname);
+ for (p= varname; *p; p++)
+ if (*p == '-')
+ *p= '_';
+ v= new (mem_root) sys_var_pluginvar(varname, o);
+ }
+ DBUG_ASSERT(v); /* check that an object was actually constructed */
+ /*
+ Add to the chain of variables.
+ Done like this for easier debugging so that the
+ pointer to v is not lost on optimized builds.
+ */
+ v->chain_sys_var(&chain);
+ } // end for
+ if (chain.first)
+ {
+ chain.last->next = NULL;
+ if (mysql_add_sys_var_chain(chain.first, NULL))
{
- chain.last->next = NULL;
- if (mysql_add_sys_var_chain(chain.first, NULL))
- {
- sql_print_error("Plugin '%s' has conflicting system variables",
- tmp->name.str);
- goto err;
- }
- tmp->system_vars= chain.first;
+ sql_print_error("Plugin '%s' has conflicting system variables",
+ tmp->name.str);
+ goto err;
}
- DBUG_RETURN(0);
+ tmp->system_vars= chain.first;
}
-
- if (enabled_saved && global_system_variables.log_warnings)
- sql_print_information("Plugin '%s' disabled by command line option",
- tmp->name.str);
+ DBUG_RETURN(0);
+
err:
if (opts)
my_cleanup_options(opts);
=== modified file 'sql/sql_plugin.h'
--- a/sql/sql_plugin.h 2008-12-17 15:45:34 +0000
+++ b/sql/sql_plugin.h 2009-04-30 13:29:27 +0000
@@ -79,6 +79,7 @@ struct st_plugin_int
void *data; /* plugin type specific, e.g. handlerton */
MEM_ROOT mem_root; /* memory for dynamic plugin structures */
sys_var *system_vars; /* server variables for this plugin */
+ bool is_mandatory; /* If true then plugin must not fail to load */
};
Attachment: [text/bzr-bundle] bzr/kristofer.pettersson@sun.com-20090430132927-i17lbc1v9pvt98xm.bundle
| Thread |
|---|
| • bzr commit into mysql-5.1-bugteam branch (kristofer.pettersson:2877)Bug#19027 | Kristofer Pettersson | 30 Apr |
