From dead2a9d04e1e479ee7dcd35707759f925db14f8 Mon Sep 17 00:00:00 2001 From: Volker Theile Date: Mon, 4 Feb 2019 17:34:14 +0100 Subject: [PATCH] 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 --- src/mgr/BaseMgrModule.cc | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/mgr/BaseMgrModule.cc b/src/mgr/BaseMgrModule.cc index eaff6b03113..00024fdfd2d 100644 --- 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 val; + if (value) { + val = value; + } + self->py_modules->set_config(self->this_module->get_name(), key, val); + + Py_RETURN_NONE; } static PyObject* -- 2.39.5