rbd-mirror: fixup to get/list mirror image status API

- embed name and rbd_mirror_image_info_t into rbd_mirror_image_status_t;
- index image status list by image id to make 'start' work correctly;
- provide rbd_mirror_image_status_list_cleanup function.

Fixes: #15771
Signed-off-by: Mykola Golub <mgolub@mirantis.com>

diff --git a/src/include/rbd/librbd.h b/src/include/rbd/librbd.h
index eefa689d63029e7a9a5354b9ae3a652abaa6717d..fb61b8f70bd3cd0b3b345d54deff5d4e6f986573 100644 (file)
--- a/src/include/rbd/librbd.h
+++ b/src/include/rbd/librbd.h
@@ -124,6 +124,8 @@ typedef enum {
 } rbd_mirror_image_status_state_t;
 
 typedef struct {
+  char *name;
+  rbd_mirror_image_info_t info;
   rbd_mirror_image_status_state_t state;
   char *description;
   time_t last_update;
@@ -228,9 +230,12 @@ CEPH_RBD_API int rbd_mirror_peer_set_cluster(rados_ioctx_t io_ctx,
                                              const char *uuid,
                                              const char *cluster_name);
 CEPH_RBD_API int rbd_mirror_image_status_list(rados_ioctx_t io_ctx,
-    const char *start, size_t max, char **image_names,
-    rbd_mirror_image_info_t *images, rbd_mirror_image_status_t *image_statuses,
-    size_t *len);
+                                             const char *start_id, size_t max,
+                                             char **image_ids,
+                                             rbd_mirror_image_status_t *images,
+                                             size_t *len);
+CEPH_RBD_API void rbd_mirror_image_status_list_cleanup(char **image_ids,
+    rbd_mirror_image_status_t *images, size_t len);
 CEPH_RBD_API int rbd_mirror_image_status_summary(rados_ioctx_t io_ctx,
     rbd_mirror_image_status_state_t *states, int *counts, size_t *maxlen);
 
@@ -633,7 +638,8 @@ CEPH_RBD_API int rbd_mirror_image_get_info(rbd_image_t image,
                                            rbd_mirror_image_info_t *mirror_image_info,
                                            size_t info_size);
 CEPH_RBD_API int rbd_mirror_image_get_status(rbd_image_t image,
-    rbd_mirror_image_status_t *mirror_image_status, size_t info_size);
+                                             rbd_mirror_image_status_t *mirror_image_status,
+                                             size_t status_size);
 
 #ifdef __cplusplus
 }

diff --git a/src/include/rbd/librbd.hpp b/src/include/rbd/librbd.hpp
index 1ead0909bad750bc1424f0b0fcebd3f396f1cffe..e4c434ab490f478f7174fff845f46421d07560cc 100644 (file)
--- a/src/include/rbd/librbd.hpp
+++ b/src/include/rbd/librbd.hpp
@@ -63,6 +63,8 @@ namespace librbd {
   typedef rbd_mirror_image_status_state_t mirror_image_status_state_t;
 
   typedef struct {
+    std::string name;
+    mirror_image_info_t info;
     mirror_image_status_state_t state;
     std::string description;
     time_t last_update;
@@ -141,9 +143,8 @@ public:
                              const std::string &client_name);
   int mirror_peer_set_cluster(IoCtx& io_ctx, const std::string &uuid,
                               const std::string &cluster_name);
-  int mirror_image_status_list(IoCtx& io_ctx, const std::string &start,
-      size_t max, std::map<std::string, mirror_image_info_t> *images,
-      std::map<std::string, mirror_image_status_t> *statuses);
+  int mirror_image_status_list(IoCtx& io_ctx, const std::string &start_id,
+      size_t max, std::map<std::string, mirror_image_status_t> *images);
   int mirror_image_status_summary(IoCtx& io_ctx,
       std::map<mirror_image_status_state_t, int> *states);
 

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 455d3b35eb4c19c61243808e886dbe1d9f24e098..f8e23aba35e5ea498db72a02082043f4bed92a26 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -2933,28 +2933,25 @@ remove_mirroring_image:
       return -ERANGE;
     }
 
+    mirror_image_info_t info;
+    int r = mirror_image_get_info(ictx, &info, sizeof(info));
+    if (r < 0) {
+      return r;
+    }
+
     cls::rbd::MirrorImageStatus
       s(cls::rbd::MIRROR_IMAGE_STATUS_STATE_UNKNOWN, "status not found");
 
-    cls::rbd::MirrorImage image;
-    int r = cls_client::mirror_image_get(&ictx->md_ctx, ictx->id, &image);
+    r = cls_client::mirror_image_status_get(&ictx->md_ctx, info.global_id, &s);
     if (r < 0 && r != -ENOENT) {
-      lderr(cct) << "failed to retrieve mirroring state: " << cpp_strerror(r)
-                << dendl;
+      lderr(cct) << "failed to retrieve image mirror status: "
+                << cpp_strerror(r) << dendl;
       return r;
     }
 
-    if (r == 0) {
-      r = cls_client::mirror_image_status_get(&ictx->md_ctx,
-                                             image.global_image_id, &s);
-      if (r < 0 && r != -ENOENT) {
-       lderr(cct) << "failed to retrieve image mirror status: "
-                  << cpp_strerror(r) << dendl;
-       return r;
-      }
-    }
-
     *status = mirror_image_status_t{
+      ictx->name,
+      info,
       static_cast<mirror_image_status_state_t>(s.state),
       s.description,
       s.last_update.sec(),
@@ -3327,9 +3324,8 @@ remove_mirroring_image:
     return 0;
   }
 
-  int mirror_image_status_list(IoCtx& io_ctx, const std::string &start,
-      size_t max, std::map<std::string, mirror_image_info_t> *images,
-      std::map<std::string, mirror_image_status_t> *statuses) {
+  int mirror_image_status_list(IoCtx& io_ctx, const std::string &start_id,
+      size_t max, std::map<std::string, mirror_image_status_t> *images) {
     CephContext *cct = reinterpret_cast<CephContext *>(io_ctx.cct());
     int r;
 
@@ -3348,7 +3344,7 @@ remove_mirroring_image:
     map<std::string, cls::rbd::MirrorImage> images_;
     map<std::string, cls::rbd::MirrorImageStatus> statuses_;
 
-    r = librbd::cls_client::mirror_image_status_list(&io_ctx, start, max,
+    r = librbd::cls_client::mirror_image_status_list(&io_ctx, start_id, max,
                                                     &images_, &statuses_);
     if (r < 0) {
       lderr(cct) << "Failed to list mirror image statuses: "
@@ -3356,8 +3352,8 @@ remove_mirroring_image:
       return r;
     }
 
-    cls::rbd::MirrorImageStatus
-      unknown_status(cls::rbd::MIRROR_IMAGE_STATUS_STATE_UNKNOWN, "status not found");
+    cls::rbd::MirrorImageStatus unknown_status(
+      cls::rbd::MIRROR_IMAGE_STATUS_STATE_UNKNOWN, "status not found");
 
     for (auto it = images_.begin(); it != images_.end(); ++it) {
       auto &image_id = it->first;
@@ -3368,13 +3364,14 @@ remove_mirroring_image:
                   << ", using image id as name" << dendl;
        image_name = image_id;
       }
-      (*images)[image_name] = mirror_image_info_t{
-       info.global_image_id,
-       static_cast<mirror_image_state_t>(info.state),
-       false}; // XXX: To set "primary" properly would require additional call.
       auto s_it = statuses_.find(image_id);
       auto &s = s_it != statuses_.end() ? s_it->second : unknown_status;
-      (*statuses)[image_name] = mirror_image_status_t{
+      (*images)[image_id] = mirror_image_status_t{
+       image_name,
+       mirror_image_info_t{
+         info.global_image_id,
+         static_cast<mirror_image_state_t>(info.state),
+         false}, // XXX: To set "primary" right would require an additional call.
        static_cast<mirror_image_status_state_t>(s.state),
        s.description,
        s.last_update.sec(),

diff --git a/src/librbd/internal.h b/src/librbd/internal.h
index 47f6f3381541af2dd4cfc049f378fca1707f2785..dcb03509d50989df95ecd2117f4f42fd09e08a0a 100644 (file)
--- a/src/librbd/internal.h
+++ b/src/librbd/internal.h
@@ -191,9 +191,8 @@ namespace librbd {
                              const std::string &client_name);
   int mirror_peer_set_cluster(IoCtx& io_ctx, const std::string &uuid,
                               const std::string &cluster_name);
-  int mirror_image_status_list(IoCtx& io_ctx, const std::string &start,
-      size_t max, std::map<std::string, mirror_image_info_t> *images,
-      std::map<std::string, mirror_image_status_t> *statuses);
+  int mirror_image_status_list(IoCtx& io_ctx, const std::string &start_id,
+      size_t max, std::map<std::string, mirror_image_status_t> *images);
   int mirror_image_status_summary(IoCtx& io_ctx,
       std::map<mirror_image_status_state_t, int> *states);
 

diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc
index c3cc7e8ef3bfbd37eb7c5c971efef93ae6cf59fe..2cb5132ed24bf927fc8cec4fc55464eb137a83c5 100644 (file)
--- a/src/librbd/librbd.cc
+++ b/src/librbd/librbd.cc
@@ -141,6 +141,8 @@ void mirror_image_info_cpp_to_c(const librbd::mirror_image_info_t &cpp_info,
 
 void mirror_image_status_cpp_to_c(const librbd::mirror_image_status_t &cpp_status,
                                  rbd_mirror_image_status_t *c_status) {
+  c_status->name = strdup(cpp_status.name.c_str());
+  mirror_image_info_cpp_to_c(cpp_status.info, &c_status->info);
   c_status->state = cpp_status.state;
   c_status->description = strdup(cpp_status.description.c_str());
   c_status->last_update = cpp_status.last_update;
@@ -426,11 +428,9 @@ namespace librbd {
     return librbd::mirror_peer_set_cluster(io_ctx, uuid, cluster_name);
   }
 
-  int RBD::mirror_image_status_list(IoCtx& io_ctx, const std::string &start,
-      size_t max, std::map<std::string, mirror_image_info_t> *images,
-      std::map<std::string, mirror_image_status_t> *statuses) {
-    return librbd::mirror_image_status_list(io_ctx, start, max, images,
-                                           statuses);
+  int RBD::mirror_image_status_list(IoCtx& io_ctx, const std::string &start_id,
+      size_t max, std::map<std::string, mirror_image_status_t> *images) {
+    return librbd::mirror_image_status_list(io_ctx, start_id, max, images);
   }
 
   int RBD::mirror_image_status_summary(IoCtx& io_ctx,
@@ -1470,16 +1470,14 @@ extern "C" int rbd_mirror_peer_set_cluster(rados_ioctx_t p, const char *uuid,
   return librbd::mirror_peer_set_cluster(io_ctx, uuid, cluster_name);
 }
 
-extern "C" int rbd_mirror_image_status_list(rados_ioctx_t p, const char *start,
-    size_t max, char **image_names, rbd_mirror_image_info_t *images,
-    rbd_mirror_image_status_t *statuses, size_t *len) {
+extern "C" int rbd_mirror_image_status_list(rados_ioctx_t p,
+    const char *start_id, size_t max, char **image_ids,
+    rbd_mirror_image_status_t *images, size_t *len) {
   librados::IoCtx io_ctx;
   librados::IoCtx::from_rados_ioctx_t(p, io_ctx);
-  std::map<std::string, librbd::mirror_image_info_t> cpp_images;
-  std::map<std::string, librbd::mirror_image_status_t> cpp_statuses;
+  std::map<std::string, librbd::mirror_image_status_t> cpp_images;
 
-  int r = librbd::mirror_image_status_list(io_ctx, start, max, &cpp_images,
-                                          &cpp_statuses);
+  int r = librbd::mirror_image_status_list(io_ctx, start_id, max, &cpp_images);
   if (r < 0) {
     return r;
   }
@@ -1487,16 +1485,25 @@ extern "C" int rbd_mirror_image_status_list(rados_ioctx_t p, const char *start,
   size_t i = 0;
   for (auto &it : cpp_images) {
     assert(i < max);
-    const std::string &image_name = it.first;
-    image_names[i] = strdup(image_name.c_str());
-    mirror_image_info_cpp_to_c(it.second, &images[i]);
-    mirror_image_status_cpp_to_c(cpp_statuses[image_name], &statuses[i]);
+    const std::string &image_id = it.first;
+    image_ids[i] = strdup(image_id.c_str());
+    mirror_image_status_cpp_to_c(it.second, &images[i]);
     i++;
   }
   *len = i;
   return 0;
 }
 
+extern "C" void rbd_mirror_image_status_list_cleanup(char **image_ids,
+    rbd_mirror_image_status_t *images, size_t len) {
+  for (size_t i = 0; i < len; i++) {
+    free(image_ids[i]);
+    free(images[i].name);
+    free(images[i].info.global_id);
+    free(images[i].description);
+  }
+}
+
 extern "C" int rbd_mirror_image_status_summary(rados_ioctx_t p,
     rbd_mirror_image_status_state_t *states, int *counts, size_t *maxlen) {
 

diff --git a/src/test/librbd/test_mirroring.cc b/src/test/librbd/test_mirroring.cc
index 75514dd0e8880ec56f17df74d4e14616d1644e88..ece07b1e9d8c1739374f28195767dfc35d1cdb14 100644 (file)
--- a/src/test/librbd/test_mirroring.cc
+++ b/src/test/librbd/test_mirroring.cc
@@ -107,11 +107,8 @@ public:
   }
 
   void check_mirroring_status(size_t *images_count) {
-    std::map<std::string, librbd::mirror_image_info_t> images;
-    std::map<std::string, librbd::mirror_image_status_t> statuses;
-    ASSERT_EQ(0, m_rbd.mirror_image_status_list(m_ioctx, "", 4096, &images,
-           &statuses));
-    ASSERT_EQ(images.size(), statuses.size());
+    std::map<std::string, librbd::mirror_image_status_t> images;
+    ASSERT_EQ(0, m_rbd.mirror_image_status_list(m_ioctx, "", 4096, &images));
 
     std::map<librbd::mirror_image_status_state_t, int> states;
     ASSERT_EQ(0, m_rbd.mirror_image_status_summary(m_ioctx, &states));
@@ -513,3 +510,28 @@ TEST_F(TestMirroring, RemoveImage_With_ImageWithoutJournal) {
                      false);
 }
 
+TEST_F(TestMirroring, MirrorStatusList) {
+  std::vector<uint64_t>
+      features_vec(5, RBD_FEATURE_EXCLUSIVE_LOCK | RBD_FEATURE_JOURNALING);
+  setup_images_with_mirror_mode(RBD_MIRROR_MODE_POOL, features_vec);
+
+  std::string last_read = "";
+  std::map<std::string, librbd::mirror_image_status_t> images;
+  ASSERT_EQ(0, m_rbd.mirror_image_status_list(m_ioctx, last_read, 2, &images));
+  ASSERT_EQ(2U, images.size());
+
+  last_read = images.rbegin()->first;
+  images.clear();
+  ASSERT_EQ(0, m_rbd.mirror_image_status_list(m_ioctx, last_read, 2, &images));
+  ASSERT_EQ(2U, images.size());
+
+  last_read = images.rbegin()->first;
+  images.clear();
+  ASSERT_EQ(0, m_rbd.mirror_image_status_list(m_ioctx, last_read, 4096, &images));
+  ASSERT_EQ(1U, images.size());
+
+  last_read = images.rbegin()->first;
+  images.clear();
+  ASSERT_EQ(0, m_rbd.mirror_image_status_list(m_ioctx, last_read, 4096, &images));
+  ASSERT_EQ(0U, images.size());
+}

diff --git a/src/tools/rbd/action/MirrorImage.cc b/src/tools/rbd/action/MirrorImage.cc
index 6311c0eac60c00f4f69d226a368fb6f6695b3164..e703e2df288c1757e242fccd80708a0b7e823771 100644 (file)
--- a/src/tools/rbd/action/MirrorImage.cc
+++ b/src/tools/rbd/action/MirrorImage.cc
@@ -220,20 +220,6 @@ int execute_status(const po::variables_map &vm) {
     return r;
   }
 
-  librbd::mirror_image_info_t mirror_image;
-  r = image.mirror_image_get_info(&mirror_image, sizeof(mirror_image));
-  if (r < 0) {
-    std::cerr << "rbd: failed to get global image id for image " << image_name
-             << ": " << cpp_strerror(r) << std::endl;
-    return r;
-  }
-
-  if (mirror_image.global_id.empty()) {
-    std::cerr << "rbd: failed to get global image id for image " << image_name
-             << std::endl;
-    return -EINVAL;
-  }
-
   librbd::mirror_image_status_t status;
   r = image.mirror_image_get_status(&status, sizeof(status));
   if (r < 0) {
@@ -248,7 +234,7 @@ int execute_status(const po::variables_map &vm) {
   if (formatter != nullptr) {
     formatter->open_object_section("image");
     formatter->dump_string("name", image_name);
-    formatter->dump_string("global_id", mirror_image.global_id);
+    formatter->dump_string("global_id", status.info.global_id);
     formatter->dump_string("state", state);
     formatter->dump_string("description", status.description);
     formatter->dump_string("last_update", last_update);
@@ -256,7 +242,7 @@ int execute_status(const po::variables_map &vm) {
     formatter->flush(std::cout);
   } else {
     std::cout << image_name << ":\n"
-             << "  global_id:   " << mirror_image.global_id << "\n"
+             << "  global_id:   " << status.info.global_id << "\n"
              << "  state:       " << state << "\n"
              << "  description: " << status.description << "\n"
              << "  last_update: " << last_update << std::endl;

diff --git a/src/tools/rbd/action/MirrorPool.cc b/src/tools/rbd/action/MirrorPool.cc
index 94f21b9238e717e57c79cf372ca5afb514cbf099..4552db9b05cbf634d0dfd1ac5cb037bc7f4a5608 100644 (file)
--- a/src/tools/rbd/action/MirrorPool.cc
+++ b/src/tools/rbd/action/MirrorPool.cc
@@ -466,19 +466,18 @@ int execute_status(const po::variables_map &vm) {
     std::string last_read = "";
     int max_read = 1024;
     do {
-      map<std::string, librbd::mirror_image_info_t> mirror_images;
-      map<std::string, librbd::mirror_image_status_t> statuses;
+      map<std::string, librbd::mirror_image_status_t> mirror_images;
       r = rbd.mirror_image_status_list(io_ctx, last_read, max_read,
-                                      &mirror_images, &statuses);
+                                      &mirror_images);
       if (r < 0) {
        std::cerr << "rbd: failed to list mirrored image directory: "
                  << cpp_strerror(r) << std::endl;
        return r;
       }
       for (auto it = mirror_images.begin(); it != mirror_images.end(); ++it) {
-       const std::string &image_name = it->first;
-       std::string &global_image_id = it->second.global_id;
-       librbd::mirror_image_status_t &status = statuses[image_name];
+       librbd::mirror_image_status_t &status = it->second;
+       const std::string &image_name = status.name;
+       std::string &global_image_id = status.info.global_id;
        std::string state = utils::mirror_image_status_state(status);
        std::string last_update = utils::timestr(status.last_update);