common/blkdev: refactor to add block_device_get_metrics returning json

Refactor block_device_run_smartctl into a helper that returns JSON.

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/common/blkdev.cc b/src/common/blkdev.cc
index 42f096d9b7e7f54f58850c157d26835cbf5d8cb2..3a4fab7b319d5e1424f301ddfdabb5329ce7d479 100644 (file)
--- a/src/common/blkdev.cc
+++ b/src/common/blkdev.cc
@@ -29,8 +29,11 @@
 //#include "common/debug.h"
 #include "include/scope_guard.h"
 #include "include/uuid.h"
+#include "include/stringify.h"
 #include "blkdev.h"
 
+#include "json_spirit/json_spirit_reader.h"
+
 int get_device_by_path(const char *path, char* partition, char* device,
                       size_t max)
 {
@@ -479,6 +482,29 @@ std::string get_device_id(const std::string& devname)
   return device_id;
 }
 
+int block_device_get_metrics(const char *device, int timeout,
+                            json_spirit::mValue *result)
+{
+  std::string s;
+  if (int r = block_device_run_smartctl(device, timeout, &s); r != 0) {
+    s = "{\"error\": \"smartctl failed\", \"dev\": \"";
+    s += device;
+    s += "\", \"smartctl_error_code\": " + stringify(r);
+    s += "\", \"smartctl_output\": \"" + s;
+    s += + "\"}";
+  }
+  if (json_spirit::read(s, *result)) {
+    return 0;
+  }
+  s = "{\"error\": \"smartctl returned invalid JSON\", \"dev\": \"";
+  s += device;
+  s += "\"}";
+  if (json_spirit::read(s, *result)) {
+    return 0;
+  }
+  return -EINVAL;
+}
+
 int block_device_run_smartctl(const char *device, int timeout,
                              std::string *result)
 {

diff --git a/src/common/blkdev.h b/src/common/blkdev.h
index 342880845dfb3c78720cdc48bc5fd1198a7e21e5..637754deb2c6650d8332a040aeb38418a84d383e 100644 (file)
--- a/src/common/blkdev.h
+++ b/src/common/blkdev.h
@@ -1,8 +1,12 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+
 #ifndef __CEPH_COMMON_BLKDEV_H
 #define __CEPH_COMMON_BLKDEV_H
 
 #include <set>
 #include <string>
+#include "json_spirit/json_spirit_value.h"
 
 enum blkdev_prop_t {
   BLKDEV_PROP_DEV,
@@ -21,6 +25,8 @@ extern std::string get_device_id(const std::string& devname);
 extern void get_dm_parents(const std::string& dev, std::set<std::string> *ls);
 extern int block_device_run_smartctl(const char *device, int timeout,
                                     std::string *result);
+extern int block_device_get_metrics(const char *device, int timeout,
+                                   json_spirit::mValue *result);
 
 // for VDO
 /// return an op fd for the sysfs stats dir, if this is a VDO device

diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc
index 660052713fcf60913002b6452fb89fb4cc554abd..f040c71297fc694eb103aa9dc86dd90cf26fecf3 100644 (file)
--- a/src/mon/Monitor.cc
+++ b/src/mon/Monitor.cc
@@ -3646,17 +3646,9 @@ void Monitor::handle_command(MonOpRequestRef op)
        "mon_smart_report_timeout");
       json_spirit::mObject json_map;
       json_spirit::mValue smart_json;
-      std::string result;
-      if (block_device_run_smartctl(("/dev/" + dev).c_str(), smart_timeout,
-                                   &result)) {
-       dout(10) << "probe_smart_device failed for /dev/" << dev << dendl;
-       result = "{\"error\": \"smartctl failed\", \"dev\": \"" + dev +
-         "\", \"smartctl_error\": \"" + result + "\"}";
-      }
-
-      if (!json_spirit::read(result, smart_json)) {
-       derr << "smartctl JSON output of /dev/" + dev + " is invalid"
-            << dendl;
+      if (block_device_get_metrics(("/dev/" + dev).c_str(), smart_timeout,
+                                   &smart_json)) {
+       dout(10) << "block_device_get_metrics failed for /dev/" << dev << dendl;
       } else {
        json_map[devid] = smart_json;
       }

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index c387b02eb67616787c3dfce4624e8be5d0bb40a9..0cd46a8812cd8a9e048948fe098680a478f5d0cb 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -6614,7 +6614,6 @@ void OSD::probe_smart(const string& only_devid, ostream& ss)
 
   // == typedef std::map<std::string, mValue> mObject;
   json_spirit::mObject json_map;
-  json_spirit::mValue smart_json;
 
   for (auto dev : devnames) {
     // smartctl works only on physical devices; filter out any logical device
@@ -6632,22 +6631,13 @@ void OSD::probe_smart(const string& only_devid, ostream& ss)
       continue;
     }
 
-    std::string result;
-    if (block_device_run_smartctl(("/dev/" + dev).c_str(), smart_timeout,
-                                 &result)) {
-      dout(10) << "probe_smart_device failed for /dev/" << dev << dendl;
-      //continue;
-      result = "{\"error\": \"smartctl failed\", \"dev\": \"" + dev +
-       "\", \"smartctl_error\": \"" + result + "\"}";
-    }
-
-    // TODO: change to read_or_throw?
-    if (!json_spirit::read(result, smart_json)) {
-      derr << "smartctl JSON output of /dev/" + dev + " is invalid" << dendl;
-    } else { //json is valid, assigning
-      json_map[devid] = smart_json;
+    json_spirit::mValue smart_json;
+    if (block_device_get_metrics(("/dev/" + dev).c_str(), smart_timeout,
+                                &smart_json)) {
+      dout(10) << "block_device_get_metrics failed for /dev/" << dev << dendl;
+      continue;
     }
-    // no need to result.clear() or clear smart_json
+    json_map[devid] = smart_json;
   }
   json_spirit::write(json_map, ss, json_spirit::pretty_print);
 }