fs,util: Fix a bug where zone locks are not handled correctly

Change signature of `ListZenFileSystems` to return status by value and file system list by reference. Reported-by: Mohit Joshi <[email protected]> Reported-by: Yura Sorokin <[email protected]> Signed-off-by: Andreas Hindborg <[email protected]>
westerndigitalcorporation · Jan 5, 2022 · 52b9c51 · 52b9c51
1 parent 756b86e
commit 52b9c51
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 13 deletions.
diff --git a/fs/fs_zenfs.cc b/fs/fs_zenfs.cc
@@ -1155,8 +1155,9 @@ Status NewZenFS(FileSystem** fs, const std::string& bdevname,
   return Status::OK();
 }
 
-std::map<std::string, std::string> ListZenFileSystems() {
+Status ListZenFileSystems(std::map<std::string, std::string>& out_list) {
   std::map<std::string, std::string> zenFileSystems;
+
   auto closedirDeleter = [](DIR* d) {
     if (d != nullptr) closedir(d);
   };
@@ -1180,6 +1181,10 @@ std::map<std::string, std::string> ListZenFileSystems() {
         for (const auto z : metazones) {
           Superblock super_block;
           std::unique_ptr<ZenMetaLog> log;
+          if (!z->Acquire()) {
+            return Status::Aborted("Could not aquire busy flag of zone" +
+                                   std::to_string(z->GetZoneNr()));
+          }
           log.reset(new ZenMetaLog(zbd.get(), z));
 
           if (!log->ReadRecord(&super_record, &scratch).ok()) continue;
@@ -1202,7 +1207,8 @@ std::map<std::string, std::string> ListZenFileSystems() {
     }
   }
 
-  return zenFileSystems;
+  out_list = std::move(zenFileSystems);
+  return Status::OK();
 }
 
 void ZenFS::GetZoneSnapshot(std::vector<ZoneSnapshot>& zones) {
@@ -1234,16 +1240,20 @@ FactoryFunc<FileSystem> zenfs_filesystem_reg =
               *errmsg = s.ToString();
             }
           } else if (devID.rfind("uuid:") == 0) {
-            std::map<std::string, std::string> zenFileSystems =
-                ListZenFileSystems();
-            devID.replace(0, strlen("uuid:"), "");
-
-            if (zenFileSystems.find(devID) == zenFileSystems.end()) {
-              *errmsg = "UUID not found";
+            std::map<std::string, std::string> zenFileSystems;
+            s = ListZenFileSystems(zenFileSystems);
+            if (!s.ok()) {
+              *errmsg = s.ToString();
             } else {
-              s = NewZenFS(&fs, zenFileSystems[devID]);
-              if (!s.ok()) {
-                *errmsg = s.ToString();
+              devID.replace(0, strlen("uuid:"), "");
+
+              if (zenFileSystems.find(devID) == zenFileSystems.end()) {
+                *errmsg = "UUID not found";
+              } else {
+                s = NewZenFS(&fs, zenFileSystems[devID]);
+                if (!s.ok()) {
+                  *errmsg = s.ToString();
+                }
               }
             }
           } else {

diff --git a/fs/fs_zenfs.h b/fs/fs_zenfs.h
@@ -373,6 +373,6 @@ class ZenFS : public FileSystemWrapper {
 Status NewZenFS(
     FileSystem** fs, const std::string& bdevname,
     std::shared_ptr<ZenFSMetrics> metrics = std::make_shared<NoZenFSMetrics>());
-std::map<std::string, std::string> ListZenFileSystems();
+Status ListZenFileSystems(std::map<std::string, std::string>& out_list);
 
 }  // namespace ROCKSDB_NAMESPACE
diff --git a/util/zenfs.cc b/util/zenfs.cc
@@ -278,7 +278,13 @@ int zenfs_tool_df() {
 }
 
 int zenfs_tool_lsuuid() {
-  std::map<std::string, std::string> zenFileSystems = ListZenFileSystems();
+  std::map<std::string, std::string> zenFileSystems;
+  Status s = ListZenFileSystems(zenFileSystems);
+  if (!s.ok()) {
+    fprintf(stderr, "Failed to enumerate file systems: %s",
+            s.ToString().c_str());
+    return 1;
+  }
 
   for (const auto &p : zenFileSystems)
     fprintf(stdout, "%s\t%s\n", p.first.c_str(), p.second.c_str());