From 7a43414513ab5040b04abacd4490b9b5597c72ac Mon Sep 17 00:00:00 2001 From: Mykola Golub Date: Wed, 21 Sep 2016 13:57:20 +0300 Subject: [PATCH 1/2] cls::rbd: async methods to set/remove metadata Signed-off-by: Mykola Golub --- src/cls/rbd/cls_rbd_client.cc | 36 ++++++++++++++++++++++++++--------- src/cls/rbd/cls_rbd_client.h | 4 ++++ 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/cls/rbd/cls_rbd_client.cc b/src/cls/rbd/cls_rbd_client.cc index 4a53acce9b17a..656db5c2333ec 100644 --- a/src/cls/rbd/cls_rbd_client.cc +++ b/src/cls/rbd/cls_rbd_client.cc @@ -1013,22 +1013,40 @@ namespace librbd { rados_op->exec("rbd", "object_map_snap_remove", in); } + void metadata_set(librados::ObjectWriteOperation *op, + const map &data) + { + bufferlist bl; + ::encode(data, bl); + + op->exec("rbd", "metadata_set", bl); + } + int metadata_set(librados::IoCtx *ioctx, const std::string &oid, const map &data) { - bufferlist in; - ::encode(data, in); - bufferlist out; - return ioctx->exec(oid, "rbd", "metadata_set", in, out); + librados::ObjectWriteOperation op; + metadata_set(&op, data); + + return ioctx->operate(oid, &op); + } + + void metadata_remove(librados::ObjectWriteOperation *op, + const std::string &key) + { + bufferlist bl; + ::encode(key, bl); + + op->exec("rbd", "metadata_remove", bl); } int metadata_remove(librados::IoCtx *ioctx, const std::string &oid, - const std::string &key) + const std::string &key) { - bufferlist in; - ::encode(key, in); - bufferlist out; - return ioctx->exec(oid, "rbd", "metadata_remove", in, out); + librados::ObjectWriteOperation op; + metadata_remove(&op, key); + + return ioctx->operate(oid, &op); } int metadata_list(librados::IoCtx *ioctx, const std::string &oid, diff --git a/src/cls/rbd/cls_rbd_client.h b/src/cls/rbd/cls_rbd_client.h index 75634ce6013b7..b8d02f5228b9b 100644 --- a/src/cls/rbd/cls_rbd_client.h +++ b/src/cls/rbd/cls_rbd_client.h @@ -153,8 +153,12 @@ namespace librbd { const std::string &start, uint64_t max_return); int metadata_list_finish(bufferlist::iterator *it, std::map *pairs); + void metadata_set(librados::ObjectWriteOperation *op, + const map &data); int metadata_set(librados::IoCtx *ioctx, const std::string &oid, const map &data); + void metadata_remove(librados::ObjectWriteOperation *op, + const std::string &key); int metadata_remove(librados::IoCtx *ioctx, const std::string &oid, const std::string &key); int metadata_get(librados::IoCtx *ioctx, const std::string &oid, From be25f843afdf0eb2c70bde975ac3daa8b3c8b072 Mon Sep 17 00:00:00 2001 From: Mykola Golub Date: Wed, 21 Sep 2016 13:58:48 +0300 Subject: [PATCH 2/2] rbd-mirror: replicate image metadata settings Fixes: http://tracker.ceph.com/issues/16212 Signed-off-by: Mykola Golub --- src/librbd/CMakeLists.txt | 2 + src/librbd/Operations.cc | 121 +++++++++++++++++- src/librbd/Operations.h | 7 + src/librbd/internal.cc | 37 ------ src/librbd/internal.h | 2 - src/librbd/journal/Replay.cc | 54 ++++++++ src/librbd/journal/Replay.h | 4 + src/librbd/journal/Types.cc | 51 ++++++++ src/librbd/journal/Types.h | 37 ++++++ src/librbd/librbd.cc | 8 +- src/librbd/operation/MetadataRemoveRequest.cc | 60 +++++++++ src/librbd/operation/MetadataRemoveRequest.h | 44 +++++++ src/librbd/operation/MetadataSetRequest.cc | 62 +++++++++ src/librbd/operation/MetadataSetRequest.h | 47 +++++++ src/test/librbd/journal/test_Replay.cc | 74 +++++++++++ src/test/librbd/journal/test_mock_Replay.cc | 112 ++++++++++++++++ src/test/librbd/mock/MockOperations.h | 5 + src/test/librbd/test_internal.cc | 14 +- .../image_sync/test_mock_ObjectCopyRequest.cc | 4 +- src/test/rbd_mirror/test_ImageReplayer.cc | 45 +++++++ 20 files changed, 737 insertions(+), 53 deletions(-) create mode 100644 src/librbd/operation/MetadataRemoveRequest.cc create mode 100644 src/librbd/operation/MetadataRemoveRequest.h create mode 100644 src/librbd/operation/MetadataSetRequest.cc create mode 100644 src/librbd/operation/MetadataSetRequest.h diff --git a/src/librbd/CMakeLists.txt b/src/librbd/CMakeLists.txt index ef8f0e68c0044..4f46c1f2bacde 100644 --- a/src/librbd/CMakeLists.txt +++ b/src/librbd/CMakeLists.txt @@ -65,6 +65,8 @@ set(librbd_internal_srcs operation/DisableFeaturesRequest.cc operation/EnableFeaturesRequest.cc operation/FlattenRequest.cc + operation/MetadataRemoveRequest.cc + operation/MetadataSetRequest.cc operation/ObjectMapIterate.cc operation/RebuildObjectMapRequest.cc operation/RenameRequest.cc diff --git a/src/librbd/Operations.cc b/src/librbd/Operations.cc index c774a54d20760..304545f8b7396 100644 --- a/src/librbd/Operations.cc +++ b/src/librbd/Operations.cc @@ -15,8 +15,10 @@ #include "librbd/operation/DisableFeaturesRequest.h" #include "librbd/operation/EnableFeaturesRequest.h" #include "librbd/operation/FlattenRequest.h" -#include "librbd/operation/RebuildObjectMapRequest.h" +#include "librbd/operation/MetadataRemoveRequest.h" +#include "librbd/operation/MetadataSetRequest.h" #include "librbd/operation/ObjectMapIterate.h" +#include "librbd/operation/RebuildObjectMapRequest.h" #include "librbd/operation/RenameRequest.h" #include "librbd/operation/ResizeRequest.h" #include "librbd/operation/SnapshotCreateRequest.h" @@ -1296,6 +1298,123 @@ void Operations::execute_update_features(uint64_t features, bool enabled, } } +template +int Operations::metadata_set(const std::string &key, + const std::string &value) { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 5) << this << " " << __func__ << ": key=" << key << ", value=" + << value << dendl; + + string start = m_image_ctx.METADATA_CONF_PREFIX; + size_t conf_prefix_len = start.size(); + + if (key.size() > conf_prefix_len && !key.compare(0, conf_prefix_len, start)) { + string subkey = key.substr(conf_prefix_len, key.size() - conf_prefix_len); + int r = cct->_conf->set_val(subkey.c_str(), value); + if (r < 0) { + return r; + } + } + + int r = m_image_ctx.state->refresh_if_required(); + if (r < 0) { + return r; + } + + if (m_image_ctx.read_only) { + return -EROFS; + } + + { + RWLock::RLocker owner_lock(m_image_ctx.owner_lock); + C_SaferCond metadata_ctx; + + if (m_image_ctx.exclusive_lock != nullptr && + !m_image_ctx.exclusive_lock->is_lock_owner()) { + C_SaferCond lock_ctx; + + m_image_ctx.exclusive_lock->request_lock(&lock_ctx); + r = lock_ctx.wait(); + if (r < 0) { + return r; + } + } + + execute_metadata_set(key, value, &metadata_ctx); + r = metadata_ctx.wait(); + } + + return r; +} + +template +void Operations::execute_metadata_set(const std::string &key, + const std::string &value, + Context *on_finish) { + assert(m_image_ctx.owner_lock.is_locked()); + + CephContext *cct = m_image_ctx.cct; + ldout(cct, 5) << this << " " << __func__ << ": key=" << key << ", value=" + << value << dendl; + + operation::MetadataSetRequest *request = + new operation::MetadataSetRequest(m_image_ctx, on_finish, key, value); + request->send(); +} + +template +int Operations::metadata_remove(const std::string &key) { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 5) << this << " " << __func__ << ": key=" << key << dendl; + + if (m_image_ctx.read_only) { + return -EROFS; + } + + int r = m_image_ctx.state->refresh_if_required(); + if (r < 0) { + return r; + } + + if (m_image_ctx.read_only) { + return -EROFS; + } + + { + RWLock::RLocker owner_lock(m_image_ctx.owner_lock); + C_SaferCond metadata_ctx; + + if (m_image_ctx.exclusive_lock != nullptr && + !m_image_ctx.exclusive_lock->is_lock_owner()) { + C_SaferCond lock_ctx; + + m_image_ctx.exclusive_lock->request_lock(&lock_ctx); + r = lock_ctx.wait(); + if (r < 0) { + return r; + } + } + + execute_metadata_remove(key, &metadata_ctx); + r = metadata_ctx.wait(); + } + + return r; +} + +template +void Operations::execute_metadata_remove(const std::string &key, + Context *on_finish) { + assert(m_image_ctx.owner_lock.is_locked()); + + CephContext *cct = m_image_ctx.cct; + ldout(cct, 5) << this << " " << __func__ << ": key=" << key << dendl; + + operation::MetadataRemoveRequest *request = + new operation::MetadataRemoveRequest(m_image_ctx, on_finish, key); + request->send(); +} + template int Operations::prepare_image_update() { assert(m_image_ctx.owner_lock.is_locked() && diff --git a/src/librbd/Operations.h b/src/librbd/Operations.h index 64ec781e6da23..6790ffeac3f31 100644 --- a/src/librbd/Operations.h +++ b/src/librbd/Operations.h @@ -74,6 +74,13 @@ class Operations { void execute_update_features(uint64_t features, bool enabled, Context *on_finish, uint64_t journal_op_tid); + int metadata_set(const std::string &key, const std::string &value); + void execute_metadata_set(const std::string &key, const std::string &value, + Context *on_finish); + + int metadata_remove(const std::string &key); + void execute_metadata_remove(const std::string &key, Context *on_finish); + int prepare_image_update(); private: diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc index 3761702119950..fe8f0f4680f5d 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -2365,43 +2365,6 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, return cls_client::metadata_get(&ictx->md_ctx, ictx->header_oid, key, value); } - int metadata_set(ImageCtx *ictx, const string &key, const string &value) - { - CephContext *cct = ictx->cct; - string start = ictx->METADATA_CONF_PREFIX; - size_t conf_prefix_len = start.size(); - - if(key.size() > conf_prefix_len && !key.compare(0,conf_prefix_len,start)) { - string subkey = key.substr(conf_prefix_len, key.size()-conf_prefix_len); - int r = cct->_conf->set_val(subkey.c_str(), value); - if (r < 0) - return r; - } - ldout(cct, 20) << "metadata_set " << ictx << " key=" << key << " value=" << value << dendl; - - int r = ictx->state->refresh_if_required(); - if (r < 0) { - return r; - } - - map data; - data[key].append(value); - return cls_client::metadata_set(&ictx->md_ctx, ictx->header_oid, data); - } - - int metadata_remove(ImageCtx *ictx, const string &key) - { - CephContext *cct = ictx->cct; - ldout(cct, 20) << "metadata_remove " << ictx << " key=" << key << dendl; - - int r = ictx->state->refresh_if_required(); - if (r < 0) { - return r; - } - - return cls_client::metadata_remove(&ictx->md_ctx, ictx->header_oid, key); - } - int metadata_list(ImageCtx *ictx, const string &start, uint64_t max, map *pairs) { CephContext *cct = ictx->cct; diff --git a/src/librbd/internal.h b/src/librbd/internal.h index 2d6e7c5dbf358..01b95464f6ad9 100644 --- a/src/librbd/internal.h +++ b/src/librbd/internal.h @@ -195,8 +195,6 @@ namespace librbd { int poll_io_events(ImageCtx *ictx, AioCompletion **comps, int numcomp); int metadata_list(ImageCtx *ictx, const string &last, uint64_t max, map *pairs); int metadata_get(ImageCtx *ictx, const std::string &key, std::string *value); - int metadata_set(ImageCtx *ictx, const std::string &key, const std::string &value); - int metadata_remove(ImageCtx *ictx, const std::string &key); int mirror_mode_get(IoCtx& io_ctx, rbd_mirror_mode_t *mirror_mode); int mirror_mode_set(IoCtx& io_ctx, rbd_mirror_mode_t mirror_mode); diff --git a/src/librbd/journal/Replay.cc b/src/librbd/journal/Replay.cc index 7e450a26a577e..9055e0b897484 100644 --- a/src/librbd/journal/Replay.cc +++ b/src/librbd/journal/Replay.cc @@ -94,6 +94,15 @@ struct ExecuteOp : public Context { on_op_complete, event.op_tid); } + void execute(const journal::MetadataSetEvent &_) { + image_ctx.operations->execute_metadata_set(event.key, event.value, + on_op_complete); + } + + void execute(const journal::MetadataRemoveEvent &_) { + image_ctx.operations->execute_metadata_remove(event.key, on_op_complete); + } + virtual void finish(int r) override { CephContext *cct = image_ctx.cct; if (r < 0) { @@ -688,6 +697,51 @@ void Replay::handle_event(const journal::UpdateFeaturesEvent &event, op_event->on_start_ready = on_ready; } +template +void Replay::handle_event(const journal::MetadataSetEvent &event, + Context *on_ready, Context *on_safe) { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 20) << ": Metadata set event" << dendl; + + Mutex::Locker locker(m_lock); + OpEvent *op_event; + Context *on_op_complete = create_op_context_callback(event.op_tid, on_ready, + on_safe, &op_event); + if (on_op_complete == nullptr) { + return; + } + + op_event->on_op_finish_event = new C_RefreshIfRequired( + m_image_ctx, new ExecuteOp( + m_image_ctx, event, on_op_complete)); + + on_ready->complete(0); +} + +template +void Replay::handle_event(const journal::MetadataRemoveEvent &event, + Context *on_ready, Context *on_safe) { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 20) << ": Metadata remove event" << dendl; + + Mutex::Locker locker(m_lock); + OpEvent *op_event; + Context *on_op_complete = create_op_context_callback(event.op_tid, on_ready, + on_safe, &op_event); + if (on_op_complete == nullptr) { + return; + } + + op_event->on_op_finish_event = new C_RefreshIfRequired( + m_image_ctx, new ExecuteOp( + m_image_ctx, event, on_op_complete)); + + // ignore errors caused due to replay + op_event->ignore_error_codes = {-ENOENT}; + + on_ready->complete(0); +} + template void Replay::handle_event(const journal::UnknownEvent &event, Context *on_ready, Context *on_safe) { diff --git a/src/librbd/journal/Replay.h b/src/librbd/journal/Replay.h index 468ed32fc3eee..5b7b7e07925dc 100644 --- a/src/librbd/journal/Replay.h +++ b/src/librbd/journal/Replay.h @@ -161,6 +161,10 @@ class Replay { Context *on_safe); void handle_event(const UpdateFeaturesEvent &event, Context *on_ready, Context *on_safe); + void handle_event(const MetadataSetEvent &event, Context *on_ready, + Context *on_safe); + void handle_event(const MetadataRemoveEvent &event, Context *on_ready, + Context *on_safe); void handle_event(const UnknownEvent &event, Context *on_ready, Context *on_safe); diff --git a/src/librbd/journal/Types.cc b/src/librbd/journal/Types.cc index c62e70d027ccb..bdd294f467b49 100644 --- a/src/librbd/journal/Types.cc +++ b/src/librbd/journal/Types.cc @@ -247,6 +247,39 @@ void UpdateFeaturesEvent::dump(Formatter *f) const { f->dump_bool("enabled", enabled); } +void MetadataSetEvent::encode(bufferlist& bl) const { + OpEventBase::encode(bl); + ::encode(key, bl); + ::encode(value, bl); +} + +void MetadataSetEvent::decode(__u8 version, bufferlist::iterator& it) { + OpEventBase::decode(version, it); + ::decode(key, it); + ::decode(value, it); +} + +void MetadataSetEvent::dump(Formatter *f) const { + OpEventBase::dump(f); + f->dump_string("key", key); + f->dump_string("value", value); +} + +void MetadataRemoveEvent::encode(bufferlist& bl) const { + OpEventBase::encode(bl); + ::encode(key, bl); +} + +void MetadataRemoveEvent::decode(__u8 version, bufferlist::iterator& it) { + OpEventBase::decode(version, it); + ::decode(key, it); +} + +void MetadataRemoveEvent::dump(Formatter *f) const { + OpEventBase::dump(f); + f->dump_string("key", key); +} + void UnknownEvent::encode(bufferlist& bl) const { assert(false); } @@ -320,6 +353,12 @@ void EventEntry::decode(bufferlist::iterator& it) { case EVENT_TYPE_UPDATE_FEATURES: event = UpdateFeaturesEvent(); break; + case EVENT_TYPE_METADATA_SET: + event = MetadataSetEvent(); + break; + case EVENT_TYPE_METADATA_REMOVE: + event = MetadataRemoveEvent(); + break; default: event = UnknownEvent(); break; @@ -376,6 +415,12 @@ void EventEntry::generate_test_instances(std::list &o) { o.push_back(new EventEntry(UpdateFeaturesEvent())); o.push_back(new EventEntry(UpdateFeaturesEvent(123, 127, true))); + + o.push_back(new EventEntry(MetadataSetEvent())); + o.push_back(new EventEntry(MetadataSetEvent(123, "key", "value"))); + + o.push_back(new EventEntry(MetadataRemoveEvent())); + o.push_back(new EventEntry(MetadataRemoveEvent(123, "key"))); } // Journal Client @@ -630,6 +675,12 @@ std::ostream &operator<<(std::ostream &out, const EventType &type) { case EVENT_TYPE_UPDATE_FEATURES: out << "UpdateFeatures"; break; + case EVENT_TYPE_METADATA_SET: + out << "MetadataSet"; + break; + case EVENT_TYPE_METADATA_REMOVE: + out << "MetadataRemove"; + break; default: out << "Unknown (" << static_cast(type) << ")"; break; diff --git a/src/librbd/journal/Types.h b/src/librbd/journal/Types.h index 4d904e4aac716..37c590556fdc7 100644 --- a/src/librbd/journal/Types.h +++ b/src/librbd/journal/Types.h @@ -38,6 +38,8 @@ enum EventType { EVENT_TYPE_DEMOTE = 13, EVENT_TYPE_SNAP_LIMIT = 14, EVENT_TYPE_UPDATE_FEATURES = 15, + EVENT_TYPE_METADATA_SET = 16, + EVENT_TYPE_METADATA_REMOVE = 17, }; struct AioDiscardEvent { @@ -306,6 +308,39 @@ struct UpdateFeaturesEvent : public OpEventBase { void dump(Formatter *f) const; }; +struct MetadataSetEvent : public OpEventBase { + static const EventType TYPE = EVENT_TYPE_METADATA_SET; + + string key; + string value; + + MetadataSetEvent() { + } + MetadataSetEvent(uint64_t op_tid, const string &_key, const string &_value) + : OpEventBase(op_tid), key(_key), value(_value) { + } + + void encode(bufferlist& bl) const; + void decode(__u8 version, bufferlist::iterator& it); + void dump(Formatter *f) const; +}; + +struct MetadataRemoveEvent : public OpEventBase { + static const EventType TYPE = EVENT_TYPE_METADATA_REMOVE; + + string key; + + MetadataRemoveEvent() { + } + MetadataRemoveEvent(uint64_t op_tid, const string &_key) + : OpEventBase(op_tid), key(_key) { + } + + void encode(bufferlist& bl) const; + void decode(__u8 version, bufferlist::iterator& it); + void dump(Formatter *f) const; +}; + struct UnknownEvent { static const EventType TYPE = static_cast(-1); @@ -330,6 +365,8 @@ typedef boost::variant Event; struct EventEntry { diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc index ac7a3dade7b9a..ebc656a7a9d8e 100644 --- a/src/librbd/librbd.cc +++ b/src/librbd/librbd.cc @@ -1399,7 +1399,7 @@ namespace librbd { { ImageCtx *ictx = (ImageCtx *)ctx; tracepoint(librbd, metadata_set_enter, ictx, key.c_str(), value.c_str()); - int r = librbd::metadata_set(ictx, key, value); + int r = ictx->operations->metadata_set(key, value); tracepoint(librbd, metadata_set_exit, r); return r; } @@ -1408,7 +1408,7 @@ namespace librbd { { ImageCtx *ictx = (ImageCtx *)ctx; tracepoint(librbd, metadata_remove_enter, ictx, key.c_str()); - int r = librbd::metadata_remove(ictx, key); + int r = ictx->operations->metadata_remove(key); tracepoint(librbd, metadata_remove_exit, r); return r; } @@ -2882,7 +2882,7 @@ extern "C" int rbd_metadata_set(rbd_image_t image, const char *key, const char * { librbd::ImageCtx *ictx = (librbd::ImageCtx *)image; tracepoint(librbd, metadata_set_enter, ictx, key, value); - int r = librbd::metadata_set(ictx, key, value); + int r = ictx->operations->metadata_set(key, value); tracepoint(librbd, metadata_set_exit, r); return r; } @@ -2891,7 +2891,7 @@ extern "C" int rbd_metadata_remove(rbd_image_t image, const char *key) { librbd::ImageCtx *ictx = (librbd::ImageCtx *)image; tracepoint(librbd, metadata_remove_enter, ictx, key); - int r = librbd::metadata_remove(ictx, key); + int r = ictx->operations->metadata_remove(key); tracepoint(librbd, metadata_remove_exit, r); return r; } diff --git a/src/librbd/operation/MetadataRemoveRequest.cc b/src/librbd/operation/MetadataRemoveRequest.cc new file mode 100644 index 0000000000000..3c5a7448dd209 --- /dev/null +++ b/src/librbd/operation/MetadataRemoveRequest.cc @@ -0,0 +1,60 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "librbd/operation/MetadataRemoveRequest.h" +#include "common/dout.h" +#include "common/errno.h" +#include "librbd/ImageCtx.h" + +#define dout_subsys ceph_subsys_rbd +#undef dout_prefix +#define dout_prefix *_dout << "librbd::MetadataRemoveRequest: " + +namespace librbd { +namespace operation { + +template +MetadataRemoveRequest::MetadataRemoveRequest(I &image_ctx, + Context *on_finish, + const std::string &key) + : Request(image_ctx, on_finish), m_key(key) { +} + +template +void MetadataRemoveRequest::send_op() { + send_metadata_remove(); +} + +template +bool MetadataRemoveRequest::should_complete(int r) { + I &image_ctx = this->m_image_ctx; + CephContext *cct = image_ctx.cct; + ldout(cct, 20) << this << " " << __func__ << "r=" << r << dendl; + + if (r < 0) { + lderr(cct) << "encountered error: " << cpp_strerror(r) << dendl; + } + return true; +} + +template +void MetadataRemoveRequest::send_metadata_remove() { + I &image_ctx = this->m_image_ctx; + assert(image_ctx.owner_lock.is_locked()); + + CephContext *cct = image_ctx.cct; + ldout(cct, 20) << this << " " << __func__ << dendl; + + librados::ObjectWriteOperation op; + cls_client::metadata_remove(&op, m_key); + + librados::AioCompletion *comp = this->create_callback_completion(); + int r = image_ctx.md_ctx.aio_operate(image_ctx.header_oid, comp, &op); + assert(r == 0); + comp->release(); +} + +} // namespace operation +} // namespace librbd + +template class librbd::operation::MetadataRemoveRequest; diff --git a/src/librbd/operation/MetadataRemoveRequest.h b/src/librbd/operation/MetadataRemoveRequest.h new file mode 100644 index 0000000000000..0b51e11bf39ea --- /dev/null +++ b/src/librbd/operation/MetadataRemoveRequest.h @@ -0,0 +1,44 @@ +// -*- mode:C++; tab-width:8; c-basic-offremove:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#ifndef CEPH_LIBRBD_OPERATION_METADATA_REMOVE_REQUEST_H +#define CEPH_LIBRBD_OPERATION_METADATA_REMOVE_REQUEST_H + +#include "librbd/operation/Request.h" +#include +#include + +class Context; + +namespace librbd { + +class ImageCtx; + +namespace operation { + +template +class MetadataRemoveRequest : public Request { +public: + MetadataRemoveRequest(ImageCtxT &image_ctx, Context *on_finish, + const std::string &key); + +protected: + virtual void send_op(); + virtual bool should_complete(int r); + + virtual journal::Event create_event(uint64_t op_tid) const { + return journal::MetadataRemoveEvent(op_tid, m_key); + } + +private: + std::string m_key; + + void send_metadata_remove(); +}; + +} // namespace operation +} // namespace librbd + +extern template class librbd::operation::MetadataRemoveRequest; + +#endif // CEPH_LIBRBD_OPERATION_METADATA_REMOVE_REQUEST_H diff --git a/src/librbd/operation/MetadataSetRequest.cc b/src/librbd/operation/MetadataSetRequest.cc new file mode 100644 index 0000000000000..0142d827c9593 --- /dev/null +++ b/src/librbd/operation/MetadataSetRequest.cc @@ -0,0 +1,62 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "librbd/operation/MetadataSetRequest.h" +#include "common/dout.h" +#include "common/errno.h" +#include "librbd/ImageCtx.h" + +#define dout_subsys ceph_subsys_rbd +#undef dout_prefix +#define dout_prefix *_dout << "librbd::MetadataSetRequest: " + +namespace librbd { +namespace operation { + +template +MetadataSetRequest::MetadataSetRequest(I &image_ctx, + Context *on_finish, + const std::string &key, + const std::string &value) + : Request(image_ctx, on_finish), m_key(key), m_value(value) { +} + +template +void MetadataSetRequest::send_op() { + send_metadata_set(); +} + +template +bool MetadataSetRequest::should_complete(int r) { + I &image_ctx = this->m_image_ctx; + CephContext *cct = image_ctx.cct; + ldout(cct, 20) << this << " " << __func__ << "r=" << r << dendl; + + if (r < 0) { + lderr(cct) << "encountered error: " << cpp_strerror(r) << dendl; + } + return true; +} + +template +void MetadataSetRequest::send_metadata_set() { + I &image_ctx = this->m_image_ctx; + assert(image_ctx.owner_lock.is_locked()); + + CephContext *cct = image_ctx.cct; + ldout(cct, 20) << this << " " << __func__ << dendl; + + m_data[m_key].append(m_value); + librados::ObjectWriteOperation op; + cls_client::metadata_set(&op, m_data); + + librados::AioCompletion *comp = this->create_callback_completion(); + int r = image_ctx.md_ctx.aio_operate(image_ctx.header_oid, comp, &op); + assert(r == 0); + comp->release(); +} + +} // namespace operation +} // namespace librbd + +template class librbd::operation::MetadataSetRequest; diff --git a/src/librbd/operation/MetadataSetRequest.h b/src/librbd/operation/MetadataSetRequest.h new file mode 100644 index 0000000000000..d6abc3b3cc4e4 --- /dev/null +++ b/src/librbd/operation/MetadataSetRequest.h @@ -0,0 +1,47 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#ifndef CEPH_LIBRBD_OPERATION_METADATA_SET_REQUEST_H +#define CEPH_LIBRBD_OPERATION_METADATA_SET_REQUEST_H + +#include "librbd/operation/Request.h" +#include "include/buffer.h" +#include +#include + +class Context; + +namespace librbd { + +class ImageCtx; + +namespace operation { + +template +class MetadataSetRequest : public Request { +public: + MetadataSetRequest(ImageCtxT &image_ctx, Context *on_finish, + const std::string &key, const std::string &value); + +protected: + virtual void send_op(); + virtual bool should_complete(int r); + + virtual journal::Event create_event(uint64_t op_tid) const { + return journal::MetadataSetEvent(op_tid, m_key, m_value); + } + +private: + std::string m_key; + std::string m_value; + std::map m_data; + + void send_metadata_set(); +}; + +} // namespace operation +} // namespace librbd + +extern template class librbd::operation::MetadataSetRequest; + +#endif // CEPH_LIBRBD_OPERATION_METADATA_SET_REQUEST_H diff --git a/src/test/librbd/journal/test_Replay.cc b/src/test/librbd/journal/test_Replay.cc index d54267e1c8478..4622c2789d006 100644 --- a/src/test/librbd/journal/test_Replay.cc +++ b/src/test/librbd/journal/test_Replay.cc @@ -680,6 +680,80 @@ TEST_F(TestJournalReplay, UpdateFeatures) { ASSERT_EQ(0, ictx->operations->update_features(features, !enabled)); } +TEST_F(TestJournalReplay, MetadataSet) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + // get current commit position + int64_t initial_tag; + int64_t initial_entry; + get_journal_commit_position(ictx, &initial_tag, &initial_entry); + + // inject metadata_set op into journal + inject_into_journal(ictx, librbd::journal::MetadataSetEvent(1, "key", "value")); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + close_image(ictx); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current_tag; + int64_t current_entry; + get_journal_commit_position(ictx, ¤t_tag, ¤t_entry); + ASSERT_EQ(initial_tag + 1, current_tag); + ASSERT_EQ(1, current_entry); + + std::string value; + ASSERT_EQ(0, librbd::metadata_get(ictx, "key", &value)); + ASSERT_EQ("value", value); + + // verify lock ordering constraints + ASSERT_EQ(0, ictx->operations->metadata_set("key2", "value")); +} + +TEST_F(TestJournalReplay, MetadataRemove) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + ASSERT_EQ(0, ictx->operations->metadata_set("key", "value")); + + // get current commit position + int64_t initial_tag; + int64_t initial_entry; + get_journal_commit_position(ictx, &initial_tag, &initial_entry); + + // inject metadata_remove op into journal + inject_into_journal(ictx, librbd::journal::MetadataRemoveEvent(1, "key")); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + close_image(ictx); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current_tag; + int64_t current_entry; + get_journal_commit_position(ictx, ¤t_tag, ¤t_entry); + ASSERT_EQ(initial_tag, current_tag); + ASSERT_EQ(initial_entry + 2, current_entry); + + std::string value; + ASSERT_EQ(-ENOENT, librbd::metadata_get(ictx, "key", &value)); + + // verify lock ordering constraints + ASSERT_EQ(0, ictx->operations->metadata_set("key", "value")); + ASSERT_EQ(0, ictx->operations->metadata_remove("key")); +} + TEST_F(TestJournalReplay, ObjectPosition) { REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); diff --git a/src/test/librbd/journal/test_mock_Replay.cc b/src/test/librbd/journal/test_mock_Replay.cc index df38b9af058e3..92250645214c7 100644 --- a/src/test/librbd/journal/test_mock_Replay.cc +++ b/src/test/librbd/journal/test_mock_Replay.cc @@ -204,6 +204,22 @@ class TestMockJournalReplay : public TestMockFixture { NotifyInvoke(&m_invoke_lock, &m_invoke_cond))); } + void expect_metadata_set(MockReplayImageCtx &mock_image_ctx, + Context **on_finish, const char *key, + const char *value) { + EXPECT_CALL(*mock_image_ctx.operations, execute_metadata_set(StrEq(key), + StrEq(value), _)) + .WillOnce(DoAll(SaveArg<2>(on_finish), + NotifyInvoke(&m_invoke_lock, &m_invoke_cond))); + } + + void expect_metadata_remove(MockReplayImageCtx &mock_image_ctx, + Context **on_finish, const char *key) { + EXPECT_CALL(*mock_image_ctx.operations, execute_metadata_remove(StrEq(key), _)) + .WillOnce(DoAll(SaveArg<1>(on_finish), + NotifyInvoke(&m_invoke_lock, &m_invoke_cond))); + } + void expect_refresh_image(MockReplayImageCtx &mock_image_ctx, bool required, int r) { EXPECT_CALL(*mock_image_ctx.state, is_refresh_required()) @@ -1290,6 +1306,102 @@ TEST_F(TestMockJournalReplay, UpdateFeaturesEvent) { ASSERT_EQ(0, on_finish_safe.wait()); } +TEST_F(TestMockJournalReplay, MetadataSetEvent) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockReplayImageCtx mock_image_ctx(*ictx); + MockJournalReplay mock_journal_replay(mock_image_ctx); + expect_op_work_queue(mock_image_ctx); + + InSequence seq; + Context *on_finish = nullptr; + expect_refresh_image(mock_image_ctx, false, 0); + expect_metadata_set(mock_image_ctx, &on_finish, "key", "value"); + + C_SaferCond on_start_ready; + C_SaferCond on_start_safe; + when_process(mock_journal_replay, EventEntry{MetadataSetEvent(123, "key", "value")}, + &on_start_ready, &on_start_safe); + ASSERT_EQ(0, on_start_ready.wait()); + + C_SaferCond on_finish_ready; + C_SaferCond on_finish_safe; + when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)}, + &on_finish_ready, &on_finish_safe); + + wait_for_op_invoked(&on_finish, 0); + ASSERT_EQ(0, on_start_safe.wait()); + ASSERT_EQ(0, on_finish_ready.wait()); + ASSERT_EQ(0, on_finish_safe.wait()); +} + +TEST_F(TestMockJournalReplay, MetadataRemoveEvent) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockReplayImageCtx mock_image_ctx(*ictx); + MockJournalReplay mock_journal_replay(mock_image_ctx); + expect_op_work_queue(mock_image_ctx); + + InSequence seq; + Context *on_finish = nullptr; + expect_refresh_image(mock_image_ctx, false, 0); + expect_metadata_remove(mock_image_ctx, &on_finish, "key"); + + C_SaferCond on_start_ready; + C_SaferCond on_start_safe; + when_process(mock_journal_replay, EventEntry{MetadataRemoveEvent(123, "key")}, + &on_start_ready, &on_start_safe); + ASSERT_EQ(0, on_start_ready.wait()); + + C_SaferCond on_finish_ready; + C_SaferCond on_finish_safe; + when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)}, + &on_finish_ready, &on_finish_safe); + + wait_for_op_invoked(&on_finish, 0); + ASSERT_EQ(0, on_start_safe.wait()); + ASSERT_EQ(0, on_finish_ready.wait()); + ASSERT_EQ(0, on_finish_safe.wait()); +} + +TEST_F(TestMockJournalReplay, MetadataRemoveEventDNE) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockReplayImageCtx mock_image_ctx(*ictx); + MockJournalReplay mock_journal_replay(mock_image_ctx); + expect_op_work_queue(mock_image_ctx); + + InSequence seq; + Context *on_finish = nullptr; + expect_refresh_image(mock_image_ctx, false, 0); + expect_metadata_remove(mock_image_ctx, &on_finish, "key"); + + C_SaferCond on_start_ready; + C_SaferCond on_start_safe; + when_process(mock_journal_replay, EventEntry{MetadataRemoveEvent(123, "key")}, + &on_start_ready, &on_start_safe); + ASSERT_EQ(0, on_start_ready.wait()); + + C_SaferCond on_finish_ready; + C_SaferCond on_finish_safe; + when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)}, + &on_finish_ready, &on_finish_safe); + + wait_for_op_invoked(&on_finish, -ENOENT); + ASSERT_EQ(0, on_start_safe.wait()); + ASSERT_EQ(0, on_finish_ready.wait()); + ASSERT_EQ(0, on_finish_safe.wait()); +} + TEST_F(TestMockJournalReplay, UnknownEvent) { REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); diff --git a/src/test/librbd/mock/MockOperations.h b/src/test/librbd/mock/MockOperations.h index f026bc4b5ec10..94e39639e8332 100644 --- a/src/test/librbd/mock/MockOperations.h +++ b/src/test/librbd/mock/MockOperations.h @@ -49,6 +49,11 @@ struct MockOperations { MOCK_METHOD4(execute_update_features, void(uint64_t features, bool enabled, Context *on_finish, uint64_t journal_op_tid)); + MOCK_METHOD3(execute_metadata_set, void(const std::string &key, + const std::string &value, + Context *on_finish)); + MOCK_METHOD2(execute_metadata_remove, void(const std::string &key, + Context *on_finish)); }; } // namespace librbd diff --git a/src/test/librbd/test_internal.cc b/src/test/librbd/test_internal.cc index 798009aefcacb..32de41a7efa8e 100644 --- a/src/test/librbd/test_internal.cc +++ b/src/test/librbd/test_internal.cc @@ -408,25 +408,25 @@ TEST_F(TestInternal, Metadata) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - r = librbd::metadata_set(ictx, it->first, "value1"); + r = ictx->operations->metadata_set(it->first, "value1"); ASSERT_EQ(0, r); ++it; - r = librbd::metadata_set(ictx, it->first, "value2"); + r = ictx->operations->metadata_set(it->first, "value2"); ASSERT_EQ(0, r); ++it; - r = librbd::metadata_set(ictx, it->first, "value3"); + r = ictx->operations->metadata_set(it->first, "value3"); ASSERT_EQ(0, r); - r = librbd::metadata_set(ictx, "abcd", "value4"); + r = ictx->operations->metadata_set("abcd", "value4"); ASSERT_EQ(0, r); - r = librbd::metadata_set(ictx, "xyz", "value5"); + r = ictx->operations->metadata_set("xyz", "value5"); ASSERT_EQ(0, r); map pairs; r = librbd::metadata_list(ictx, "", 0, &pairs); ASSERT_EQ(0, r); ASSERT_EQ(5u, pairs.size()); - r = librbd::metadata_remove(ictx, "abcd"); + r = ictx->operations->metadata_remove("abcd"); ASSERT_EQ(0, r); - r = librbd::metadata_remove(ictx, "xyz"); + r = ictx->operations->metadata_remove("xyz"); ASSERT_EQ(0, r); pairs.clear(); r = librbd::metadata_list(ictx, "", 0, &pairs); 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..3bb09fb24fa1f 100644 --- a/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc +++ b/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc @@ -472,8 +472,8 @@ TEST_F(TestMockImageSyncObjectCopyRequest, WriteSnaps) { } TEST_F(TestMockImageSyncObjectCopyRequest, Trim) { - ASSERT_EQ(0, metadata_set(m_remote_image_ctx, - "conf_rbd_skip_partial_discard", "false")); + ASSERT_EQ(0, m_remote_image_ctx->operations->metadata_set( + "conf_rbd_skip_partial_discard", "false")); // scribble some data interval_set one; scribble(m_remote_image_ctx, 10, 102400, &one); diff --git a/src/test/rbd_mirror/test_ImageReplayer.cc b/src/test/rbd_mirror/test_ImageReplayer.cc index 4c39fd1b2bcb5..24d0709999754 100644 --- a/src/test/rbd_mirror/test_ImageReplayer.cc +++ b/src/test/rbd_mirror/test_ImageReplayer.cc @@ -1005,3 +1005,48 @@ TEST_F(TestImageReplayer, UpdateFeatures) stop(); } + +TEST_F(TestImageReplayer, MetadataSetRemove) +{ + const std::string KEY = "test_key"; + const std::string VALUE = "test_value"; + + librbd::ImageCtx *ictx; + std::string value; + + bootstrap(); + + start(); + + // Test metadata_set replication + + open_remote_image(&ictx); + ASSERT_EQ(0, ictx->operations->metadata_set(KEY, VALUE)); + value.clear(); + ASSERT_EQ(0, librbd::metadata_get(ictx, KEY, &value)); + ASSERT_EQ(VALUE, value); + close_image(ictx); + + wait_for_replay_complete(); + + open_local_image(&ictx); + value.clear(); + ASSERT_EQ(0, librbd::metadata_get(ictx, KEY, &value)); + ASSERT_EQ(VALUE, value); + close_image(ictx); + + // Test metadata_remove replication + + open_remote_image(&ictx); + ASSERT_EQ(0, ictx->operations->metadata_remove(KEY)); + ASSERT_EQ(-ENOENT, librbd::metadata_get(ictx, KEY, &value)); + close_image(ictx); + + wait_for_replay_complete(); + + open_local_image(&ictx); + ASSERT_EQ(-ENOENT, librbd::metadata_get(ictx, KEY, &value)); + close_image(ictx); + + stop(); +}