From b579df9016743d2efe016ba69ff3e83f994eb687 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 (cherry picked from commit 08ea7d62ba6eedf614d72ff9d33f2e6a1c0b81fe) Conflicts: src/librbd/image/CreateRequest.cc src/librbd/image/CreateRequest.h - luminous uses m_ioctx where master has m_io_ctx - luminous IoCtx does not have get_namespace/set_namespace - in luminuos CreateRequest state machine is a little different than in the master (see the diagram in CreateRequest.h). In luminous the next state after VALIDATE_DATA_POOL is CREATE_ID_OBJECT, so call create_id_object() instead of add_image_to_directory(). --- qa/workunits/rbd/verify_pool.sh | 4 +- src/librbd/image/CreateRequest.cc | 120 ++++++++---------------------- src/librbd/image/CreateRequest.h | 12 +-- 3 files changed, 36 insertions(+), 100 deletions(-) diff --git a/qa/workunits/rbd/verify_pool.sh b/qa/workunits/rbd/verify_pool.sh index f008fb6b3cf..65a61199bba 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 75af7061efc..b4e25e08425 100644 --- a/src/librbd/image/CreateRequest.cc +++ b/src/librbd/image/CreateRequest.cc @@ -73,25 +73,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; @@ -250,24 +231,29 @@ void CreateRequest::send() { return; } - r = validate_data_pool(m_cct, m_ioctx, 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_ioctx; + if ((m_features & RBD_FEATURE_DATA_POOL) != 0) { + librados::Rados rados(m_ioctx); + 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(); + } + if (!m_cct->_conf->get_val("rbd_validate_pool")) { create_id_object(); return; @@ -277,27 +263,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_ioctx.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)) { + create_id_object(); 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; } @@ -308,8 +296,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_ioctx.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; @@ -322,7 +312,7 @@ void CreateRequest::handle_validate_pool(int r) { return; } - r = m_ioctx.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. @@ -330,56 +320,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_ioctx; - if (m_data_pool_id != -1) { - librados::Rados rados(m_ioctx); - 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; - } - } - - 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)) { - create_id_object(); - 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 9d77838dd31..42f50b8ff1a 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) CREATE ID OBJECT. . < . . . . . * _______<_______ | * | | v @@ -133,11 +130,8 @@ private: rbd_mirror_mode_t m_mirror_mode; 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 create_id_object(); void handle_create_id_object(int r); -- 2.47.3