From 053be9847ff0052348aa259520d641923e57537d Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Fri, 8 Sep 2017 16:43:58 -0400 Subject: [PATCH] rbd: mirror "get" actions now have cleaner error messages Fixes: http://tracker.ceph.com/issues/21319 Signed-off-by: Jason Dillaman --- src/librbd/api/Mirror.cc | 8 +++- src/tools/rbd/action/MirrorImage.cc | 39 +++++++++++++++++ src/tools/rbd/action/MirrorPool.cc | 67 ++++++++++++++++++++++------- 3 files changed, 97 insertions(+), 17 deletions(-) diff --git a/src/librbd/api/Mirror.cc b/src/librbd/api/Mirror.cc index 5b63e0f83c031..25ffea03e8c05 100644 --- 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 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::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::image_status_summary(librados::IoCtx& io_ctx, std::map 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 70cd91f394c7d..e0151c9780e90 100644 --- 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 4314b1ed6b46d..ba179d054ce3f 100644 --- 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 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 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 counter = { 0 }; ImageRequestGenerator generator(io_ctx, &counter, vm["force"].as()); @@ -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 counter { 0 }; ImageRequestGenerator generator(io_ctx, &counter); r = generator.execute(); -- 2.39.5