Skip to content

Commit

Permalink
librbd: default features should be negotiated with the OSD
Browse files Browse the repository at this point in the history
Fixes: http://tracker.ceph.com/issues/17010
Signed-off-by: Mykola Golub <mgolub@mirantis.com>
  • Loading branch information
Mykola Golub committed Nov 6, 2016
1 parent 0822a7e commit 445dafe
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 12 deletions.
20 changes: 18 additions & 2 deletions src/librbd/Utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
#include "include/rbd_types.h"
#include "include/stringify.h"
#include "include/rbd/features.h"
#include "cls/rbd/cls_rbd_client.h"
#include "common/dout.h"
#include "common/errno.h"

#define dout_subsys ceph_subsys_rbd
#undef dout_prefix
Expand Down Expand Up @@ -64,7 +66,7 @@ std::string generate_image_id(librados::IoCtx &ioctx) {
return id;
}

uint64_t parse_rbd_default_features(CephContext* cct)
uint64_t parse_rbd_default_features(CephContext *cct)
{
int ret = 0;
uint64_t value = 0;
Expand All @@ -89,7 +91,7 @@ uint64_t parse_rbd_default_features(CephContext* cct)
value += conf_vals[feature];
} else {
ret = -EINVAL;
ldout(cct, 1) << "Warning: unknown rbd feature " << feature << dendl;
lderr(cct) << "unknown rbd feature " << feature << dendl;
}
}
if (value == 0 && ret == -EINVAL)
Expand All @@ -98,6 +100,20 @@ uint64_t parse_rbd_default_features(CephContext* cct)
return value;
}

uint64_t get_rbd_default_features(librados::IoCtx &io_ctx)
{
CephContext *cct = (CephContext *)io_ctx.cct();
uint64_t features = parse_rbd_default_features(cct);
uint64_t all_features = RBD_FEATURES_ALL;
int r = cls_client::get_all_features(&io_ctx, RBD_DIRECTORY, &all_features);
if (r < 0) {
ldout(cct, 1) << "failed retrieving supported features list: "
<< cpp_strerror(r) << dendl;
}
features &= all_features;
return features;
}

} // namespace util

} // namespace librbd
3 changes: 2 additions & 1 deletion src/librbd/Utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ class AsyncOpTracker {
Context *m_on_finish = nullptr;
};

uint64_t parse_rbd_default_features(CephContext* cct);
uint64_t parse_rbd_default_features(CephContext *cct);
uint64_t get_rbd_default_features(librados::IoCtx &io_ctx);

} // namespace util

Expand Down
43 changes: 43 additions & 0 deletions src/librbd/image/CreateRequest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ CreateRequest<I>::CreateRequest(IoCtx &ioctx, const std::string &image_name,

if (image_options.get(RBD_IMAGE_OPTION_FEATURES, &m_features) != 0) {
m_features = util::parse_rbd_default_features(m_cct);
m_negotiate_features = true;
}

uint64_t features_clear = 0;
Expand Down Expand Up @@ -381,6 +382,48 @@ Context *CreateRequest<I>::handle_add_image_to_directory(int *result) {
return nullptr;
}

negotiate_features();
return nullptr;
}

template<typename I>
void CreateRequest<I>::negotiate_features() {
if (!m_negotiate_features) {
create_image();
return;
}

ldout(m_cct, 20) << this << " " << __func__ << dendl;

librados::ObjectReadOperation op;
cls_client::get_all_features_start(&op);

using klass = CreateRequest<I>;
librados::AioCompletion *comp =
create_rados_ack_callback<klass, &klass::handle_negotiate_features>(this);
int r = m_ioctx.aio_operate(RBD_DIRECTORY, comp, &op, &m_outbl);
assert(r == 0);
comp->release();
}

template<typename I>
Context *CreateRequest<I>::handle_negotiate_features(int *result) {
ldout(m_cct, 20) << __func__ << ": r=" << *result << dendl;

uint64_t all_features;
if (*result == 0) {
bufferlist::iterator it = m_outbl.begin();
*result = cls_client::get_all_features_finish(&it, &all_features);
}
if (*result < 0) {
ldout(m_cct, 10) << "error retrieving server supported features set: "
<< cpp_strerror(*result) << dendl;
} else if (all_features < m_features) {
m_features &= all_features;
ldout(m_cct, 10) << "limiting default features set to server supported: "
<< m_features << dendl;
}

create_image();
return nullptr;
}
Expand Down
9 changes: 8 additions & 1 deletion src/librbd/image/CreateRequest.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ class CreateRequest {
* | | v
* | | ADD IMAGE TO DIRECTORY
* | | / |
* | REMOVE ID OBJECT<-------/ v (stripingv2 disabled)
* | REMOVE ID OBJECT<-------/ v
* | | NEGOTIATE FEATURES (when using default features)
* | | |
* | | v (stripingv2 disabled)
* | | CREATE IMAGE. . . . > . . . .
* v | / | .
* | REMOVE FROM DIR<--------/ v .
Expand Down Expand Up @@ -112,6 +115,7 @@ class CreateRequest {
int64_t m_data_pool_id = -1;
const std::string m_non_primary_global_image_id;
const std::string m_primary_mirror_uuid;
bool m_negotiate_features = false;

ContextWQ *m_op_work_queue;
Context *m_on_finish;
Expand All @@ -135,6 +139,9 @@ class CreateRequest {
void add_image_to_directory();
Context *handle_add_image_to_directory(int *result);

void negotiate_features();
Context *handle_negotiate_features(int *result);

void create_image();
Context *handle_create_image(int *result);

Expand Down
2 changes: 1 addition & 1 deletion src/librbd/internal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
{
CephContext *cct = (CephContext *)io_ctx.cct();
bool old_format = cct->_conf->rbd_default_format == 1;
uint64_t features = old_format ? 0 : librbd::util::parse_rbd_default_features(cct);
uint64_t features = old_format ? 0 : librbd::util::get_rbd_default_features(io_ctx);
return create(io_ctx, imgname, size, old_format, features, order, 0, 0);
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/rbd_mirror/test_ImageReplayer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ class TestImageReplayer : public ::testing::Test {
m_remote_ioctx));

m_image_name = get_temp_image_name();
uint64_t features = librbd::util::parse_rbd_default_features(g_ceph_context);
uint64_t features = librbd::util::get_rbd_default_features(m_remote_ioctx);
features |= RBD_FEATURE_EXCLUSIVE_LOCK | RBD_FEATURE_JOURNALING;
int order = 0;
EXPECT_EQ(0, librbd::create(m_remote_ioctx, m_image_name.c_str(), 1 << 22,
Expand Down
11 changes: 5 additions & 6 deletions src/test/rbd_mirror/test_PoolWatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,14 @@ TestPoolWatcher() : m_lock("TestPoolWatcherLock"),

void create_image(const string &pool_name, bool mirrored=true,
string *image_name=nullptr) {
uint64_t features = librbd::util::parse_rbd_default_features(g_ceph_context);
librados::IoCtx ioctx;
ASSERT_EQ(0, m_cluster->ioctx_create(pool_name.c_str(), ioctx));
int order = 0;
uint64_t features = librbd::util::get_rbd_default_features(ioctx);
string name = "image" + stringify(++m_image_number);
if (mirrored) {
features |= RBD_FEATURE_EXCLUSIVE_LOCK | RBD_FEATURE_JOURNALING;
}

librados::IoCtx ioctx;
ASSERT_EQ(0, m_cluster->ioctx_create(pool_name.c_str(), ioctx));
int order = 0;
ASSERT_EQ(0, librbd::create(ioctx, name.c_str(), 1 << 22, false,
features, &order, 0, 0));
if (mirrored) {
Expand Down Expand Up @@ -134,7 +133,7 @@ TestPoolWatcher() : m_lock("TestPoolWatcherLock"),
ictx->state->close();
}

uint64_t features = librbd::util::parse_rbd_default_features(g_ceph_context);
uint64_t features = librbd::util::get_rbd_default_features(cioctx);
string name = "clone" + stringify(++m_image_number);
if (mirrored) {
features |= RBD_FEATURE_EXCLUSIVE_LOCK | RBD_FEATURE_JOURNALING;
Expand Down

0 comments on commit 445dafe

Please sign in to comment.