From 725722bb5efc6bf331d7ec804e90bcb44eca9d44 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 11 Jan 2018 00:24:37 -0500 Subject: [PATCH] cls/rbd: combined group_snap_add and group_snap_update Signed-off-by: Jason Dillaman --- src/cls/rbd/cls_rbd.cc | 87 ++++++----------------------- src/cls/rbd/cls_rbd_client.cc | 15 +---- src/cls/rbd/cls_rbd_client.h | 4 +- src/librbd/api/Group.cc | 4 +- src/test/cls_rbd/test_cls_rbd.cc | 96 ++++++-------------------------- 5 files changed, 38 insertions(+), 168 deletions(-) diff --git a/src/cls/rbd/cls_rbd.cc b/src/cls/rbd/cls_rbd.cc index bfb29c28912..e4c6735f963 100644 --- 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 8af0fe7c069..693d18bac73 100644 --- 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 6a9bd04046a..14ce01c437f 100644 --- 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 4c0b8ab2e9a..86424d2d374 100644 --- a/src/librbd/api/Group.cc +++ b/src/librbd/api/Group.cc @@ -710,7 +710,7 @@ int Group::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::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 3217c9d9dc1..e33ab86bd07 100644 --- 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 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 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 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 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 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)); -- 2.39.5