Re: [Maria-developers] Rev 3876: MDEV-4472 Audit-plugin. Server-related part of the task
Hi, Holyfoot! Mostly ok. There are few comments, see below: On Sep 03, holyfoot@askmonty.org wrote:
------------------------------------------------------------ revno: 3876 revision-id: holyfoot@askmonty.org-20130903082521-xpia2wond6oggpsk parent: igor@askmonty.org-20130831163309-vqtls4oubhvz9zcz committer: Alexey Botchkov <holyfoot@askmonty.org> branch nick: 55-4472 timestamp: Tue 2013-09-03 13:25:21 +0500 message: MDEV-4472 Audit-plugin. Server-related part of the task. file_logger became the service. Data like query_id now are sent to the audit plugin. Fix for MDEV-4770 ported from 10.0. Fix added for the read_maria_plugin_info().
=== modified file 'include/mysql/plugin_audit.h' --- a/include/mysql/plugin_audit.h 2013-04-19 10:50:16 +0000 +++ b/include/mysql/plugin_audit.h 2013-09-03 08:25:21 +0000 @@ -25,7 +25,7 @@
#define MYSQL_AUDIT_CLASS_MASK_SIZE 1
-#define MYSQL_AUDIT_INTERFACE_VERSION 0x0301 +#define MYSQL_AUDIT_INTERFACE_VERSION 0x0302
/************************************************************************* @@ -59,6 +59,10 @@ struct mysql_event_general struct charset_info_st *general_charset; unsigned long long general_time; unsigned long long general_rows; + /* Added for 0x302 */
A bit confusing. Better "Added in version 0x0302"
+ long long query_id;
should it rather be unsigned?
+ const char *database; + unsigned int database_length; };
@@ -140,6 +144,8 @@ struct mysql_event_table unsigned int new_database_length; const char *new_table; unsigned int new_table_length; + /* Added for 0x302 */ + long long query_id; };
/*************************************************************************
=== renamed file 'plugin/sql_errlog/sql_logger.cc' => 'mysys/file_logger.c' --- a/plugin/sql_errlog/sql_logger.cc 2013-06-22 12:02:03 +0000 +++ b/mysys/file_logger.c 2013-09-03 08:25:21 +0000 @@ -51,6 +53,12 @@ LOGGER_HANDLE *logger_open(const char *p unsigned int rotations) { LOGGER_HANDLE new_log, *l_perm; + char *local_mysql_data_home; +#ifdef _WIN32 + local_mysql_data_home= (char *)GetProcAddress(0, "mysql_data_home"); +#else + local_mysql_data_home= mysql_data_home; +#endif /*_WIN32*/
You shouldn't need it, file_logger.c is part of the server, not a dynamically loaded plugin.
/* I don't think we ever need more rotations, === modified file 'sql/sql_audit.h' --- a/sql/sql_audit.h 2013-04-19 10:50:16 +0000 +++ b/sql/sql_audit.h 2013-09-03 08:25:21 +0000 @@ -93,7 +93,8 @@ void mysql_audit_general_log(THD *thd, t
mysql_audit_notify(thd, MYSQL_AUDIT_GENERAL_CLASS, MYSQL_AUDIT_GENERAL_LOG, 0, time, user, userlen, cmd, cmdlen, - query, querylen, clientcs, 0); + query, querylen, clientcs, (ha_rows) 0, + thd->db, thd->db_length);
no query_id?
} }
@@ -139,7 +140,8 @@ void mysql_audit_general(THD *thd, uint
mysql_audit_notify(thd, MYSQL_AUDIT_GENERAL_CLASS, event_subtype, error_code, time, user, userlen, msg, msglen, - query.str(), query.length(), query.charset(), rows); + query.str(), query.length(), query.charset(), rows, + thd->db, thd->db_length);
no query_id?
} }
=== modified file 'sql/sql_plugin.cc' --- a/sql/sql_plugin.cc 2013-06-13 18:19:11 +0000 +++ b/sql/sql_plugin.cc 2013-09-03 08:25:21 +0000 @@ -2645,13 +2647,16 @@ static void update_func_longlong(THD *th static void update_func_str(THD *thd, struct st_mysql_sys_var *var, void *tgt, const void *save) { - char *old= *(char **) tgt; - *(char **)tgt= *(char **) save; + char *value= *(char**) save; if (var->flags & PLUGIN_VAR_MEMALLOC) { - *(char **)tgt= my_strdup(*(char **) save, MYF(0)); + char *old= *(char**) tgt; + if (value) + *(char**) tgt= my_strdup(value, MYF(0));
add else *(char**) tgt= 0;
my_free(old); } + else + *(char**) tgt= value; }
Regards, Sergei
03.09.2013 16:02, Sergei Golubchik wrote:
Hi, Holyfoot!
unsigned long long general_rows; + /* Added for 0x302 */ A bit confusing. Better "Added in version 0x0302"
Fixed.
+ long long query_id; should it rather be unsigned?
For the THD structure we have: typedef int64 query_id_t; So that it's originally unsigned.
+ char *local_mysql_data_home; +#ifdef _WIN32 + local_mysql_data_home= (char *)GetProcAddress(0, "mysql_data_home"); +#else + local_mysql_data_home= mysql_data_home; +#endif /*_WIN32*/ You shouldn't need it, file_logger.c is part of the server, not a dynamically loaded plugin.
For now the plugin supposed to handle older Maria's and even MySQL. So we have to include the file-logger code in there.
/* I don't think we ever need more rotations, === modified file 'sql/sql_audit.h' --- a/sql/sql_audit.h 2013-04-19 10:50:16 +0000 +++ b/sql/sql_audit.h 2013-09-03 08:25:21 +0000 @@ -93,7 +93,8 @@ void mysql_audit_general_log(THD *thd, t
mysql_audit_notify(thd, MYSQL_AUDIT_GENERAL_CLASS, MYSQL_AUDIT_GENERAL_LOG, 0, time, user, userlen, cmd, cmdlen, - query, querylen, clientcs, 0); + query, querylen, clientcs, (ha_rows) 0, + thd->db, thd->db_length);
no query_id?
} }
@@ -139,7 +140,8 @@ void mysql_audit_general(THD *thd, uint
mysql_audit_notify(thd, MYSQL_AUDIT_GENERAL_CLASS, event_subtype, error_code, time, user, userlen, msg, msglen, - query.str(), query.length(), query.charset(), rows); + query.str(), query.length(), query.charset(), rows, + thd->db, thd->db_length);
no query_id?
No query_id. An appropriate 'handler' is called inside the mysql_audit_notify for each type of events, receiving the 'thd' as a parameter. There the event->query_id is actually set. See general_class_handler() in sql/sql_audit.cc for example.
} }
=== modified file 'sql/sql_plugin.cc' --- a/sql/sql_plugin.cc 2013-06-13 18:19:11 +0000 +++ b/sql/sql_plugin.cc 2013-09-03 08:25:21 +0000 @@ -2645,13 +2647,16 @@ static void update_func_longlong(THD *th static void update_func_str(THD *thd, struct st_mysql_sys_var *var, void *tgt, const void *save) { - char *old= *(char **) tgt; - *(char **)tgt= *(char **) save; + char *value= *(char**) save; if (var->flags & PLUGIN_VAR_MEMALLOC) { - *(char **)tgt= my_strdup(*(char **) save, MYF(0)); + char *old= *(char**) tgt; + if (value) + *(char**) tgt= my_strdup(value, MYF(0));
add
else *(char**) tgt= 0;
my_free(old); } + else + *(char**) tgt= value; }
Added. Best regards. HF
Hi, Alexey! On Sep 04, Alexey Botchkov wrote:
03.09.2013 16:02, Sergei Golubchik wrote:
+ long long query_id; should it rather be unsigned?
For the THD structure we have: typedef int64 query_id_t; So that it's originally unsigned.
You mean "originally signed". Yes, I know. But I think that for API purposes we'd better make it unsigned.
+ char *local_mysql_data_home; +#ifdef _WIN32 + local_mysql_data_home= (char *)GetProcAddress(0, "mysql_data_home"); +#else + local_mysql_data_home= mysql_data_home; +#endif /*_WIN32*/ You shouldn't need it, file_logger.c is part of the server, not a dynamically loaded plugin.
For now the plugin supposed to handle older Maria's and even MySQL. So we have to include the file-logger code in there.
Yes. But this file is part of the server code, not part of the plugin. The plugin is not even in the tree.
=== modified file 'sql/sql_audit.h' --- a/sql/sql_audit.h 2013-04-19 10:50:16 +0000 +++ b/sql/sql_audit.h 2013-09-03 08:25:21 +0000 @@ -139,7 +140,8 @@ void mysql_audit_general(THD *thd, uint
mysql_audit_notify(thd, MYSQL_AUDIT_GENERAL_CLASS, event_subtype, error_code, time, user, userlen, msg, msglen, - query.str(), query.length(), query.charset(), rows); + query.str(), query.length(), query.charset(), rows, + thd->db, thd->db_length); no query_id?
No query_id. An appropriate 'handler' is called inside the mysql_audit_notify for each type of events, receiving the 'thd' as a parameter. There the event->query_id is actually set. See general_class_handler() in sql/sql_audit.cc for example.
Right. Sorry. I've missed that. Regards, Sergei
04.09.2013 16:06, Sergei Golubchik wrote:
Hi, Alexey!
On Sep 04, Alexey Botchkov wrote:
03.09.2013 16:02, Sergei Golubchik wrote:
+ long long query_id; should it rather be unsigned? For the THD structure we have: typedef int64 query_id_t; So that it's originally unsigned. You mean "originally signed". Yes, I know. But I think that for API purposes we'd better make it unsigned.
Ok, the event->query_id is of 'unsigned long long' type now.
+ char *local_mysql_data_home; +#ifdef _WIN32 + local_mysql_data_home= (char *)GetProcAddress(0, "mysql_data_home"); +#else + local_mysql_data_home= mysql_data_home; +#endif /*_WIN32*/ You shouldn't need it, file_logger.c is part of the server, not a dynamically loaded plugin. For now the plugin supposed to handle older Maria's and even MySQL. So we have to include the file-logger code in there. Yes. But this file is part of the server code, not part of the plugin. The plugin is not even in the tree.
I removed the local_mysql_data_home. Just the declaration and the use of the mysql_data_home left. Best regards. HF
participants (2)
-
Alexey Botchkov -
Sergei Golubchik