Merge pull request #9640 from dillaman/wip-16235

librbd: recursive lock possible when disabling journaling

Reviewed-by: Mykola Golub <mgolub@mirantis.com>
This commit is contained in:
Mykola Golub 2016-06-12 20:39:25 +03:00 committed by GitHub
commit 9dd0487af9
5 changed files with 149 additions and 44 deletions

View File

@ -1646,15 +1646,6 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
return -EINVAL;
}
RWLock::RLocker owner_locker(ictx->owner_lock);
r = ictx->aio_work_queue->block_writes();
BOOST_SCOPE_EXIT_ALL( (ictx) ) {
ictx->aio_work_queue->unblock_writes();
};
if (r < 0) {
return r;
}
uint64_t disable_mask = (RBD_FEATURES_MUTABLE |
RBD_FEATURES_DISABLE_ONLY);
if ((enabled && (features & RBD_FEATURES_MUTABLE) != features) ||
@ -1666,6 +1657,35 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
return -EINVAL;
}
rbd_mirror_mode_t mirror_mode = RBD_MIRROR_MODE_DISABLED;
if ((features & RBD_FEATURE_JOURNALING) != 0) {
r = librbd::mirror_mode_get(ictx->md_ctx, &mirror_mode);
if (r < 0) {
lderr(cct) << "error in retrieving pool mirroring status: "
<< cpp_strerror(r) << dendl;
return r;
}
// mark mirroring as disabling and prune all sync snapshots
// before acquiring locks
if (mirror_mode == RBD_MIRROR_MODE_POOL && !enabled) {
r = mirror_image_disable_internal(ictx, false, false);
if (r < 0) {
lderr(cct) << "error disabling image mirroring: "
<< cpp_strerror(r) << dendl;
}
}
}
RWLock::RLocker owner_locker(ictx->owner_lock);
r = ictx->aio_work_queue->block_writes();
BOOST_SCOPE_EXIT_ALL( (ictx) ) {
ictx->aio_work_queue->unblock_writes();
};
if (r < 0) {
return r;
}
// avoid accepting new requests from peers while we manipulate
// the image features
if (ictx->exclusive_lock != nullptr) {
@ -1749,14 +1769,6 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
return r;
}
rbd_mirror_mode_t mirror_mode;
r = librbd::mirror_mode_get(ictx->md_ctx, &mirror_mode);
if (r < 0) {
lderr(cct) << "error in retrieving pool mirroring status: "
<< cpp_strerror(r) << dendl;
return r;
}
enable_mirroring = (mirror_mode == RBD_MIRROR_MODE_POOL);
}
@ -1793,14 +1805,6 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
disable_flags |= RBD_FLAG_FAST_DIFF_INVALID;
}
if ((features & RBD_FEATURE_JOURNALING) != 0) {
rbd_mirror_mode_t mirror_mode;
r = librbd::mirror_mode_get(ictx->md_ctx, &mirror_mode);
if (r < 0) {
lderr(cct) << "error in retrieving pool mirroring status: "
<< cpp_strerror(r) << dendl;
return r;
}
if (mirror_mode == RBD_MIRROR_MODE_IMAGE) {
cls::rbd::MirrorImage mirror_image;
r = cls_client::mirror_image_get(&ictx->md_ctx, ictx->id,
@ -1816,10 +1820,11 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
return -EINVAL;
}
} else if (mirror_mode == RBD_MIRROR_MODE_POOL) {
r = mirror_image_disable_internal(ictx, false);
r = cls_client::mirror_image_remove(&ictx->md_ctx, ictx->id);
if (r < 0) {
lderr(cct) << "error disabling image mirroring: "
<< cpp_strerror(r) << dendl;
lderr(cct) << "failed to remove image from mirroring directory: "
<< cpp_strerror(r) << dendl;
return r;
}
}

View File

@ -82,22 +82,6 @@ static int get_features(bool *old_format, uint64_t *features)
return 0;
}
static int get_image_id(librbd::Image &image, std::string *image_id)
{
librbd::image_info_t info;
int r = image.stat(info, sizeof(info));
if (r < 0) {
return r;
}
char prefix[RBD_MAX_BLOCK_NAME_SIZE + 1];
strncpy(prefix, info.block_name_prefix, RBD_MAX_BLOCK_NAME_SIZE);
prefix[RBD_MAX_BLOCK_NAME_SIZE] = '\0';
*image_id = std::string(prefix + strlen(RBD_DATA_PREFIX));
return 0;
}
static int create_image_full(rados_ioctx_t ioctx, const char *name,
uint64_t size, int *order, int old_format,
uint64_t features)

View File

@ -22,6 +22,8 @@
#include "librbd/internal.h"
#include "librbd/ObjectMap.h"
#include "librbd/Operations.h"
#include "librbd/journal/Types.h"
#include "journal/Journaler.h"
#include <boost/scope_exit.hpp>
#include <boost/assign/list_of.hpp>
#include <utility>
@ -255,6 +257,30 @@ public:
ASSERT_EQ(0, m_rbd.mirror_mode_set(m_ioctx, RBD_MIRROR_MODE_DISABLED));
}
void setup_mirror_peer(librados::IoCtx &io_ctx, librbd::Image &image) {
ASSERT_EQ(0, image.snap_create("sync-point-snap"));
std::string image_id;
ASSERT_EQ(0, get_image_id(image, &image_id));
librbd::journal::MirrorPeerClientMeta peer_client_meta(
"remote-image-id", {{"sync-point-snap", boost::none}}, {});
librbd::journal::ClientData client_data(peer_client_meta);
journal::Journaler journaler(io_ctx, image_id, "peer-client", 5);
C_SaferCond init_ctx;
journaler.init(&init_ctx);
ASSERT_EQ(-ENOENT, init_ctx.wait());
bufferlist client_data_bl;
::encode(client_data, client_data_bl);
ASSERT_EQ(0, journaler.register_client(client_data_bl));
C_SaferCond shut_down_ctx;
journaler.shut_down(&shut_down_ctx);
ASSERT_EQ(0, shut_down_ctx.wait());
}
};
TEST_F(TestMirroring, EnableImageMirror_In_MirrorModeImage) {
@ -311,6 +337,77 @@ TEST_F(TestMirroring, DisableImageMirror_In_MirrorModeDisabled) {
RBD_MIRROR_IMAGE_DISABLED);
}
TEST_F(TestMirroring, DisableImageMirrorWithPeer) {
REQUIRE_FEATURE(RBD_FEATURE_JOURNALING);
ASSERT_EQ(0, m_rbd.mirror_mode_set(m_ioctx, RBD_MIRROR_MODE_IMAGE));
uint64_t features = RBD_FEATURE_EXCLUSIVE_LOCK | RBD_FEATURE_JOURNALING;
int order = 20;
ASSERT_EQ(0, m_rbd.create2(m_ioctx, image_name.c_str(), 4096, features,
&order));
librbd::Image image;
ASSERT_EQ(0, m_rbd.open(m_ioctx, image, image_name.c_str()));
ASSERT_EQ(0, image.mirror_image_enable());
setup_mirror_peer(m_ioctx, image);
ASSERT_EQ(0, image.mirror_image_disable(false));
std::vector<librbd::snap_info_t> snaps;
ASSERT_EQ(0, image.snap_list(snaps));
ASSERT_TRUE(snaps.empty());
librbd::mirror_image_info_t mirror_image;
ASSERT_EQ(0, image.mirror_image_get_info(&mirror_image,
sizeof(mirror_image)));
ASSERT_EQ(RBD_MIRROR_IMAGE_DISABLED, mirror_image.state);
librbd::mirror_image_status_t status;
ASSERT_EQ(0, image.mirror_image_get_status(&status, sizeof(status)));
ASSERT_EQ(MIRROR_IMAGE_STATUS_STATE_UNKNOWN, status.state);
ASSERT_EQ(0, image.close());
ASSERT_EQ(0, m_rbd.remove(m_ioctx, image_name.c_str()));
ASSERT_EQ(0, m_rbd.mirror_mode_set(m_ioctx, RBD_MIRROR_MODE_DISABLED));
}
TEST_F(TestMirroring, DisableJournalingWithPeer) {
REQUIRE_FEATURE(RBD_FEATURE_JOURNALING);
ASSERT_EQ(0, m_rbd.mirror_mode_set(m_ioctx, RBD_MIRROR_MODE_POOL));
uint64_t features = RBD_FEATURE_EXCLUSIVE_LOCK | RBD_FEATURE_JOURNALING;
int order = 20;
ASSERT_EQ(0, m_rbd.create2(m_ioctx, image_name.c_str(), 4096, features,
&order));
librbd::Image image;
ASSERT_EQ(0, m_rbd.open(m_ioctx, image, image_name.c_str()));
setup_mirror_peer(m_ioctx, image);
ASSERT_EQ(0, image.update_features(RBD_FEATURE_JOURNALING, false));
std::vector<librbd::snap_info_t> snaps;
ASSERT_EQ(0, image.snap_list(snaps));
ASSERT_TRUE(snaps.empty());
librbd::mirror_image_info_t mirror_image;
ASSERT_EQ(0, image.mirror_image_get_info(&mirror_image,
sizeof(mirror_image)));
ASSERT_EQ(RBD_MIRROR_IMAGE_DISABLED, mirror_image.state);
librbd::mirror_image_status_t status;
ASSERT_EQ(0, image.mirror_image_get_status(&status, sizeof(status)));
ASSERT_EQ(MIRROR_IMAGE_STATUS_STATE_UNKNOWN, status.state);
ASSERT_EQ(0, image.close());
ASSERT_EQ(0, m_rbd.remove(m_ioctx, image_name.c_str()));
ASSERT_EQ(0, m_rbd.mirror_mode_set(m_ioctx, RBD_MIRROR_MODE_DISABLED));
}
TEST_F(TestMirroring, EnableImageMirror_WithoutJournaling) {
uint64_t features = 0;
features |= RBD_FEATURE_OBJECT_MAP;

View File

@ -1,6 +1,7 @@
// -*- mode:C; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
// vim: ts=8 sw=2 smarttab
#include "test/librbd/test_support.h"
#include "include/rbd_types.h"
#include <sstream>
bool get_features(uint64_t *features) {
@ -37,3 +38,20 @@ int create_image_pp(librbd::RBD &rbd, librados::IoCtx &ioctx,
return rbd.create2(ioctx, name.c_str(), size, features, &order);
}
}
int get_image_id(librbd::Image &image, std::string *image_id)
{
librbd::image_info_t info;
int r = image.stat(info, sizeof(info));
if (r < 0) {
return r;
}
char prefix[RBD_MAX_BLOCK_NAME_SIZE + 1];
strncpy(prefix, info.block_name_prefix, RBD_MAX_BLOCK_NAME_SIZE);
prefix[RBD_MAX_BLOCK_NAME_SIZE] = '\0';
*image_id = std::string(prefix + strlen(RBD_DATA_PREFIX));
return 0;
}

View File

@ -9,6 +9,7 @@ bool get_features(uint64_t *features);
bool is_feature_enabled(uint64_t feature);
int create_image_pp(librbd::RBD &rbd, librados::IoCtx &ioctx,
const std::string &name, uint64_t size);
int get_image_id(librbd::Image &image, std::string *image_id);
#define REQUIRE_FEATURE(feature) { \
if (!is_feature_enabled(feature)) { \