cls/rbd: combined group_snap_add and group_snap_update

Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/cls/rbd/cls_rbd.cc b/src/cls/rbd/cls_rbd.cc
index bfb29c289126f9493fdf9302788937f2bf7d131f..e4c6735f9635f9983075294d433af30c3d343fbf 100644 (file)
--- a/src/cls/rbd/cls_rbd.cc
+++ b/src/cls/rbd/cls_rbd.cc
@@ -5148,19 +5148,6 @@ static int check_duplicate_snap_name(cls_method_context_t hctx,
   return 0;
 }
 
-static int check_duplicate_snap_id(cls_method_context_t hctx,
-                                  std::string snap_key)
-{
-  bufferlist bl;
-  int r = cls_cxx_map_get_val(hctx, snap_key, &bl);
-  if (r == -ENOENT) {
-    return 0;
-  } else if (r < 0) {
-    return r;
-  }
-  return -EEXIST;
-}
-
 }
 
 /**
@@ -5172,10 +5159,10 @@ static int check_duplicate_snap_id(cls_method_context_t hctx,
  * Output:
  * @return 0 on success, negative error code on failure
  */
-int group_snap_add(cls_method_context_t hctx,
+int group_snap_set(cls_method_context_t hctx,
                   bufferlist *in, bufferlist *out)
 {
-  CLS_LOG(20, "group_snap_add");
+  CLS_LOG(20, "group_snap_set");
   cls::rbd::GroupSnapshot group_snap;
   try {
     bufferlist::iterator iter = in->begin();
@@ -5183,6 +5170,7 @@ int group_snap_add(cls_method_context_t hctx,
   } catch (const buffer::error &err) {
     return -EINVAL;
   }
+
   if (group_snap.name.empty()) {
     CLS_ERR("group snapshot name is empty");
     return -EINVAL;
@@ -5192,65 +5180,26 @@ int group_snap_add(cls_method_context_t hctx,
     return -EINVAL;
   }
 
-  int r = group::check_duplicate_snap_name(hctx, group_snap.name, group_snap.id);
-  if (r < 0) {
-    return r;
-  }
-
-  r = group::check_duplicate_snap_id(hctx, group::snap_key(group_snap.id));
+  int r = group::check_duplicate_snap_name(hctx, group_snap.name,
+                                           group_snap.id);
   if (r < 0) {
     return r;
   }
 
   std::string key = group::snap_key(group_snap.id);
-
-  bufferlist obl;
-  ::encode(group_snap, obl);
-  r = cls_cxx_map_set_val(hctx, key, &obl);
-  return r;
-}
-
-/**
- * Update snapshot record.
- *
- * Input:
- * @param GroupSnapshot
- *
- * Output:
- * @return 0 on success, negative error code on failure
- */
-int group_snap_update(cls_method_context_t hctx,
-                     bufferlist *in, bufferlist *out)
-{
-  CLS_LOG(20, "group_snap_update");
-  cls::rbd::GroupSnapshot group_snap;
-  try {
-    bufferlist::iterator iter = in->begin();
-    ::decode(group_snap, iter);
-  } catch (const buffer::error &err) {
-    return -EINVAL;
-  }
-  if (group_snap.name.empty()) {
-    CLS_ERR("group snapshot name is empty");
-    return -EINVAL;
-  }
-
-  int r = group::check_duplicate_snap_name(hctx, group_snap.name, group_snap.id);
-  if (r < 0) {
-    return r;
-  }
-
-  if (group_snap.id.empty()) {
-    CLS_ERR("group snapshot id is empty");
-    return -EINVAL;
+  if (group_snap.state == cls::rbd::GROUP_SNAPSHOT_STATE_INCOMPLETE) {
+    bufferlist snap_bl;
+    r = cls_cxx_map_get_val(hctx, key, &snap_bl);
+    if (r < 0 && r != -ENOENT) {
+      return r;
+    } else if (r >= 0) {
+      return -EEXIST;
+    }
   }
 
-  std::string key = group::snap_key(group_snap.id);
-
   bufferlist obl;
   ::encode(group_snap, obl);
   r = cls_cxx_map_set_val(hctx, key, &obl);
-
   return r;
 }
 
@@ -5661,8 +5610,7 @@ CLS_INIT(rbd)
   cls_method_handle_t h_image_add_group;
   cls_method_handle_t h_image_remove_group;
   cls_method_handle_t h_image_get_group;
-  cls_method_handle_t h_group_snap_add;
-  cls_method_handle_t h_group_snap_update;
+  cls_method_handle_t h_group_snap_set;
   cls_method_handle_t h_group_snap_remove;
   cls_method_handle_t h_group_snap_get_by_id;
   cls_method_handle_t h_group_snap_list;
@@ -5944,12 +5892,9 @@ CLS_INIT(rbd)
   cls_register_cxx_method(h_class, "image_get_group",
                          CLS_METHOD_RD,
                          image_get_group, &h_image_get_group);
-  cls_register_cxx_method(h_class, "group_snap_add",
-                         CLS_METHOD_RD | CLS_METHOD_WR,
-                         group_snap_add, &h_group_snap_add);
-  cls_register_cxx_method(h_class, "group_snap_update",
+  cls_register_cxx_method(h_class, "group_snap_set",
                          CLS_METHOD_RD | CLS_METHOD_WR,
-                         group_snap_update, &h_group_snap_update);
+                         group_snap_set, &h_group_snap_set);
   cls_register_cxx_method(h_class, "group_snap_remove",
                          CLS_METHOD_RD | CLS_METHOD_WR,
                          group_snap_remove, &h_group_snap_remove);

diff --git a/src/cls/rbd/cls_rbd_client.cc b/src/cls/rbd/cls_rbd_client.cc
index 8af0fe7c06918e5915fd05e7a9fd41e82d2e715f..693d18bac732d7b15d651628532121b2a252e157 100644 (file)
--- a/src/cls/rbd/cls_rbd_client.cc
+++ b/src/cls/rbd/cls_rbd_client.cc
@@ -2064,25 +2064,14 @@ namespace librbd {
       return image_get_group_finish(&iter, group_spec);
     }
 
-    int group_snap_add(librados::IoCtx *ioctx, const std::string &oid,
+    int group_snap_set(librados::IoCtx *ioctx, const std::string &oid,
                       const cls::rbd::GroupSnapshot &snapshot)
     {
 
       bufferlist inbl, outbl;
 
       ::encode(snapshot, inbl);
-      int r = ioctx->exec(oid, "rbd", "group_snap_add", inbl, outbl);
-      return r;
-    }
-
-    int group_snap_update(librados::IoCtx *ioctx, const std::string &oid,
-                         const cls::rbd::GroupSnapshot &snapshot)
-    {
-
-      bufferlist inbl, outbl;
-
-      ::encode(snapshot, inbl);
-      int r = ioctx->exec(oid, "rbd", "group_snap_update", inbl, outbl);
+      int r = ioctx->exec(oid, "rbd", "group_snap_set", inbl, outbl);
       return r;
     }
 

diff --git a/src/cls/rbd/cls_rbd_client.h b/src/cls/rbd/cls_rbd_client.h
index 6a9bd04046a56d1f6c29e66960fabb716b6749ce..14ce01c437f8db4a1e9a007258bc13a5855a255c 100644 (file)
--- a/src/cls/rbd/cls_rbd_client.h
+++ b/src/cls/rbd/cls_rbd_client.h
@@ -423,10 +423,8 @@ namespace librbd {
                                cls::rbd::GroupSpec *group_spec);
     int image_get_group(librados::IoCtx *ioctx, const std::string &oid,
                        cls::rbd::GroupSpec *group_spec);
-    int group_snap_add(librados::IoCtx *ioctx, const std::string &oid,
+    int group_snap_set(librados::IoCtx *ioctx, const std::string &oid,
                       const cls::rbd::GroupSnapshot &snapshot);
-    int group_snap_update(librados::IoCtx *ioctx, const std::string &oid,
-                         const cls::rbd::GroupSnapshot &snapshot);
     int group_snap_remove(librados::IoCtx *ioctx, const std::string &oid,
                          const std::string &snap_id);
     int group_snap_get_by_id(librados::IoCtx *ioctx, const std::string &oid,

diff --git a/src/librbd/api/Group.cc b/src/librbd/api/Group.cc
index 4c0b8ab2e9afd74a41f12f7234171645aebdd8f6..86424d2d374dc45e0983ddf760e37f07a15bd600 100644 (file)
--- a/src/librbd/api/Group.cc
+++ b/src/librbd/api/Group.cc
@@ -710,7 +710,7 @@ int Group<I>::snap_create(librados::IoCtx& group_ioctx,
   cls::rbd::GroupSnapshotNamespace ne{group_ioctx.get_id(), group_id,
                                       group_snap.id};
 
-  r = cls_client::group_snap_add(&group_ioctx, group_header_oid, group_snap);
+  r = cls_client::group_snap_set(&group_ioctx, group_header_oid, group_snap);
   if (r == -EEXIST) {
     lderr(cct) << "snapshot with this name already exists: "
               << cpp_strerror(r)
@@ -835,7 +835,7 @@ int Group<I>::snap_create(librados::IoCtx& group_ioctx,
   group_snap.snaps = image_snaps;
   group_snap.state = cls::rbd::GROUP_SNAPSHOT_STATE_COMPLETE;
 
-  r = cls_client::group_snap_update(&group_ioctx, group_header_oid, group_snap);
+  r = cls_client::group_snap_set(&group_ioctx, group_header_oid, group_snap);
   if (r < 0) {
     ret_code = r;
     goto remove_image_snaps;

diff --git a/src/test/cls_rbd/test_cls_rbd.cc b/src/test/cls_rbd/test_cls_rbd.cc
index 3217c9d9dc1e5d37716ff77fe806397f987e02a3..e33ab86bd0715801de2b68beb527245cc1dafd1a 100644 (file)
--- a/src/test/cls_rbd/test_cls_rbd.cc
+++ b/src/test/cls_rbd/test_cls_rbd.cc
@@ -2223,7 +2223,7 @@ TEST_F(TestClsRbd, image_get_group) {
   ASSERT_EQ(pool_id, spec.pool_id);
 }
 
-TEST_F(TestClsRbd, group_snap_add_empty_name) {
+TEST_F(TestClsRbd, group_snap_set_empty_name) {
   librados::IoCtx ioctx;
   ASSERT_EQ(0, _rados.ioctx_create(_pool_name.c_str(), ioctx));
 
@@ -2234,10 +2234,10 @@ TEST_F(TestClsRbd, group_snap_add_empty_name) {
 
   string snap_id = "snap_id";
   cls::rbd::GroupSnapshot snap = {snap_id, "", cls::rbd::GROUP_SNAPSHOT_STATE_INCOMPLETE};
-  ASSERT_EQ(-EINVAL, group_snap_add(&ioctx, group_id, snap));
+  ASSERT_EQ(-EINVAL, group_snap_set(&ioctx, group_id, snap));
 }
 
-TEST_F(TestClsRbd, group_snap_add_empty_id) {
+TEST_F(TestClsRbd, group_snap_set_empty_id) {
   librados::IoCtx ioctx;
   ASSERT_EQ(0, _rados.ioctx_create(_pool_name.c_str(), ioctx));
 
@@ -2248,10 +2248,10 @@ TEST_F(TestClsRbd, group_snap_add_empty_id) {
 
   string snap_id = "snap_id";
   cls::rbd::GroupSnapshot snap = {"", "snap_name", cls::rbd::GROUP_SNAPSHOT_STATE_INCOMPLETE};
-  ASSERT_EQ(-EINVAL, group_snap_add(&ioctx, group_id, snap));
+  ASSERT_EQ(-EINVAL, group_snap_set(&ioctx, group_id, snap));
 }
 
-TEST_F(TestClsRbd, group_snap_add_duplicate_id) {
+TEST_F(TestClsRbd, group_snap_set_duplicate_id) {
   librados::IoCtx ioctx;
   ASSERT_EQ(0, _rados.ioctx_create(_pool_name.c_str(), ioctx));
 
@@ -2262,13 +2262,13 @@ TEST_F(TestClsRbd, group_snap_add_duplicate_id) {
 
   string snap_id = "snap_id";
   cls::rbd::GroupSnapshot snap = {snap_id, "snap_name", cls::rbd::GROUP_SNAPSHOT_STATE_INCOMPLETE};
-  ASSERT_EQ(0, group_snap_add(&ioctx, group_id, snap));
+  ASSERT_EQ(0, group_snap_set(&ioctx, group_id, snap));
 
   cls::rbd::GroupSnapshot snap1 = {snap_id, "snap_name1", cls::rbd::GROUP_SNAPSHOT_STATE_INCOMPLETE};
-  ASSERT_EQ(-EEXIST, group_snap_add(&ioctx, group_id, snap1));
+  ASSERT_EQ(-EEXIST, group_snap_set(&ioctx, group_id, snap1));
 }
 
-TEST_F(TestClsRbd, group_snap_add_duplicate_name) {
+TEST_F(TestClsRbd, group_snap_set_duplicate_name) {
   librados::IoCtx ioctx;
   ASSERT_EQ(0, _rados.ioctx_create(_pool_name.c_str(), ioctx));
 
@@ -2279,14 +2279,14 @@ TEST_F(TestClsRbd, group_snap_add_duplicate_name) {
 
   string snap_id1 = "snap_id1";
   cls::rbd::GroupSnapshot snap = {snap_id1, "snap_name", cls::rbd::GROUP_SNAPSHOT_STATE_INCOMPLETE};
-  ASSERT_EQ(0, group_snap_add(&ioctx, group_id, snap));
+  ASSERT_EQ(0, group_snap_set(&ioctx, group_id, snap));
 
   string snap_id2 = "snap_id2";
   cls::rbd::GroupSnapshot snap1 = {snap_id2, "snap_name", cls::rbd::GROUP_SNAPSHOT_STATE_INCOMPLETE};
-  ASSERT_EQ(-EEXIST, group_snap_add(&ioctx, group_id, snap1));
+  ASSERT_EQ(-EEXIST, group_snap_set(&ioctx, group_id, snap1));
 }
 
-TEST_F(TestClsRbd, group_snap_add) {
+TEST_F(TestClsRbd, group_snap_set) {
   librados::IoCtx ioctx;
   ASSERT_EQ(0, _rados.ioctx_create(_pool_name.c_str(), ioctx));
 
@@ -2297,7 +2297,7 @@ TEST_F(TestClsRbd, group_snap_add) {
 
   string snap_id = "snap_id";
   cls::rbd::GroupSnapshot snap = {snap_id, "test_snapshot", cls::rbd::GROUP_SNAPSHOT_STATE_INCOMPLETE};
-  ASSERT_EQ(0, group_snap_add(&ioctx, group_id, snap));
+  ASSERT_EQ(0, group_snap_set(&ioctx, group_id, snap));
 
   set<string> keys;
   ASSERT_EQ(0, ioctx.omap_get_keys(group_id, "", 10, &keys));
@@ -2320,11 +2320,11 @@ TEST_F(TestClsRbd, group_snap_list) {
 
   string snap_id1 = "snap_id1";
   cls::rbd::GroupSnapshot snap1 = {snap_id1, "test_snapshot1", cls::rbd::GROUP_SNAPSHOT_STATE_INCOMPLETE};
-  ASSERT_EQ(0, group_snap_add(&ioctx, group_id, snap1));
+  ASSERT_EQ(0, group_snap_set(&ioctx, group_id, snap1));
 
   string snap_id2 = "snap_id2";
   cls::rbd::GroupSnapshot snap2 = {snap_id2, "test_snapshot2", cls::rbd::GROUP_SNAPSHOT_STATE_INCOMPLETE};
-  ASSERT_EQ(0, group_snap_add(&ioctx, group_id, snap2));
+  ASSERT_EQ(0, group_snap_set(&ioctx, group_id, snap2));
 
   std::vector<cls::rbd::GroupSnapshot> snapshots;
   ASSERT_EQ(0, group_snap_list(&ioctx, group_id, cls::rbd::GroupSnapshot(), 10, &snapshots));
@@ -2354,7 +2354,7 @@ TEST_F(TestClsRbd, group_snap_list_max_return) {
     cls::rbd::GroupSnapshot snap = {snap_id,
                                    "test_snapshot" + hexify(i),
                                    cls::rbd::GROUP_SNAPSHOT_STATE_INCOMPLETE};
-    ASSERT_EQ(0, group_snap_add(&ioctx, group_id, snap));
+    ASSERT_EQ(0, group_snap_set(&ioctx, group_id, snap));
   }
 
   std::vector<cls::rbd::GroupSnapshot> snapshots;
@@ -2376,68 +2376,6 @@ TEST_F(TestClsRbd, group_snap_list_max_return) {
   }
 }
 
-TEST_F(TestClsRbd, group_snap_update) {
-  librados::IoCtx ioctx;
-  ASSERT_EQ(0, _rados.ioctx_create(_pool_name.c_str(), ioctx));
-
-  string image_id = "image_id_snap_update";
-
-  string group_id = "group_id_snap_update";
-  ASSERT_EQ(0, ioctx.create(group_id, true));
-
-  string snap_id = "snap_id";
-  cls::rbd::GroupSnapshot snap = {snap_id, "test_snapshot", cls::rbd::GROUP_SNAPSHOT_STATE_INCOMPLETE};
-  ASSERT_EQ(0, group_snap_add(&ioctx, group_id, snap));
-
-  snap.state = cls::rbd::GROUP_SNAPSHOT_STATE_COMPLETE;
-
-  ASSERT_EQ(0, group_snap_update(&ioctx, group_id, snap));
-
-  std::vector<cls::rbd::GroupSnapshot> snapshots;
-  ASSERT_EQ(0, group_snap_list(&ioctx, group_id, cls::rbd::GroupSnapshot(), 10, &snapshots));
-  ASSERT_EQ(1U, snapshots.size());
-  ASSERT_EQ(snap_id, snapshots[0].id);
-  ASSERT_EQ(cls::rbd::GROUP_SNAPSHOT_STATE_COMPLETE, snapshots[0].state);
-}
-
-TEST_F(TestClsRbd, group_snap_update_empty_name) {
-  librados::IoCtx ioctx;
-  ASSERT_EQ(0, _rados.ioctx_create(_pool_name.c_str(), ioctx));
-
-  string image_id = "image_id_snap_update_empty_name";
-
-  string group_id = "group_id_snap_update_empty_name";
-  ASSERT_EQ(0, ioctx.create(group_id, true));
-
-  string snap_id = "snap_id";
-  cls::rbd::GroupSnapshot snap = {snap_id, "test_snapshot", cls::rbd::GROUP_SNAPSHOT_STATE_INCOMPLETE};
-  ASSERT_EQ(0, group_snap_add(&ioctx, group_id, snap));
-
-  snap.name = "";
-  snap.state = cls::rbd::GROUP_SNAPSHOT_STATE_COMPLETE;
-
-  ASSERT_EQ(-EINVAL, group_snap_update(&ioctx, group_id, snap));
-}
-
-TEST_F(TestClsRbd, group_snap_update_empty_id) {
-  librados::IoCtx ioctx;
-  ASSERT_EQ(0, _rados.ioctx_create(_pool_name.c_str(), ioctx));
-
-  string image_id = "image_id_snap_update_empty_id";
-
-  string group_id = "group_id_snap_update_empty_id";
-  ASSERT_EQ(0, ioctx.create(group_id, true));
-
-  string snap_id = "snap_id";
-  cls::rbd::GroupSnapshot snap = {snap_id, "test_snapshot", cls::rbd::GROUP_SNAPSHOT_STATE_INCOMPLETE};
-  ASSERT_EQ(0, group_snap_add(&ioctx, group_id, snap));
-
-  snap.id = "";
-  snap.state = cls::rbd::GROUP_SNAPSHOT_STATE_COMPLETE;
-
-  ASSERT_EQ(-EINVAL, group_snap_update(&ioctx, group_id, snap));
-}
-
 TEST_F(TestClsRbd, group_snap_remove) {
   librados::IoCtx ioctx;
 
@@ -2450,7 +2388,7 @@ TEST_F(TestClsRbd, group_snap_remove) {
 
   string snap_id = "snap_id";
   cls::rbd::GroupSnapshot snap = {snap_id, "test_snapshot", cls::rbd::GROUP_SNAPSHOT_STATE_INCOMPLETE};
-  ASSERT_EQ(0, group_snap_add(&ioctx, group_id, snap));
+  ASSERT_EQ(0, group_snap_set(&ioctx, group_id, snap));
 
   set<string> keys;
   ASSERT_EQ(0, ioctx.omap_get_keys(group_id, "", 10, &keys));
@@ -2484,7 +2422,7 @@ TEST_F(TestClsRbd, group_snap_get_by_id) {
   cls::rbd::GroupSnapshot snap = {snap_id,
                                   "test_snapshot",
                                   cls::rbd::GROUP_SNAPSHOT_STATE_INCOMPLETE};
-  ASSERT_EQ(0, group_snap_add(&ioctx, group_id, snap));
+  ASSERT_EQ(0, group_snap_set(&ioctx, group_id, snap));
 
   cls::rbd::GroupSnapshot received_snap;
   ASSERT_EQ(0, group_snap_get_by_id(&ioctx, group_id, snap_id, &received_snap));