crushtool: reorder order of operations

It is currently possible to get very confusing results, e.g.

 crushtool -i foo --set-straw-calc-version 1 --reweight -o bar

will reweight before adjusting the tunable.  Or,

 crushtool -i foo --tree --reweight

will display the map before modifying it.

Rearrange everything into distinct phases:

 - input/build
 - tunables adjustments
 - modifications
 - display/test
 - output

Backport: firefly
Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/test/cli/crushtool/set-choose.t b/src/test/cli/crushtool/set-choose.t
index b40494d018faf831a7b6a33562a1f1de849bb741..42df9f21e260cb9cb77dde5948a93c25281ddc9c 100644 (file)
--- a/src/test/cli/crushtool/set-choose.t
+++ b/src/test/cli/crushtool/set-choose.t
@@ -1,6 +1,5 @@
   $ crushtool -c "$TESTDIR/set-choose.crushmap.txt" -o set-choose.crushmap
   $ crushtool -i set-choose.crushmap --test --show-mappings --show-statistics --set-straw-calc-version 0
-  crushtool successfully built or modified map.  Use '-o <file>' to write it out.
   rule 0 (choose), x = 0..1023, numrep = 2..3
   CRUSH rule 0 x 0 [0,3]
   CRUSH rule 0 x 1 [0,8]
@@ -12307,8 +12306,8 @@
   CRUSH rule 5 x 1022 [1,6,4]
   CRUSH rule 5 x 1023 [3,2,8]
   rule 5 (chooseleaf-set) num_rep 3 result size == 3:\t1024/1024 (esc)
-  $ crushtool -i set-choose.crushmap --test --show-mappings --show-statistics --weight 0 0 --weight 1 0 --weight 3 0 --weight 4 0 --set-straw-calc-version 0
   crushtool successfully built or modified map.  Use '-o <file>' to write it out.
+  $ crushtool -i set-choose.crushmap --test --show-mappings --show-statistics --weight 0 0 --weight 1 0 --weight 3 0 --weight 4 0 --set-straw-calc-version 0
   rule 0 (choose), x = 0..1023, numrep = 2..3
   CRUSH rule 0 x 0 [2,5]
   CRUSH rule 0 x 1 [2,8]
@@ -24620,8 +24619,8 @@
   CRUSH rule 5 x 1022 [2,6,5]
   CRUSH rule 5 x 1023 [5,2,8]
   rule 5 (chooseleaf-set) num_rep 3 result size == 3:\t1024/1024 (esc)
-  $ crushtool -i set-choose.crushmap --test --show-mappings --show-statistics --weight 0 0 --weight 3 0 --weight 4 .5 --weight 5 0 --weight 6 .1 --weight 7 0 --set-straw-calc-version 0
   crushtool successfully built or modified map.  Use '-o <file>' to write it out.
+  $ crushtool -i set-choose.crushmap --test --show-mappings --show-statistics --weight 0 0 --weight 3 0 --weight 4 .5 --weight 5 0 --weight 6 .1 --weight 7 0 --set-straw-calc-version 0
   rule 0 (choose), x = 0..1023, numrep = 2..3
   CRUSH rule 0 x 0 [2,4]
   CRUSH rule 0 x 1 [2,8]
@@ -36942,3 +36941,4 @@
   CRUSH rule 5 x 1023 [4,2,8]
   rule 5 (chooseleaf-set) num_rep 3 result size == 2:\t501/1024 (esc)
   rule 5 (chooseleaf-set) num_rep 3 result size == 3:\t523/1024 (esc)
+  crushtool successfully built or modified map.  Use '-o <file>' to write it out.

diff --git a/src/test/cli/crushtool/test-map-bobtail-tunables.t b/src/test/cli/crushtool/test-map-bobtail-tunables.t
index 77f2ce86243ebba8655a2020315684c2700ea956..5037cfd8e87190b70165ebfa7f593b3df967583a 100644 (file)
--- a/src/test/cli/crushtool/test-map-bobtail-tunables.t
+++ b/src/test/cli/crushtool/test-map-bobtail-tunables.t
@@ -1,5 +1,4 @@
   $ crushtool -i "$TESTDIR/test-map-a.crushmap" --test --show-mappings --show-statistics --rule 0 --set-choose-local-tries 0 --set-choose-local-fallback-tries 0 --set-choose-total-tries 50 --set-chooseleaf-descend-once 1
-  crushtool successfully built or modified map.  Use '-o <file>' to write it out.
   rule 0 (data), x = 0..1023, numrep = 1..10
   CRUSH rule 0 x 0 [36]
   CRUSH rule 0 x 1 [876]
@@ -10251,3 +10250,4 @@
   CRUSH rule 0 x 1022 [967,829,973,640,703,470,871,828,440,449]
   CRUSH rule 0 x 1023 [488,257,614,859,325,419,50,560,595,554]
   rule 0 (data) num_rep 10 result size == 10:\t1024/1024 (esc)
+  crushtool successfully built or modified map.  Use '-o <file>' to write it out.

diff --git a/src/test/cli/crushtool/test-map-firefly-tunables.t b/src/test/cli/crushtool/test-map-firefly-tunables.t
index a75e89fb1c59e3c0ef0c08c49c2fbc38361f131c..93bba48d787e2d9f269d165f138b6d64e8bf77fb 100644 (file)
--- a/src/test/cli/crushtool/test-map-firefly-tunables.t
+++ b/src/test/cli/crushtool/test-map-firefly-tunables.t
@@ -1,5 +1,4 @@
   $ crushtool -i "$TESTDIR/test-map-vary-r.crushmap" --test --show-mappings --show-statistics --rule 0 --set-choose-local-tries 0 --set-choose-local-fallback-tries 0 --set-choose-total-tries 50 --set-chooseleaf-descend-once 1 --set-chooseleaf-vary-r 1 --weight 12 0 --weight 20 0 --weight 30 0
-  crushtool successfully built or modified map.  Use '-o <file>' to write it out.
   rule 0 (data), x = 0..1023, numrep = 1..10
   CRUSH rule 0 x 0 [101]
   CRUSH rule 0 x 1 [80]
@@ -10257,3 +10256,4 @@
   CRUSH rule 0 x 1022 [73,21,36]
   CRUSH rule 0 x 1023 [0,16,3]
   rule 0 (data) num_rep 10 result size == 3:\t1024/1024 (esc)
+  crushtool successfully built or modified map.  Use '-o <file>' to write it out.

diff --git a/src/test/cli/crushtool/test-map-indep.t b/src/test/cli/crushtool/test-map-indep.t
index f4ee371696a3b6c4716bc05b3312db057437b08e..37eb7b51d116a6af0b2b583676cc436bba5ee686 100644 (file)
--- a/src/test/cli/crushtool/test-map-indep.t
+++ b/src/test/cli/crushtool/test-map-indep.t
@@ -1,5 +1,4 @@
   $ crushtool -i "$TESTDIR/test-map-indep.crushmap" --test --show-mappings --show-statistics --rule 1 --set-choose-local-tries 0 --set-choose-local-fallback-tries 0 --set-choose-total-tries 50 --set-chooseleaf-descend-once 2
-  crushtool successfully built or modified map.  Use '-o <file>' to write it out.
   rule 1 (metadata), x = 0..1023, numrep = 1..10
   CRUSH rule 1 x 0 [36]
   CRUSH rule 1 x 1 [876]
@@ -10251,3 +10250,4 @@
   CRUSH rule 1 x 1022 [967,829,973,640,703,470,871,828,440,449]
   CRUSH rule 1 x 1023 [488,257,614,859,325,419,50,560,595,554]
   rule 1 (metadata) num_rep 10 result size == 10:\t1024/1024 (esc)
+  crushtool successfully built or modified map.  Use '-o <file>' to write it out.

diff --git a/src/test/cli/crushtool/test-map-vary-r-0.t b/src/test/cli/crushtool/test-map-vary-r-0.t
index eefd862ddb9102e646c5bd02040ff7f9c7690d4e..389e3cf9e766c27bf41f227bd6c549702d3567b9 100644 (file)
--- a/src/test/cli/crushtool/test-map-vary-r-0.t
+++ b/src/test/cli/crushtool/test-map-vary-r-0.t
@@ -1,5 +1,4 @@
   $ crushtool -i "$TESTDIR/test-map-vary-r.crushmap" --test --show-mappings --show-statistics --rule 3 --set-chooseleaf-vary-r 0 --weight 0 0 --weight 4 0 --weight 9 0
-  crushtool successfully built or modified map.  Use '-o <file>' to write it out.
   rule 3 (delltestrule), x = 0..1023, numrep = 2..4
   CRUSH rule 3 x 0 [94,85]
   CRUSH rule 3 x 1 [73,78]
@@ -3079,3 +3078,4 @@
   CRUSH rule 3 x 1023 [59,88]
   rule 3 (delltestrule) num_rep 4 result size == 1:\t27/1024 (esc)
   rule 3 (delltestrule) num_rep 4 result size == 2:\t997/1024 (esc)
+  crushtool successfully built or modified map.  Use '-o <file>' to write it out.

diff --git a/src/test/cli/crushtool/test-map-vary-r-1.t b/src/test/cli/crushtool/test-map-vary-r-1.t
index a21b9d5f43d5821c5c69639b382afdfe3ff18ac9..f58886056b68ab64b85de40c7ab5bb3b338b4f04 100644 (file)
--- a/src/test/cli/crushtool/test-map-vary-r-1.t
+++ b/src/test/cli/crushtool/test-map-vary-r-1.t
@@ -1,5 +1,4 @@
   $ crushtool -i "$TESTDIR/test-map-vary-r.crushmap" --test --show-mappings --show-statistics --rule 3 --set-chooseleaf-vary-r 1 --weight 0 0 --weight 4 0 --weight 9 0
-  crushtool successfully built or modified map.  Use '-o <file>' to write it out.
   rule 3 (delltestrule), x = 0..1023, numrep = 2..4
   CRUSH rule 3 x 0 [94,6]
   CRUSH rule 3 x 1 [73,52]
@@ -3076,3 +3075,4 @@
   CRUSH rule 3 x 1022 [73,34]
   CRUSH rule 3 x 1023 [88,79]
   rule 3 (delltestrule) num_rep 4 result size == 2:\t1024/1024 (esc)
+  crushtool successfully built or modified map.  Use '-o <file>' to write it out.

diff --git a/src/test/cli/crushtool/test-map-vary-r-2.t b/src/test/cli/crushtool/test-map-vary-r-2.t
index eaf0542916ddc11aeefe75f45c8503fcaaf0af17..efc1932ec4a35f090d3d46c7d8d94bbb5126ac19 100644 (file)
--- a/src/test/cli/crushtool/test-map-vary-r-2.t
+++ b/src/test/cli/crushtool/test-map-vary-r-2.t
@@ -1,5 +1,4 @@
   $ crushtool -i "$TESTDIR/test-map-vary-r.crushmap" --test --show-mappings --show-statistics --rule 3 --set-chooseleaf-vary-r 2 --weight 0 0 --weight 4 0 --weight 9 0
-  crushtool successfully built or modified map.  Use '-o <file>' to write it out.
   rule 3 (delltestrule), x = 0..1023, numrep = 2..4
   CRUSH rule 3 x 0 [94,45]
   CRUSH rule 3 x 1 [73,78]
@@ -3076,3 +3075,4 @@
   CRUSH rule 3 x 1022 [73,106]
   CRUSH rule 3 x 1023 [88,89]
   rule 3 (delltestrule) num_rep 4 result size == 2:\t1024/1024 (esc)
+  crushtool successfully built or modified map.  Use '-o <file>' to write it out.

diff --git a/src/test/cli/crushtool/test-map-vary-r-3.t b/src/test/cli/crushtool/test-map-vary-r-3.t
index 31943b2fff169948d20f0a32edfa4eae0cc97adf..ec41ecbf37062e45f3197fb4c332534404b44401 100644 (file)
--- a/src/test/cli/crushtool/test-map-vary-r-3.t
+++ b/src/test/cli/crushtool/test-map-vary-r-3.t
@@ -1,5 +1,4 @@
   $ crushtool -i "$TESTDIR/test-map-vary-r.crushmap" --test --show-mappings --show-statistics --rule 3 --set-chooseleaf-vary-r 3 --weight 0 0 --weight 4 0 --weight 9 0
-  crushtool successfully built or modified map.  Use '-o <file>' to write it out.
   rule 3 (delltestrule), x = 0..1023, numrep = 2..4
   CRUSH rule 3 x 0 [94,85]
   CRUSH rule 3 x 1 [73,78]
@@ -3076,3 +3075,4 @@
   CRUSH rule 3 x 1022 [73,28]
   CRUSH rule 3 x 1023 [83,88]
   rule 3 (delltestrule) num_rep 4 result size == 2:\t1024/1024 (esc)
+  crushtool successfully built or modified map.  Use '-o <file>' to write it out.

diff --git a/src/test/cli/crushtool/test-map-vary-r-4.t b/src/test/cli/crushtool/test-map-vary-r-4.t
index 24cf0ba28921d6fe6db00a335102c2d462112e2b..e32a19446807ab9d6d04ec23fe5ace44f65899f6 100644 (file)
--- a/src/test/cli/crushtool/test-map-vary-r-4.t
+++ b/src/test/cli/crushtool/test-map-vary-r-4.t
@@ -1,5 +1,4 @@
   $ crushtool -i "$TESTDIR/test-map-vary-r.crushmap" --test --show-mappings --show-statistics --rule 3 --set-chooseleaf-vary-r 4 --weight 0 0 --weight 4 0 --weight 9 0
-  crushtool successfully built or modified map.  Use '-o <file>' to write it out.
   rule 3 (delltestrule), x = 0..1023, numrep = 2..4
   CRUSH rule 3 x 0 [94,85]
   CRUSH rule 3 x 1 [73,78]
@@ -3076,3 +3075,4 @@
   CRUSH rule 3 x 1022 [73,28]
   CRUSH rule 3 x 1023 [59,88]
   rule 3 (delltestrule) num_rep 4 result size == 2:\t1024/1024 (esc)
+  crushtool successfully built or modified map.  Use '-o <file>' to write it out.

diff --git a/src/tools/crushtool.cc b/src/tools/crushtool.cc
index fa678c384522ba7f3a97d3cf0e41baf95add5e29..f894c1f5b0522c49d0c92db57c4de5af3e36bccc 100644 (file)
--- a/src/tools/crushtool.cc
+++ b/src/tools/crushtool.cc
@@ -485,6 +485,8 @@ int main(int argc, const char **argv)
 
   bool modified = false;
 
+  // input ----
+
   if (!infn.empty()) {
     bufferlist bl;
     std::string error;
@@ -498,36 +500,6 @@ int main(int argc, const char **argv)
     crush.decode(p);
   }
 
-  if (full_location >= 0) {
-    map<string, string> loc = crush.get_full_location(full_location);
-    for (map<string,string>::iterator p = loc.begin();
-        p != loc.end();
-        ++p) {
-      cout << p->first << "\t" << p->second << std::endl;
-    }
-    exit(0);
-  }
-  if (decompile) {
-    CrushCompiler cc(crush, cerr, verbose);
-    if (!outfn.empty()) {
-      ofstream o;
-      o.open(outfn.c_str(), ios::out | ios::binary | ios::trunc);
-      if (!o.is_open()) {
-       cerr << me << ": error writing '" << outfn << "'" << std::endl;
-       exit(1);
-      }
-      cc.decompile(o);
-      o.close();
-    } else {
-      cc.decompile(cout);
-    }
-  }
-  if (tree) {
-    ostringstream oss;
-    crush.dump_tree(&oss, NULL);
-    dout(1) << "\n" << oss.str() << dendl;
-  }
-
   if (compile) {
     crush.create();
 
@@ -670,6 +642,37 @@ int main(int argc, const char **argv)
     modified = true;
   }
 
+  // mutate ----
+
+  if (choose_local_tries >= 0) {
+    crush.set_choose_local_tries(choose_local_tries);
+    modified = true;
+  }
+  if (choose_local_fallback_tries >= 0) {
+    crush.set_choose_local_fallback_tries(choose_local_fallback_tries);
+    modified = true;
+  }
+  if (choose_total_tries >= 0) {
+    crush.set_choose_total_tries(choose_total_tries);
+    modified = true;
+  }
+  if (chooseleaf_descend_once >= 0) {
+    crush.set_chooseleaf_descend_once(chooseleaf_descend_once);
+    modified = true;
+  }
+  if (chooseleaf_vary_r >= 0) {
+    crush.set_chooseleaf_vary_r(chooseleaf_vary_r);
+    modified = true;
+  }
+  if (straw_calc_version >= 0) {
+    crush.set_straw_calc_version(straw_calc_version);
+    modified = true;
+  }
+  if (allowed_bucket_algs >= 0) {
+    crush.set_allowed_bucket_algs(allowed_bucket_algs);
+    modified = true;
+  }
+
   if (!reweight_name.empty()) {
     cout << me << " reweighting item " << reweight_name << " to " << reweight_weight << std::endl;
     int r;
@@ -686,8 +689,8 @@ int main(int argc, const char **argv)
       cerr << me << " " << cpp_strerror(r) << std::endl;
       return r;
     }
-        
   }
+
   if (!remove_name.empty()) {
     cout << me << " removing item " << remove_name << std::endl;
     int r;
@@ -705,6 +708,7 @@ int main(int argc, const char **argv)
       return r;
     }
   }
+
   if (add_item >= 0) {
     int r;
     if (update_item) {
@@ -719,39 +723,56 @@ int main(int argc, const char **argv)
       return r;
     }
   }
+
   if (reweight) {
     crush.reweight(g_ceph_context);
     modified = true;
   }
 
-  if (choose_local_tries >= 0) {
-    crush.set_choose_local_tries(choose_local_tries);
-    modified = true;
-  }
-  if (choose_local_fallback_tries >= 0) {
-    crush.set_choose_local_fallback_tries(choose_local_fallback_tries);
-    modified = true;
-  }
-  if (choose_total_tries >= 0) {
-    crush.set_choose_total_tries(choose_total_tries);
-    modified = true;
-  }
-  if (chooseleaf_descend_once >= 0) {
-    crush.set_chooseleaf_descend_once(chooseleaf_descend_once);
-    modified = true;
+
+  // display ---
+  if (full_location >= 0) {
+    map<string, string> loc = crush.get_full_location(full_location);
+    for (map<string,string>::iterator p = loc.begin();
+        p != loc.end();
+        ++p) {
+      cout << p->first << "\t" << p->second << std::endl;
+    }
   }
-  if (chooseleaf_vary_r >= 0) {
-    crush.set_chooseleaf_vary_r(chooseleaf_vary_r);
-    modified = true;
+
+  if (tree) {
+    ostringstream oss;
+    crush.dump_tree(&oss, NULL);
+    dout(1) << "\n" << oss.str() << dendl;
   }
-  if (straw_calc_version >= 0) {
-    crush.set_straw_calc_version(straw_calc_version);
-    modified = true;
+
+  if (decompile) {
+    CrushCompiler cc(crush, cerr, verbose);
+    if (!outfn.empty()) {
+      ofstream o;
+      o.open(outfn.c_str(), ios::out | ios::binary | ios::trunc);
+      if (!o.is_open()) {
+       cerr << me << ": error writing '" << outfn << "'" << std::endl;
+       exit(1);
+      }
+      cc.decompile(o);
+      o.close();
+    } else {
+      cc.decompile(cout);
+    }
   }
-  if (allowed_bucket_algs >= 0) {
-    crush.set_allowed_bucket_algs(allowed_bucket_algs);
-    modified = true;
+
+  if (test) {
+    if (tester.get_output_utilization_all() ||
+       tester.get_output_utilization())
+      tester.set_output_statistics(true);
+
+    int r = tester.test();
+    if (r < 0)
+      exit(1);
   }
+
+  // output ---
   if (modified) {
     crush.finalize();
 
@@ -770,16 +791,6 @@ int main(int argc, const char **argv)
     }
   }
 
-  if (test) {
-    if (tester.get_output_utilization_all() ||
-       tester.get_output_utilization())
-      tester.set_output_statistics(true);
-
-    int r = tester.test();
-    if (r < 0)
-      exit(1);
-  }
-
   return 0;
 }
 /*