From 1e9b62c1b609fa5db3e57a257f24e2b7908cf974 Mon Sep 17 00:00:00 2001
From: John Spray <john.spray@redhat.com>
Date: Wed, 12 Jul 2017 19:54:59 -0400
Subject: [PATCH] tools: avoid max() calls on rbd config options

These were awkward for typing of the '1' literal vs.
the int64_t settings.  The whole max() thing is also
unnecessary now, if we set proper bounds on the option
definitions.

Signed-off-by: John Spray <john.spray@redhat.com>
---
 src/common/options.cc                    |  3 +++
 src/common/options.h                     | 11 +++++++++--
 src/tools/rbd/action/Export.cc           |  2 +-
 src/tools/rbd/action/Import.cc           |  4 ++--
 src/tools/rbd_mirror/InstanceReplayer.cc |  3 +--
 src/tools/rbd_mirror/Instances.cc        |  2 +-
 src/tools/rbd_mirror/LeaderWatcher.cc    |  3 +--
 7 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/src/common/options.cc b/src/common/options.cc
index a930f362cd3ef..e5540b6717a39 100644
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -4345,6 +4345,7 @@ const std::vector<Option> ceph_options = {
 
   Option("rbd_concurrent_management_ops", Option::TYPE_INT, Option::LEVEL_ADVANCED)
   .set_default(10)
+  .set_min(1)
   .set_description(""),
 
   Option("rbd_balance_snap_reads", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
@@ -4513,10 +4514,12 @@ const std::vector<Option> ceph_options = {
 
   Option("rbd_mirror_image_state_check_interval", Option::TYPE_INT, Option::LEVEL_ADVANCED)
   .set_default(30)
+  .set_min(1)
   .set_description(""),
 
   Option("rbd_mirror_leader_heartbeat_interval", Option::TYPE_INT, Option::LEVEL_ADVANCED)
   .set_default(5)
+  .set_min(1)
   .set_description(""),
 
   Option("rbd_mirror_leader_max_missed_heartbeats", Option::TYPE_INT, Option::LEVEL_ADVANCED)
diff --git a/src/common/options.h b/src/common/options.h
index b3f9b331218fc..99240d411b29c 100644
--- a/src/common/options.h
+++ b/src/common/options.h
@@ -205,10 +205,17 @@ struct Option {
     desc = new_desc;
     return *this;
   }
+
+  template<typename T>
+  Option& set_min(const T& mi) {
+    set_value(min, mi);
+    return *this;
+  }
+
   template<typename T>
   Option& set_min_max(const T& mi, const T& ma) {
-    min = mi;
-    max = ma;
+    set_value(min, mi);
+    set_value(max, ma);
     return *this;
   }
 
diff --git a/src/tools/rbd/action/Export.cc b/src/tools/rbd/action/Export.cc
index 1be951028b4aa..708663a43104f 100644
--- a/src/tools/rbd/action/Export.cc
+++ b/src/tools/rbd/action/Export.cc
@@ -509,7 +509,7 @@ static int do_export(librbd::Image& image, const char *path, bool no_progress, i
     fd = STDOUT_FILENO;
     max_concurrent_ops = 1;
   } else {
-    max_concurrent_ops = max(g_conf->rbd_concurrent_management_ops, 1);
+    max_concurrent_ops = g_conf->rbd_concurrent_management_ops;
     fd = open(path, O_WRONLY | O_CREAT | O_EXCL, 0644);
     if (fd < 0) {
       return -errno;
diff --git a/src/tools/rbd/action/Import.cc b/src/tools/rbd/action/Import.cc
index 5bea7805da738..0f31d810394be 100644
--- a/src/tools/rbd/action/Import.cc
+++ b/src/tools/rbd/action/Import.cc
@@ -37,7 +37,7 @@ struct ImportDiffContext {
   ImportDiffContext(librbd::Image *image, int fd, size_t size, bool no_progress)
     : image(image), fd(fd), size(size), pc("Importing image diff", no_progress),
       throttle((fd == STDIN_FILENO) ? 1 :
-               max(g_conf->rbd_concurrent_management_ops, 1), false), last_offset(0) {
+               g_conf->rbd_concurrent_management_ops, false), last_offset(0) {
   }
 
   void update_size(size_t new_size)
@@ -653,7 +653,7 @@ static int do_import_v1(int fd, librbd::Image &image, uint64_t size,
     throttle.reset(new SimpleThrottle(1, false));
   } else {
     throttle.reset(new SimpleThrottle(
-                     max(g_conf->rbd_concurrent_management_ops, 1), false));
+                     g_conf->rbd_concurrent_management_ops, false));
   }
 
   reqlen = min<uint64_t>(reqlen, size);
diff --git a/src/tools/rbd_mirror/InstanceReplayer.cc b/src/tools/rbd_mirror/InstanceReplayer.cc
index 2b732617d8ae3..5c2fa13a7ab60 100644
--- a/src/tools/rbd_mirror/InstanceReplayer.cc
+++ b/src/tools/rbd_mirror/InstanceReplayer.cc
@@ -486,8 +486,7 @@ void InstanceReplayer<I>::schedule_image_state_check_task() {
       start_image_replayers();
     });
 
-  int after =
-    max(1, g_ceph_context->_conf->rbd_mirror_image_state_check_interval);
+  int after = g_ceph_context->_conf->rbd_mirror_image_state_check_interval;
 
   dout(20) << "scheduling image state check after " << after << " sec (task "
            << m_image_state_check_task << ")" << dendl;
diff --git a/src/tools/rbd_mirror/Instances.cc b/src/tools/rbd_mirror/Instances.cc
index 0adc4f3428e7d..302700e51bb61 100644
--- a/src/tools/rbd_mirror/Instances.cc
+++ b/src/tools/rbd_mirror/Instances.cc
@@ -228,7 +228,7 @@ void Instances<I>::schedule_remove_task(Instance &instance) {
 
   cancel_remove_task(instance);
 
-  int after = max(1, m_cct->_conf->rbd_mirror_leader_heartbeat_interval) *
+  int after = m_cct->_conf->rbd_mirror_leader_heartbeat_interval *
     (1 + m_cct->_conf->rbd_mirror_leader_max_missed_heartbeats +
      m_cct->_conf->rbd_mirror_leader_max_acquire_attempts_before_break);
 
diff --git a/src/tools/rbd_mirror/LeaderWatcher.cc b/src/tools/rbd_mirror/LeaderWatcher.cc
index ca0783679baee..9e99d6b6f6e8b 100644
--- a/src/tools/rbd_mirror/LeaderWatcher.cc
+++ b/src/tools/rbd_mirror/LeaderWatcher.cc
@@ -370,8 +370,7 @@ void LeaderWatcher<I>::schedule_timer_task(const std::string &name,
       m_timer_gate->timer_callback = timer_callback;
     });
 
-  int after = delay_factor *
-    max(1, m_cct->_conf->rbd_mirror_leader_heartbeat_interval);
+  int after = delay_factor * m_cct->_conf->rbd_mirror_leader_heartbeat_interval;
 
   dout(20) << "scheduling " << name << " after " << after << " sec (task "
            << m_timer_task << ")" << dendl;
-- 
2.39.5