[tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module
- From: Kirill Shcherbatov <kshcherbatov@xxxxxxxxxxxxx>
- To: tarantool-patches@xxxxxxxxxxxxx, Vladimir Davydov <vdavydov.dev@xxxxxxxxx>
- Date: Wed, 11 Jul 2018 17:50:31 +0300
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()
Other related posts:
