osd: class load shouldn't hang when failed to load

Fixes bug #332. This happened if either the class existed, and it was either
corrupted, or failed to load from any other reason.

diff --git a/src/common/ClassHandler.cc b/src/common/ClassHandler.cc
index 7d94d53579663088f3c16026d18330ce97adac14..f694f89ccb23311006d181d618160d2ca9fa3494 100644 (file)
--- a/src/common/ClassHandler.cc
+++ b/src/common/ClassHandler.cc
@@ -18,8 +18,9 @@
 
 static ClassHandler::ClassData null_cls_data;
 
-void ClassHandler::_load_class(ClassData &cls)
+int ClassHandler::_load_class(ClassData &cls)
 {
+  int ret;
   dout(10) << "load_class " << cls.name << dendl;
 
   cls_deps_t *(*cls_deps)();
@@ -28,6 +29,10 @@ void ClassHandler::_load_class(ClassData &cls)
   snprintf(fname, sizeof(fname), "%s/class-XXXXXX", g_conf.osd_class_tmp);
 
   int fd = mkstemp(fname);
+  if (fd < 0) {
+   dout(0) << "could not create temp file " << fname << dendl;
+   return -errno;
+  }
   cls.impl.binary.write_fd(fd);
   close(fd);
 
@@ -35,6 +40,7 @@ void ClassHandler::_load_class(ClassData &cls)
 
   if (!cls.handle) {
     dout(0) << "could not open class (dlopen failed): " << dlerror() << dendl;
+    ret = -EIO;
     goto done;
   }
   cls_deps = (cls_deps_t *(*)())dlsym(cls.handle, "class_deps");
@@ -47,22 +53,26 @@ void ClassHandler::_load_class(ClassData &cls)
       deps++;
     }
   }
-  cls.load();
+  ret = cls.load();
 done:
   unlink(fname);
-  return;
+  return ret;
 }
 
-void ClassHandler::load_class(const string& cname)
+int ClassHandler::load_class(const string& cname)
 {
+  int ret;
+
   ClassData& cls = get_obj(cname);
   if (&cls == &null_cls_data) {
     dout(0) << "ERROR: can't load null class data" << dendl;
-    return;
+    return -EINVAL;
   }
   cls.mutex->Lock();
-  _load_class(cls);
+  ret = _load_class(cls);
   cls.mutex->Unlock();
+
+  return ret;
 }
 
 ClassHandler::ClassData& ClassHandler::get_obj(const string& cname)
@@ -96,6 +106,7 @@ ClassHandler::ClassData *ClassHandler::get_class(const string& cname, ClassVersi
   Mutex::Locker lock(*class_data->mutex);
 
   switch (class_data->status) {
+  case ClassData::CLASS_ERROR:
   case ClassData::CLASS_INVALID:
   case ClassData::CLASS_LOADED:
     if (class_data->cache_timed_out()) {
@@ -144,7 +155,11 @@ void ClassHandler::handle_class(MClass *m)
        dout(10) << "added class '" << info_iter->name << "'" << dendl;
        data.impl = *impl_iter;
        ++impl_iter;
-       _load_class(data);
+       int ret = _load_class(data);
+        if (ret < 0) {
+          data.set_status(ClassData::CLASS_ERROR);
+          osd->got_class(info_iter->name);
+        }
     } else {
        dout(10) << "response of an invalid class '" << info_iter->name << "'" << dendl;
         data.set_status(ClassData::CLASS_INVALID);
@@ -194,12 +209,23 @@ void ClassHandler::unregister_class(ClassHandler::ClassData *cls)
 }
 
 
-void ClassHandler::ClassData::load()
+int ClassHandler::ClassData::load()
 {
-  if (status == CLASS_INVALID) {
+  int ret;
+  switch (status) {
+    case CLASS_INVALID:
+      ret = -EINVAL;
+      break;
+    case CLASS_ERROR:
+      ret = -EIO;
+      break;
+    default:
+      ret = 0;
+  }
+  if (ret) {
     /* if we're invalid, we should just notify osd */
     osd->got_class(name);
-    return;
+    return ret;
   }
 
   if (!has_missing_deps()) {
@@ -214,6 +240,8 @@ void ClassHandler::ClassData::load()
     ClassData *cls = *iter;
     cls->satisfy_dependency(this);
   }
+
+  return 0;
 }
 
 void ClassHandler::ClassData::init()
@@ -250,9 +278,10 @@ bool ClassHandler::ClassData::_add_dependency(cls_deps_t *dep)
   }
   cls_dep._add_dependent(*this);
 
-  if (cls_dep.status == CLASS_INVALID) {
-    dout(0) << "ouch! depending on invalid class" << dendl;
-    set_status(CLASS_INVALID); /* we have an invalid dependency, we're invalid */
+  if ((cls_dep.status == CLASS_INVALID) ||
+      (cls_dep.status == CLASS_ERROR))  {
+    dout(0) << "ouch! depending on bad class" << dendl;
+    set_status(cls_dep.status); /* we have an invalid dependency, we're invalid */
   }
 
   return true;
@@ -339,6 +368,7 @@ void ClassHandler::ClassData::set_status(ClassHandler::ClassData::Status _status
   status = _status;
 
   switch (status) {
+    case CLASS_ERROR:
     case CLASS_INVALID:
     case CLASS_LOADED:
       set_timeout();

diff --git a/src/common/ClassHandler.h b/src/common/ClassHandler.h
index 871144ab55729f176a026d24f107966f43b61d7e..a2e6a0baff5ac474d85711715932b0e1e381bdbf 100644 (file)
--- a/src/common/ClassHandler.h
+++ b/src/common/ClassHandler.h
@@ -44,10 +44,11 @@ public:
     void satisfy_dependency(ClassData *cls);
 
     enum Status { 
-      CLASS_UNKNOWN, 
-      CLASS_INVALID, 
-      CLASS_LOADED, 
-      CLASS_REQUESTED, 
+      CLASS_UNKNOWN,
+      CLASS_INVALID,
+      CLASS_ERROR,
+      CLASS_LOADED,
+      CLASS_REQUESTED,
     } status;
     ClassVersion version;
     time_t timeout;
@@ -73,7 +74,7 @@ public:
     ClassMethod *get_method(const char *mname);
     void unregister_method(ClassMethod *method);
 
-    void load();
+    int load();
     void init();
 
     void set_status(Status _status);
@@ -85,8 +86,8 @@ public:
 
   ClassData& get_obj(const string& cname);
 
-  void load_class(const string& cname);
-  void _load_class(ClassData &data);
+  int load_class(const string& cname);
+  int _load_class(ClassData &data);
 
   ClassHandler(OSD *_osd) : osd(_osd), mutex("ClassHandler") {}
 

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index ddfae632aae2a8db41622799ab291a4979a2fd56..9a948fd1a1be9a2a1fe6c0fcd58e480f12a8c814 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -4810,6 +4810,9 @@ int OSD::get_class(const string& cname, ClassVersion& version, pg_t pgid, Messag
     case ClassHandler::ClassData::CLASS_INVALID:
       dout(0) << "class not supported" << dendl;
       return -EOPNOTSUPP;
+    case ClassHandler::ClassData::CLASS_ERROR:
+      dout(0) << "error loading class!" << dendl;
+      return -EIO;
     default:
       assert(0);
     }