From edc78e46cee356da1e45247c38b7428dd6c965cb Mon Sep 17 00:00:00 2001
From: Patrick Donnelly <pdonnell@redhat.com>
Date: Mon, 10 Oct 2016 18:16:16 -0400
Subject: [PATCH] mds: use parse_filesystem in parse_role

This allows us to reuse code in parse_filesystem and avoid
get_filesystem which may fail if the fscid does not exist. This would
result in the program (mon) aborting due to the uncaught exception.

Fixes: http://tracker.ceph.com/issues/17518

Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
---
 src/mds/FSMap.cc | 75 ++++++++++++++++--------------------------------
 1 file changed, 25 insertions(+), 50 deletions(-)

diff --git a/src/mds/FSMap.cc b/src/mds/FSMap.cc
index 3a66f6eca48ab..ad2fb634bcb4d 100644
--- a/src/mds/FSMap.cc
+++ b/src/mds/FSMap.cc
@@ -846,66 +846,41 @@ int FSMap::parse_role(
     mds_role_t *role,
     std::ostream &ss) const
 {
-  auto colon_pos = role_str.find(":");
-
-  if (colon_pos != std::string::npos && colon_pos != role_str.size()) {
-    auto fs_part = role_str.substr(0, colon_pos);
-    auto rank_part = role_str.substr(colon_pos + 1);
-
-    std::string err;
-    fs_cluster_id_t fs_id = FS_CLUSTER_ID_NONE;
-    long fs_id_i = strict_strtol(fs_part.c_str(), 10, &err);
-    if (fs_id_i < 0 || !err.empty()) {
-      // Try resolving as name
-      auto fs = get_filesystem(fs_part);
-      if (fs == nullptr) {
-        ss << "Unknown filesystem name '" << fs_part << "'";
-        return -EINVAL;
-      } else {
-        fs_id = fs->fscid;
-      }
-    } else {
-      fs_id = fs_id_i;
-    }
-
-    mds_rank_t rank;
-    long rank_i = strict_strtol(rank_part.c_str(), 10, &err);
-    if (rank_i < 0 || !err.empty()) {
-      ss << "Invalid rank '" << rank_part << "'";
-      return -EINVAL;
-    } else {
-      rank = rank_i;
-    }
-
-    *role = {fs_id, rank};
-  } else {
-    std::string err;
-    long who_i = strict_strtol(role_str.c_str(), 10, &err);
-    if (who_i < 0 || !err.empty()) {
-      ss << "Invalid rank '" << role_str << "'";
-      return -EINVAL;
-    }
-
+  size_t colon_pos = role_str.find(":");
+  size_t rank_pos;
+  std::shared_ptr<const Filesystem> fs;
+  if (colon_pos == std::string::npos) {
     if (legacy_client_fscid == FS_CLUSTER_ID_NONE) {
       ss << "No filesystem selected";
       return -ENOENT;
-    } else {
-      *role = mds_role_t(legacy_client_fscid, who_i);
     }
+    fs = get_filesystem(legacy_client_fscid);
+    rank_pos = 0;
+  } else {
+    if (parse_filesystem(role_str.substr(0, colon_pos), &fs) < 0) {
+      ss << "Invalid filesystem";
+      return -ENOENT;
+    }
+    rank_pos = colon_pos+1;
   }
 
-  // Now check that the role actually exists
-  if (get_filesystem(role->fscid) == nullptr) {
-    ss << "Filesystem with ID '" << role->fscid << "' not found";
-    return -ENOENT;
+  mds_rank_t rank;
+  std::string err;
+  std::string rank_str = role_str.substr(rank_pos);
+  long rank_i = strict_strtol(rank_str.c_str(), 10, &err);
+  if (rank_i < 0 || !err.empty()) {
+    ss << "Invalid rank '" << rank_str << "'";
+    return -EINVAL;
+  } else {
+    rank = rank_i;
   }
 
-  auto fs = get_filesystem(role->fscid);
-  if (fs->mds_map.in.count(role->rank) == 0) {
-    ss << "Rank '" << role->rank << "' not found";
+  if (fs->mds_map.in.count(rank) == 0) {
+    ss << "Rank '" << rank << "' not found";
     return -ENOENT;
   }
 
+  *role = {fs->fscid, rank};
+
   return 0;
 }
-
-- 
2.39.5