mgr: remove _ceph_get_module_option_ex() method

It seems that the current implementation has some side effects which results in segfaults in the Mgr. It looks like boxing parameters for feeding them into a Python function written in C++ does not work very well or we did not handle the GIL correctly here. Because of that the old code from the ceph_(get|set)_module_option functions are used again (see #25651). Thus we have some lines of more or less duplicate code, but we get rid of the problems that arise from the current implementation.

Previous improvements like #26112, #26240 and #26149 are still useful for the ceph_(get|set)_module_option_ex functions.

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

diff --git a/src/mgr/BaseMgrModule.cc b/src/mgr/BaseMgrModule.cc
index eaff6b031134db774ac1861d3b852bc72040b961..00024fdfd2d34bb499f2c26bbf45f2369707f704 100644 (file)
--- a/src/mgr/BaseMgrModule.cc
+++ b/src/mgr/BaseMgrModule.cc
@@ -390,12 +390,12 @@ static PyObject*
 ceph_get_module_option_ex(BaseMgrModule *self, PyObject *args)
 {
   char *module = nullptr;
-  char *what = nullptr;
-  if (!PyArg_ParseTuple(args, "ss:ceph_get_module_option_ex", &module, &what)) {
+  char *key = nullptr;
+  if (!PyArg_ParseTuple(args, "ss:ceph_get_module_option_ex", &module, &key)) {
     derr << "Invalid args!" << dendl;
     return nullptr;
   }
-  auto pResult = self->py_modules->get_typed_config(module, what);
+  auto pResult = self->py_modules->get_typed_config(module, key);
   return pResult;
 }
 
@@ -407,11 +407,20 @@ ceph_get_module_option(BaseMgrModule *self, PyObject *args)
     derr << "Invalid args!" << dendl;
     return nullptr;
   }
-  auto pArgs = Py_BuildValue("(ss)", self->this_module->get_name().c_str(),
-    key);
-  auto pResult = ceph_get_module_option_ex(self, pArgs);
-  Py_DECREF(pArgs);
-  return pResult;
+
+  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*
@@ -456,11 +465,13 @@ ceph_set_module_option(BaseMgrModule *self, PyObject *args)
     derr << "Invalid args!" << dendl;
     return nullptr;
   }
-  auto pArgs = Py_BuildValue("(ssz)", self->this_module->get_name().c_str(),
-    key, value);
-  auto pResult = ceph_set_module_option_ex(self, pArgs);
-  Py_DECREF(pArgs);
-  return pResult;
+  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*