From 20b43380e272ab09c500125a07d303623e50944b Mon Sep 17 00:00:00 2001 From: Mykola Golub Date: Thu, 2 Jan 2020 14:36:49 +0000 Subject: [PATCH] librbd: when unlinking peer from mirror snaps do it in all namespaces Signed-off-by: Mykola Golub --- src/librbd/api/Mirror.cc | 117 ++++++++++++++++++------------ src/test/librbd/test_mirroring.cc | 32 ++++++++ 2 files changed, 102 insertions(+), 47 deletions(-) diff --git a/src/librbd/api/Mirror.cc b/src/librbd/api/Mirror.cc index decafb7f825ed..a343d1c1a4ab8 100644 --- a/src/librbd/api/Mirror.cc +++ b/src/librbd/api/Mirror.cc @@ -15,6 +15,7 @@ #include "librbd/Operations.h" #include "librbd/Utils.h" #include "librbd/api/Image.h" +#include "librbd/api/Namespace.h" #include "librbd/mirror/DemoteRequest.h" #include "librbd/mirror/DisableRequest.h" #include "librbd/mirror/EnableRequest.h" @@ -1306,71 +1307,93 @@ int Mirror::peer_site_remove(librados::IoCtx& io_ctx, return r; } - std::set image_ids; - r = list_mirror_images(io_ctx, image_ids); + vector names; + r = Namespace::list(io_ctx, &names); if (r < 0) { - lderr(cct) << "failed listing images: " << cpp_strerror(r) << dendl; return r; } - for (const auto& image_id : image_ids) { - cls::rbd::MirrorImage mirror_image; - r = cls_client::mirror_image_get(&io_ctx, image_id, &mirror_image); - if (r < 0) { - lderr(cct) << "error getting mirror info for image " << image_id - << ": " << cpp_strerror(r) << dendl; - return r; - } + names.push_back(""); - if (mirror_image.mode != cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT) { - continue; - } + librados::IoCtx ns_io_ctx; + ns_io_ctx.dup(io_ctx); - // Snapshot based mirroring. Unlink the peer from mirroring snapshots. - // TODO: optimize. + for (auto &name : names) { + ns_io_ctx.set_namespace(name); - I *img_ctx = I::create("", image_id, nullptr, io_ctx, false); - r = img_ctx->state->open(0); + std::set image_ids; + r = list_mirror_images(ns_io_ctx, image_ids); if (r < 0) { - lderr(cct) << "error opening image " << image_id << ": " + lderr(cct) << "failed listing images in " + << (name.empty() ? "default" : name) << " namespace : " << cpp_strerror(r) << dendl; return r; } - std::list snap_ids; - { - std::shared_lock image_locker{img_ctx->image_lock}; - for (auto &it : img_ctx->snap_info) { - auto info = boost::get( - &it.second.snap_namespace); - if (info && info->mirror_peer_uuids.count(uuid)) { - snap_ids.push_back(it.first); - } + for (const auto& image_id : image_ids) { + cls::rbd::MirrorImage mirror_image; + r = cls_client::mirror_image_get(&ns_io_ctx, image_id, &mirror_image); + if (r == -ENOENT) { + continue; } - } - for (auto snap_id : snap_ids) { - C_SaferCond cond; - auto req = mirror::snapshot::UnlinkPeerRequest::create( - img_ctx, snap_id, uuid, &cond); - req->send(); - r = cond.wait(); + if (r < 0) { + lderr(cct) << "error getting mirror info for image " << image_id + << ": " << cpp_strerror(r) << dendl; + return r; + } + if (mirror_image.mode != cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT) { + continue; + } + + // Snapshot based mirroring. Unlink the peer from mirroring snapshots. + // TODO: optimize. + + I *img_ctx = I::create("", image_id, nullptr, ns_io_ctx, false); + r = img_ctx->state->open(0); if (r == -ENOENT) { - r = 0; + continue; } if (r < 0) { - break; + lderr(cct) << "error opening image " << image_id << ": " + << cpp_strerror(r) << dendl; + return r; } - } - int close_r = img_ctx->state->close(); - if (r < 0) { - lderr(cct) << "error unlinking peer for image " << image_id << ": " - << cpp_strerror(r) << dendl; - return r; - } else if (close_r < 0) { - lderr(cct) << "failed to close image " << image_id << ": " - << cpp_strerror(close_r) << dendl; - return close_r; + std::list snap_ids; + { + std::shared_lock image_locker{img_ctx->image_lock}; + for (auto &it : img_ctx->snap_info) { + auto info = boost::get( + &it.second.snap_namespace); + if (info && info->mirror_peer_uuids.count(uuid)) { + snap_ids.push_back(it.first); + } + } + } + for (auto snap_id : snap_ids) { + C_SaferCond cond; + auto req = mirror::snapshot::UnlinkPeerRequest::create( + img_ctx, snap_id, uuid, &cond); + req->send(); + r = cond.wait(); + if (r == -ENOENT) { + r = 0; + } + if (r < 0) { + break; + } + } + + int close_r = img_ctx->state->close(); + if (r < 0) { + lderr(cct) << "error unlinking peer for image " << image_id << ": " + << cpp_strerror(r) << dendl; + return r; + } else if (close_r < 0) { + lderr(cct) << "failed to close image " << image_id << ": " + << cpp_strerror(close_r) << dendl; + return close_r; + } } } diff --git a/src/test/librbd/test_mirroring.cc b/src/test/librbd/test_mirroring.cc index 4f9cbdba234ab..c805fb778fe31 100644 --- a/src/test/librbd/test_mirroring.cc +++ b/src/test/librbd/test_mirroring.cc @@ -20,6 +20,7 @@ #include "librbd/ObjectMap.h" #include "librbd/Operations.h" #include "librbd/api/Image.h" +#include "librbd/api/Namespace.h" #include "librbd/io/AioCompletion.h" #include "librbd/io/ImageRequest.h" #include "librbd/io/ImageRequestWQ.h" @@ -1268,6 +1269,26 @@ TEST_F(TestMirroring, SnapshotUnlinkPeer) ASSERT_EQ(1, mirror_snap.mirror_peer_uuids.count(peer2_uuid)); ASSERT_EQ(1, mirror_snap.mirror_peer_uuids.count(peer3_uuid)); + ASSERT_EQ(0, librbd::api::Namespace<>::create(m_ioctx, "ns1")); + librados::IoCtx ns_ioctx; + ns_ioctx.dup(m_ioctx); + ns_ioctx.set_namespace("ns1"); + ASSERT_EQ(0, m_rbd.mirror_mode_set(ns_ioctx, RBD_MIRROR_MODE_IMAGE)); + ASSERT_EQ(0, m_rbd.create2(ns_ioctx, image_name.c_str(), 4096, features, + &order)); + + librbd::Image ns_image; + ASSERT_EQ(0, m_rbd.open(ns_ioctx, ns_image, image_name.c_str())); + ASSERT_EQ(0, ns_image.mirror_image_enable2(RBD_MIRROR_IMAGE_MODE_SNAPSHOT)); + uint64_t ns_snap_id; + ASSERT_EQ(0, ns_image.mirror_image_create_snapshot(&ns_snap_id)); + ASSERT_EQ(0, ns_image.snap_get_mirror_primary_namespace( + ns_snap_id, &mirror_snap, sizeof(mirror_snap))); + ASSERT_EQ(3U, mirror_snap.mirror_peer_uuids.size()); + ASSERT_EQ(1, mirror_snap.mirror_peer_uuids.count(peer1_uuid)); + ASSERT_EQ(1, mirror_snap.mirror_peer_uuids.count(peer2_uuid)); + ASSERT_EQ(1, mirror_snap.mirror_peer_uuids.count(peer3_uuid)); + ASSERT_EQ(0, m_rbd.mirror_peer_site_remove(m_ioctx, peer3_uuid)); ASSERT_EQ(0, image.snap_get_mirror_primary_namespace(snap_id, &mirror_snap, @@ -1275,6 +1296,12 @@ TEST_F(TestMirroring, SnapshotUnlinkPeer) ASSERT_EQ(1U, mirror_snap.mirror_peer_uuids.size()); ASSERT_EQ(1, mirror_snap.mirror_peer_uuids.count(peer2_uuid)); + ASSERT_EQ(0, ns_image.snap_get_mirror_primary_namespace( + ns_snap_id, &mirror_snap, sizeof(mirror_snap))); + ASSERT_EQ(2U, mirror_snap.mirror_peer_uuids.size()); + ASSERT_EQ(1, mirror_snap.mirror_peer_uuids.count(peer1_uuid)); + ASSERT_EQ(1, mirror_snap.mirror_peer_uuids.count(peer2_uuid)); + C_SaferCond cond2; req = librbd::mirror::snapshot::UnlinkPeerRequest<>::create( ictx, snap_id, peer2_uuid, &cond2); @@ -1287,6 +1314,11 @@ TEST_F(TestMirroring, SnapshotUnlinkPeer) ictx = nullptr; ASSERT_EQ(0, image.close()); ASSERT_EQ(0, m_rbd.remove(m_ioctx, image_name.c_str())); + + ASSERT_EQ(0, ns_image.close()); + ASSERT_EQ(0, m_rbd.remove(ns_ioctx, image_name.c_str())); + ASSERT_EQ(0, librbd::api::Namespace<>::remove(m_ioctx, "ns1")); + ASSERT_EQ(0, m_rbd.mirror_peer_site_remove(m_ioctx, peer1_uuid)); ASSERT_EQ(0, m_rbd.mirror_peer_site_remove(m_ioctx, peer2_uuid)); ASSERT_EQ(0, m_rbd.mirror_mode_set(m_ioctx, RBD_MIRROR_MODE_DISABLED)); -- 2.39.5