diff --git a/src/librbd/AioObjectRequest.cc b/src/librbd/AioObjectRequest.cc index cf7617640b2c2..36ec0e7334cac 100644 --- a/src/librbd/AioObjectRequest.cc +++ b/src/librbd/AioObjectRequest.cc @@ -454,12 +454,10 @@ void AbstractAioObjectWrite::send() { void AbstractAioObjectWrite::send_pre() { assert(m_ictx->owner_lock.is_locked()); - bool write = false; { RWLock::RLocker snap_lock(m_ictx->snap_lock); if (m_ictx->object_map == nullptr) { m_object_exist = true; - write = true; } else { // should have been flushed prior to releasing lock assert(m_ictx->exclusive_lock->is_lock_owner()); @@ -469,27 +467,20 @@ void AbstractAioObjectWrite::send_pre() { pre_object_map_update(&new_state); RWLock::WLocker object_map_locker(m_ictx->object_map_lock); - if (m_ictx->object_map->update_required(m_object_no, new_state)) { - ldout(m_ictx->cct, 20) << "send_pre " << this << " " << m_oid << " " - << m_object_off << "~" << m_object_len - << dendl; - m_state = LIBRBD_AIO_WRITE_PRE; - - Context *ctx = util::create_context_callback(this); - bool updated = m_ictx->object_map->aio_update(m_object_no, new_state, - {}, ctx); - assert(updated); - } else { - write = true; + ldout(m_ictx->cct, 20) << "send_pre " << this << " " << m_oid << " " + << m_object_off << "~" << m_object_len + << dendl; + m_state = LIBRBD_AIO_WRITE_PRE; + + if (m_ictx->object_map->aio_update( + CEPH_NOSNAP, m_object_no, new_state, {}, this)) { + return; } } } - // avoid possible recursive lock attempts - if (write) { - // no object map update required - send_write(); - } + // no object map update required + send_write(); } bool AbstractAioObjectWrite::send_post() { @@ -503,20 +494,16 @@ bool AbstractAioObjectWrite::send_post() { assert(m_ictx->exclusive_lock->is_lock_owner()); RWLock::WLocker object_map_locker(m_ictx->object_map_lock); - if (!m_ictx->object_map->update_required(m_object_no, OBJECT_NONEXISTENT)) { - return true; - } - ldout(m_ictx->cct, 20) << "send_post " << this << " " << m_oid << " " << m_object_off << "~" << m_object_len << dendl; m_state = LIBRBD_AIO_WRITE_POST; - Context *ctx = util::create_context_callback(this); - bool updated = m_ictx->object_map->aio_update(m_object_no, - OBJECT_NONEXISTENT, - OBJECT_PENDING, ctx); - assert(updated); - return false; + if (m_ictx->object_map->aio_update( + CEPH_NOSNAP, m_object_no, OBJECT_NONEXISTENT, OBJECT_PENDING, this)) { + return false; + } + + return true; } void AbstractAioObjectWrite::send_write() { diff --git a/src/librbd/BlockGuard.h b/src/librbd/BlockGuard.h new file mode 100644 index 0000000000000..36eb8be2eb18f --- /dev/null +++ b/src/librbd/BlockGuard.h @@ -0,0 +1,172 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#ifndef CEPH_LIBRBD_IO_BLOCK_GUARD_H +#define CEPH_LIBRBD_IO_BLOCK_GUARD_H + +#include "include/int_types.h" +#include "common/dout.h" +#include "common/Mutex.h" +#include +#include +#include +#include +#include "include/assert.h" + +#define dout_subsys ceph_subsys_rbd +#undef dout_prefix +#define dout_prefix *_dout << "librbd::BlockGuard: " << this << " " \ + << __func__ << ": " + +namespace librbd { + +struct BlockExtent { + uint64_t block_start = 0; + uint64_t block_end = 0; + + BlockExtent() { + } + BlockExtent(uint64_t block_start, uint64_t block_end) + : block_start(block_start), block_end(block_end) { + } +}; + +struct BlockGuardCell { +}; + +/** + * Helper class to restrict and order concurrent IO to the same block. The + * definition of a block is dependent upon the user of this class. It might + * represent a backing object, 512 byte sectors, etc. + */ +template +class BlockGuard { +private: + struct DetainedBlockExtent; + +public: + typedef std::list BlockOperations; + + BlockGuard(CephContext *cct) + : m_cct(cct), m_lock("librbd::BlockGuard::m_lock") { + } + + BlockGuard(const BlockGuard&) = delete; + BlockGuard &operator=(const BlockGuard&) = delete; + + /** + * Detain future IO for a range of blocks. the guard will assume + * ownership of the provided operation if the operation is blocked. + * @return 0 upon success and IO can be issued + * >0 if the IO is blocked, + * <0 upon error + */ + int detain(const BlockExtent &block_extent, BlockOperation *block_operation, + BlockGuardCell **cell) { + Mutex::Locker locker(m_lock); + ldout(m_cct, 20) << "block_start=" << block_extent.block_start << ", " + << "block_end=" << block_extent.block_end << ", " + << "free_slots=" << m_free_detained_block_extents.size() + << dendl; + + DetainedBlockExtent *detained_block_extent; + auto it = m_detained_block_extents.find(block_extent); + if (it != m_detained_block_extents.end()) { + // request against an already detained block + detained_block_extent = &(*it); + if (block_operation != nullptr) { + detained_block_extent->block_operations.emplace_back( + std::move(*block_operation)); + } + + // alert the caller that the IO was detained + *cell = nullptr; + return detained_block_extent->block_operations.size(); + } else { + if (!m_free_detained_block_extents.empty()) { + detained_block_extent = &m_free_detained_block_extents.front(); + detained_block_extent->block_operations.clear(); + m_free_detained_block_extents.pop_front(); + } else { + ldout(m_cct, 20) << "no free detained block cells" << dendl; + m_detained_block_extent_pool.emplace_back(); + detained_block_extent = &m_detained_block_extent_pool.back(); + } + + detained_block_extent->block_extent = block_extent; + m_detained_block_extents.insert(*detained_block_extent); + *cell = reinterpret_cast(detained_block_extent); + return 0; + } + } + + /** + * Release any detained IO operations from the provided cell. + */ + void release(BlockGuardCell *cell, BlockOperations *block_operations) { + Mutex::Locker locker(m_lock); + + assert(cell != nullptr); + auto &detained_block_extent = reinterpret_cast( + *cell); + ldout(m_cct, 20) << "block_start=" + << detained_block_extent.block_extent.block_start << ", " + << "block_end=" + << detained_block_extent.block_extent.block_end << ", " + << "pending_ops=" + << (detained_block_extent.block_operations.empty() ? + 0 : detained_block_extent.block_operations.size() - 1) + << dendl; + + *block_operations = std::move(detained_block_extent.block_operations); + m_detained_block_extents.erase(detained_block_extent.block_extent); + m_free_detained_block_extents.push_back(detained_block_extent); + } + +private: + struct DetainedBlockExtent : public boost::intrusive::list_base_hook<>, + public boost::intrusive::set_base_hook<> { + DetainedBlockExtent() { + } + DetainedBlockExtent(const BlockExtent &block_extent) + : block_extent(block_extent) { + } + + BlockExtent block_extent; + BlockOperations block_operations; + }; + + struct DetainedBlockExtentCompare { + bool operator()(const DetainedBlockExtent &lhs, + const DetainedBlockExtent &rhs) const { + // check for range overlap (lhs < rhs) + if (lhs.block_extent.block_end <= rhs.block_extent.block_start) { + return true; + } + return false; + } + }; + + typedef std::deque DetainedBlockExtentsPool; + typedef boost::intrusive::list DetainedBlockExtents; + typedef boost::intrusive::set< + DetainedBlockExtent, + boost::intrusive::compare > + BlockExtentToDetainedBlockExtents; + + CephContext *m_cct; + + Mutex m_lock; + DetainedBlockExtentsPool m_detained_block_extent_pool; + DetainedBlockExtents m_free_detained_block_extents; + BlockExtentToDetainedBlockExtents m_detained_block_extents; + +}; + +} // namespace librbd + +#undef dout_subsys +#undef dout_prefix +#define dout_prefix *_dout + +#endif // CEPH_LIBRBD_IO_BLOCK_GUARD_H diff --git a/src/librbd/CopyupRequest.cc b/src/librbd/CopyupRequest.cc index b95544b28494a..d9750b6e52438 100644 --- a/src/librbd/CopyupRequest.cc +++ b/src/librbd/CopyupRequest.cc @@ -46,9 +46,8 @@ class UpdateObjectMap : public C_AsyncObjectThrottle<> { RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock); assert(m_image_ctx.exclusive_lock->is_lock_owner()); assert(m_image_ctx.object_map != nullptr); - bool sent = m_image_ctx.object_map->aio_update(m_object_no, OBJECT_EXISTS, - boost::optional(), - this); + bool sent = m_image_ctx.object_map->aio_update( + CEPH_NOSNAP, m_object_no, OBJECT_EXISTS, {}, this); return (sent ? 0 : 1); } @@ -64,8 +63,9 @@ class UpdateObjectMap : public C_AsyncObjectThrottle<> { return 1; } - m_image_ctx.object_map->aio_update(snap_id, m_object_no, m_object_no + 1, - state, boost::optional(), this); + bool sent = m_image_ctx.object_map->aio_update( + snap_id, m_object_no, state, {}, this); + assert(sent); return 0; } diff --git a/src/librbd/DiffIterate.cc b/src/librbd/DiffIterate.cc index c0165c72774a7..cd61be86528ed 100644 --- a/src/librbd/DiffIterate.cc +++ b/src/librbd/DiffIterate.cc @@ -385,8 +385,8 @@ int DiffIterate::diff_object_map(uint64_t from_snap_id, uint64_t to_snap_id, } BitVector<2> object_map; - std::string oid(ObjectMap::object_map_name(m_image_ctx.id, - current_snap_id)); + std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, + current_snap_id)); r = cls_client::object_map_load(&m_image_ctx.md_ctx, oid, &object_map); if (r < 0) { lderr(cct) << "diff_object_map: failed to load object map " << oid diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc index 4226d42747138..863a1235e4aa8 100644 --- a/src/librbd/ImageCtx.cc +++ b/src/librbd/ImageCtx.cc @@ -995,8 +995,8 @@ struct C_InvalidateCache : public Context { return new ExclusiveLock(*this); } - ObjectMap *ImageCtx::create_object_map(uint64_t snap_id) { - return new ObjectMap(*this, snap_id); + ObjectMap *ImageCtx::create_object_map(uint64_t snap_id) { + return new ObjectMap(*this, snap_id); } Journal *ImageCtx::create_journal() { diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h index b8a3bf6cf24a8..c1cec019d8088 100644 --- a/src/librbd/ImageCtx.h +++ b/src/librbd/ImageCtx.h @@ -45,7 +45,7 @@ namespace librbd { template class ImageWatcher; template class Journal; class LibrbdAdminSocketHook; - class ObjectMap; + template class ObjectMap; template class Operations; class LibrbdWriteback; @@ -141,7 +141,7 @@ namespace librbd { Operations *operations; ExclusiveLock *exclusive_lock; - ObjectMap *object_map; + ObjectMap *object_map; xlist*> resize_reqs; @@ -287,7 +287,7 @@ namespace librbd { void apply_metadata(const std::map &meta); ExclusiveLock *create_exclusive_lock(); - ObjectMap *create_object_map(uint64_t snap_id); + ObjectMap *create_object_map(uint64_t snap_id); Journal *create_journal(); void clear_pending_completions(); diff --git a/src/librbd/Makefile.am b/src/librbd/Makefile.am index d6bc3581e3f55..8cc8fe47dfc88 100644 --- a/src/librbd/Makefile.am +++ b/src/librbd/Makefile.am @@ -97,6 +97,7 @@ noinst_HEADERS += \ librbd/AsyncObjectThrottle.h \ librbd/AsyncOperation.h \ librbd/AsyncRequest.h \ + librbd/BlockGuard.h \ librbd/CopyupRequest.h \ librbd/DiffIterate.h \ librbd/ExclusiveLock.h \ diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc index b5d659ef8a2e8..c59366eceb0b4 100644 --- a/src/librbd/ObjectMap.cc +++ b/src/librbd/ObjectMap.cc @@ -1,6 +1,8 @@ // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- // vim: ts=8 sw=2 smarttab + #include "librbd/ObjectMap.h" +#include "librbd/BlockGuard.h" #include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/object_map/RefreshRequest.h" @@ -25,21 +27,30 @@ #define dout_subsys ceph_subsys_rbd #undef dout_prefix -#define dout_prefix *_dout << "librbd::ObjectMap: " +#define dout_prefix *_dout << "librbd::ObjectMap: " << this << " " << __func__ \ + << ": " namespace librbd { -ObjectMap::ObjectMap(ImageCtx &image_ctx, uint64_t snap_id) - : m_image_ctx(image_ctx), m_snap_id(snap_id) -{ +template +ObjectMap::ObjectMap(I &image_ctx, uint64_t snap_id) + : m_image_ctx(image_ctx), m_snap_id(snap_id), + m_update_guard(new UpdateGuard(m_image_ctx.cct)) { +} + +template +ObjectMap::~ObjectMap() { + delete m_update_guard; } -int ObjectMap::remove(librados::IoCtx &io_ctx, const std::string &image_id) { +template +int ObjectMap::remove(librados::IoCtx &io_ctx, const std::string &image_id) { return io_ctx.remove(object_map_name(image_id, CEPH_NOSNAP)); } -std::string ObjectMap::object_map_name(const std::string &image_id, - uint64_t snap_id) { +template +std::string ObjectMap::object_map_name(const std::string &image_id, + uint64_t snap_id) { std::string oid(RBD_OBJECT_MAP_PREFIX + image_id); if (snap_id != CEPH_NOSNAP) { std::stringstream snap_suffix; @@ -50,26 +61,30 @@ std::string ObjectMap::object_map_name(const std::string &image_id, return oid; } -bool ObjectMap::is_compatible(const file_layout_t& layout, uint64_t size) { +template +bool ObjectMap::is_compatible(const file_layout_t& layout, uint64_t size) { uint64_t object_count = Striper::get_num_objects(layout, size); return (object_count <= cls::rbd::MAX_OBJECT_MAP_OBJECT_COUNT); } -ceph::BitVector<2u>::Reference ObjectMap::operator[](uint64_t object_no) +template +ceph::BitVector<2u>::Reference ObjectMap::operator[](uint64_t object_no) { assert(m_image_ctx.object_map_lock.is_wlocked()); assert(object_no < m_object_map.size()); return m_object_map[object_no]; } -uint8_t ObjectMap::operator[](uint64_t object_no) const +template +uint8_t ObjectMap::operator[](uint64_t object_no) const { assert(m_image_ctx.object_map_lock.is_locked()); assert(object_no < m_object_map.size()); return m_object_map[object_no]; } -bool ObjectMap::object_may_exist(uint64_t object_no) const +template +bool ObjectMap::object_may_exist(uint64_t object_no) const { assert(m_image_ctx.snap_lock.is_locked()); @@ -84,13 +99,13 @@ bool ObjectMap::object_may_exist(uint64_t object_no) const RWLock::RLocker l(m_image_ctx.object_map_lock); uint8_t state = (*this)[object_no]; bool exists = (state != OBJECT_NONEXISTENT); - ldout(m_image_ctx.cct, 20) << &m_image_ctx << " object_may_exist: " - << "object_no=" << object_no << " r=" << exists + ldout(m_image_ctx.cct, 20) << "object_no=" << object_no << " r=" << exists << dendl; return exists; } -bool ObjectMap::update_required(uint64_t object_no, uint8_t new_state) { +template +bool ObjectMap::update_required(uint64_t object_no, uint8_t new_state) { assert(m_image_ctx.object_map_lock.is_wlocked()); uint8_t state = (*this)[object_no]; @@ -102,24 +117,26 @@ bool ObjectMap::update_required(uint64_t object_no, uint8_t new_state) { return true; } -void ObjectMap::open(Context *on_finish) { - object_map::RefreshRequest<> *req = new object_map::RefreshRequest<>( +template +void ObjectMap::open(Context *on_finish) { + auto req = object_map::RefreshRequest::create( m_image_ctx, &m_object_map, m_snap_id, on_finish); req->send(); } -void ObjectMap::close(Context *on_finish) { +template +void ObjectMap::close(Context *on_finish) { if (m_snap_id != CEPH_NOSNAP) { m_image_ctx.op_work_queue->queue(on_finish, 0); return; } - object_map::UnlockRequest<> *req = new object_map::UnlockRequest<>( - m_image_ctx, on_finish); + auto req = object_map::UnlockRequest::create(m_image_ctx, on_finish); req->send(); } -void ObjectMap::rollback(uint64_t snap_id, Context *on_finish) { +template +void ObjectMap::rollback(uint64_t snap_id, Context *on_finish) { assert(m_image_ctx.snap_lock.is_locked()); assert(m_image_ctx.object_map_lock.is_wlocked()); @@ -128,7 +145,8 @@ void ObjectMap::rollback(uint64_t snap_id, Context *on_finish) { req->send(); } -void ObjectMap::snapshot_add(uint64_t snap_id, Context *on_finish) { +template +void ObjectMap::snapshot_add(uint64_t snap_id, Context *on_finish) { assert(m_image_ctx.snap_lock.is_locked()); assert((m_image_ctx.features & RBD_FEATURE_OBJECT_MAP) != 0); assert(snap_id != CEPH_NOSNAP); @@ -139,7 +157,8 @@ void ObjectMap::snapshot_add(uint64_t snap_id, Context *on_finish) { req->send(); } -void ObjectMap::snapshot_remove(uint64_t snap_id, Context *on_finish) { +template +void ObjectMap::snapshot_remove(uint64_t snap_id, Context *on_finish) { assert(m_image_ctx.snap_lock.is_wlocked()); assert((m_image_ctx.features & RBD_FEATURE_OBJECT_MAP) != 0); assert(snap_id != CEPH_NOSNAP); @@ -150,7 +169,8 @@ void ObjectMap::snapshot_remove(uint64_t snap_id, Context *on_finish) { req->send(); } -void ObjectMap::aio_save(Context *on_finish) { +template +void ObjectMap::aio_save(Context *on_finish) { assert(m_image_ctx.owner_lock.is_locked()); assert(m_image_ctx.snap_lock.is_locked()); assert(m_image_ctx.test_features(RBD_FEATURE_OBJECT_MAP, @@ -171,8 +191,9 @@ void ObjectMap::aio_save(Context *on_finish) { comp->release(); } -void ObjectMap::aio_resize(uint64_t new_size, uint8_t default_object_state, - Context *on_finish) { +template +void ObjectMap::aio_resize(uint64_t new_size, uint8_t default_object_state, + Context *on_finish) { assert(m_image_ctx.owner_lock.is_locked()); assert(m_image_ctx.snap_lock.is_locked()); assert(m_image_ctx.test_features(RBD_FEATURE_OBJECT_MAP, @@ -187,61 +208,110 @@ void ObjectMap::aio_resize(uint64_t new_size, uint8_t default_object_state, req->send(); } -bool ObjectMap::aio_update(uint64_t object_no, uint8_t new_state, - const boost::optional ¤t_state, - Context *on_finish) -{ - return aio_update(object_no, object_no + 1, new_state, current_state, - on_finish); +template +void ObjectMap::detained_aio_update(UpdateOperation &&op) { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 20) << dendl; + + assert(m_image_ctx.snap_lock.is_locked()); + assert(m_image_ctx.object_map_lock.is_wlocked()); + + BlockGuardCell *cell; + int r = m_update_guard->detain({op.start_object_no, op.end_object_no}, + &op, &cell); + if (r < 0) { + lderr(cct) << "failed to detain object map update: " << cpp_strerror(r) + << dendl; + m_image_ctx.op_work_queue->queue(op.on_finish, r); + return; + } else if (r > 0) { + ldout(cct, 20) << "detaining object map update due to in-flight update: " + << "start=" << op.start_object_no << ", " + << "end=" << op.end_object_no << ", " + << (op.current_state ? + stringify(static_cast(*op.current_state)) : + "") + << "->" << static_cast(op.new_state) << dendl; + return; + } + + ldout(cct, 20) << "in-flight update cell: " << cell << dendl; + Context *on_finish = op.on_finish; + Context *ctx = new FunctionContext([this, cell, on_finish](int r) { + handle_detained_aio_update(cell, r, on_finish); + }); + aio_update(CEPH_NOSNAP, op.start_object_no, op.end_object_no, op.new_state, + op.current_state, ctx); } -bool ObjectMap::aio_update(uint64_t start_object_no, uint64_t end_object_no, - uint8_t new_state, - const boost::optional ¤t_state, - Context *on_finish) -{ +template +void ObjectMap::handle_detained_aio_update(BlockGuardCell *cell, int r, + Context *on_finish) { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 20) << "cell=" << cell << ", r=" << r << dendl; + + typename UpdateGuard::BlockOperations block_ops; + m_update_guard->release(cell, &block_ops); + + { + RWLock::RLocker owner_locker(m_image_ctx.owner_lock); + RWLock::RLocker snap_locker(m_image_ctx.snap_lock); + RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock); + for (auto &op : block_ops) { + detained_aio_update(std::move(op)); + } + } + + on_finish->complete(r); +} + +template +void ObjectMap::aio_update(uint64_t snap_id, uint64_t start_object_no, + uint64_t end_object_no, uint8_t new_state, + const boost::optional ¤t_state, + Context *on_finish) { assert(m_image_ctx.snap_lock.is_locked()); assert((m_image_ctx.features & RBD_FEATURE_OBJECT_MAP) != 0); - assert(m_image_ctx.owner_lock.is_locked()); + assert(snap_id != CEPH_NOSNAP || m_image_ctx.owner_lock.is_locked()); assert(m_image_ctx.image_watcher != NULL); assert(m_image_ctx.exclusive_lock == nullptr || m_image_ctx.exclusive_lock->is_lock_owner()); - assert(m_image_ctx.object_map_lock.is_wlocked()); + assert(snap_id != CEPH_NOSNAP || m_image_ctx.object_map_lock.is_wlocked()); assert(start_object_no < end_object_no); CephContext *cct = m_image_ctx.cct; - ldout(cct, 20) << &m_image_ctx << " aio_update: start=" << start_object_no - << ", end=" << end_object_no << ", " + ldout(cct, 20) << "start=" << start_object_no << ", " + << "end=" << end_object_no << ", " << (current_state ? stringify(static_cast(*current_state)) : "") << "->" << static_cast(new_state) << dendl; - if (end_object_no > m_object_map.size()) { - ldout(cct, 20) << "skipping update of invalid object map" << dendl; - return false; - } + if (snap_id == CEPH_NOSNAP) { + if (end_object_no > m_object_map.size()) { + ldout(cct, 20) << "skipping update of invalid object map" << dendl; + m_image_ctx.op_work_queue->queue(on_finish, 0); + return; + } - for (uint64_t object_no = start_object_no; object_no < end_object_no; - ++object_no) { - uint8_t state = m_object_map[object_no]; - if ((!current_state || state == *current_state || - (*current_state == OBJECT_EXISTS && state == OBJECT_EXISTS_CLEAN)) && - state != new_state) { - aio_update(m_snap_id, start_object_no, end_object_no, new_state, - current_state, on_finish); - return true; + uint64_t object_no; + for (object_no = start_object_no; object_no < end_object_no; ++object_no) { + if (update_required(object_no, new_state)) { + break; + } + } + if (object_no == end_object_no) { + ldout(cct, 20) << "object map update not required" << dendl; + m_image_ctx.op_work_queue->queue(on_finish, 0); + return; } } - return false; -} -void ObjectMap::aio_update(uint64_t snap_id, uint64_t start_object_no, - uint64_t end_object_no, uint8_t new_state, - const boost::optional ¤t_state, - Context *on_finish) { - object_map::UpdateRequest *req = new object_map::UpdateRequest( + auto req = object_map::UpdateRequest::create( m_image_ctx, &m_object_map, snap_id, start_object_no, end_object_no, new_state, current_state, on_finish); req->send(); } } // namespace librbd + +template class librbd::ObjectMap; + diff --git a/src/librbd/ObjectMap.h b/src/librbd/ObjectMap.h index 5d99180e771a3..a3d5ea74e5396 100644 --- a/src/librbd/ObjectMap.h +++ b/src/librbd/ObjectMap.h @@ -1,5 +1,6 @@ // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- // vim: ts=8 sw=2 smarttab + #ifndef CEPH_LIBRBD_OBJECT_MAP_H #define CEPH_LIBRBD_OBJECT_MAP_H @@ -7,6 +8,7 @@ #include "include/fs_types.h" #include "include/rbd/object_map_types.h" #include "common/bit_vector.hpp" +#include "librbd/Utils.h" #include class Context; @@ -17,11 +19,19 @@ namespace librados { namespace librbd { +template class BlockGuard; +struct BlockGuardCell; class ImageCtx; +template class ObjectMap { public: - ObjectMap(ImageCtx &image_ctx, uint64_t snap_id); + static ObjectMap *create(ImageCtxT &image_ctx, uint64_t snap_id) { + return new ObjectMap(image_ctx, snap_id); + } + + ObjectMap(ImageCtxT &image_ctx, uint64_t snap_id); + ~ObjectMap(); static int remove(librados::IoCtx &io_ctx, const std::string &image_id); static std::string object_map_name(const std::string &image_id, @@ -39,35 +49,95 @@ class ObjectMap { void close(Context *on_finish); bool object_may_exist(uint64_t object_no) const; - bool update_required(uint64_t object_no, uint8_t new_state); void aio_save(Context *on_finish); void aio_resize(uint64_t new_size, uint8_t default_object_state, Context *on_finish); - bool aio_update(uint64_t object_no, uint8_t new_state, - const boost::optional ¤t_state, - Context *on_finish); - bool aio_update(uint64_t start_object_no, uint64_t end_object_no, - uint8_t new_state, - const boost::optional ¤t_state, - Context *on_finish); - void aio_update(uint64_t snap_id, uint64_t start_object_no, + template + bool aio_update(uint64_t snap_id, uint64_t start_object_no, uint8_t new_state, + const boost::optional ¤t_state, + T *callback_object) { + return aio_update(snap_id, start_object_no, start_object_no + 1, + new_state, current_state, callback_object); + } + + template + bool aio_update(uint64_t snap_id, uint64_t start_object_no, uint64_t end_object_no, uint8_t new_state, const boost::optional ¤t_state, - Context *on_finish); + T *callback_object) { + assert(start_object_no < end_object_no); + if (snap_id == CEPH_NOSNAP) { + uint64_t object_no; + for (object_no = start_object_no; object_no < end_object_no; + ++object_no) { + if (update_required(object_no, new_state)) { + break; + } + } + + if (object_no == end_object_no) { + return false; + } + + UpdateOperation update_operation(start_object_no, end_object_no, + new_state, current_state, + util::create_context_callback( + callback_object)); + detained_aio_update(std::move(update_operation)); + } else { + aio_update(snap_id, start_object_no, end_object_no, new_state, + current_state, + util::create_context_callback(callback_object)); + } + return true; + } void rollback(uint64_t snap_id, Context *on_finish); void snapshot_add(uint64_t snap_id, Context *on_finish); void snapshot_remove(uint64_t snap_id, Context *on_finish); private: - ImageCtx &m_image_ctx; + struct UpdateOperation { + uint64_t start_object_no; + uint64_t end_object_no; + uint8_t new_state; + boost::optional current_state; + Context *on_finish; + + UpdateOperation(uint64_t start_object_no, uint64_t end_object_no, + uint8_t new_state, + const boost::optional ¤t_state, + Context *on_finish) + : start_object_no(start_object_no), end_object_no(end_object_no), + new_state(new_state), current_state(current_state), + on_finish(on_finish) { + } + }; + + typedef BlockGuard UpdateGuard; + + ImageCtxT &m_image_ctx; ceph::BitVector<2> m_object_map; uint64_t m_snap_id; + UpdateGuard *m_update_guard = nullptr; + + void detained_aio_update(UpdateOperation &&update_operation); + void handle_detained_aio_update(BlockGuardCell *cell, int r, + Context *on_finish); + + void aio_update(uint64_t snap_id, uint64_t start_object_no, + uint64_t end_object_no, uint8_t new_state, + const boost::optional ¤t_state, + Context *on_finish); + bool update_required(uint64_t object_no, uint8_t new_state); + }; } // namespace librbd +extern template class librbd::ObjectMap; + #endif // CEPH_LIBRBD_OBJECT_MAP_H diff --git a/src/librbd/Operations.cc b/src/librbd/Operations.cc index 31a503f4d4f16..030ed6d70b2b6 100644 --- a/src/librbd/Operations.cc +++ b/src/librbd/Operations.cc @@ -544,7 +544,7 @@ int Operations::resize(uint64_t size, ProgressContext& prog_ctx) { } if (m_image_ctx.test_features(RBD_FEATURE_OBJECT_MAP) && - !ObjectMap::is_compatible(m_image_ctx.layout, size)) { + !ObjectMap<>::is_compatible(m_image_ctx.layout, size)) { lderr(cct) << "New size not compatible with object map" << dendl; return -EINVAL; } @@ -582,7 +582,7 @@ void Operations::execute_resize(uint64_t size, ProgressContext &prog_ctx, return; } else if (m_image_ctx.test_features(RBD_FEATURE_OBJECT_MAP, m_image_ctx.snap_lock) && - !ObjectMap::is_compatible(m_image_ctx.layout, size)) { + !ObjectMap<>::is_compatible(m_image_ctx.layout, size)) { m_image_ctx.snap_lock.put_read(); on_finish->complete(-EINVAL); return; diff --git a/src/librbd/image/SetSnapRequest.cc b/src/librbd/image/SetSnapRequest.cc index 50d485ce54e72..def86f99a5d8e 100644 --- a/src/librbd/image/SetSnapRequest.cc +++ b/src/librbd/image/SetSnapRequest.cc @@ -262,7 +262,7 @@ Context *SetSnapRequest::send_open_object_map(int *result) { using klass = SetSnapRequest; Context *ctx = create_context_callback< klass, &klass::handle_open_object_map>(this); - m_object_map = new ObjectMap(m_image_ctx, m_snap_id); + m_object_map = ObjectMap::create(m_image_ctx, m_snap_id); m_object_map->open(ctx); return nullptr; } diff --git a/src/librbd/image/SetSnapRequest.h b/src/librbd/image/SetSnapRequest.h index 1e2df49bcf14b..b64fa68afa0c1 100644 --- a/src/librbd/image/SetSnapRequest.h +++ b/src/librbd/image/SetSnapRequest.h @@ -12,7 +12,7 @@ namespace librbd { template class ExclusiveLock; class ImageCtx; -class ObjectMap; +template class ObjectMap; namespace image { @@ -84,7 +84,7 @@ class SetSnapRequest { uint64_t m_snap_id; ExclusiveLock *m_exclusive_lock; - ObjectMap *m_object_map; + ObjectMap *m_object_map; RefreshParentRequest *m_refresh_parent; bool m_writes_blocked; diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc index 68d195be5e2f7..f02c186b19c70 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -76,7 +76,7 @@ int remove_object_map(ImageCtx *ictx) { int r; for (std::map::iterator it = ictx->snap_info.begin(); it != ictx->snap_info.end(); ++it) { - std::string oid(ObjectMap::object_map_name(ictx->id, it->first)); + std::string oid(ObjectMap<>::object_map_name(ictx->id, it->first)); r = ictx->md_ctx.remove(oid); if (r < 0 && r != -ENOENT) { lderr(cct) << "failed to remove object map " << oid << ": " @@ -85,7 +85,7 @@ int remove_object_map(ImageCtx *ictx) { } } - r = ictx->md_ctx.remove(ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP)); + r = ictx->md_ctx.remove(ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP)); if (r < 0 && r != -ENOENT) { lderr(cct) << "failed to remove object map: " << cpp_strerror(r) << dendl; return r; @@ -107,7 +107,7 @@ int create_object_map(ImageCtx *ictx) { snap_ids.push_back(it->first); } - if (!ObjectMap::is_compatible(ictx->layout, max_size)) { + if (!ObjectMap<>::is_compatible(ictx->layout, max_size)) { lderr(cct) << "image size not compatible with object map" << dendl; return -EINVAL; } @@ -115,7 +115,7 @@ int create_object_map(ImageCtx *ictx) { for (std::vector::iterator it = snap_ids.begin(); it != snap_ids.end(); ++it) { librados::ObjectWriteOperation op; - std::string oid(ObjectMap::object_map_name(ictx->id, *it)); + std::string oid(ObjectMap<>::object_map_name(ictx->id, *it)); uint64_t snap_size = ictx->get_image_size(*it); cls_client::object_map_resize(&op, Striper::get_num_objects(ictx->layout, snap_size), OBJECT_NONEXISTENT); @@ -1139,7 +1139,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, layout.stripe_count = stripe_count; } - if (!ObjectMap::is_compatible(layout, size)) { + if (!ObjectMap<>::is_compatible(layout, size)) { lderr(cct) << "image size not compatible with object map" << dendl; goto err_remove_header; } @@ -1147,7 +1147,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, librados::ObjectWriteOperation op; cls_client::object_map_resize(&op, Striper::get_num_objects(layout, size), OBJECT_NONEXISTENT); - r = io_ctx.operate(ObjectMap::object_map_name(id, CEPH_NOSNAP), &op); + r = io_ctx.operate(ObjectMap<>::object_map_name(id, CEPH_NOSNAP), &op); if (r < 0) { lderr(cct) << "error creating initial object map: " << cpp_strerror(r) << dendl; @@ -1204,7 +1204,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, err_remove_object_map: if ((features & RBD_FEATURE_OBJECT_MAP) != 0) { - remove_r = ObjectMap::remove(io_ctx, id); + remove_r = ObjectMap<>::remove(io_ctx, id); if (remove_r < 0) { lderr(cct) << "error cleaning up object map after creation failed: " << cpp_strerror(remove_r) << dendl; @@ -2247,7 +2247,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, } ldout(cct, 10) << "removing object map..." << dendl; - r = ObjectMap::remove(io_ctx, id); + r = ObjectMap<>::remove(io_ctx, id); if (r < 0 && r != -ENOENT) { lderr(cct) << "error removing image object map" << dendl; return r; diff --git a/src/librbd/object_map/LockRequest.cc b/src/librbd/object_map/LockRequest.cc index 3af50735e431b..213799aabc5ca 100644 --- a/src/librbd/object_map/LockRequest.cc +++ b/src/librbd/object_map/LockRequest.cc @@ -32,7 +32,7 @@ void LockRequest::send() { template void LockRequest::send_lock() { CephContext *cct = m_image_ctx.cct; - std::string oid(ObjectMap::object_map_name(m_image_ctx.id, CEPH_NOSNAP)); + std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, CEPH_NOSNAP)); ldout(cct, 10) << this << " " << __func__ << ": oid=" << oid << dendl; librados::ObjectWriteOperation op; @@ -68,7 +68,7 @@ Context *LockRequest::handle_lock(int *ret_val) { template void LockRequest::send_get_lock_info() { CephContext *cct = m_image_ctx.cct; - std::string oid(ObjectMap::object_map_name(m_image_ctx.id, CEPH_NOSNAP)); + std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, CEPH_NOSNAP)); ldout(cct, 10) << this << " " << __func__ << ": oid=" << oid << dendl; librados::ObjectReadOperation op; @@ -113,7 +113,7 @@ Context *LockRequest::handle_get_lock_info(int *ret_val) { template void LockRequest::send_break_locks() { CephContext *cct = m_image_ctx.cct; - std::string oid(ObjectMap::object_map_name(m_image_ctx.id, CEPH_NOSNAP)); + std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, CEPH_NOSNAP)); ldout(cct, 10) << this << " " << __func__ << ": oid=" << oid << ", " << "num_lockers=" << m_lockers.size() << dendl; diff --git a/src/librbd/object_map/RefreshRequest.cc b/src/librbd/object_map/RefreshRequest.cc index 9421c1216c4b1..c7b4834d2f1a9 100644 --- a/src/librbd/object_map/RefreshRequest.cc +++ b/src/librbd/object_map/RefreshRequest.cc @@ -72,7 +72,7 @@ void RefreshRequest::send_lock() { return; } - std::string oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id)); + std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id)); ldout(cct, 10) << this << " " << __func__ << ": oid=" << oid << dendl; using klass = RefreshRequest; @@ -96,7 +96,7 @@ Context *RefreshRequest::handle_lock(int *ret_val) { template void RefreshRequest::send_load() { CephContext *cct = m_image_ctx.cct; - std::string oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id)); + std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id)); ldout(cct, 10) << this << " " << __func__ << ": oid=" << oid << dendl; librados::ObjectReadOperation op; @@ -122,7 +122,7 @@ Context *RefreshRequest::handle_load(int *ret_val) { &m_on_disk_object_map); } - std::string oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id)); + std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id)); if (*ret_val == -EINVAL) { // object map is corrupt on-disk -- clear it and properly size it // so future IO can keep the object map in sync @@ -220,7 +220,7 @@ Context *RefreshRequest::handle_resize_invalidate(int *ret_val) { template void RefreshRequest::send_resize() { CephContext *cct = m_image_ctx.cct; - std::string oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id)); + std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id)); ldout(cct, 10) << this << " " << __func__ << ": oid=" << oid << dendl; librados::ObjectWriteOperation op; diff --git a/src/librbd/object_map/RefreshRequest.h b/src/librbd/object_map/RefreshRequest.h index 4c2b059fa268f..24a7998ae1bc7 100644 --- a/src/librbd/object_map/RefreshRequest.h +++ b/src/librbd/object_map/RefreshRequest.h @@ -19,6 +19,12 @@ namespace object_map { template class RefreshRequest { public: + static RefreshRequest *create(ImageCtxT &image_ctx, + ceph::BitVector<2> *object_map, + uint64_t snap_id, Context *on_finish) { + return new RefreshRequest(image_ctx, object_map, snap_id, on_finish); + } + RefreshRequest(ImageCtxT &image_ctx, ceph::BitVector<2> *object_map, uint64_t snap_id, Context *on_finish); diff --git a/src/librbd/object_map/Request.cc b/src/librbd/object_map/Request.cc index 1725cbf907541..e54aed0b21e0a 100644 --- a/src/librbd/object_map/Request.cc +++ b/src/librbd/object_map/Request.cc @@ -28,10 +28,7 @@ bool Request::should_complete(int r) { return invalidate(); } - { - RWLock::WLocker l2(m_image_ctx.object_map_lock); - finish_request(); - } + finish_request(); return true; case STATE_INVALIDATE: diff --git a/src/librbd/object_map/ResizeRequest.cc b/src/librbd/object_map/ResizeRequest.cc index e574a18472966..3182c8aef91ca 100644 --- a/src/librbd/object_map/ResizeRequest.cc +++ b/src/librbd/object_map/ResizeRequest.cc @@ -30,7 +30,7 @@ void ResizeRequest::send() { RWLock::WLocker l(m_image_ctx.object_map_lock); m_num_objs = Striper::get_num_objects(m_image_ctx.layout, m_new_size); - std::string oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id)); + std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id)); ldout(cct, 5) << this << " resizing on-disk object map: " << "ictx=" << &m_image_ctx << ", " << "oid=" << oid << ", num_objs=" << m_num_objs << dendl; @@ -48,10 +48,12 @@ void ResizeRequest::send() { } void ResizeRequest::finish_request() { - CephContext *cct = m_image_ctx.cct; + RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock); + CephContext *cct = m_image_ctx.cct; ldout(cct, 5) << this << " resizing in-memory object map: " << m_num_objs << dendl; + resize(m_object_map, m_num_objs, m_default_object_state); } diff --git a/src/librbd/object_map/SnapshotCreateRequest.cc b/src/librbd/object_map/SnapshotCreateRequest.cc index 6408973416fa0..5d77d2630d385 100644 --- a/src/librbd/object_map/SnapshotCreateRequest.cc +++ b/src/librbd/object_map/SnapshotCreateRequest.cc @@ -79,7 +79,7 @@ void SnapshotCreateRequest::send_read_map() { assert(m_image_ctx.get_snap_info(m_snap_id) != NULL); CephContext *cct = m_image_ctx.cct; - std::string oid(ObjectMap::object_map_name(m_image_ctx.id, CEPH_NOSNAP)); + std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, CEPH_NOSNAP)); ldout(cct, 5) << this << " " << __func__ << ": oid=" << oid << dendl; m_state = STATE_READ_MAP; @@ -96,7 +96,7 @@ void SnapshotCreateRequest::send_read_map() { void SnapshotCreateRequest::send_write_map() { CephContext *cct = m_image_ctx.cct; - std::string snap_oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id)); + std::string snap_oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id)); ldout(cct, 5) << this << " " << __func__ << ": snap_oid=" << snap_oid << dendl; m_state = STATE_WRITE_MAP; @@ -117,7 +117,7 @@ bool SnapshotCreateRequest::send_add_snapshot() { } CephContext *cct = m_image_ctx.cct; - std::string oid(ObjectMap::object_map_name(m_image_ctx.id, CEPH_NOSNAP)); + std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, CEPH_NOSNAP)); ldout(cct, 5) << this << " " << __func__ << ": oid=" << oid << dendl; m_state = STATE_ADD_SNAPSHOT; diff --git a/src/librbd/object_map/SnapshotRemoveRequest.cc b/src/librbd/object_map/SnapshotRemoveRequest.cc index 94c0952504a14..821dd5b8781bb 100644 --- a/src/librbd/object_map/SnapshotRemoveRequest.cc +++ b/src/librbd/object_map/SnapshotRemoveRequest.cc @@ -112,7 +112,7 @@ bool SnapshotRemoveRequest::should_complete(int r) { void SnapshotRemoveRequest::send_load_map() { CephContext *cct = m_image_ctx.cct; - std::string snap_oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id)); + std::string snap_oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id)); ldout(cct, 5) << this << " " << __func__ << ": snap_oid=" << snap_oid << dendl; m_state = STATE_LOAD_MAP; @@ -129,7 +129,7 @@ void SnapshotRemoveRequest::send_load_map() { void SnapshotRemoveRequest::send_remove_snapshot() { CephContext *cct = m_image_ctx.cct; - std::string oid(ObjectMap::object_map_name(m_image_ctx.id, m_next_snap_id)); + std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_next_snap_id)); ldout(cct, 5) << this << " " << __func__ << ": oid=" << oid << dendl; m_state = STATE_REMOVE_SNAPSHOT; @@ -161,7 +161,7 @@ void SnapshotRemoveRequest::send_invalidate_next_map() { void SnapshotRemoveRequest::send_remove_map() { CephContext *cct = m_image_ctx.cct; - std::string oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id)); + std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id)); ldout(cct, 5) << this << " " << __func__ << ": oid=" << oid << dendl; m_state = STATE_REMOVE_MAP; diff --git a/src/librbd/object_map/SnapshotRollbackRequest.cc b/src/librbd/object_map/SnapshotRollbackRequest.cc index 10eb59136d381..0da4fd4ae8e3b 100644 --- a/src/librbd/object_map/SnapshotRollbackRequest.cc +++ b/src/librbd/object_map/SnapshotRollbackRequest.cc @@ -76,7 +76,7 @@ bool SnapshotRollbackRequest::should_complete(int r) { } void SnapshotRollbackRequest::send_read_map() { - std::string snap_oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id)); + std::string snap_oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id)); CephContext *cct = m_image_ctx.cct; ldout(cct, 5) << this << " " << __func__ << ": snap_oid=" << snap_oid @@ -97,7 +97,8 @@ void SnapshotRollbackRequest::send_write_map() { RWLock::RLocker owner_locker(m_image_ctx.owner_lock); CephContext *cct = m_image_ctx.cct; - std::string snap_oid(ObjectMap::object_map_name(m_image_ctx.id, CEPH_NOSNAP)); + std::string snap_oid(ObjectMap<>::object_map_name(m_image_ctx.id, + CEPH_NOSNAP)); ldout(cct, 5) << this << " " << __func__ << ": snap_oid=" << snap_oid << dendl; m_state = STATE_WRITE_MAP; diff --git a/src/librbd/object_map/UnlockRequest.cc b/src/librbd/object_map/UnlockRequest.cc index c7ae9801d3ce0..3458bbed17f23 100644 --- a/src/librbd/object_map/UnlockRequest.cc +++ b/src/librbd/object_map/UnlockRequest.cc @@ -31,7 +31,7 @@ void UnlockRequest::send() { template void UnlockRequest::send_unlock() { CephContext *cct = m_image_ctx.cct; - std::string oid(ObjectMap::object_map_name(m_image_ctx.id, CEPH_NOSNAP)); + std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, CEPH_NOSNAP)); ldout(cct, 10) << this << " " << __func__ << ": oid=" << oid << dendl; librados::ObjectWriteOperation op; diff --git a/src/librbd/object_map/UnlockRequest.h b/src/librbd/object_map/UnlockRequest.h index b52a3d0f89e97..ae1d9e93425de 100644 --- a/src/librbd/object_map/UnlockRequest.h +++ b/src/librbd/object_map/UnlockRequest.h @@ -15,6 +15,10 @@ namespace object_map { template class UnlockRequest { public: + static UnlockRequest *create(ImageCtxT &image_ctx, Context *on_finish) { + return new UnlockRequest(image_ctx, on_finish); + } + UnlockRequest(ImageCtxT &image_ctx, Context *on_finish); void send(); diff --git a/src/librbd/object_map/UpdateRequest.cc b/src/librbd/object_map/UpdateRequest.cc index 51dbc48827f5b..e88085add4115 100644 --- a/src/librbd/object_map/UpdateRequest.cc +++ b/src/librbd/object_map/UpdateRequest.cc @@ -17,14 +17,15 @@ namespace librbd { namespace object_map { -void UpdateRequest::send() { +template +void UpdateRequest::send() { assert(m_image_ctx.snap_lock.is_locked()); assert(m_image_ctx.object_map_lock.is_locked()); CephContext *cct = m_image_ctx.cct; // safe to update in-memory state first without handling rollback since any // failures will invalidate the object map - std::string oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id)); + std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id)); ldout(cct, 20) << this << " updating object map" << ": ictx=" << &m_image_ctx << ", oid=" << oid << ", [" << m_start_object_no << "," << m_end_object_no << ") = " @@ -33,20 +34,6 @@ void UpdateRequest::send() { << "->" << static_cast(m_new_state) << dendl; - // rebuilding the object map might update on-disk only - if (m_snap_id == m_image_ctx.snap_id) { - assert(m_image_ctx.object_map_lock.is_wlocked()); - for (uint64_t object_no = m_start_object_no; - object_no < MIN(m_end_object_no, m_object_map.size()); - ++object_no) { - uint8_t state = m_object_map[object_no]; - if (!m_current_state || state == *m_current_state || - (*m_current_state == OBJECT_EXISTS && state == OBJECT_EXISTS_CLEAN)) { - m_object_map[object_no] = m_new_state; - } - } - } - librados::ObjectWriteOperation op; if (m_snap_id == CEPH_NOSNAP) { rados::cls::lock::assert_locked(&op, RBD_LOCK_NAME, LOCK_EXCLUSIVE, "", ""); @@ -60,10 +47,28 @@ void UpdateRequest::send() { rados_completion->release(); } -void UpdateRequest::finish_request() { +template +void UpdateRequest::finish_request() { + RWLock::RLocker snap_locker(m_image_ctx.snap_lock); + RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock); ldout(m_image_ctx.cct, 20) << this << " on-disk object map updated" << dendl; + + // rebuilding the object map might update on-disk only + if (m_snap_id == m_image_ctx.snap_id) { + for (uint64_t object_no = m_start_object_no; + object_no < MIN(m_end_object_no, m_object_map.size()); + ++object_no) { + uint8_t state = m_object_map[object_no]; + if (!m_current_state || state == *m_current_state || + (*m_current_state == OBJECT_EXISTS && state == OBJECT_EXISTS_CLEAN)) { + m_object_map[object_no] = m_new_state; + } + } + } } } // namespace object_map } // namespace librbd + +template class librbd::object_map::UpdateRequest; diff --git a/src/librbd/object_map/UpdateRequest.h b/src/librbd/object_map/UpdateRequest.h index d1ce4075dd0d4..5f3ae5cd53f51 100644 --- a/src/librbd/object_map/UpdateRequest.h +++ b/src/librbd/object_map/UpdateRequest.h @@ -17,13 +17,25 @@ class ImageCtx; namespace object_map { +template class UpdateRequest : public Request { public: + static UpdateRequest *create(ImageCtx &image_ctx, + ceph::BitVector<2> *object_map, + uint64_t snap_id, uint64_t start_object_no, + uint64_t end_object_no, uint8_t new_state, + const boost::optional ¤t_state, + Context *on_finish) { + return new UpdateRequest(image_ctx, object_map, snap_id, start_object_no, + end_object_no, new_state, current_state, + on_finish); + } + UpdateRequest(ImageCtx &image_ctx, ceph::BitVector<2> *object_map, uint64_t snap_id, uint64_t start_object_no, uint64_t end_object_no, uint8_t new_state, const boost::optional ¤t_state, - Context *on_finish) + Context *on_finish) : Request(image_ctx, snap_id, on_finish), m_object_map(*object_map), m_start_object_no(start_object_no), m_end_object_no(end_object_no), m_new_state(new_state), m_current_state(current_state) @@ -46,4 +58,6 @@ class UpdateRequest : public Request { } // namespace object_map } // namespace librbd +extern template class librbd::object_map::UpdateRequest; + #endif // CEPH_LIBRBD_OBJECT_MAP_UPDATE_REQUEST_H diff --git a/src/librbd/operation/TrimRequest.cc b/src/librbd/operation/TrimRequest.cc index 3992fb75e21c2..e99e0e95a21b7 100644 --- a/src/librbd/operation/TrimRequest.cc +++ b/src/librbd/operation/TrimRequest.cc @@ -254,12 +254,9 @@ void TrimRequest::send_pre_remove() { return; } - bool remove_objects = false; { RWLock::RLocker snap_locker(image_ctx.snap_lock); - if (image_ctx.object_map == nullptr) { - remove_objects = true; - } else { + if (image_ctx.object_map != nullptr) { ldout(image_ctx.cct, 5) << this << " send_pre_remove: " << " delete_start=" << m_delete_start << " num_objects=" << m_num_objects << dendl; @@ -268,22 +265,17 @@ void TrimRequest::send_pre_remove() { assert(image_ctx.exclusive_lock->is_lock_owner()); // flag the objects as pending deletion - Context *ctx = this->create_callback_context(); RWLock::WLocker object_map_locker(image_ctx.object_map_lock); - if (!image_ctx.object_map->aio_update(m_delete_start, m_num_objects, - OBJECT_PENDING, OBJECT_EXISTS, - ctx)) { - delete ctx; - remove_objects = true; + if (image_ctx.object_map->template aio_update >( + CEPH_NOSNAP, m_delete_start, m_num_objects, OBJECT_PENDING, + OBJECT_EXISTS, this)) { + return; } } } - // avoid possible recursive lock attempts - if (remove_objects) { - // no object map update required - send_remove_objects(); - } + // no object map update required + send_remove_objects(); } template @@ -291,12 +283,9 @@ void TrimRequest::send_post_remove() { I &image_ctx = this->m_image_ctx; assert(image_ctx.owner_lock.is_locked()); - bool clean_boundary = false; { RWLock::RLocker snap_locker(image_ctx.snap_lock); - if (image_ctx.object_map == nullptr) { - clean_boundary = true; - } else { + if (image_ctx.object_map != nullptr) { ldout(image_ctx.cct, 5) << this << " send_post_remove: " << " delete_start=" << m_delete_start << " num_objects=" << m_num_objects << dendl; @@ -305,22 +294,17 @@ void TrimRequest::send_post_remove() { assert(image_ctx.exclusive_lock->is_lock_owner()); // flag the pending objects as removed - Context *ctx = this->create_callback_context(); RWLock::WLocker object_map_locker(image_ctx.object_map_lock); - if (!image_ctx.object_map->aio_update(m_delete_start, m_num_objects, - OBJECT_NONEXISTENT, - OBJECT_PENDING, ctx)) { - delete ctx; - clean_boundary = true; + if (image_ctx.object_map->template aio_update >( + CEPH_NOSNAP, m_delete_start, m_num_objects, OBJECT_NONEXISTENT, + OBJECT_PENDING, this)) { + return; } } } - // avoid possible recursive lock attempts - if (clean_boundary) { - // no object map update required - send_clean_boundary(); - } + // no object map update required + send_clean_boundary(); } template diff --git a/src/test/Makefile-client.am b/src/test/Makefile-client.am index 6eade231ff8d9..bf20a35b5f5f8 100644 --- a/src/test/Makefile-client.am +++ b/src/test/Makefile-client.am @@ -383,6 +383,7 @@ librbd_test_mock_la_CXXFLAGS = $(UNITTEST_CXXFLAGS) noinst_LTLIBRARIES += librbd_test_mock.la unittest_librbd_SOURCES = \ + test/librbd/test_BlockGuard.cc \ test/librbd/test_main.cc \ test/librbd/test_mock_fixture.cc \ test/librbd/test_mock_ExclusiveLock.cc \ diff --git a/src/test/librbd/CMakeLists.txt b/src/test/librbd/CMakeLists.txt index aff27a628ca57..06d091092b888 100644 --- a/src/test/librbd/CMakeLists.txt +++ b/src/test/librbd/CMakeLists.txt @@ -15,11 +15,13 @@ set_target_properties(rbd_test PROPERTIES COMPILE_FLAGS ${UNITTEST_CXX_FLAGS}) # unittest_librbd # doesn't use add_ceph_test because it is called by run-rbd-unit-tests.sh set(unittest_librbd_srcs + test_BlockGuard.cc test_main.cc test_mock_fixture.cc test_mock_AioImageRequest.cc test_mock_ExclusiveLock.cc test_mock_Journal.cc + test_mock_ObjectMap.cc test_mock_ObjectWatcher.cc exclusive_lock/test_mock_AcquireRequest.cc exclusive_lock/test_mock_ReleaseRequest.cc diff --git a/src/test/librbd/mock/MockObjectMap.h b/src/test/librbd/mock/MockObjectMap.h index 25d14ed3c085c..dd274237908cd 100644 --- a/src/test/librbd/mock/MockObjectMap.h +++ b/src/test/librbd/mock/MockObjectMap.h @@ -5,6 +5,7 @@ #define CEPH_TEST_LIBRBD_MOCK_OBJECT_MAP_H #include "common/RWLock.h" +#include "librbd/Utils.h" #include "gmock/gmock.h" namespace librbd { @@ -17,7 +18,25 @@ struct MockObjectMap { MOCK_METHOD3(aio_resize, void(uint64_t new_size, uint8_t default_object_state, Context *on_finish)); - MOCK_METHOD6(aio_update, void(uint64_t snap_id, uint64_t start_object_no, + + template + bool aio_update(uint64_t snap_id, uint64_t start_object_no, uint8_t new_state, + const boost::optional ¤t_state, + T *callback_object) { + return aio_update(snap_id, start_object_no, start_object_no + 1, + new_state, current_state, callback_object); + } + + template + bool aio_update(uint64_t snap_id, uint64_t start_object_no, + uint64_t end_object_no, uint8_t new_state, + const boost::optional ¤t_state, + T *callback_object) { + return aio_update(snap_id, start_object_no, end_object_no, new_state, + current_state, + util::create_context_callback(callback_object)); + } + MOCK_METHOD6(aio_update, bool(uint64_t snap_id, uint64_t start_object_no, uint64_t end_object_no, uint8_t new_state, const boost::optional ¤t_state, Context *on_finish)); diff --git a/src/test/librbd/object_map/test_mock_LockRequest.cc b/src/test/librbd/object_map/test_mock_LockRequest.cc index 0ee782bce1337..819b0125c897f 100644 --- a/src/test/librbd/object_map/test_mock_LockRequest.cc +++ b/src/test/librbd/object_map/test_mock_LockRequest.cc @@ -27,14 +27,16 @@ class TestMockObjectMapLockRequest : public TestMockFixture { typedef LockRequest MockLockRequest; void expect_lock(MockImageCtx &mock_image_ctx, int r) { - std::string oid(ObjectMap::object_map_name(mock_image_ctx.id, CEPH_NOSNAP)); + std::string oid(ObjectMap<>::object_map_name(mock_image_ctx.id, + CEPH_NOSNAP)); EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), exec(oid, _, StrEq("lock"), StrEq("lock"), _, _, _)) .WillOnce(Return(r)); } void expect_get_lock_info(MockImageCtx &mock_image_ctx, int r) { - std::string oid(ObjectMap::object_map_name(mock_image_ctx.id, CEPH_NOSNAP)); + std::string oid(ObjectMap<>::object_map_name(mock_image_ctx.id, + CEPH_NOSNAP)); auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), exec(oid, _, StrEq("lock"), StrEq("get_info"), _, _, _)); if (r < 0) { @@ -59,7 +61,8 @@ class TestMockObjectMapLockRequest : public TestMockFixture { } void expect_break_lock(MockImageCtx &mock_image_ctx, int r) { - std::string oid(ObjectMap::object_map_name(mock_image_ctx.id, CEPH_NOSNAP)); + std::string oid(ObjectMap<>::object_map_name(mock_image_ctx.id, + CEPH_NOSNAP)); auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), exec(oid, _, StrEq("lock"), StrEq("break_lock"), _, _, _)); if (r < 0) { diff --git a/src/test/librbd/object_map/test_mock_RefreshRequest.cc b/src/test/librbd/object_map/test_mock_RefreshRequest.cc index be982ce684e89..8d67a1a4f5d4a 100644 --- a/src/test/librbd/object_map/test_mock_RefreshRequest.cc +++ b/src/test/librbd/object_map/test_mock_RefreshRequest.cc @@ -86,7 +86,7 @@ class TestMockObjectMapRefreshRequest : public TestMockFixture { void expect_object_map_load(MockObjectMapImageCtx &mock_image_ctx, ceph::BitVector<2> *object_map, uint64_t snap_id, int r) { - std::string oid(ObjectMap::object_map_name(mock_image_ctx.id, snap_id)); + std::string oid(ObjectMap<>::object_map_name(mock_image_ctx.id, snap_id)); auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), exec(oid, _, StrEq("rbd"), StrEq("object_map_load"), _, _, _)); if (r < 0) { @@ -117,14 +117,16 @@ class TestMockObjectMapRefreshRequest : public TestMockFixture { } void expect_truncate_request(MockObjectMapImageCtx &mock_image_ctx) { - std::string oid(ObjectMap::object_map_name(mock_image_ctx.id, TEST_SNAP_ID)); + std::string oid(ObjectMap<>::object_map_name(mock_image_ctx.id, + TEST_SNAP_ID)); EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), truncate(oid, 0, _)) .WillOnce(Return(0)); } void expect_object_map_resize(MockObjectMapImageCtx &mock_image_ctx, uint64_t num_objects, int r) { - std::string oid(ObjectMap::object_map_name(mock_image_ctx.id, TEST_SNAP_ID)); + std::string oid(ObjectMap<>::object_map_name(mock_image_ctx.id, + TEST_SNAP_ID)); auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), exec(oid, _, StrEq("rbd"), StrEq("object_map_resize"), _, _, _)); expect.WillOnce(Return(r)); diff --git a/src/test/librbd/object_map/test_mock_ResizeRequest.cc b/src/test/librbd/object_map/test_mock_ResizeRequest.cc index 42007d34487a4..44fd714353ba1 100644 --- a/src/test/librbd/object_map/test_mock_ResizeRequest.cc +++ b/src/test/librbd/object_map/test_mock_ResizeRequest.cc @@ -22,7 +22,7 @@ using ::testing::StrEq; class TestMockObjectMapResizeRequest : public TestMockFixture { public: void expect_resize(librbd::ImageCtx *ictx, uint64_t snap_id, int r) { - std::string oid(ObjectMap::object_map_name(ictx->id, snap_id)); + std::string oid(ObjectMap<>::object_map_name(ictx->id, snap_id)); if (snap_id == CEPH_NOSNAP) { EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), exec(oid, _, StrEq("lock"), StrEq("assert_locked"), _, _, _)) diff --git a/src/test/librbd/object_map/test_mock_SnapshotCreateRequest.cc b/src/test/librbd/object_map/test_mock_SnapshotCreateRequest.cc index 3dd62989f2970..823a32de9fe16 100644 --- a/src/test/librbd/object_map/test_mock_SnapshotCreateRequest.cc +++ b/src/test/librbd/object_map/test_mock_SnapshotCreateRequest.cc @@ -31,11 +31,11 @@ class TestMockObjectMapSnapshotCreateRequest : public TestMockFixture { void expect_read_map(librbd::ImageCtx *ictx, int r) { if (r < 0) { EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), - read(ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP), + read(ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP), 0, 0, _)).WillOnce(Return(r)); } else { EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), - read(ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP), + read(ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP), 0, 0, _)).WillOnce(DoDefault()); } } @@ -44,18 +44,18 @@ class TestMockObjectMapSnapshotCreateRequest : public TestMockFixture { if (r < 0) { EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), write_full( - ObjectMap::object_map_name(ictx->id, snap_id), _, _)) + ObjectMap<>::object_map_name(ictx->id, snap_id), _, _)) .WillOnce(Return(r)); } else { EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), write_full( - ObjectMap::object_map_name(ictx->id, snap_id), _, _)) + ObjectMap<>::object_map_name(ictx->id, snap_id), _, _)) .WillOnce(DoDefault()); } } void expect_add_snapshot(librbd::ImageCtx *ictx, int r) { - std::string oid(ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP)); + std::string oid(ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP)); if (r < 0) { EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), exec(oid, _, StrEq("lock"), StrEq("assert_locked"), _, _, _)) diff --git a/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc b/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc index 215c214eb8b10..270b267c53fed 100644 --- a/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc +++ b/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc @@ -23,7 +23,7 @@ using ::testing::StrEq; class TestMockObjectMapSnapshotRemoveRequest : public TestMockFixture { public: void expect_load_map(librbd::ImageCtx *ictx, uint64_t snap_id, int r) { - std::string snap_oid(ObjectMap::object_map_name(ictx->id, snap_id)); + std::string snap_oid(ObjectMap<>::object_map_name(ictx->id, snap_id)); if (r < 0) { EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), exec(snap_oid, _, StrEq("rbd"), StrEq("object_map_load"), _, _, _)) @@ -36,7 +36,7 @@ class TestMockObjectMapSnapshotRemoveRequest : public TestMockFixture { } void expect_remove_snapshot(librbd::ImageCtx *ictx, int r) { - std::string oid(ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP)); + std::string oid(ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP)); if (r < 0) { EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), exec(oid, _, StrEq("lock"), StrEq("assert_locked"), _, _, _)) @@ -52,7 +52,7 @@ class TestMockObjectMapSnapshotRemoveRequest : public TestMockFixture { } void expect_remove_map(librbd::ImageCtx *ictx, uint64_t snap_id, int r) { - std::string snap_oid(ObjectMap::object_map_name(ictx->id, snap_id)); + std::string snap_oid(ObjectMap<>::object_map_name(ictx->id, snap_id)); if (r < 0) { EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), remove(snap_oid, _)) .WillOnce(Return(r)); diff --git a/src/test/librbd/object_map/test_mock_SnapshotRollbackRequest.cc b/src/test/librbd/object_map/test_mock_SnapshotRollbackRequest.cc index 51fc932c0b07a..ff3e06a8d70c5 100644 --- a/src/test/librbd/object_map/test_mock_SnapshotRollbackRequest.cc +++ b/src/test/librbd/object_map/test_mock_SnapshotRollbackRequest.cc @@ -24,29 +24,29 @@ class TestMockObjectMapSnapshotRollbackRequest : public TestMockFixture { void expect_read_map(librbd::ImageCtx *ictx, uint64_t snap_id, int r) { if (r < 0) { EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), - read(ObjectMap::object_map_name(ictx->id, snap_id), + read(ObjectMap<>::object_map_name(ictx->id, snap_id), 0, 0, _)).WillOnce(Return(r)); } else { EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), - read(ObjectMap::object_map_name(ictx->id, snap_id), + read(ObjectMap<>::object_map_name(ictx->id, snap_id), 0, 0, _)).WillOnce(DoDefault()); } } void expect_write_map(librbd::ImageCtx *ictx, int r) { EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), - exec(ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP), _, + exec(ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP), _, StrEq("lock"), StrEq("assert_locked"), _, _, _)) .WillOnce(DoDefault()); if (r < 0) { EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), write_full( - ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP), _, _)) + ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP), _, _)) .WillOnce(Return(r)); } else { EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), write_full( - ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP), _, _)) + ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP), _, _)) .WillOnce(DoDefault()); } } diff --git a/src/test/librbd/object_map/test_mock_UnlockRequest.cc b/src/test/librbd/object_map/test_mock_UnlockRequest.cc index 7f834ca8dba18..a834a5004fa31 100644 --- a/src/test/librbd/object_map/test_mock_UnlockRequest.cc +++ b/src/test/librbd/object_map/test_mock_UnlockRequest.cc @@ -25,7 +25,8 @@ class TestMockObjectMapUnlockRequest : public TestMockFixture { typedef UnlockRequest MockUnlockRequest; void expect_unlock(MockImageCtx &mock_image_ctx, int r) { - std::string oid(ObjectMap::object_map_name(mock_image_ctx.id, CEPH_NOSNAP)); + std::string oid(ObjectMap<>::object_map_name(mock_image_ctx.id, + CEPH_NOSNAP)); EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), exec(oid, _, StrEq("lock"), StrEq("unlock"), _, _, _)) .WillOnce(Return(r)); diff --git a/src/test/librbd/object_map/test_mock_UpdateRequest.cc b/src/test/librbd/object_map/test_mock_UpdateRequest.cc index 45879f2f83e60..f82ffb82ac21d 100644 --- a/src/test/librbd/object_map/test_mock_UpdateRequest.cc +++ b/src/test/librbd/object_map/test_mock_UpdateRequest.cc @@ -8,6 +8,7 @@ #include "librbd/ImageState.h" #include "librbd/internal.h" #include "librbd/ObjectMap.h" +#include "librbd/Operations.h" #include "librbd/object_map/UpdateRequest.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -23,7 +24,7 @@ using ::testing::StrEq; class TestMockObjectMapUpdateRequest : public TestMockFixture { public: void expect_update(librbd::ImageCtx *ictx, uint64_t snap_id, int r) { - std::string oid(ObjectMap::object_map_name(ictx->id, snap_id)); + std::string oid(ObjectMap<>::object_map_name(ictx->id, snap_id)); if (snap_id == CEPH_NOSNAP) { EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), exec(oid, _, StrEq("lock"), StrEq("assert_locked"), _, _, _)) @@ -56,16 +57,19 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateInMemory) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + librbd::NoOpProgressContext no_progress; + ASSERT_EQ(0, ictx->operations->resize(4 << ictx->order, no_progress)); ASSERT_EQ(0, acquire_exclusive_lock(*ictx)); ceph::BitVector<2> object_map; - object_map.resize(1024); + object_map.resize(4); for (uint64_t i = 0; i < object_map.size(); ++i) { object_map[i] = i % 4; } C_SaferCond cond_ctx; - AsyncRequest<> *req = new UpdateRequest( + AsyncRequest<> *req = new UpdateRequest<>( *ictx, &object_map, CEPH_NOSNAP, 0, object_map.size(), OBJECT_NONEXISTENT, OBJECT_EXISTS, &cond_ctx); { @@ -97,7 +101,7 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateHeadOnDisk) { object_map.resize(1); C_SaferCond cond_ctx; - AsyncRequest<> *req = new UpdateRequest( + AsyncRequest<> *req = new UpdateRequest<>( *ictx, &object_map, CEPH_NOSNAP, 0, object_map.size(), OBJECT_NONEXISTENT, OBJECT_EXISTS, &cond_ctx); { @@ -125,7 +129,7 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateSnapOnDisk) { object_map.resize(1); C_SaferCond cond_ctx; - AsyncRequest<> *req = new UpdateRequest( + AsyncRequest<> *req = new UpdateRequest<>( *ictx, &object_map, snap_id, 0, object_map.size(), OBJECT_NONEXISTENT, OBJECT_EXISTS, &cond_ctx); { @@ -152,7 +156,7 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateOnDiskError) { object_map.resize(1); C_SaferCond cond_ctx; - AsyncRequest<> *req = new UpdateRequest( + AsyncRequest<> *req = new UpdateRequest<>( *ictx, &object_map, CEPH_NOSNAP, 0, object_map.size(), OBJECT_NONEXISTENT, OBJECT_EXISTS, &cond_ctx); { @@ -182,7 +186,7 @@ TEST_F(TestMockObjectMapUpdateRequest, RebuildSnapOnDisk) { object_map.resize(1); C_SaferCond cond_ctx; - AsyncRequest<> *req = new UpdateRequest( + AsyncRequest<> *req = new UpdateRequest<>( *ictx, &object_map, snap_id, 0, object_map.size(), OBJECT_EXISTS_CLEAN, boost::optional(), &cond_ctx); { diff --git a/src/test/librbd/test_BlockGuard.cc b/src/test/librbd/test_BlockGuard.cc new file mode 100644 index 0000000000000..b16188b04a474 --- /dev/null +++ b/src/test/librbd/test_BlockGuard.cc @@ -0,0 +1,98 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "test/librbd/test_fixture.h" +#include "test/librbd/test_support.h" +#include "librbd/BlockGuard.h" + +namespace librbd { + +class TestIOBlockGuard : public TestFixture { +public: + static uint32_t s_index; + + struct Operation { + uint32_t index; + Operation() : index(++s_index) { + } + Operation(Operation &&rhs) : index(rhs.index) { + } + Operation(const Operation &) = delete; + + Operation& operator=(Operation &&rhs) { + index = rhs.index; + return *this; + } + + bool operator==(const Operation &rhs) const { + return index == rhs.index; + } + }; + + typedef std::list Operations; + + typedef BlockGuard OpBlockGuard; + + virtual void SetUp() override { + TestFixture::SetUp(); + m_cct = reinterpret_cast(m_ioctx.cct()); + } + + CephContext *m_cct; +}; + +TEST_F(TestIOBlockGuard, NonDetainedOps) { + OpBlockGuard op_block_guard(m_cct); + + Operation op1; + BlockGuardCell *cell1; + ASSERT_EQ(0, op_block_guard.detain({1, 3}, &op1, &cell1)); + + Operation op2; + BlockGuardCell *cell2; + ASSERT_EQ(0, op_block_guard.detain({0, 1}, &op2, &cell2)); + + Operation op3; + BlockGuardCell *cell3; + ASSERT_EQ(0, op_block_guard.detain({3, 6}, &op3, &cell3)); + + Operations released_ops; + op_block_guard.release(cell1, &released_ops); + ASSERT_TRUE(released_ops.empty()); + + op_block_guard.release(cell2, &released_ops); + ASSERT_TRUE(released_ops.empty()); + + op_block_guard.release(cell3, &released_ops); + ASSERT_TRUE(released_ops.empty()); +} + +TEST_F(TestIOBlockGuard, DetainedOps) { + OpBlockGuard op_block_guard(m_cct); + + Operation op1; + BlockGuardCell *cell1; + ASSERT_EQ(0, op_block_guard.detain({1, 3}, &op1, &cell1)); + + Operation op2; + BlockGuardCell *cell2; + ASSERT_EQ(1, op_block_guard.detain({2, 6}, &op2, &cell2)); + ASSERT_EQ(nullptr, cell2); + + Operation op3; + BlockGuardCell *cell3; + ASSERT_EQ(2, op_block_guard.detain({0, 2}, &op3, &cell3)); + ASSERT_EQ(nullptr, cell3); + + Operations expected_ops; + expected_ops.push_back(std::move(op2)); + expected_ops.push_back(std::move(op3)); + Operations released_ops; + op_block_guard.release(cell1, &released_ops); + ASSERT_EQ(expected_ops, released_ops); +} + +uint32_t TestIOBlockGuard::s_index = 0; + +} // namespace librbd + diff --git a/src/test/librbd/test_ObjectMap.cc b/src/test/librbd/test_ObjectMap.cc index b3b19e45ccd30..1e8566e9c5fe5 100644 --- a/src/test/librbd/test_ObjectMap.cc +++ b/src/test/librbd/test_ObjectMap.cc @@ -18,7 +18,7 @@ class TestObjectMap : public TestFixture { int when_open_object_map(librbd::ImageCtx *ictx) { C_SaferCond ctx; - librbd::ObjectMap object_map(*ictx, ictx->snap_id); + librbd::ObjectMap<> object_map(*ictx, ictx->snap_id); object_map.open(&ctx); return ctx.wait(); } @@ -38,7 +38,7 @@ TEST_F(TestObjectMap, RefreshInvalidatesWhenCorrupt) { } ASSERT_EQ(0, lock_ctx.wait()); - std::string oid = librbd::ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP); + std::string oid = librbd::ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP); bufferlist bl; bl.append("corrupt"); ASSERT_EQ(0, ictx->data_ctx.write_full(oid, bl)); @@ -64,8 +64,8 @@ TEST_F(TestObjectMap, RefreshInvalidatesWhenTooSmall) { librados::ObjectWriteOperation op; librbd::cls_client::object_map_resize(&op, 0, OBJECT_NONEXISTENT); - std::string oid = librbd::ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP); - ASSERT_EQ(0, ictx->data_ctx.operate(oid, &op)); + std::string oid = librbd::ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP); + ASSERT_EQ(0, ictx->md_ctx.operate(oid, &op)); ASSERT_EQ(0, when_open_object_map(ictx)); ASSERT_TRUE(ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID)); @@ -85,7 +85,7 @@ TEST_F(TestObjectMap, InvalidateFlagOnDisk) { } ASSERT_EQ(0, lock_ctx.wait()); - std::string oid = librbd::ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP); + std::string oid = librbd::ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP); bufferlist bl; bl.append("corrupt"); ASSERT_EQ(0, ictx->data_ctx.write_full(oid, bl)); @@ -104,7 +104,7 @@ TEST_F(TestObjectMap, InvalidateFlagInMemoryOnly) { ASSERT_EQ(0, open_image(m_image_name, &ictx)); ASSERT_FALSE(ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID)); - std::string oid = librbd::ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP); + std::string oid = librbd::ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP); bufferlist valid_bl; ASSERT_LT(0, ictx->data_ctx.read(oid, valid_bl, 0, 0)); diff --git a/src/test/librbd/test_internal.cc b/src/test/librbd/test_internal.cc index e5890520c8d17..97c2dadf253ca 100644 --- a/src/test/librbd/test_internal.cc +++ b/src/test/librbd/test_internal.cc @@ -558,7 +558,7 @@ TEST_F(TestInternal, SnapshotCopyup) state = OBJECT_EXISTS_CLEAN; } - librbd::ObjectMap object_map(*ictx2, ictx2->snap_id); + librbd::ObjectMap<> object_map(*ictx2, ictx2->snap_id); C_SaferCond ctx; object_map.open(&ctx); ASSERT_EQ(0, ctx.wait()); diff --git a/src/test/librbd/test_mock_ObjectMap.cc b/src/test/librbd/test_mock_ObjectMap.cc new file mode 100644 index 0000000000000..f14c6b9a5b274 --- /dev/null +++ b/src/test/librbd/test_mock_ObjectMap.cc @@ -0,0 +1,272 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "test/librbd/test_mock_fixture.h" +#include "test/librbd/test_support.h" +#include "test/librbd/mock/MockImageCtx.h" +#include "librbd/ObjectMap.h" +#include "librbd/object_map/RefreshRequest.h" +#include "librbd/object_map/UnlockRequest.h" +#include "librbd/object_map/UpdateRequest.h" + +namespace librbd { + +namespace { + +struct MockTestImageCtx : public MockImageCtx { + MockTestImageCtx(ImageCtx &image_ctx) : MockImageCtx(image_ctx) { + } +}; + +} // anonymous namespace + +namespace object_map { + +template <> +struct RefreshRequest { + Context *on_finish = nullptr; + ceph::BitVector<2u> *object_map = nullptr; + static RefreshRequest *s_instance; + static RefreshRequest *create(MockTestImageCtx &image_ctx, + ceph::BitVector<2u> *object_map, + uint64_t snap_id, Context *on_finish) { + assert(s_instance != nullptr); + s_instance->on_finish = on_finish; + s_instance->object_map = object_map; + return s_instance; + } + + MOCK_METHOD0(send, void()); + + RefreshRequest() { + s_instance = this; + } +}; + +template <> +struct UnlockRequest { + Context *on_finish = nullptr; + static UnlockRequest *s_instance; + static UnlockRequest *create(MockTestImageCtx &image_ctx, + Context *on_finish) { + assert(s_instance != nullptr); + s_instance->on_finish = on_finish; + return s_instance; + } + + MOCK_METHOD0(send, void()); + UnlockRequest() { + s_instance = this; + } +}; + +template <> +struct UpdateRequest { + Context *on_finish = nullptr; + static UpdateRequest *s_instance; + static UpdateRequest *create(MockTestImageCtx &image_ctx, + ceph::BitVector<2u> *object_map, + uint64_t snap_id, + uint64_t start_object_no, uint64_t end_object_no, + uint8_t new_state, + const boost::optional ¤t_state, + Context *on_finish) { + assert(s_instance != nullptr); + s_instance->on_finish = on_finish; + s_instance->construct(snap_id, start_object_no, end_object_no, new_state, + current_state); + return s_instance; + } + + MOCK_METHOD5(construct, void(uint64_t snap_id, uint64_t start_object_no, + uint64_t end_object_no, uint8_t new_state, + const boost::optional ¤t_state)); + MOCK_METHOD0(send, void()); + UpdateRequest() { + s_instance = this; + } +}; + +RefreshRequest *RefreshRequest::s_instance = nullptr; +UnlockRequest *UnlockRequest::s_instance = nullptr; +UpdateRequest *UpdateRequest::s_instance = nullptr; + +} // namespace object_map +} // namespace librbd + +#include "librbd/ObjectMap.cc" + +namespace librbd { + +using testing::_; +using testing::InSequence; +using testing::Invoke; + +class TestMockObjectMap : public TestMockFixture { +public: + typedef ObjectMap MockObjectMap; + typedef object_map::RefreshRequest MockRefreshRequest; + typedef object_map::UnlockRequest MockUnlockRequest; + typedef object_map::UpdateRequest MockUpdateRequest; + + void expect_refresh(MockTestImageCtx &mock_image_ctx, + MockRefreshRequest &mock_refresh_request, + const ceph::BitVector<2u> &object_map, int r) { + EXPECT_CALL(mock_refresh_request, send()) + .WillOnce(Invoke([&mock_image_ctx, &mock_refresh_request, &object_map, r]() { + *mock_refresh_request.object_map = object_map; + mock_image_ctx.image_ctx->op_work_queue->queue(mock_refresh_request.on_finish, r); + })); + } + + void expect_unlock(MockTestImageCtx &mock_image_ctx, + MockUnlockRequest &mock_unlock_request, int r) { + EXPECT_CALL(mock_unlock_request, send()) + .WillOnce(Invoke([&mock_image_ctx, &mock_unlock_request, r]() { + mock_image_ctx.image_ctx->op_work_queue->queue(mock_unlock_request.on_finish, r); + })); + } + + void expect_update(MockTestImageCtx &mock_image_ctx, + MockUpdateRequest &mock_update_request, + uint64_t snap_id, uint64_t start_object_no, + uint64_t end_object_no, uint8_t new_state, + const boost::optional ¤t_state, + Context **on_finish) { + EXPECT_CALL(mock_update_request, construct(snap_id, start_object_no, + end_object_no, new_state, + current_state)) + .Times(1); + EXPECT_CALL(mock_update_request, send()) + .WillOnce(Invoke([&mock_image_ctx, &mock_update_request, on_finish]() { + *on_finish = mock_update_request.on_finish; + })); + } + +}; + +TEST_F(TestMockObjectMap, NonDetainedUpdate) { + REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + + InSequence seq; + ceph::BitVector<2u> object_map; + object_map.resize(4); + MockRefreshRequest mock_refresh_request; + expect_refresh(mock_image_ctx, mock_refresh_request, object_map, 0); + + MockUpdateRequest mock_update_request; + Context *finish_update_1; + expect_update(mock_image_ctx, mock_update_request, CEPH_NOSNAP, + 0, 1, 1, {}, &finish_update_1); + Context *finish_update_2; + expect_update(mock_image_ctx, mock_update_request, CEPH_NOSNAP, + 1, 2, 1, {}, &finish_update_2); + + MockUnlockRequest mock_unlock_request; + expect_unlock(mock_image_ctx, mock_unlock_request, 0); + + MockObjectMap mock_object_map(mock_image_ctx, CEPH_NOSNAP); + C_SaferCond open_ctx; + mock_object_map.open(&open_ctx); + ASSERT_EQ(0, open_ctx.wait()); + + C_SaferCond update_ctx1; + C_SaferCond update_ctx2; + { + RWLock::RLocker snap_locker(mock_image_ctx.snap_lock); + RWLock::WLocker object_map_locker(mock_image_ctx.object_map_lock); + mock_object_map.aio_update(CEPH_NOSNAP, 0, 1, {}, &update_ctx1); + mock_object_map.aio_update(CEPH_NOSNAP, 1, 1, {}, &update_ctx2); + } + + finish_update_2->complete(0); + ASSERT_EQ(0, update_ctx2.wait()); + + finish_update_1->complete(0); + ASSERT_EQ(0, update_ctx1.wait()); + + C_SaferCond close_ctx; + mock_object_map.close(&close_ctx); + ASSERT_EQ(0, close_ctx.wait()); +} + +TEST_F(TestMockObjectMap, DetainedUpdate) { + REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + + InSequence seq; + ceph::BitVector<2u> object_map; + object_map.resize(4); + MockRefreshRequest mock_refresh_request; + expect_refresh(mock_image_ctx, mock_refresh_request, object_map, 0); + + MockUpdateRequest mock_update_request; + Context *finish_update_1; + expect_update(mock_image_ctx, mock_update_request, CEPH_NOSNAP, + 1, 4, 1, {}, &finish_update_1); + Context *finish_update_2 = nullptr; + expect_update(mock_image_ctx, mock_update_request, CEPH_NOSNAP, + 1, 3, 1, {}, &finish_update_2); + Context *finish_update_3 = nullptr; + expect_update(mock_image_ctx, mock_update_request, CEPH_NOSNAP, + 2, 3, 1, {}, &finish_update_3); + Context *finish_update_4 = nullptr; + expect_update(mock_image_ctx, mock_update_request, CEPH_NOSNAP, + 0, 2, 1, {}, &finish_update_4); + + MockUnlockRequest mock_unlock_request; + expect_unlock(mock_image_ctx, mock_unlock_request, 0); + + MockObjectMap mock_object_map(mock_image_ctx, CEPH_NOSNAP); + C_SaferCond open_ctx; + mock_object_map.open(&open_ctx); + ASSERT_EQ(0, open_ctx.wait()); + + C_SaferCond update_ctx1; + C_SaferCond update_ctx2; + C_SaferCond update_ctx3; + C_SaferCond update_ctx4; + { + RWLock::RLocker snap_locker(mock_image_ctx.snap_lock); + RWLock::WLocker object_map_locker(mock_image_ctx.object_map_lock); + mock_object_map.aio_update(CEPH_NOSNAP, 1, 4, 1, {}, &update_ctx1); + mock_object_map.aio_update(CEPH_NOSNAP, 1, 3, 1, {}, &update_ctx2); + mock_object_map.aio_update(CEPH_NOSNAP, 2, 3, 1, {}, &update_ctx3); + mock_object_map.aio_update(CEPH_NOSNAP, 0, 2, 1, {}, &update_ctx4); + } + + // updates 2, 3, and 4 are blocked on update 1 + ASSERT_EQ(nullptr, finish_update_2); + finish_update_1->complete(0); + ASSERT_EQ(0, update_ctx1.wait()); + + // updates 3 and 4 are blocked on update 2 + ASSERT_NE(nullptr, finish_update_2); + ASSERT_EQ(nullptr, finish_update_3); + ASSERT_EQ(nullptr, finish_update_4); + finish_update_2->complete(0); + ASSERT_EQ(0, update_ctx2.wait()); + + ASSERT_NE(nullptr, finish_update_3); + ASSERT_NE(nullptr, finish_update_4); + finish_update_3->complete(0); + finish_update_4->complete(0); + ASSERT_EQ(0, update_ctx3.wait()); + ASSERT_EQ(0, update_ctx4.wait()); + + C_SaferCond close_ctx; + mock_object_map.close(&close_ctx); + ASSERT_EQ(0, close_ctx.wait()); +} + +} // namespace librbd + diff --git a/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc b/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc index b018f16c21b30..5a2fa9e5a65c6 100644 --- a/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc +++ b/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc @@ -174,17 +174,18 @@ class TestMockImageSyncObjectCopyRequest : public TestMockFixture { if (mock_image_ctx.image_ctx->object_map != nullptr) { auto &expect = EXPECT_CALL(mock_object_map, aio_update(snap_id, 0, 1, state, _, _)); if (r < 0) { - expect.WillOnce(WithArg<5>(Invoke([this, r](Context *ctx) { - m_threads->work_queue->queue(ctx, r); - }))); + expect.WillOnce(DoAll(WithArg<5>(Invoke([this, r](Context *ctx) { + m_threads->work_queue->queue(ctx, r); + })), + Return(true))); } else { - expect.WillOnce(WithArg<5>(Invoke([&mock_image_ctx, snap_id, state, r](Context *ctx) { - assert(mock_image_ctx.image_ctx->snap_lock.is_locked()); - assert(mock_image_ctx.image_ctx->object_map_lock.is_wlocked()); - mock_image_ctx.image_ctx->object_map->aio_update(snap_id, 0, 1, - state, - boost::none, ctx); - }))); + expect.WillOnce(DoAll(WithArg<5>(Invoke([&mock_image_ctx, snap_id, state, r](Context *ctx) { + assert(mock_image_ctx.image_ctx->snap_lock.is_locked()); + assert(mock_image_ctx.image_ctx->object_map_lock.is_wlocked()); + mock_image_ctx.image_ctx->object_map->aio_update( + snap_id, 0, 1, state, boost::none, ctx); + })), + Return(true))); } } } diff --git a/src/test/rbd_mirror/image_sync/test_mock_SnapshotCreateRequest.cc b/src/test/rbd_mirror/image_sync/test_mock_SnapshotCreateRequest.cc index c0105aeaa210b..c54aa2c223b53 100644 --- a/src/test/rbd_mirror/image_sync/test_mock_SnapshotCreateRequest.cc +++ b/src/test/rbd_mirror/image_sync/test_mock_SnapshotCreateRequest.cc @@ -91,8 +91,8 @@ class TestMockImageSyncSnapshotCreateRequest : public TestMockFixture { void expect_object_map_resize(librbd::MockTestImageCtx &mock_image_ctx, librados::snap_t snap_id, int r) { - std::string oid(librbd::ObjectMap::object_map_name(mock_image_ctx.id, - snap_id)); + std::string oid(librbd::ObjectMap<>::object_map_name(mock_image_ctx.id, + snap_id)); EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), exec(oid, _, StrEq("rbd"), StrEq("object_map_resize"), _, _, _)) .WillOnce(Return(r)); diff --git a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc index 9975b5bf81576..5c024738e1f92 100644 --- a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc +++ b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc @@ -255,14 +255,11 @@ void ObjectCopyRequest::send_update_object_map() { << dendl; RWLock::WLocker object_map_locker(m_local_image_ctx->object_map_lock); - Context *ctx = create_context_callback< + bool sent = m_local_image_ctx->object_map->template aio_update< ObjectCopyRequest, &ObjectCopyRequest::handle_update_object_map>( + snap_object_state.first, m_object_number, snap_object_state.second, {}, this); - m_local_image_ctx->object_map->aio_update(snap_object_state.first, - m_object_number, - m_object_number + 1, - snap_object_state.second, - boost::none, ctx); + assert(sent); m_local_image_ctx->snap_lock.put_read(); } diff --git a/src/tools/rbd_mirror/image_sync/SnapshotCreateRequest.cc b/src/tools/rbd_mirror/image_sync/SnapshotCreateRequest.cc index ee0c80fbad24b..ede82db4d0dd2 100644 --- a/src/tools/rbd_mirror/image_sync/SnapshotCreateRequest.cc +++ b/src/tools/rbd_mirror/image_sync/SnapshotCreateRequest.cc @@ -222,7 +222,7 @@ void SnapshotCreateRequest::send_create_object_map() { librados::snap_t local_snap_id = snap_it->second; m_local_image_ctx->snap_lock.put_read(); - std::string object_map_oid(librbd::ObjectMap::object_map_name( + std::string object_map_oid(librbd::ObjectMap<>::object_map_name( m_local_image_ctx->id, local_snap_id)); uint64_t object_count = Striper::get_num_objects(m_local_image_ctx->layout, m_size);