blkdev: Add dedicated accessors for all property functions

Hide get_block_device_string_property and get_block_device_int_property
to lower risk of misuse.

Signed-off-by: Alan Somers <asomers@gmail.com>

diff --git a/src/common/blkdev.cc b/src/common/blkdev.cc
index 197cf895120db1b0fd23c81fdce7e863d10cc9e8..0534a4d85923288045bc7759a60b5ba921c395b0 100644 (file)
--- a/src/common/blkdev.cc
+++ b/src/common/blkdev.cc
@@ -174,7 +174,7 @@ int get_block_device_base(const char *dev, char *out, size_t out_len)
  * return 0 on success
  * return negative error on error
  */
-int64_t get_block_device_string_property(const char *devname,
+static int64_t get_block_device_string_property(const char *devname,
                                         blkdev_prop_t prop,
                                         char *val, size_t maxlen)
 {
@@ -211,7 +211,7 @@ int64_t get_block_device_string_property(const char *devname,
  * return the value (we assume it is positive)
  * return negative error on error
  */
-int64_t get_block_device_int_property(const char *devname, blkdev_prop_t prop)
+static int64_t get_block_device_int_property(const char *devname, blkdev_prop_t prop)
 {
   char buff[256] = {0};
   int r = get_block_device_string_property(devname, prop, buff, sizeof(buff));
@@ -242,6 +242,15 @@ int block_device_discard(int fd, int64_t offset, int64_t len)
   return ioctl(fd, BLKDISCARD, range);
 }
 
+bool block_device_is_nvme(const char *devname)
+{
+  char vendor[80];
+  // nvme has a device/device/vendor property; infer from that.  There is
+  // probably a better way?
+  int r = get_block_device_string_property(devname, BLKDEV_PROP_VENDOR, vendor, 80);
+  return (r > 0);
+}
+
 bool block_device_is_rotational(const char *devname)
 {
   return get_block_device_int_property(devname, BLKDEV_PROP_ROTATIONAL) > 0;
@@ -252,6 +261,11 @@ int block_device_vendor(const char *devname, char *vendor, size_t max)
   return get_block_device_string_property(devname, BLKDEV_PROP_VENDOR, vendor, max);
 }
 
+int block_device_dev(const char *devname, char *dev, size_t max)
+{
+  return get_block_device_string_property(devname, BLKDEV_PROP_DEV, dev, max);
+}
+
 int block_device_model(const char *devname, char *model, size_t max)
 {
   return get_block_device_string_property(devname, BLKDEV_PROP_MODEL, model,
@@ -259,6 +273,13 @@ int block_device_model(const char *devname, char *model, size_t max)
 }
 
 int block_device_serial(const char *devname, char *serial, size_t max)
+{
+  return get_block_device_string_property(devname, BLKDEV_PROP_SERIAL, serial,
+                                          max);
+}
+
+int get_device_by_uuid(uuid_d dev_uuid, const char* label, char* partition,
+       char* device)
 {
   return get_block_device_string_property(devname, BLKDEV_PROP_SERIAL, serial, max);
 }
@@ -536,6 +557,11 @@ int block_device_discard(int fd, int64_t offset, int64_t len)
   return -EOPNOTSUPP;
 }
 
+bool block_device_is_nvme(const char *devname)
+{
+  return false;
+}
+
 bool block_device_is_rotational(const char *devname)
 {
   return false;
@@ -581,6 +607,11 @@ int block_device_discard(int fd, int64_t offset, int64_t len)
   return -EOPNOTSUPP;
 }
 
+bool block_device_is_nvme(const char *devname)
+{
+  return false;
+}
+
 bool block_device_is_rotational(const char *devname)
 {
   return false;
@@ -638,6 +669,11 @@ int block_device_discard(int fd, int64_t offset, int64_t len)
   return -EOPNOTSUPP;
 }
 
+bool block_device_is_nvme(const char *devname)
+{
+  return false;
+}
+
 bool block_device_is_rotational(const char *devname)
 {
   return false;

diff --git a/src/common/blkdev.h b/src/common/blkdev.h
index a2569c23fc9101def1597bd6d9cd406eeddda67b..81525ea6634135191c0fbba99409b349e66fbacb 100644 (file)
--- a/src/common/blkdev.h
+++ b/src/common/blkdev.h
@@ -27,12 +27,8 @@ extern int get_device_by_path(const char *path, char* partition, char* device, s
 extern int get_device_by_fd(int fd, char* partition, char* device, size_t max);
 
 // from a device (e.g., "sdb")
-extern int64_t get_block_device_int_property(
-       const char *devname, blkdev_prop_t prop);
-extern int64_t get_block_device_string_property(
-       const char *devname, blkdev_prop_t prop,
-       char *val, size_t maxlen);
 extern bool block_device_support_discard(const char *devname);
+extern bool block_device_is_nvme(const char *devname);
 extern bool block_device_is_rotational(const char *devname);
 extern int block_device_vendor(const char *devname, char *vendor, size_t max);
 extern int block_device_model(const char *devname, char *model, size_t max);
@@ -50,5 +46,8 @@ extern int block_device_run_smartctl(const char *device, int timeout,
 extern int get_vdo_stats_handle(const char *devname, std::string *vdo_name);
 extern int64_t get_vdo_stat(int fd, const char *property);
 extern bool get_vdo_utilization(int fd, uint64_t *total, uint64_t *avail);
+extern int block_device_dev(const char *devname, char *dev, size_t max);
+extern int block_device_model(const char *devname, char *model, size_t max);
+extern int block_device_serial(const char *devname, char *serial, size_t max);
 
 #endif

diff --git a/src/os/bluestore/KernelDevice.cc b/src/os/bluestore/KernelDevice.cc
index 0bf470c537fdaecbeb4f94cfa3d355717e9599df..5f86a439a0a482b7fa6ef4db9cbfb2807b26f8a8 100644 (file)
--- a/src/os/bluestore/KernelDevice.cc
+++ b/src/os/bluestore/KernelDevice.cc
@@ -209,13 +209,6 @@ void KernelDevice::close()
   path.clear();
 }
 
-static string get_dev_property(const char *dev, blkdev_prop_t property)
-{
-  char val[1024] = {0};
-  get_block_device_string_property(dev, property, val, sizeof(val));
-  return val;
-}
-
 int KernelDevice::collect_metadata(const string& prefix, map<string,string> *pm) const
 {
   (*pm)[prefix + "rotational"] = stringify((int)(bool)rotational);
@@ -255,21 +248,23 @@ int KernelDevice::collect_metadata(const string& prefix, map<string,string> *pm)
       break;
     default:
       {
+       char buffer[1024] = {0};
        (*pm)[prefix + "partition_path"] = string(partition_path);
        (*pm)[prefix + "dev_node"] = string(dev_node);
-       (*pm)[prefix + "model"] = get_dev_property(dev_node, BLKDEV_PROP_MODEL);
-       (*pm)[prefix + "dev"] = get_dev_property(dev_node, BLKDEV_PROP_DEV);
 
+       block_device_model(dev_node, buffer, sizeof(buffer));
+       (*pm)[prefix + "model"] = buffer;
+       
+       buffer[0] = '\0';
+       block_device_dev(dev_node, buffer, sizeof(buffer));
+       (*pm)[prefix + "dev"] = buffer;
+       
        // nvme exposes a serial number
-       string serial = get_dev_property(dev_node, BLKDEV_PROP_SERIAL);
-       if (serial.length()) {
-         (*pm)[prefix + "serial"] = serial;
-       }
+       buffer[0] = '\0';
+       block_device_serial(dev_node, buffer, sizeof(buffer));
+       (*pm)[prefix + "serial"] = buffer;
 
-       // nvme has a device/device/* structure; infer from that.  there
-       // is probably a better way?
-       string nvme_vendor = get_dev_property(dev_node, BLKDEV_PROP_VENDOR);
-       if (nvme_vendor.length()) {
+       if (block_device_is_nvme(dev_node)) {
          (*pm)[prefix + "type"] = "nvme";
        }
       }

diff --git a/src/os/bluestore/PMEMDevice.cc b/src/os/bluestore/PMEMDevice.cc
index a711c84a7eb61a1088f9239942bd4edb27287303..ee6bf771508cfd91b1283ebe43e0be9e0df94ddb 100644 (file)
--- a/src/os/bluestore/PMEMDevice.cc
+++ b/src/os/bluestore/PMEMDevice.cc
@@ -128,13 +128,6 @@ void PMEMDevice::close()
   path.clear();
 }
 
-static string get_dev_property(const char *dev, blkdev_prop_t property)
-{
-  char val[1024] = {0};
-  get_block_device_string_property(dev, property, val, sizeof(val));
-  return val;
-}
-
 int PMEMDevice::collect_metadata(const string& prefix, map<string,string> *pm) const
 {
   (*pm)[prefix + "rotational"] = stringify((int)(bool)rotational);
@@ -164,21 +157,23 @@ int PMEMDevice::collect_metadata(const string& prefix, map<string,string> *pm) c
       break;
     default:
       {
+       char buffer[1024] = {0};
        (*pm)[prefix + "partition_path"] = string(partition_path);
        (*pm)[prefix + "dev_node"] = string(dev_node);
-       (*pm)[prefix + "model"] = get_dev_property(dev_node, BLKDEV_PROP_MODEL);
-       (*pm)[prefix + "dev"] = get_dev_property(dev_node, BLKDEV_PROP_DEV);
 
+       block_device_model(dev_node, buffer, sizeof(buffer));
+       (*pm)[prefix + "model"] = buffer;
+       
+       buffer[0] = '\0';
+       block_device_dev(dev_node, buffer, sizeof(buffer));
+       (*pm)[prefix + "dev"] = buffer;
+       
        // nvme exposes a serial number
-       string serial = get_dev_property(dev_node, BLKDEV_PROP_SERIAL);
-       if (serial.length()) {
-         (*pm)[prefix + "serial"] = serial;
-       }
+       buffer[0] = '\0';
+       block_device_serial(dev_node, buffer, sizeof(buffer));
+       (*pm)[prefix + "serial"] = buffer;
 
-       // nvme has a device/device/* structure; infer from that.  there
-       // is probably a better way?
-       string nvme_vendor = get_dev_property(dev_node, BLKDEV_PROP_VENDOR);
-       if (nvme_vendor.length()) {
+       if (block_device_nvme(dev_node)) {
          (*pm)[prefix + "type"] = "nvme";
        }
       }

diff --git a/src/test/common/test_blkdev.cc b/src/test/common/test_blkdev.cc
index fbca85c37c05c3e2c5c0a6447fd1b77286317de7..625b84bca0574929950d940e26d6d0de9a070df1 100644 (file)
--- a/src/test/common/test_blkdev.cc
+++ b/src/test/common/test_blkdev.cc
@@ -48,8 +48,7 @@ TEST(blkdev, get_block_device_base) {
       ASSERT_EQ(0, get_block_device_base(base, buf3, sizeof(buf3)));
       printf("  got '%s' expected '%s'\n", buf3, de->d_name);
       ASSERT_EQ(0, strcmp(de->d_name, buf3));
-      printf("  discard granularity = %lld .. supported = %d\n",
-            (long long)get_block_device_int_property(base, BLKDEV_PROP_DISCARD_GRANULARITY),
+      printf("  discard  supported = %d\n",
             (int)block_device_support_discard(base));
 
       char subdirfn[PATH_MAX];
@@ -75,8 +74,7 @@ TEST(blkdev, get_block_device_base) {
        ASSERT_EQ(0, get_block_device_base(part, buf3, sizeof(buf3)));
        printf("  got '%s' expected '%s'\n", buf3, de->d_name);
        ASSERT_EQ(0, strcmp(buf3, de->d_name));
-       printf("  discard granularity = %lld .. supported = %d\n",
-              (long long)get_block_device_int_property(part, BLKDEV_PROP_DISCARD_GRANULARITY),
+       printf("  discard  supported = %d\n",
               (int)block_device_support_discard(part));
       }
 
@@ -101,21 +99,6 @@ TEST(blkdev, device_model)
   ASSERT_EQ(strcmp(model, "myfancymodel"), 0);
 }
 
-TEST(blkdev, get_block_device_string_property)
-{
-  const char* env = getenv("CEPH_ROOT");
-  ASSERT_NE(env, nullptr) << "Environment Variable CEPH_ROOT not found!";
-  string root = string(env) + "/src/test/common/test_blkdev_sys_block";
-  set_block_device_sandbox_dir(root.c_str());
-
-  char val[1000] = {0};
-  int rc = get_block_device_string_property("sda", BLKDEV_PROP_MODEL,
-                                           val, sizeof(val));
-  ASSERT_EQ(0, rc);
-  printf("val '%s'\n", val);
-  ASSERT_EQ(strcmp(val, "myfancymodel"), 0);
-}
-
 TEST(blkdev, is_rotational)
 {
   const char* env = getenv("CEPH_ROOT");
@@ -126,5 +109,3 @@ TEST(blkdev, is_rotational)
   ASSERT_FALSE(block_device_is_rotational("sda"));
   ASSERT_TRUE(block_device_is_rotational("sdb"));
 }
-
-