In iSCSI scenario (via tcmu-runner), after a client has acquired exclusive
lock on an image, users will no longer be able to set metadata on the image.
This commit try to fix it by forward the request to the lock owner.
Steps to reproduce:
1. Client A:
```
>>> import rbd, rados;client=rados.Rados(conffile='');client.connect();ioctx=client.open_ioctx('rbd');rbd_inst=rbd.RBD();image=rbd.Image(ioctx, 'img1')
>>> from rbd import RBD_LOCK_MODE_EXCLUSIVE
>>> image.lock_acquire(RBD_LOCK_MODE_EXCLUSIVE)
>>>
```
2. Client B:
```
$ rbd image-meta set img1 conf_rbd_qos_iops_limit 10000
2020-09-12T15:19:58.325+0800
7f161affd700 -1 librbd::ManagedLock: 0x7f15f4001d48 handle_acquire_lock: failed to acquire exclusive lock:(30) Read-only file system
failed to set metadata conf_rbd_qos_iops_limit of image : (30) Read-only file system
rbd: setting metadata failed: (30) Read-only file system
$
```
Signed-off-by: songweibin <song.weibin@zte.com.cn>
send_notify(new UnquiescePayload(async_request_id), on_finish);
}
+template <typename I>
+void ImageWatcher<I>::notify_metadata_set(const std::string &key,
+ const std::string &value,
+ Context *on_finish) {
+ ceph_assert(ceph_mutex_is_locked(m_image_ctx.owner_lock));
+ ceph_assert(m_image_ctx.exclusive_lock &&
+ !m_image_ctx.exclusive_lock->is_lock_owner());
+
+ notify_lock_owner(new MetadataUpdatePayload(key,
+ std::optional<std::string>{value}),
+ on_finish);
+}
+
+template <typename I>
+void ImageWatcher<I>::notify_metadata_remove(const std::string &key,
+ Context *on_finish) {
+ ceph_assert(ceph_mutex_is_locked(m_image_ctx.owner_lock));
+ ceph_assert(m_image_ctx.exclusive_lock &&
+ !m_image_ctx.exclusive_lock->is_lock_owner());
+
+ notify_lock_owner(new MetadataUpdatePayload(key, std::nullopt),
+ on_finish);
+}
+
template <typename I>
void ImageWatcher<I>::schedule_cancel_async_requests() {
auto ctx = new LambdaContext(
return true;
}
+template <typename I>
+bool ImageWatcher<I>::handle_payload(const MetadataUpdatePayload &payload,
+ C_NotifyAck *ack_ctx) {
+ std::shared_lock l{m_image_ctx.owner_lock};
+ if (m_image_ctx.exclusive_lock != nullptr) {
+ int r;
+ if (m_image_ctx.exclusive_lock->accept_request(
+ exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, &r)) {
+ if (payload.value) {
+ ldout(m_image_ctx.cct, 10) << this << " remote metadata_set request: key="
+ << payload.key << ", value="
+ << *payload.value << dendl;
+
+ m_image_ctx.operations->execute_metadata_set(payload.key, *payload.value,
+ new C_ResponseMessage(ack_ctx));
+ return false;
+ } else {
+ ldout(m_image_ctx.cct, 10) << this << " remote metadata_remove request: key="
+ << payload.key << dendl;
+
+ m_image_ctx.operations->execute_metadata_remove(payload.key,
+ new C_ResponseMessage(ack_ctx));
+ return false;
+ }
+ } else if (r < 0) {
+ encode(ResponseMessage(r), ack_ctx->out);
+ }
+ }
+ return true;
+}
+
template <typename I>
bool ImageWatcher<I>::handle_payload(const UnknownPayload &payload,
C_NotifyAck *ack_ctx) {
case NOTIFY_OP_UNQUIESCE:
complete = handle_payload(*(static_cast<UnquiescePayload *>(payload)), ctx);
break;
+ case NOTIFY_OP_METADATA_UPDATE:
+ complete = handle_payload(*(static_cast<MetadataUpdatePayload *>(payload)), ctx);
+ break;
default:
ceph_assert(payload->get_notify_op() == static_cast<NotifyOp>(-1));
complete = handle_payload(*(static_cast<UnknownPayload *>(payload)), ctx);
Context *on_finish);
void notify_unquiesce(uint64_t request_id, Context *on_finish);
+ void notify_metadata_set(const std::string &key, const std::string &value,
+ Context *on_finish);
+ void notify_metadata_remove(const std::string &key, Context *on_finish);
+
private:
enum TaskCode {
TASK_CODE_REQUEST_LOCK,
C_NotifyAck *ctx);
bool handle_payload(const watch_notify::UnquiescePayload& payload,
C_NotifyAck *ctx);
+ bool handle_payload(const watch_notify::MetadataUpdatePayload& payload,
+ C_NotifyAck *ctx);
bool handle_payload(const watch_notify::UnknownPayload& payload,
C_NotifyAck *ctx);
void process_payload(uint64_t notify_id, uint64_t handle,
return -EROFS;
}
- C_SaferCond metadata_ctx;
- {
- std::shared_lock owner_lock{m_image_ctx.owner_lock};
- r = prepare_image_update(exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL,
- true);
- if (r < 0) {
- return r;
- }
-
- execute_metadata_set(key, value, &metadata_ctx);
- }
+ r = invoke_async_request("metadata_set",
+ exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL,
+ true,
+ boost::bind(&Operations<I>::execute_metadata_set,
+ this, key, value, _1),
+ boost::bind(&ImageWatcher<I>::notify_metadata_set,
+ m_image_ctx.image_watcher,
+ key, value, _1));
- r = metadata_ctx.wait();
if (config_override && r >= 0) {
// apply new config key immediately
r = m_image_ctx.state->refresh_if_required();
}
+ ldout(cct, 20) << "metadata_set finished" << dendl;
return r;
}
if(r < 0)
return r;
- C_SaferCond metadata_ctx;
- {
- std::shared_lock owner_lock{m_image_ctx.owner_lock};
- r = prepare_image_update(exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL,
- true);
- if (r < 0) {
- return r;
- }
-
- execute_metadata_remove(key, &metadata_ctx);
+ r = invoke_async_request("metadata_remove",
+ exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL,
+ true,
+ boost::bind(&Operations<I>::execute_metadata_remove,
+ this, key, _1),
+ boost::bind(&ImageWatcher<I>::notify_metadata_remove,
+ m_image_ctx.image_watcher, key, _1));
+ if (r == -ENOENT) {
+ r = 0;
}
- r = metadata_ctx.wait();
-
std::string config_key;
if (util::is_metadata_config_override(key, &config_key) && r >= 0) {
// apply new config key immediately
r = m_image_ctx.state->refresh_if_required();
}
+ ldout(cct, 20) << "metadata_remove finished" << dendl;
return r;
}
f->dump_unsigned("sparse_size", sparse_size);
}
+void MetadataUpdatePayload::encode(bufferlist &bl) const {
+ using ceph::encode;
+ encode(key, bl);
+ encode(value, bl);
+}
+
+void MetadataUpdatePayload::decode(__u8 version, bufferlist::const_iterator &iter) {
+ using ceph::decode;
+ decode(key, iter);
+ decode(value, iter);
+}
+
+void MetadataUpdatePayload::dump(Formatter *f) const {
+ f->dump_string("key", key);
+ f->dump_string("value", *value);
+}
+
void UnknownPayload::encode(bufferlist &bl) const {
ceph_abort();
}
case NOTIFY_OP_UNQUIESCE:
payload.reset(new UnquiescePayload());
break;
+ case NOTIFY_OP_METADATA_UPDATE:
+ payload.reset(new MetadataUpdatePayload());
+ break;
}
payload->decode(struct_v, iter);
o.push_back(new NotifyMessage(new SparsifyPayload(AsyncRequestId(ClientId(0, 1), 2), 1)));
o.push_back(new NotifyMessage(new QuiescePayload(AsyncRequestId(ClientId(0, 1), 2))));
o.push_back(new NotifyMessage(new UnquiescePayload(AsyncRequestId(ClientId(0, 1), 2))));
+ o.push_back(new NotifyMessage(new MetadataUpdatePayload("foo", std::optional<std::string>{"xyz"})));
}
void ResponseMessage::encode(bufferlist& bl) const {
case NOTIFY_OP_UNQUIESCE:
out << "Unquiesce";
break;
+ case NOTIFY_OP_METADATA_UPDATE:
+ out << "MetadataUpdate";
+ break;
default:
out << "Unknown (" << static_cast<uint32_t>(op) << ")";
break;
NOTIFY_OP_SPARSIFY = 17,
NOTIFY_OP_QUIESCE = 18,
NOTIFY_OP_UNQUIESCE = 19,
+ NOTIFY_OP_METADATA_UPDATE = 20,
};
struct Payload {
}
};
+struct MetadataUpdatePayload : public Payload {
+ std::string key;
+ std::optional<std::string> value;
+ MetadataUpdatePayload() {}
+ MetadataUpdatePayload(std::string key, std::optional<std::string> value)
+ : key(key), value(value) {}
+
+ NotifyOp get_notify_op() const override {
+ return NOTIFY_OP_METADATA_UPDATE;
+ }
+ bool check_for_refresh() const override {
+ return false;
+ }
+
+ void encode(bufferlist &bl) const;
+ void decode(__u8 version, bufferlist::const_iterator &iter);
+ void dump(Formatter *f) const;
+};
+
struct UnknownPayload : public Payload {
NotifyOp get_notify_op() const override {
return static_cast<NotifyOp>(-1);
projects / ceph.git / commitdiff