From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module References: <8846b566-24c5-9f11-ca29-18baed551921@tarantool.org> <20180711124656.pzuh254bjku3k2xc@esperanza> <20180711125953.eevnmjygj2s7swwj@esperanza> From: Kirill Shcherbatov Message-ID: <5ec8d1d0-a3ed-987a-79bf-341c67d703ab@tarantool.org> Date: Wed, 11 Jul 2018 17:50:31 +0300 MIME-Version: 1.0 In-Reply-To: <20180711125953.eevnmjygj2s7swwj@esperanza> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit To: tarantool-patches@freelists.org, Vladimir Davydov List-ID: >>> AFAIU a user may reload a whole module only if he has the global EXECUTE >>> privilege (because we don't have such an entity as module in our data >>> dictionary to grant access rights for). access_check_func(), which is >>> called by func_reload(), already checks the global EXECUTE privilege and >>> returns 0 (success) if it is set, no matter if the function was found or >>> not. So all you have to do is call module_reload() from func_reload() if >>> access_check_func() returned func = NULL, no? >> >> No, this call iterates through all loaded functions of specified module and >> reload all that belongs to initiator if any. > > I understand what you code does, but I wonder why it does that. As we have verbally discussed, ================================== diff --git a/src/box/call.c b/src/box/call.c index 438be19..b322c03 100644 --- a/src/box/call.c +++ b/src/box/call.c @@ -139,14 +139,24 @@ box_func_reload(const char *name) if ((access_check_func(name, name_len, &func)) != 0) return -1; if (func == NULL) { + /* Try to interpret name as a module name. */ + struct module *module; + if ((effective_user()->universal_access & (PRIV_X | PRIV_U)) == + (PRIV_X | PRIV_U) && + module_reload(name, name + name_len, &module) == 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 diff --git a/src/box/call.h b/src/box/call.h index eabba69..14844fc 100644 --- a/src/box/call.h +++ b/src/box/call.h @@ -35,8 +35,11 @@ extern "C" { #endif /* defined(__cplusplus) */ +#include + struct port; struct call_request; +struct func; struct box_function_ctx { struct port *port; diff --git a/src/box/func.c b/src/box/func.c index dfbc5f3..ccbc12b 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 /** diff --git a/src/box/func.h b/src/box/func.h index 0957546..3b67b06 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); + #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()