[tarantool-patches] [PATCH v1 1/1] box: support reload whole module

[tarantool-patches] [PATCH v1 1/1] box: support reload whole module

[Kirill Shcherbatov]

@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!

Closes #2946.
---
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                | 21 ++++++++++++++++++---
 src/box/func.h                | 15 +++++++++++++++
 test/box/func_reload.result   |  7 +++++++
 test/box/func_reload.test.lua |  2 ++
 6 files changed, 76 insertions(+), 24 deletions(-)

diff --git a/src/box/call.c b/src/box/call.c
index 438be19..dfe15dd 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)
+			return 0; /* Nothing to do */
+		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..dfab5d0 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,10 +455,11 @@ 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;
 	}
+
 	return 0;
 }
 
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()
-- 
2.7.4

[tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module

[Vladislav Shpilevoy]

On 09/07/2018 19:54, Kirill Shcherbatov wrote:
> @TarantoolBot document

Please, put the doc request after 'Closes'. The request must be at the
very end of the commit message. Else 'Closes' is included into the
request.

> 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!
> 
> Closes #2946.
> ---

[tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module

[Kirill Shcherbatov]

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()
-- 
2.7.4

[tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module

[Georgy Kirichenko]

[-- Attachment #1: Type: text/plain, Size: 8446 bytes --]

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()


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module

[Vladimir Davydov]

On Tue, Jul 10, 2018 at 09:49:14AM +0300, Kirill Shcherbatov wrote:
> @@ -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;
> +

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?

Re: [tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module

[Kirill Shcherbatov]

On 11.07.2018 15:46, Vladimir Davydov wrote:
> 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.

Re: [tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module

[Vladimir Davydov]

On Wed, Jul 11, 2018 at 03:52:46PM +0300, Kirill Shcherbatov wrote:
> On 11.07.2018 15:46, Vladimir Davydov wrote:
> > 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.

Re: [tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module

[Kirill Shcherbatov]

>>> 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 <inttypes.h>
+
 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 <dlfcn.h>
 
 /**
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()

Re: [tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module

[Vladimir Davydov]

On Wed, Jul 11, 2018 at 03:46:56PM +0300, Vladimir Davydov wrote:
> On Tue, Jul 10, 2018 at 09:49:14AM +0300, Kirill Shcherbatov wrote:
> > @@ -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;
> > +
> 
> 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?

During our verbal discussion with Kirill, he noticed that the fact that
box.schema.func.reload() reloads the whole module when it is passed a
function name is rather confusing: the user may not know that and call
box.schema.func.reload() once per each used function, in which case he
will effectively reload the whole module that contains those functions
multiple times, which is pointless. Turns out Kirill isn't the only one
who finds such an API weird, see

  https://github.com/tarantool/tarantool/issues/910#issuecomment-331435227

I talked to Kostja and he doesn't mind banning this API, i.e. making
box.schema.func.reload() interpret its argument only as a module name.

So let's rework this patch so that box_func_reload() directly calls
module_reload(), and remove func_reload() altogether. As for the access
checks, box_func_reload() should work only if the caller has the global
execute privilege.

Re: [tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module

[Kirill Shcherbatov]

> During our verbal discussion with Kirill, he noticed that the fact that
> box.schema.func.reload() reloads the whole module when it is passed a
> function name is rather confusing: the user may not know that and call
> box.schema.func.reload() once per each used function, in which case he
> will effectively reload the whole module that contains those functions
> multiple times, which is pointless. Turns out Kirill isn't the only one
> who finds such an API weird, see
> 
>   https://github.com/tarantool/tarantool/issues/910#issuecomment-331435227
> 
> I talked to Kostja and he doesn't mind banning this API, i.e. making
> box.schema.func.reload() interpret its argument only as a module name.
> 
> So let's rework this patch so that box_func_reload() directly calls
> module_reload(), and remove func_reload() altogether. As for the access
> checks, box_func_reload() should work only if the caller has the global
> execute privilege.

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. Legacy method including finction
name is forbidden.
box.schema.func.reload("utils")       -- ok since now
box.schema.func.reload("utils.func1") -- forbidden 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                | 23 +++++++++++++----------
 src/box/call.h                |  9 ++++++++-
 src/box/errcode.h             |  1 +
 src/box/func.c                | 16 ----------------
 src/box/func.h                | 11 ++++++++++-
 src/box/lua/call.c            |  6 +++---
 src/box/lua/schema.lua        |  2 +-
 test/box/func_reload.result   | 23 ++++++++---------------
 test/box/func_reload.test.lua | 14 ++++++--------
 test/box/misc.result          |  1 +
 10 files changed, 51 insertions(+), 55 deletions(-)

diff --git a/src/box/call.c b/src/box/call.c
index 438be19..b9750c5 100644
--- a/src/box/call.c
+++ b/src/box/call.c
@@ -132,20 +132,23 @@ box_c_call(struct func *func, struct call_request *request, struct port *port)
 }
 
 int
-box_func_reload(const char *name)
+box_module_reload(const char *name)
 {
-	size_t name_len = strlen(name);
-	struct func *func = NULL;
-	if ((access_check_func(name, name_len, &func)) != 0)
-		return -1;
-	if (func == NULL) {
-		diag_set(ClientError, ER_NO_SUCH_FUNCTION, name);
+	struct credentials *credentials = effective_user();
+	if ((credentials->universal_access & (PRIV_X | PRIV_U)) !=
+	    (PRIV_X | PRIV_U)) {
+		struct user *user = user_find(credentials->uid);
+		if (user != NULL)
+			diag_set(AccessDeniedError, priv_name(PRIV_U),
+				 schema_object_name(SC_UNIVERSE), "",
+				 user->def->name);
 		return -1;
 	}
-	if (func->def->language != FUNC_LANGUAGE_C || func->func == NULL)
-		return 0; /* Nothing to do */
-	if (func_reload(func) == 0)
+	struct module *module = NULL;
+	if (module_reload(name, name + strlen(name), &module) == 0 &&
+	    module != NULL)
 		return 0;
+	diag_set(ClientError, ER_NO_SUCH_MODULE, name);
 	return -1;
 }
 
diff --git a/src/box/call.h b/src/box/call.h
index eabba69..1b54551 100644
--- a/src/box/call.h
+++ b/src/box/call.h
@@ -42,8 +42,15 @@ struct box_function_ctx {
 	struct port *port;
 };
 
+/**
+ * Reload loadable module by name.
+ *
+ * @param name of the module to reload.
+ * @retval -1 on error.
+ * @retval 0 on success.
+ */
 int
-box_func_reload(const char *name);
+box_module_reload(const char *name);
 
 int
 box_process_call(struct call_request *request, struct port *port);
diff --git a/src/box/errcode.h b/src/box/errcode.h
index c76018c..64f9af1 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -215,6 +215,7 @@ struct errcode_record {
 	/*160 */_(ER_ACTION_MISMATCH,		"Field %d contains %s on conflict action, but %s in index parts") \
 	/*161 */_(ER_VIEW_MISSING_SQL,		"Space declared as a view must have SQL statement") \
 	/*162 */_(ER_FOREIGN_KEY_CONSTRAINT,	"Can not commit transaction: deferred foreign keys violations are not resolved") \
+	/*163 */_(ER_NO_SUCH_MODULE,		"Module '%s' does not exist") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/func.c b/src/box/func.c
index dfbc5f3..a817851 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -298,9 +298,6 @@ module_sym(struct module *module, const char *name)
 	return f;
 }
 
-/*
- * Reload a dso.
- */
 int
 module_reload(const char *package, const char *package_end, struct module **module)
 {
@@ -436,19 +433,6 @@ func_load(struct func *func)
 }
 
 int
-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) {
-		diag_log();
-		return -1;
-	}
-	return 0;
-}
-
-int
 func_call(struct func *func, box_function_ctx_t *ctx, const char *args,
 	  const char *args_end)
 {
diff --git a/src/box/func.h b/src/box/func.h
index 0957546..8dcd61d 100644
--- a/src/box/func.h
+++ b/src/box/func.h
@@ -113,8 +113,17 @@ int
 func_call(struct func *func, box_function_ctx_t *ctx, const char *args,
 	  const char *args_end);
 
+/**
+ * Reload dynamically loadable module.
+ *
+ * @param package name begin pointer.
+ * @param package_end package_end name end pointer.
+ * @param[out] module a pointer to store module object on success.
+ * @retval -1 on error.
+ * @retval 0 on success.
+ */
 int
-func_reload(struct func *func);
+module_reload(const char *package, const char *package_end, struct module **module);
 
 #if defined(__cplusplus)
 } /* extern "C" */
diff --git a/src/box/lua/call.c b/src/box/lua/call.c
index 8f3461a..d20cbbb 100644
--- a/src/box/lua/call.c
+++ b/src/box/lua/call.c
@@ -458,10 +458,10 @@ box_lua_eval(struct call_request *request, struct port *port)
 }
 
 static int
-lbox_func_reload(lua_State *L)
+lbox_module_reload(lua_State *L)
 {
 	const char *name = luaL_checkstring(L, 1);
-	if (box_func_reload(name) != 0)
+	if (box_module_reload(name) != 0)
 		return luaT_error(L);
 	return 0;
 }
@@ -469,7 +469,7 @@ lbox_func_reload(lua_State *L)
 static const struct luaL_Reg boxlib_internal[] = {
 	{"call_loadproc",  lbox_call_loadproc},
 	{"sql_create_function",  lbox_sql_create_function},
-	{"func_reload", lbox_func_reload},
+	{"module_reload", lbox_module_reload},
 	{NULL, NULL}
 };
 
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 87c79bd..ab8072c 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -1951,7 +1951,7 @@ function box.schema.func.exists(name_or_id)
     return tuple ~= nil
 end
 
-box.schema.func.reload = internal.func_reload
+box.schema.func.reload = internal.module_reload
 
 box.internal.collation = {}
 box.internal.collation.create = function(name, coll_type, locale, opts)
diff --git a/test/box/func_reload.result b/test/box/func_reload.result
index 5aeb85c..b024cd1 100644
--- a/test/box/func_reload.result
+++ b/test/box/func_reload.result
@@ -51,8 +51,9 @@ fio.symlink(reload1_path, reload_path)
 - true
 ...
 --check not fail on non-load func
-box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
 ---
+- error: Module 'reload' does not exist
 ...
 -- test of usual case reload. No hanging calls
 box.space.test:insert{0}
@@ -74,7 +75,7 @@ fio.symlink(reload2_path, reload_path)
 ---
 - true
 ...
-box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
 ---
 ...
 c:call("reload.foo")
@@ -98,7 +99,7 @@ fio.symlink(reload1_path, reload_path)
 ---
 - true
 ...
-box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
 ---
 ...
 fibers = 10
@@ -111,7 +112,7 @@ while box.space.test:count() < fibers do fiber.sleep(0.001) end
 ---
 ...
 -- double reload doesn't fail waiting functions
-box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
 ---
 ...
 _ = fio.unlink(reload_path)
@@ -121,7 +122,7 @@ fio.symlink(reload2_path, reload_path)
 ---
 - true
 ...
-box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
 ---
 ...
 c:call("reload.foo")
@@ -208,7 +209,7 @@ _ = fiber.create(function() ch:put(c:call("reload.test_reload")) end)
 while s:get({1}) == nil do fiber.yield(0.0001) end
 ---
 ...
-box.schema.func.reload("reload.test_reload")
+box.schema.func.reload("reload")
 ---
 ...
 _ = fiber.create(function() ch:put(c:call("reload.test_reload")) end)
@@ -242,14 +243,6 @@ fio.symlink(reload1_path, reload_path)
 ---
 - true
 ...
-s, e = pcall(box.schema.func.reload, "reload.test_reload")
----
-...
-s, string.find(tostring(e), 'test_reload_fail') ~= nil
----
-- false
-- true
-...
 c:call("reload.test_reload")
 ---
 - [[2]]
@@ -273,5 +266,5 @@ box.schema.func.reload()
 ...
 box.schema.func.reload("non-existing")
 ---
-- error: Function 'non-existing' does not exist
+- error: Module 'non-existing' does not exist
 ...
diff --git a/test/box/func_reload.test.lua b/test/box/func_reload.test.lua
index dc56d84..8906898 100644
--- a/test/box/func_reload.test.lua
+++ b/test/box/func_reload.test.lua
@@ -20,7 +20,7 @@ _ = fio.unlink(reload_path)
 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}
@@ -29,7 +29,7 @@ box.space.test:delete{0}
 _ = 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()
@@ -37,18 +37,18 @@ box.space.test:truncate()
 -- test case with hanging calls
 _ = fio.unlink(reload_path)
 fio.symlink(reload1_path, reload_path)
-box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
 
 fibers = 10
 for i = 1, fibers do fiber.create(function() c:call("reload.foo", {i}) end) end
 
 while box.space.test:count() < fibers do fiber.sleep(0.001) end
 -- double reload doesn't fail waiting functions
-box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
 
 _ = 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")
 
 while box.space.test:count() < 2 * fibers + 1 do fiber.sleep(0.001) end
@@ -71,7 +71,7 @@ _ = fio.unlink(reload_path)
 fio.symlink(reload2_path, reload_path)
 _ = fiber.create(function() ch:put(c:call("reload.test_reload")) end)
 while s:get({1}) == nil do fiber.yield(0.0001) end
-box.schema.func.reload("reload.test_reload")
+box.schema.func.reload("reload")
 _ = fiber.create(function() ch:put(c:call("reload.test_reload")) end)
 ch:get()
 ch:get()
@@ -82,8 +82,6 @@ box.schema.user.grant('guest', 'execute', 'function', 'reload.test_reload_fail')
 c:call("reload.test_reload_fail")
 _ = fio.unlink(reload_path)
 fio.symlink(reload1_path, reload_path)
-s, e = pcall(box.schema.func.reload, "reload.test_reload")
-s, string.find(tostring(e), 'test_reload_fail') ~= nil
 c:call("reload.test_reload")
 c:call("reload.test_reload_fail")
 
diff --git a/test/box/misc.result b/test/box/misc.result
index a00d033..777b40b 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -487,6 +487,7 @@ t;
   160: box.error.ACTION_MISMATCH
   161: box.error.VIEW_MISSING_SQL
   162: box.error.FOREIGN_KEY_CONSTRAINT
+  163: box.error.NO_SUCH_MODULE
 ...
 test_run:cmd("setopt delimiter ''");
 ---
-- 
2.7.4

Re: [tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module

[Vladimir Davydov]

On Thu, Jul 12, 2018 at 11:27:13AM +0300, Kirill Shcherbatov wrote:
> > During our verbal discussion with Kirill, he noticed that the fact that
> > box.schema.func.reload() reloads the whole module when it is passed a
> > function name is rather confusing: the user may not know that and call
> > box.schema.func.reload() once per each used function, in which case he
> > will effectively reload the whole module that contains those functions
> > multiple times, which is pointless. Turns out Kirill isn't the only one
> > who finds such an API weird, see
> > 
> >   https://github.com/tarantool/tarantool/issues/910#issuecomment-331435227
> > 
> > I talked to Kostja and he doesn't mind banning this API, i.e. making
> > box.schema.func.reload() interpret its argument only as a module name.
> > 
> > So let's rework this patch so that box_func_reload() directly calls
> > module_reload(), and remove func_reload() altogether. As for the access
> > checks, box_func_reload() should work only if the caller has the global
> > execute privilege.
> 
> 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. Legacy method including finction
> name is forbidden.
> box.schema.func.reload("utils")       -- ok since now
> box.schema.func.reload("utils.func1") -- forbidden 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                | 23 +++++++++++++----------
>  src/box/call.h                |  9 ++++++++-
>  src/box/errcode.h             |  1 +
>  src/box/func.c                | 16 ----------------
>  src/box/func.h                | 11 ++++++++++-
>  src/box/lua/call.c            |  6 +++---
>  src/box/lua/schema.lua        |  2 +-
>  test/box/func_reload.result   | 23 ++++++++---------------
>  test/box/func_reload.test.lua | 14 ++++++--------
>  test/box/misc.result          |  1 +
>  10 files changed, 51 insertions(+), 55 deletions(-)

Looks good to me.

[tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module

[Kirill Yukhin]

Hello,
On 09 июл 19:54, Kirill Shcherbatov wrote:
> @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!
> 
> Closes #2946.
> ---
> https://github.com/tarantool/tarantool/compare/kshch/gh-2946-module-reload
> https://github.com/tarantool/tarantool/issues/2946
I've checked in the patch into 2.0 branch.

--
Regards, Kirill Yukhin