crush: disable modification API when choose_args is not empty

Adding, removing or move items / buckets via the CrushWrapper API when
choose_args is not empty is unlikely to produce the desired outcome. The
caller should instead add, remove or move items / buckets in a
decompiled crushmap, update the associated choose_arg and upload the new
crushmap.

Signed-off-by: Loic Dachary <loic@dachary.org>

diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc
index 45f95f7a5c8d78ab3b4a46189da2e9e67814d557..4a32ccb7325c8b1423a204fe8e3a37749fe1cd57 100644 (file)
--- a/src/crush/CrushWrapper.cc
+++ b/src/crush/CrushWrapper.cc
@@ -288,6 +288,11 @@ int CrushWrapper::remove_root(int item, bool unused)
 
 int CrushWrapper::remove_item(CephContext *cct, int item, bool unlink_only)
 {
+  if (choose_args.size() > 0) {
+    ldout(cct, 1) << "remove_item not implemented when choose_args is not empty" << dendl;
+    return -EDOM;
+  }
+
   ldout(cct, 5) << "remove_item " << item << (unlink_only ? " unlink_only":"") << dendl;
 
   int ret = -ENOENT;
@@ -641,6 +646,10 @@ int CrushWrapper::get_children(int id, list<int> *children)
 int CrushWrapper::insert_item(CephContext *cct, int item, float weight, string name,
                              const map<string,string>& loc)  // typename -> bucketname
 {
+  if (choose_args.size() > 0) {
+    ldout(cct, 1) << "insert_item not implemented when choose_args is not empty" << dendl;
+    return -EDOM;
+  }
 
   ldout(cct, 5) << "insert_item item " << item << " weight " << weight
                << " name " << name << " loc " << loc << dendl;
@@ -745,6 +754,11 @@ int CrushWrapper::insert_item(CephContext *cct, int item, float weight, string n
 
 int CrushWrapper::move_bucket(CephContext *cct, int id, const map<string,string>& loc)
 {
+  if (choose_args.size() > 0) {
+    ldout(cct, 1) << "move_bucket not implemented when choose_args is not empty" << dendl;
+    return -EDOM;
+  }
+
   // sorry this only works for buckets
   if (id >= 0)
     return -EINVAL;
@@ -764,6 +778,11 @@ int CrushWrapper::move_bucket(CephContext *cct, int id, const map<string,string>
 
 int CrushWrapper::link_bucket(CephContext *cct, int id, const map<string,string>& loc)
 {
+  if (choose_args.size() > 0) {
+    ldout(cct, 1) << "link_bucket not implemented when choose_args is not empty" << dendl;
+    return -EDOM;
+  }
+
   // sorry this only works for buckets
   if (id >= 0)
     return -EINVAL;
@@ -783,6 +802,11 @@ int CrushWrapper::link_bucket(CephContext *cct, int id, const map<string,string>
 int CrushWrapper::create_or_move_item(CephContext *cct, int item, float weight, string name,
                                      const map<string,string>& loc)  // typename -> bucketname
 {
+  if (choose_args.size() > 0) {
+    ldout(cct, 1) << "create_or_move_item not implemented when choose_args is not empty" << dendl;
+    return -EDOM;
+  }
+
   int ret = 0;
   int old_iweight;
 
@@ -809,6 +833,11 @@ int CrushWrapper::create_or_move_item(CephContext *cct, int item, float weight,
 int CrushWrapper::update_item(CephContext *cct, int item, float weight, string name,
                              const map<string,string>& loc)  // typename -> bucketname
 {
+  if (choose_args.size() > 0) {
+    ldout(cct, 1) << "update_item not implemented when choose_args is not empty" << dendl;
+    return -EDOM;
+  }
+
   ldout(cct, 5) << "update_item item " << item << " weight " << weight
                << " name " << name << " loc " << loc << dendl;
   int ret = 0;

diff --git a/src/test/crush/CrushWrapper.cc b/src/test/crush/CrushWrapper.cc
index 1d73fea5f16d0bfdcc921851f1039632cd69ecda..6e6fb05e5e7c8ac38bd49934cba4ca982239bc12 100644 (file)
--- a/src/test/crush/CrushWrapper.cc
+++ b/src/test/crush/CrushWrapper.cc
@@ -712,6 +712,20 @@ TEST(CrushWrapper, insert_item) {
   delete c;
 }
 
+TEST(CrushWrapper, choose_args_disabled) {
+  auto *c = new CrushWrapper;
+  c->choose_args[0] = crush_choose_arg_map();
+
+  map<string,string> loc;
+  ASSERT_EQ(-EDOM, c->remove_item(g_ceph_context, 0, true));
+  ASSERT_EQ(-EDOM, c->insert_item(g_ceph_context, 0, 0.0, "", loc));
+  ASSERT_EQ(-EDOM, c->move_bucket(g_ceph_context, 0, loc));
+  ASSERT_EQ(-EDOM, c->link_bucket(g_ceph_context, 0, loc));
+  ASSERT_EQ(-EDOM, c->create_or_move_item(g_ceph_context, 0, 0.0, "", loc));
+
+  delete c;
+}
+
 TEST(CrushWrapper, remove_item) {
   auto *c = new CrushWrapper;