mgr: Remove _ceph_(g|s)et_module_option_ex() method

This PR replaces the _ceph_(get|set)_module_option functions with the specialized _ceph_(get|set)_module_option_ex functions and improves the MgrModule Python class to call the new _ceph_(get|set)_module_option functions.

This is done to reduce the complexity of the previous implementation and will prevent the use of boxing parameters for feeding them into a Python function written in C++.

Signed-off-by: Volker Theile <vtheile@suse.com>

diff --git a/src/mgr/BaseMgrModule.cc b/src/mgr/BaseMgrModule.cc
index 00024fdfd2d34bb499f2c26bbf45f2369707f704..9976733c9267774c22873a552a6c131be9b75be9 100644 (file)
--- a/src/mgr/BaseMgrModule.cc
+++ b/src/mgr/BaseMgrModule.cc
@@ -387,11 +387,11 @@ ceph_option_get(BaseMgrModule *self, PyObject *args)
 }
 
 static PyObject*
-ceph_get_module_option_ex(BaseMgrModule *self, PyObject *args)
+ceph_get_module_option(BaseMgrModule *self, PyObject *args)
 {
   char *module = nullptr;
   char *key = nullptr;
-  if (!PyArg_ParseTuple(args, "ss:ceph_get_module_option_ex", &module, &key)) {
+  if (!PyArg_ParseTuple(args, "ss:ceph_get_module_option", &module, &key)) {
     derr << "Invalid args!" << dendl;
     return nullptr;
   }
@@ -399,30 +399,6 @@ ceph_get_module_option_ex(BaseMgrModule *self, PyObject *args)
   return pResult;
 }
 
-static PyObject*
-ceph_get_module_option(BaseMgrModule *self, PyObject *args)
-{
-  char *key = nullptr;
-  if (!PyArg_ParseTuple(args, "s:ceph_get_module_option", &key)) {
-    derr << "Invalid args!" << dendl;
-    return nullptr;
-  }
-
-  PyThreadState *tstate = PyEval_SaveThread();
-  std::string value;
-  bool found = self->py_modules->get_config(self->this_module->get_name(),
-      key, &value);
-  PyEval_RestoreThread(tstate);
-
-  if (found) {
-    dout(10) << __func__ << " " << key << " found: " << value.c_str() << dendl;
-    return self->this_module->py_module->get_typed_option_value(key, value);
-  } else {
-    dout(4) << __func__ << " " << key << " not found " << dendl;
-    Py_RETURN_NONE;
-  }
-}
-
 static PyObject*
 ceph_store_get_prefix(BaseMgrModule *self, PyObject *args)
 {
@@ -437,12 +413,12 @@ ceph_store_get_prefix(BaseMgrModule *self, PyObject *args)
 }
 
 static PyObject*
-ceph_set_module_option_ex(BaseMgrModule *self, PyObject *args)
+ceph_set_module_option(BaseMgrModule *self, PyObject *args)
 {
   char *module = nullptr;
   char *key = nullptr;
   char *value = nullptr;
-  if (!PyArg_ParseTuple(args, "ssz:ceph_set_module_option_ex",
+  if (!PyArg_ParseTuple(args, "ssz:ceph_set_module_option",
         &module, &key, &value)) {
     derr << "Invalid args!" << dendl;
     return nullptr;
@@ -456,24 +432,6 @@ ceph_set_module_option_ex(BaseMgrModule *self, PyObject *args)
   Py_RETURN_NONE;
 }
 
-static PyObject*
-ceph_set_module_option(BaseMgrModule *self, PyObject *args)
-{
-  char *key = nullptr;
-  char *value = nullptr;
-  if (!PyArg_ParseTuple(args, "sz:ceph_set_module_option", &key, &value)) {
-    derr << "Invalid args!" << dendl;
-    return nullptr;
-  }
-  boost::optional<string> val;
-  if (value) {
-    val = value;
-  }
-  self->py_modules->set_config(self->this_module->get_name(), key, val);
-
-  Py_RETURN_NONE;
-}
-
 static PyObject*
 ceph_store_get(BaseMgrModule *self, PyObject *args)
 {
@@ -495,8 +453,6 @@ ceph_store_get(BaseMgrModule *self, PyObject *args)
   }
 }
 
-
-
 static PyObject*
 ceph_store_set(BaseMgrModule *self, PyObject *args)
 {
@@ -1053,18 +1009,12 @@ PyMethodDef BaseMgrModule_methods[] = {
   {"_ceph_get_module_option", (PyCFunction)ceph_get_module_option, METH_VARARGS,
    "Get a module configuration option value"},
 
-  {"_ceph_get_module_option_ex", (PyCFunction)ceph_get_module_option_ex, METH_VARARGS,
-   "Get a module configuration option value from the specified module"},
-
   {"_ceph_get_store_prefix", (PyCFunction)ceph_store_get_prefix, METH_VARARGS,
    "Get all KV store values with a given prefix"},
 
   {"_ceph_set_module_option", (PyCFunction)ceph_set_module_option, METH_VARARGS,
    "Set a module configuration option value"},
 
-  {"_ceph_set_module_option_ex", (PyCFunction)ceph_set_module_option_ex, METH_VARARGS,
-   "Set a module configuration option value for the specified module"},
-
   {"_ceph_get_store", (PyCFunction)ceph_store_get, METH_VARARGS,
    "Get a stored field"},
 

diff --git a/src/pybind/mgr/mgr_module.py b/src/pybind/mgr/mgr_module.py
index 3f67946bd7e48a7c512b97b0573e837e4d754244..916e65c946d0f662659b7f55233c4354417ba0b9 100644 (file)
--- a/src/pybind/mgr/mgr_module.py
+++ b/src/pybind/mgr/mgr_module.py
@@ -905,7 +905,7 @@ class MgrModule(ceph_module.BaseMgrModule):
                                format(key, self.__class__.__name__))
 
     def _get_module_option(self, key, default):
-        r = self._ceph_get_module_option(key)
+        r = self._ceph_get_module_option(self.module_name, key)
         if r is None:
             final_key = key.split('/')[-1]
             return self.MODULE_OPTION_DEFAULTS.get(final_key, default)
@@ -937,7 +937,7 @@ class MgrModule(ceph_module.BaseMgrModule):
         """
         if module == self.module_name:
             self._validate_module_option(key)
-        r = self._ceph_get_module_option_ex(module, key)
+        r = self._ceph_get_module_option(module, key)
         return default if r is None else r
 
     def get_store_prefix(self, key_prefix):
@@ -971,7 +971,7 @@ class MgrModule(ceph_module.BaseMgrModule):
         return self._get_localized(key, default, self._get_module_option)
 
     def _set_module_option(self, key, val):
-        return self._ceph_set_module_option(key, str(val))
+        return self._ceph_set_module_option(self.module_name, key, str(val))
 
     def set_module_option(self, key, val):
         """
@@ -994,7 +994,7 @@ class MgrModule(ceph_module.BaseMgrModule):
         """
         if module == self.module_name:
             self._validate_module_option(key)
-        return self._ceph_set_module_option_ex(module, key, str(val))
+        return self._ceph_set_module_option(module, key, str(val))
 
     def set_localized_module_option(self, key, val):
         """