crush: revise JSON format for 'item' type

Commit a7e9a7b648 changed the JSON format of CRUSH rules
such that the 'item' attribute on a step was sometimes
an integer and sometimes a string.

This commit separates the integer and string representations
so that tools which rely on a 'item' consistently being an
integer ID will work.

Signed-off-by: John Spray <john.spray@inktank.com>

diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc
index 8db506a6f2d00aab3a85f86c7ee8046cf5fdb4b7..61b8a8a564d77e7688d34a51fab7e27314063f86 100644 (file)
--- a/src/crush/CrushWrapper.cc
+++ b/src/crush/CrushWrapper.cc
@@ -1202,11 +1202,10 @@ void CrushWrapper::dump_rule(int ruleset, Formatter *f) const
       f->dump_string("op", "take");
       {
         int item = get_rule_arg1(ruleset, j);
+        f->dump_int("item", item);
+
         const char *name = get_item_name(item);
-        if (name == NULL)
-          f->dump_int("item", item);
-        else
-          f->dump_string("item", name);
+        f->dump_string("item_name", name ? name : "");
       }
       break;
     case CRUSH_RULE_EMIT:

diff --git a/src/test/crush/TestCrushWrapper.cc b/src/test/crush/TestCrushWrapper.cc
index 3b2990d76b732ea968103ee67426ae74f2ea1c34..73e0398bc2a70b704670e99e01d17d7c5551a794 100644 (file)
--- a/src/test/crush/TestCrushWrapper.cc
+++ b/src/test/crush/TestCrushWrapper.cc
@@ -534,7 +534,7 @@ TEST(CrushWrapper, dump_rules) {
     delete f;
     EXPECT_EQ((unsigned)0, ss.str().find("<rule><rule_id>0</rule_id><rule_name>NAME</rule_name>"));
     EXPECT_NE(string::npos,
-             ss.str().find("<step><op>take</op><item>default</item></step>"));
+             ss.str().find("<item_name>default</item_name></step>"));
   }
 
   delete c;

diff --git a/src/test/mon/osd-crush.sh b/src/test/mon/osd-crush.sh
index efe7fa52864ad2ccc134c4d9af27e42861e41dc2..2e42fa6ce7da9914954ed05b195f649c5270795e 100755 (executable)
--- a/src/test/mon/osd-crush.sh
+++ b/src/test/mon/osd-crush.sh
@@ -38,7 +38,7 @@ function run() {
 function TEST_crush_rule_create_simple() {
     local dir=$1
     ./ceph --format xml osd crush rule dump replicated_ruleset | \
-        grep '<op>take</op><item>default</item>' | \
+        egrep '<op>take</op><item>[^<]+</item><item_name>default</item_name>' | \
         grep '<op>chooseleaf_firstn</op><num>0</num><type>host</type>' || return 1
     local ruleset=ruleset0
     local root=host1
@@ -48,7 +48,7 @@ function TEST_crush_rule_create_simple() {
     ./ceph osd crush rule create-simple $ruleset $root $failure_domain 2>&1 | \
         grep "$ruleset already exists" || return 1
     ./ceph --format xml osd crush rule dump $ruleset | \
-        grep '<op>take</op><item>'$root'</item>' | \
+        egrep '<op>take</op><item>[^<]+</item><item_name>'$root'</item_name>' | \
         grep '<op>choose_firstn</op><num>0</num><type>'$failure_domain'</type>' || return 1
     ./ceph osd crush rule rm $ruleset || return 1
 }