[tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module
Georgy Kirichenko
georgy at tarantool.org
Wed Jul 11 08:25:13 MSK 2018
Ok for me
On Tuesday, July 10, 2018 9:49:14 AM MSK Kirill Shcherbatov wrote:
> Tnx for comment; I've changed commit message and fixed few
> codestyle things which I missed yesterday.
>
> Closes #2946.
>
> @TarantoolBot document
> Title: fixed module reload
> There was a bug in tarantool documentation:
> https://tarantool.io/en/doc/1.7/book/box/
> box_schema/#lua-function.box.schema.func.reload
> Now it is allowed to reload all functions in loadable
> module via one method.
> box.schema.func.reload("utils") -- ok since now
>
> Global reload is still unsupported because it seems
> to be useless.
> box.schema.func.reload() -- invalid!
> ---
> https://github.com/tarantool/tarantool/compare/kshch/gh-2946-module-reload
> https://github.com/tarantool/tarantool/issues/2946
>
> src/box/call.c | 37 ++++++++++++++++---------------------
> src/box/call.h | 18 ++++++++++++++++++
> src/box/func.c | 20 +++++++++++++++++---
> src/box/func.h | 15 +++++++++++++++
> test/box/func_reload.result | 7 +++++++
> test/box/func_reload.test.lua | 2 ++
> 6 files changed, 75 insertions(+), 24 deletions(-)
>
> diff --git a/src/box/call.c b/src/box/call.c
> index 438be19..ef30abc 100644
> --- a/src/box/call.c
> +++ b/src/box/call.c
> @@ -42,20 +42,8 @@
> #include "rmean.h"
> #include "small/obuf.h"
>
> -/**
> - * Find a function by name and check "EXECUTE" permissions.
> - *
> - * @param name function name
> - * @param name_len length of @a name
> - * @param[out] funcp function object
> - * Sic: *pfunc == NULL means that perhaps the user has a global
> - * "EXECUTE" privilege, so no specific grant to a function.
> - *
> - * @retval -1 on access denied
> - * @retval 0 on success
> - */
> -static inline int
> -access_check_func(const char *name, uint32_t name_len, struct func **funcp)
> +int
> +box_func_check_access(const char *name, uint32_t name_len, struct func
> **funcp) {
> struct func *func = func_by_name(name, name_len);
> struct credentials *credentials = effective_user();
> @@ -136,17 +124,24 @@ box_func_reload(const char *name)
> {
> size_t name_len = strlen(name);
> struct func *func = NULL;
> - if ((access_check_func(name, name_len, &func)) != 0)
> + if ((box_func_check_access(name, name_len, &func)) != 0)
> return -1;
> if (func == NULL) {
> + /* Try to interpret name as a module name. */
> + struct module *module;
> + if (module_reload(name, name + name_len, &module, true) == 0 &&
> + module != NULL)
> + return 0;
> diag_set(ClientError, ER_NO_SUCH_FUNCTION, name);
> return -1;
> + } else {
> + /* Such function exists. Try to reload it. */
> + if (func->def->language != FUNC_LANGUAGE_C || func->func == NULL)
> + return 0;
> + if (func_reload(func) == 0)
> + return 0;
> + return -1;
> }
> - if (func->def->language != FUNC_LANGUAGE_C || func->func == NULL)
> - return 0; /* Nothing to do */
> - if (func_reload(func) == 0)
> - return 0;
> - return -1;
> }
>
> int
> @@ -165,7 +160,7 @@ box_process_call(struct call_request *request, struct
> port *port) * Sic: func == NULL means that perhaps the user has a global
> * "EXECUTE" privilege, so no specific grant to a function.
> */
> - if (access_check_func(name, name_len, &func) != 0)
> + if (box_func_check_access(name, name_len, &func) != 0)
> return -1; /* permission denied */
>
> /**
> diff --git a/src/box/call.h b/src/box/call.h
> index eabba69..55cf925 100644
> --- a/src/box/call.h
> +++ b/src/box/call.h
> @@ -35,8 +35,11 @@
> extern "C" {
> #endif /* defined(__cplusplus) */
>
> +#include <inttypes.h>
> +
> struct port;
> struct call_request;
> +struct func;
>
> struct box_function_ctx {
> struct port *port;
> @@ -51,6 +54,21 @@ box_process_call(struct call_request *request, struct
> port *port); int
> box_process_eval(struct call_request *request, struct port *port);
>
> +/**
> + * Find a function by name and check "EXECUTE" permissions.
> + *
> + * @param name function name
> + * @param name_len length of @a name
> + * @param[out] funcp function object
> + * Sic: *pfunc == NULL means that perhaps the user has a global
> + * "EXECUTE" privilege, so no specific grant to a function.
> + *
> + * @retval -1 on access denied.
> + * @retval 0 on success.
> + */
> +int
> +box_func_check_access(const char *name, uint32_t name_len, struct func
> **funcp); +
> #if defined(__cplusplus)
> } /* extern "C" */
> #endif /* defined(__cplusplus) */
> diff --git a/src/box/func.c b/src/box/func.c
> index dfbc5f3..245dbe7 100644
> --- a/src/box/func.c
> +++ b/src/box/func.c
> @@ -34,6 +34,7 @@
> #include "lua/utils.h"
> #include "error.h"
> #include "diag.h"
> +#include "call.h"
> #include <dlfcn.h>
>
> /**
> @@ -302,7 +303,8 @@ module_sym(struct module *module, const char *name)
> * Reload a dso.
> */
> int
> -module_reload(const char *package, const char *package_end, struct module
> **module) +module_reload(const char *package, const char *package_end,
> + struct module **module, bool check_access)
> {
> struct module *old_module = module_cache_find(package, package_end);
> if (old_module == NULL) {
> @@ -318,7 +320,19 @@ module_reload(const char *package, const char
> *package_end, struct module **modu struct func *func, *tmp_func;
> rlist_foreach_entry_safe(func, &old_module->funcs, item, tmp_func) {
> struct func_name name;
> - func_split_name(func->def->name, &name);
> + const char *func_name = func->def->name;
> + func_split_name(func_name, &name);
> +
> + /*
> + * Allow to reload only functions that belongs to
> + * current user. Skip other.
> + */
> + struct func *dummy;
> + if (check_access &&
> + box_func_check_access(func_name, strlen(func_name),
> + &dummy) != 0)
> + continue;
> +
> func->func = module_sym(new_module, name.sym);
> if (func->func == NULL)
> goto restore;
> @@ -441,7 +455,7 @@ func_reload(struct func *func)
> struct func_name name;
> func_split_name(func->def->name, &name);
> struct module *module = NULL;
> - if (module_reload(name.package, name.package_end, &module) != 0) {
> + if (module_reload(name.package, name.package_end, &module, false) != 0) {
> diag_log();
> return -1;
> }
> diff --git a/src/box/func.h b/src/box/func.h
> index 0957546..168c2fc 100644
> --- a/src/box/func.h
> +++ b/src/box/func.h
> @@ -116,6 +116,21 @@ func_call(struct func *func, box_function_ctx_t *ctx,
> const char *args, int
> func_reload(struct func *func);
>
> +/**
> + * Reload dynamically loadable module.
> + *
> + * @param package name begin pointer.
> + * @param package_end name end pointer.
> + * @param[out] module a pointer to store module object on success.
> + * @param check_access do ACL checks if specified.
> + *
> + * @retval -1 on error
> + * @retval 0 on success
> + */
> +int
> +module_reload(const char *package, const char *package_end,
> + struct module **module, bool check_access);
> +
> #if defined(__cplusplus)
> } /* extern "C" */
> #endif /* defined(__cplusplus) */
> diff --git a/test/box/func_reload.result b/test/box/func_reload.result
> index 5aeb85c..9c05555 100644
> --- a/test/box/func_reload.result
> +++ b/test/box/func_reload.result
> @@ -54,6 +54,10 @@ fio.symlink(reload1_path, reload_path)
> box.schema.func.reload("reload.foo")
> ---
> ...
> +box.schema.func.reload("reload")
> +---
> +- error: Function 'reload' does not exist
> +...
> -- test of usual case reload. No hanging calls
> box.space.test:insert{0}
> ---
> @@ -77,6 +81,9 @@ fio.symlink(reload2_path, reload_path)
> box.schema.func.reload("reload.foo")
> ---
> ...
> +box.schema.func.reload("reload")
> +---
> +...
> c:call("reload.foo")
> ---
> - []
> diff --git a/test/box/func_reload.test.lua b/test/box/func_reload.test.lua
> index dc56d84..5569dcd 100644
> --- a/test/box/func_reload.test.lua
> +++ b/test/box/func_reload.test.lua
> @@ -21,6 +21,7 @@ fio.symlink(reload1_path, reload_path)
>
> --check not fail on non-load func
> box.schema.func.reload("reload.foo")
> +box.schema.func.reload("reload")
>
> -- test of usual case reload. No hanging calls
> box.space.test:insert{0}
> @@ -30,6 +31,7 @@ _ = fio.unlink(reload_path)
> fio.symlink(reload2_path, reload_path)
>
> box.schema.func.reload("reload.foo")
> +box.schema.func.reload("reload")
> c:call("reload.foo")
> box.space.test:select{}
> box.space.test:truncate()
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20180711/d5ba4555/attachment.sig>
More information about the Tarantool-patches
mailing list