From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 0157321BA2 for ; Wed, 11 Jul 2018 01:25:25 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id a-Vzi0jf9ER3 for ; Wed, 11 Jul 2018 01:25:24 -0400 (EDT) Received: from smtp37.i.mail.ru (smtp37.i.mail.ru [94.100.177.97]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 11C3121B4C for ; Wed, 11 Jul 2018 01:25:23 -0400 (EDT) From: Georgy Kirichenko Subject: [tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module Date: Wed, 11 Jul 2018 08:25:13 +0300 Message-ID: <2085339.gJ1XVEfBm1@home.lan> In-Reply-To: References: <8846b566-24c5-9f11-ca29-18baed551921@tarantool.org> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart2963590.DgZyVkHRBC"; micalg="pgp-sha256"; protocol="application/pgp-signature" Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Kirill Shcherbatov --nextPart2963590.DgZyVkHRBC Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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 > + > 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 > > /** > @@ -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() --nextPart2963590.DgZyVkHRBC Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part. Content-Transfer-Encoding: 7Bit -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEBJFDbU76LsBbgHBsvKOmCX79zb4FAltFlLkACgkQvKOmCX79 zb6Yuwf/eIDwVUiZzU2zWNWDIs9HAurkCNrZV5WgIiE55gJhruNWFPh3TZ4z+H4O ndpLtkcJo7G3s26Ovtee66hsnOdA+jt9x1yCDiHX/T+v9Qo8GKpC9yw0U1YE6fCJ 7HDtnKErhk04gZ+0ob01ZN+HKadzUpTPm3ULsy0WX2Q35LkQm75IEw2UY+sRB3x+ MNYpGgCd0dXPyRFedVCtYf9TFv5jsM7KEYD7tmiYT7c48NmvgJ1gCtp4R+vfLMuT 1NMeXRo656E1Aeo2Sf9mR4O5UKLa3th5y7Lk+TkziplMItXpm2q7+j/RTIOHZ5hD J3qlj61Bx4Fa0Nf/RdynsD0wnuviCQ== =gk3U -----END PGP SIGNATURE----- --nextPart2963590.DgZyVkHRBC--