Re: [PATCH] rbd: Add --json flag for the showmapped command

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/13/2012 07:37 AM, Stratos Psomadakis wrote:
Signed-off-by: Stratos Psomadakis <psomas@xxxxxxxx>
---
Hi Josh,

This patch adds the '--json' flag to enable dumping the showmapped output in
json format (as you suggested). I'm not sure if any other rbd subcommands could
make use of this flag (so the patch is showmapped-specific atm).

Thanks, this looks good overall. Mainly some style nits below.

Other subcommands that could use json output would be:

info
ls [-l]
snap ls
lock list
children

Comments?

Thanks,
Stratos

  man/rbd.8  |    5 +++++
  src/rbd.cc |   54 +++++++++++++++++++++++++++++++++++++++++-------------
  2 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/man/rbd.8 b/man/rbd.8
index 6004244..7bf4c39 100644
--- a/man/rbd.8
+++ b/man/rbd.8
@@ -132,6 +132,11 @@ be open from more than one client at once, like during
  live migration of a virtual machine, or for use underneath
  a clustered filesystem.
  .UNINDENT
+.INDENT 0.0
+.TP
+.B \-\-json
+Dump output in json format (currently used only by showmapped).
+.UNINDENT
  .SH COMMANDS
  .INDENT 0.0
  .TP

These man pages are actually generated from rst in ceph.git.
In this case the source file is docs/man/8/rbd.rst. Building
the documentation rebuilds the man pages as well [1], although
this could be more explicit in the docs.

If you send the rst change, I can rebuild the man pages.

diff --git a/src/rbd.cc b/src/rbd.cc
index 3db8978..82a72ba 100644
--- a/src/rbd.cc
+++ b/src/rbd.cc
@@ -47,6 +47,8 @@
  #include "include/rbd_types.h"
  #include "common/TextTable.h"

+#include "common/Formatter.h"
+
  #if defined(__linux__)
  #include <linux/fs.h>
  #endif
@@ -126,7 +128,8 @@ void usage()
  "                               format 2 supports cloning\n"
  "  --id <username>              rados user (without 'client.' prefix) to authenticate as\n"
  "  --keyfile <path>             file containing secret key for use with cephx\n"
-"  --shared <tag>               take a shared (rather than exclusive) lock\n";
+"  --shared <tag>               take a shared (rather than exclusive) lock\n"
+"  --json                       dump output in json format (currently used only by showmapped)\n";
  }

  static string feature_str(uint64_t features)
@@ -1198,7 +1201,7 @@ void do_closedir(DIR *dp)
      closedir(dp);
  }

-static int do_kernel_showmapped()
+static int do_kernel_showmapped(bool dump_json)
  {
    int r;
    bool have_output = false;
@@ -1220,12 +1223,18 @@ static int do_kernel_showmapped()
      return r;
    }

+  JSONFormatter jsf(false);
    TextTable tbl;
-  tbl.define_column("id", TextTable::LEFT, TextTable::LEFT);
-  tbl.define_column("pool", TextTable::LEFT, TextTable::LEFT);
-  tbl.define_column("image", TextTable::LEFT, TextTable::LEFT);
-  tbl.define_column("snap", TextTable::LEFT, TextTable::LEFT);
-  tbl.define_column("device", TextTable::LEFT, TextTable::LEFT);
+  if(dump_json == false) {

Missing space between if and (. Swapping the cases so if (dump_json)
came first would be a little nicer to read.

+    tbl.define_column("id", TextTable::LEFT, TextTable::LEFT);
+    tbl.define_column("pool", TextTable::LEFT, TextTable::LEFT);
+    tbl.define_column("image", TextTable::LEFT, TextTable::LEFT);
+    tbl.define_column("snap", TextTable::LEFT, TextTable::LEFT);
+    tbl.define_column("device", TextTable::LEFT, TextTable::LEFT);
+  }
+  else {

} else {

+    jsf.open_object_section("devices");
+  }

    do {
      if (strcmp(dent->d_name, ".") == 0 || strcmp(dent->d_name, "..") == 0)
@@ -1262,13 +1271,30 @@ static int do_kernel_showmapped()
  	   << cpp_strerror(-r) << std::endl;
        continue;
      }
-
-    tbl << dent->d_name << pool << name << snap << dev << TextTable::endrow;
+
+    if (dump_json == false) {

same comment about swapping the cases

+      tbl << dent->d_name << pool << name << snap << dev << TextTable::endrow;
+    }
+    else {

} else {

+      jsf.open_object_section(dent->d_name);
+      jsf.dump_string("pool", pool);
+      jsf.dump_string("name", name);
+      jsf.dump_string("snap", snap);
+      jsf.dump_string("device", dev);
+      jsf.close_section();
+    }
      have_output = true;

    } while ((dent = readdir(device_dir.get())));
-  if (have_output)
-    cout << tbl;
+
+  if (dump_json == false) {

same comment about swapping the cases

+    if (have_output)
+      cout << tbl;
+  }
+  else {

} else {

+    jsf.close_section();
+    jsf.flush(cout);
+  }

    return 0;
  }
@@ -1505,7 +1531,7 @@ int main(int argc, const char **argv)
    const char *poolname = NULL;
    uint64_t size = 0;  // in bytes
    int order = 0;
-  bool format_specified = false;
+  bool format_specified = false, dump_json = false;
    int format = 1;
    uint64_t features = RBD_FEATURE_LAYERING;
    const char *imgname = NULL, *snapname = NULL, *destname = NULL,
@@ -1579,6 +1605,8 @@ int main(int argc, const char **argv)
        imgname = strdup(val.c_str());
      } else if (ceph_argparse_witharg(args, i, &val, "--shared", (char *)NULL)) {
        lock_tag = strdup(val.c_str());
+    } else if (ceph_argparse_flag(args, i, "--json", (char *) NULL)) {
+      dump_json = true;
      } else {
        ++i;
      }
@@ -2130,7 +2158,7 @@ if (!set_conf_param(v, p1, p2, p3)) { \
      break;

    case OPT_SHOWMAPPED:
-    r = do_kernel_showmapped();
+    r = do_kernel_showmapped(dump_json);
      if (r < 0) {
        cerr << "rbd: showmapped failed: " << cpp_strerror(-r) << std::endl;
        return EXIT_FAILURE;


It'd be nice to report an error and fail if the --json option (or
whatever it ends up being) is provided for a command that doesn't use
it.

Josh

[1] http://ceph.com/docs/master/dev/generatedocs/
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [CEPH Users]     [Ceph Large]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux