From 08ea7d62ba6eedf614d72ff9d33f2e6a1c0b81fe Mon Sep 17 00:00:00 2001 From: Mykola Golub Date: Wed, 27 Jun 2018 17:18:24 +0300 Subject: [PATCH] librbd: validate data pool for self-managed snapshot support Fixes: https://tracker.ceph.com/issues/24675 Signed-off-by: Mykola Golub --- qa/workunits/rbd/verify_pool.sh | 4 +- src/librbd/image/CreateRequest.cc | 122 ++++++++---------------------- src/librbd/image/CreateRequest.h | 12 +-- 3 files changed, 37 insertions(+), 101 deletions(-) diff --git a/qa/workunits/rbd/verify_pool.sh b/qa/workunits/rbd/verify_pool.sh index 569b65a6d34b5..08bcca5065627 100755 --- a/qa/workunits/rbd/verify_pool.sh +++ b/qa/workunits/rbd/verify_pool.sh @@ -20,8 +20,8 @@ set_up # creating an image in a pool-managed snapshot pool should fail rbd create --pool $POOL_NAME --size 1 foo && exit 1 || true -# should succeed if images already exist in the pool -rados --pool $POOL_NAME create rbd_directory +# should succeed if the pool already marked as validated +printf "overwrite validated" | rados --pool $POOL_NAME put rbd_info - rbd create --pool $POOL_NAME --size 1 foo echo OK diff --git a/src/librbd/image/CreateRequest.cc b/src/librbd/image/CreateRequest.cc index c4e17921b722f..15aa19df68fec 100644 --- a/src/librbd/image/CreateRequest.cc +++ b/src/librbd/image/CreateRequest.cc @@ -78,25 +78,6 @@ int validate_striping(CephContext *cct, uint8_t order, uint64_t stripe_unit, return 0; } -int validate_data_pool(CephContext *cct, IoCtx &io_ctx, uint64_t features, - const std::string &data_pool, int64_t *data_pool_id) { - if ((features & RBD_FEATURE_DATA_POOL) == 0) { - return 0; - } - - librados::Rados rados(io_ctx); - librados::IoCtx data_io_ctx; - int r = rados.ioctx_create(data_pool.c_str(), data_io_ctx); - if (r < 0) { - lderr(cct) << "data pool " << data_pool << " does not exist" << dendl; - return -ENOENT; - } - - *data_pool_id = data_io_ctx.get_id(); - return 0; -} - - bool validate_layout(CephContext *cct, uint64_t size, file_layout_t &layout) { if (!librbd::ObjectMap<>::is_compatible(layout, size)) { lderr(cct) << "image size not compatible with object map" << dendl; @@ -261,24 +242,30 @@ void CreateRequest::send() { return; } - r = validate_data_pool(m_cct, m_io_ctx, m_features, m_data_pool, - &m_data_pool_id); - if (r < 0) { - complete(r); - return; - } - if (((m_features & RBD_FEATURE_OBJECT_MAP) != 0) && (!validate_layout(m_cct, m_size, m_layout))) { complete(-EINVAL); return; } - validate_pool(); + validate_data_pool(); } -template -void CreateRequest::validate_pool() { +template +void CreateRequest::validate_data_pool() { + m_data_io_ctx = m_io_ctx; + if ((m_features & RBD_FEATURE_DATA_POOL) != 0) { + librados::Rados rados(m_io_ctx); + int r = rados.ioctx_create(m_data_pool.c_str(), m_data_io_ctx); + if (r < 0) { + lderr(m_cct) << "data pool " << m_data_pool << " does not exist" << dendl; + complete(r); + return; + } + m_data_pool_id = m_data_io_ctx.get_id(); + m_data_io_ctx.set_namespace(m_io_ctx.get_namespace()); + } + if (!m_cct->_conf.get_val("rbd_validate_pool")) { add_image_to_directory(); return; @@ -288,27 +275,29 @@ void CreateRequest::validate_pool() { using klass = CreateRequest; librados::AioCompletion *comp = - create_rados_callback(this); + create_rados_callback(this); librados::ObjectReadOperation op; - op.stat(NULL, NULL, NULL); + op.read(0, 0, nullptr, nullptr); m_outbl.clear(); - int r = m_io_ctx.aio_operate(RBD_DIRECTORY, comp, &op, &m_outbl); + int r = m_data_io_ctx.aio_operate(RBD_INFO, comp, &op, &m_outbl); assert(r == 0); comp->release(); } -template -void CreateRequest::handle_validate_pool(int r) { +template +void CreateRequest::handle_validate_data_pool(int r) { ldout(m_cct, 20) << "r=" << r << dendl; - if (r == 0) { - validate_overwrite(); + bufferlist bl; + bl.append("overwrite validated"); + + if (r >= 0 && m_outbl.contents_equal(bl)) { + add_image_to_directory(); return; } else if ((r < 0) && (r != -ENOENT)) { - lderr(m_cct) << "failed to stat RBD directory: " << cpp_strerror(r) - << dendl; + lderr(m_cct) << "failed to read RBD info: " << cpp_strerror(r) << dendl; complete(r); return; } @@ -319,8 +308,10 @@ void CreateRequest::handle_validate_pool(int r) { // try hard to make it asynchronous (and it's pretty safe not to cause // deadlocks). + ldout(m_cct, 10) << "validating self-managed RBD snapshot support" << dendl; + uint64_t snap_id; - r = m_io_ctx.selfmanaged_snap_create(&snap_id); + r = m_data_io_ctx.selfmanaged_snap_create(&snap_id); if (r == -EINVAL) { lderr(m_cct) << "pool not configured for self-managed RBD snapshot support" << dendl; @@ -333,7 +324,7 @@ void CreateRequest::handle_validate_pool(int r) { return; } - r = m_io_ctx.selfmanaged_snap_remove(snap_id); + r = m_data_io_ctx.selfmanaged_snap_remove(snap_id); if (r < 0) { // we've already switched to self-managed snapshots -- no need to // error out in case of failure here. @@ -341,57 +332,8 @@ void CreateRequest::handle_validate_pool(int r) { << ": " << cpp_strerror(r) << dendl; } - validate_overwrite(); -} - -template -void CreateRequest::validate_overwrite() { - ldout(m_cct, 20) << dendl; - - m_data_io_ctx = m_io_ctx; - if (m_data_pool_id != -1) { - librados::Rados rados(m_io_ctx); - int r = rados.ioctx_create2(m_data_pool_id, m_data_io_ctx); - if (r < 0) { - lderr(m_cct) << "data pool " << m_data_pool << " does not exist" << dendl; - complete(r); - return; - } - m_data_io_ctx.set_namespace(m_io_ctx.get_namespace()); - } - - using klass = CreateRequest; - librados::AioCompletion *comp = - create_rados_callback(this); - - librados::ObjectReadOperation op; - op.read(0, 0, nullptr, nullptr); - - m_outbl.clear(); - int r = m_data_io_ctx.aio_operate(RBD_INFO, comp, &op, &m_outbl); - assert(r == 0); - comp->release(); -} - -template -void CreateRequest::handle_validate_overwrite(int r) { - ldout(m_cct, 20) << "r=" << r << dendl; - - bufferlist bl; - bl.append("overwrite validated"); - - if (r == 0 && m_outbl.contents_equal(bl)) { - add_image_to_directory(); - return; - } else if ((r < 0) && (r != -ENOENT)) { - lderr(m_cct) << "failed to read RBD info: " << cpp_strerror(r) << dendl; - complete(r); - return; - } - - // validate the pool supports overwrites. We cannot use rbd_directory - // since the v1 images store the directory as tmap data within the object. ldout(m_cct, 10) << "validating overwrite support" << dendl; + bufferlist initial_bl; initial_bl.append("validate"); r = m_data_io_ctx.write(RBD_INFO, initial_bl, initial_bl.length(), 0); diff --git a/src/librbd/image/CreateRequest.h b/src/librbd/image/CreateRequest.h index 00a0818d40652..24aa0eb3f1b3c 100644 --- a/src/librbd/image/CreateRequest.h +++ b/src/librbd/image/CreateRequest.h @@ -54,12 +54,9 @@ private: * . . . . > . . . . . * | . * v . - * VALIDATE POOL v (pool validation + * VALIDATE DATA POOL v (pool validation * | . disabled) * v . - * VALIDATE OVERWRITE . - * | . - * v . * (error: bottom up) ADD IMAGE TO DIRECTORY < . . . . * _______<_______ | * | | v @@ -133,11 +130,8 @@ private: rbd_mirror_mode_t m_mirror_mode = RBD_MIRROR_MODE_DISABLED; cls::rbd::MirrorImage m_mirror_image_internal; - void validate_pool(); - void handle_validate_pool(int r); - - void validate_overwrite(); - void handle_validate_overwrite(int r); + void validate_data_pool(); + void handle_validate_data_pool(int r); void add_image_to_directory(); void handle_add_image_to_directory(int r); -- 2.39.5