rbd: mirror "get" actions now have cleaner error messages

Fixes: http://tracker.ceph.com/issues/21319
Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/api/Mirror.cc b/src/librbd/api/Mirror.cc
index 5b63e0f83c031a5de7748988cecd48f07922754f..25ffea03e8c05f3315b4420726eb1d9f8a452823 100644 (file)
--- a/src/librbd/api/Mirror.cc
+++ b/src/librbd/api/Mirror.cc
@@ -60,7 +60,7 @@ int list_mirror_images(librados::IoCtx& io_ctx,
     std::map<std::string, std::string> mirror_images;
     r =  cls_client::mirror_image_list(&io_ctx, last_read, max_read,
                                        &mirror_images);
-    if (r < 0) {
+    if (r < 0 && r != -ENOENT) {
       lderr(cct) << "error listing mirrored image directory: "
                  << cpp_strerror(r) << dendl;
       return r;
@@ -815,6 +815,10 @@ int Mirror<I>::image_status_list(librados::IoCtx& io_ctx,
   for (auto it = images_.begin(); it != images_.end(); ++it) {
     auto &image_id = it->first;
     auto &info = it->second;
+    if (info.state == cls::rbd::MIRROR_IMAGE_STATE_DISABLED) {
+      continue;
+    }
+
     auto &image_name = id_to_name[image_id];
     if (image_name.empty()) {
       lderr(cct) << "failed to find image name for image " << image_id << ", "
@@ -845,7 +849,7 @@ int Mirror<I>::image_status_summary(librados::IoCtx& io_ctx,
 
   std::map<cls::rbd::MirrorImageStatusState, int> states_;
   int r = cls_client::mirror_image_status_get_summary(&io_ctx, &states_);
-  if (r < 0) {
+  if (r < 0 && r != -ENOENT) {
     lderr(cct) << "failed to get mirror status summary: "
                << cpp_strerror(r) << dendl;
     return r;

diff --git a/src/tools/rbd/action/MirrorImage.cc b/src/tools/rbd/action/MirrorImage.cc
index 70cd91f394c7d30867a93c9ecb487966649e949c..e0151c9780e90cac38f4c3c6a807bcb19325de6d 100644 (file)
--- a/src/tools/rbd/action/MirrorImage.cc
+++ b/src/tools/rbd/action/MirrorImage.cc
@@ -31,6 +31,25 @@ namespace mirror_image {
 namespace at = argument_types;
 namespace po = boost::program_options;
 
+namespace {
+
+int validate_mirroring_enabled(librbd::Image& image) {
+  librbd::mirror_image_info_t mirror_image;
+  int r = image.mirror_image_get_info(&mirror_image, sizeof(mirror_image));
+  if (r < 0) {
+    std::cerr << "rbd: failed to retrieve mirror mode: "
+              << cpp_strerror(r) << std::endl;
+    return r;
+  }
+
+  if (mirror_image.state != RBD_MIRROR_IMAGE_ENABLED) {
+    std::cerr << "rbd: mirroring not enabled on the image" << std::endl;
+    return -EINVAL;
+  }
+  return 0;
+}
+
+} // anonymous namespace
 
 void get_arguments(po::options_description *positional,
                            po::options_description *options) {
@@ -115,6 +134,11 @@ int execute_promote(const po::variables_map &vm) {
     return r;
   }
 
+  r = validate_mirroring_enabled(image);
+  if (r < 0) {
+    return r;
+  }
+
   r = image.mirror_image_promote(force);
   if (r < 0) {
     std::cerr << "rbd: error promoting image to primary" << std::endl;
@@ -146,6 +170,11 @@ int execute_demote(const po::variables_map &vm) {
     return r;
   }
 
+  r = validate_mirroring_enabled(image);
+  if (r < 0) {
+    return r;
+  }
+
   r = image.mirror_image_demote();
   if (r < 0) {
     std::cerr << "rbd: error demoting image to non-primary" << std::endl;
@@ -177,6 +206,11 @@ int execute_resync(const po::variables_map &vm) {
     return r;
   }
 
+  r = validate_mirroring_enabled(image);
+  if (r < 0) {
+    return r;
+  }
+
   r = image.mirror_image_resync();
   if (r < 0) {
     std::cerr << "rbd: error flagging image resync" << std::endl;
@@ -220,6 +254,11 @@ int execute_status(const po::variables_map &vm) {
     return r;
   }
 
+  r = validate_mirroring_enabled(image);
+  if (r < 0) {
+    return r;
+  }
+
   librbd::mirror_image_status_t status;
   r = image.mirror_image_get_status(&status, sizeof(status));
   if (r < 0) {

diff --git a/src/tools/rbd/action/MirrorPool.cc b/src/tools/rbd/action/MirrorPool.cc
index 4314b1ed6b46ddc3c8b8944a359414d24dda0422..ba179d054ce3f2e50fd32a6c4c89511298e5db78 100644 (file)
--- a/src/tools/rbd/action/MirrorPool.cc
+++ b/src/tools/rbd/action/MirrorPool.cc
@@ -36,6 +36,23 @@ namespace po = boost::program_options;
 
 namespace {
 
+int validate_mirroring_enabled(librados::IoCtx& io_ctx) {
+  librbd::RBD rbd;
+  rbd_mirror_mode_t mirror_mode;
+  int r = rbd.mirror_mode_get(io_ctx, &mirror_mode);
+  if (r < 0) {
+    std::cerr << "rbd: failed to retrieve mirror mode: "
+              << cpp_strerror(r) << std::endl;
+    return r;
+  }
+
+  if (mirror_mode == RBD_MIRROR_MODE_DISABLED) {
+    std::cerr << "rbd: mirroring not enabled on the pool" << std::endl;
+    return -EINVAL;
+  }
+  return 0;
+}
+
 int validate_uuid(const std::string &uuid) {
   boost::regex pattern("^[A-F0-9]{8}-[A-F0-9]{4}-[A-F0-9]{4}-[A-F0-9]{4}-[A-F0-9]{12}$",
                        boost::regex::icase);
@@ -328,7 +345,7 @@ public:
 
 protected:
   bool skip_action(const librbd::mirror_image_info_t &info) const override {
-    return info.primary;
+    return (info.state != RBD_MIRROR_IMAGE_ENABLED || info.primary);
   }
 
   void execute_action(librbd::Image &image,
@@ -340,6 +357,7 @@ protected:
     if (r >= 0) {
       (*m_counter)++;
     }
+    ImageRequestBase::handle_execute_action(r);
   }
 
   std::string get_action_type() const override {
@@ -360,7 +378,7 @@ public:
 
 protected:
   bool skip_action(const librbd::mirror_image_info_t &info) const override {
-    return !info.primary;
+    return (info.state != RBD_MIRROR_IMAGE_ENABLED || !info.primary);
   }
 
   void execute_action(librbd::Image &image,
@@ -403,6 +421,10 @@ protected:
   }
 
   void finalize_action() override {
+    if (m_mirror_image_status.info.global_id.empty()) {
+      return;
+    }
+
     std::string state = utils::mirror_image_status_state(m_mirror_image_status);
     std::string last_update = (
       m_mirror_image_status.last_update == 0 ?
@@ -529,25 +551,15 @@ int execute_peer_add(const po::variables_map &vm) {
   if (r < 0) {
     return r;
   }
-  
-  librbd::RBD rbd;
-  rbd_mirror_mode_t mirror_mode;
-  r = rbd.mirror_mode_get(io_ctx, &mirror_mode);
+
+  r = validate_mirroring_enabled(io_ctx);
   if (r < 0) {
-    std::cerr << "rbd: failed to retrieve mirror mode: " 
-              << cpp_strerror(r) << std::endl;
     return r;
   }
-  
-  if (mirror_mode == RBD_MIRROR_MODE_DISABLED) {
-    std::cerr << "rbd: failed to add mirror peer: "
-              << "mirroring must be enabled on the pool " 
-              << pool_name << std::endl;
-    return -EINVAL;
-  }
 
   // TODO: temporary restriction to prevent adding multiple peers
   // until rbd-mirror daemon can properly handle the scenario
+  librbd::RBD rbd;
   std::vector<librbd::mirror_peer_t> mirror_peers;
   r = rbd.mirror_peer_list(io_ctx, &mirror_peers);
   if (r < 0) {
@@ -593,6 +605,11 @@ int execute_peer_remove(const po::variables_map &vm) {
     return r;
   }
 
+  r = validate_mirroring_enabled(io_ctx);
+  if (r < 0) {
+    return r;
+  }
+
   librbd::RBD rbd;
   r = rbd.mirror_peer_remove(io_ctx, uuid);
   if (r < 0) {
@@ -639,6 +656,11 @@ int execute_peer_set(const po::variables_map &vm) {
     return r;
   }
 
+  r = validate_mirroring_enabled(io_ctx);
+  if (r < 0) {
+    return r;
+  }
+
   librbd::RBD rbd;
   if (key == "client") {
     r = rbd.mirror_peer_set_client(io_ctx, uuid.c_str(), value.c_str());
@@ -839,6 +861,11 @@ int execute_status(const po::variables_map &vm) {
     return r;
   }
 
+  r = validate_mirroring_enabled(io_ctx);
+  if (r < 0) {
+    return r;
+  }
+
   librbd::RBD rbd;
 
   std::map<librbd::mirror_image_status_state_t, int> states;
@@ -932,6 +959,11 @@ int execute_promote(const po::variables_map &vm) {
     return r;
   }
 
+  r = validate_mirroring_enabled(io_ctx);
+  if (r < 0) {
+    return r;
+  }
+
   std::atomic<unsigned> counter = { 0 };
   ImageRequestGenerator<PromoteImageRequest> generator(io_ctx, &counter,
                                                        vm["force"].as<bool>());
@@ -957,6 +989,11 @@ int execute_demote(const po::variables_map &vm) {
     return r;
   }
 
+  r = validate_mirroring_enabled(io_ctx);
+  if (r < 0) {
+    return r;
+  }
+
   std::atomic<unsigned> counter { 0 };
   ImageRequestGenerator<DemoteImageRequest> generator(io_ctx, &counter);
   r = generator.execute();