From: Ilya Dryomov <idryomov@gmail.com>
Date: Thu, 6 May 2021 09:20:23 +0000 (+0200)
Subject: rbd: don't attempt to interpret image cache state json
X-Git-Tag: v16.2.5~116^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=a9c8aa1cc09b367aa8025ccbca58e0e50ebaa6ba;p=ceph.git

rbd: don't attempt to interpret image cache state json

There are several issues with the current code:

- the value of "present" field is lost to the value of "clean" -- if the
  cache is present but dirty, we report that there is no cache
- the value of "clean" field is lost -- we always report false
- for a cache bigger than 2G, size is reported incorrectly -- 32-bit int
  overflow
- there is no way to get the type of the cache (rwl vs ssd)

Finally, even with all of the above fixed, image cache state output
would still be confusing because it is effectively a snapshot from
the time the cache was loaded.  It is not updated until the cache is
orderly closed.

Given that more changes would be needed to make "rbd status" output
useful and in the meantime it just needs to be correct to allow for
basic troubleshooting, just dump raw json.

Fixes: https://tracker.ceph.com/issues/50613
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit f70bb26a66cccf560065eb7bdf083361a1caec1a)
---

diff --git a/src/tools/rbd/action/Status.cc b/src/tools/rbd/action/Status.cc
index 7c3a02207dec..b63c455dd7b0 100644
--- a/src/tools/rbd/action/Status.cc
+++ b/src/tools/rbd/action/Status.cc
@@ -1,7 +1,6 @@
 // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
 // vim: ts=8 sw=2 smarttab
 
-#include "common/ceph_json.h"
 #include "common/errno.h"
 #include "common/Formatter.h"
 #include "tools/rbd/ArgumentTypes.h"
@@ -23,39 +22,6 @@ namespace {
 
 const std::string IMAGE_CACHE_STATE = ".librbd/image_cache_state";
 
-struct ImageCacheState {
-  bool present = false;
-  bool clean = false;
-  int size = 0;
-  std::string host;
-  std::string path;
-};
-
-bool image_cache_parse(const std::string& s, ImageCacheState &cache_state) {
-  JSONParser p;
-  JSONFormattable f;
-  bool success = p.parse(s.c_str(), s.size());
-  if (success) {
-    decode_json_obj(f, &p);
-    if ((success = f.exists("present"))) {
-      cache_state.present = (bool)f["present"];
-    }
-    if (success && (success = f.exists("clean"))) {
-      cache_state.present = (bool)f["clean"];
-    }
-    if (success && (success = f.exists("pwl_size"))) {
-      cache_state.size = (int)f["pwl_size"];
-    }
-    if (success && (success = f.exists("pwl_host"))) {
-      cache_state.host = (std::string)f["pwl_host"];
-    }
-    if (success && (success = f.exists("pwl_path"))) {
-      cache_state.path = (std::string)f["pwl_path"];
-    }
-  }
-  return success;
-}
-
 } // anonymous namespace
 
 static int do_show_status(librados::IoCtx& io_ctx, const std::string &image_name,
@@ -137,19 +103,13 @@ static int do_show_status(librados::IoCtx& io_ctx, const std::string &image_name
     }
   }
 
-  ImageCacheState cache_state;
+  std::string image_cache_str;
   if (features & RBD_FEATURE_DIRTY_CACHE) {
-    std::string image_cache_str;
     r = image.metadata_get(IMAGE_CACHE_STATE, &image_cache_str);
     if (r < 0) {
-      std::cerr << "rbd: getting image cache status failed: " << cpp_strerror(r)
+      std::cerr << "rbd: getting image cache state failed: " << cpp_strerror(r)
                 << std::endl;
-    } else {
-      r = image_cache_parse(image_cache_str, cache_state);
-      if (r < 0) {
-        std::cerr << "rbd: image cache metadata is corrupted: " << cpp_strerror(r)
-                  << std::endl;
-      }
+      // not fatal
     }
   }
 
@@ -186,13 +146,8 @@ static int do_show_status(librados::IoCtx& io_ctx, const std::string &image_name
       f->dump_string("state_description", migration_status.state_description);
       f->close_section(); // migration
     }
-    if (cache_state.present) {
-      f->open_object_section("image_cache_state");
-      f->dump_bool("clean", cache_state.clean);
-      f->dump_int("size", cache_state.size);
-      f->dump_string("host", cache_state.host);
-      f->dump_string("path", cache_state.path);
-      f->close_section(); // image_cache_state
+    if (!image_cache_str.empty()) {
+      f->dump_string("image_cache_state", image_cache_str);
     }
   } else {
     if (watchers.size()) {
@@ -234,13 +189,8 @@ static int do_show_status(librados::IoCtx& io_ctx, const std::string &image_name
       std::cout << std::endl;
     }
 
-    if (cache_state.present) {
-        std::cout << "Image cache state:" << std::endl;
-        std::cout << "\tclean: " << (cache_state.clean ? "true" : "false")
-                  << std::endl
-                  << "\tsize: " << byte_u_t(cache_state.size) << std::endl
-                  << "\thost: " << cache_state.host << std::endl
-                  << "\tpath: " << cache_state.path << std::endl;
+    if (!image_cache_str.empty()) {
+        std::cout << "Image cache state: " << image_cache_str << std::endl;
     }
   }