From fa52dfaff272421cd23d59a5cbafc2c1f2355179 Mon Sep 17 00:00:00 2001
From: Loic Dachary <ldachary@redhat.com>
Date: Sun, 16 Apr 2017 15:00:03 +0200
Subject: [PATCH] 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>
---
 src/crush/CrushWrapper.cc      | 29 +++++++++++++++++++++++++++++
 src/test/crush/CrushWrapper.cc | 14 ++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc
index 45f95f7a5c8..4a32ccb7325 100644
--- 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 1d73fea5f16..6e6fb05e5e7 100644
--- 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;
 
-- 
2.39.5