From ad295e90a6b5dc55e66e71a356bff2730a379a08 Mon Sep 17 00:00:00 2001 From: Patrick Donnelly Date: Tue, 1 Aug 2023 14:51:21 -0400 Subject: [PATCH] osd/OSDMonitor: check svc is writeable before changing pending Fixes: https://tracker.ceph.com/issues/59813 Signed-off-by: Patrick Donnelly --- src/mon/AuthMonitor.cc | 10 ++++----- src/mon/AuthMonitor.h | 6 +++--- src/mon/KVMonitor.cc | 3 +++ src/mon/OSDMonitor.cc | 46 ++++++++++++++++++++++++++++++------------ src/mon/OSDMonitor.h | 4 ++-- 5 files changed, 46 insertions(+), 23 deletions(-) diff --git a/src/mon/AuthMonitor.cc b/src/mon/AuthMonitor.cc index 8cb6789394b..9fe7f409f47 100644 --- a/src/mon/AuthMonitor.cc +++ b/src/mon/AuthMonitor.cc @@ -1123,11 +1123,12 @@ int AuthMonitor::validate_osd_destroy( return 0; } -int AuthMonitor::do_osd_destroy( - const EntityName& cephx_entity, - const EntityName& lockbox_entity) +void AuthMonitor::do_osd_destroy( + const EntityName& cephx_entity, + const EntityName& lockbox_entity) { ceph_assert(paxos.is_plugged()); + ceph_assert(is_writeable()); dout(10) << __func__ << " cephx " << cephx_entity << " lockbox " << lockbox_entity << dendl; @@ -1150,14 +1151,13 @@ int AuthMonitor::do_osd_destroy( if (!removed) { dout(10) << __func__ << " entities do not exist -- no-op." << dendl; - return 0; + return; } // given we have paxos plugged, this will not result in a proposal // being triggered, but it will still be needed so that we get our // pending state encoded into the paxos' pending transaction. propose_pending(); - return 0; } int _create_auth( diff --git a/src/mon/AuthMonitor.h b/src/mon/AuthMonitor.h index 51fba994977..b0c7dbe8bb4 100644 --- a/src/mon/AuthMonitor.h +++ b/src/mon/AuthMonitor.h @@ -231,9 +231,9 @@ private: EntityName& cephx_entity, EntityName& lockbox_entity, std::stringstream& ss); - int do_osd_destroy( - const EntityName& cephx_entity, - const EntityName& lockbox_entity); + void do_osd_destroy( + const EntityName& cephx_entity, + const EntityName& lockbox_entity); int do_osd_new( const auth_entity_t& cephx_entity, diff --git a/src/mon/KVMonitor.cc b/src/mon/KVMonitor.cc index 37a81a8048d..0e9511c7c96 100644 --- a/src/mon/KVMonitor.cc +++ b/src/mon/KVMonitor.cc @@ -368,6 +368,8 @@ int KVMonitor::validate_osd_destroy( void KVMonitor::do_osd_destroy(int32_t id, uuid_d& uuid) { + ceph_assert(is_writeable()); + string dmcrypt_prefix = _get_dmcrypt_prefix(uuid, ""); string daemon_prefix = "daemon-private/osd." + stringify(id) + "/"; @@ -416,6 +418,7 @@ void KVMonitor::do_osd_new( const string& dmcrypt_key) { ceph_assert(paxos.is_plugged()); + ceph_assert(is_writeable()); string dmcrypt_key_prefix = _get_dmcrypt_prefix(uuid, "luks"); bufferlist dmcrypt_key_value; diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc index 4ad44d0309d..f3eacea69b6 100644 --- a/src/mon/OSDMonitor.cc +++ b/src/mon/OSDMonitor.cc @@ -9783,6 +9783,7 @@ static int parse_reweights(CephContext *cct, } int OSDMonitor::prepare_command_osd_destroy( + MonOpRequestRef op, int32_t id, stringstream& ss) { @@ -9814,10 +9815,11 @@ int OSDMonitor::prepare_command_osd_destroy( EntityName cephx_entity, lockbox_entity; bool idempotent_auth = false, idempotent_cks = false; - int err = mon.authmon()->validate_osd_destroy(id, uuid, - cephx_entity, - lockbox_entity, - ss); + auto&& authmon = mon.authmon(); + int err = authmon->validate_osd_destroy(id, uuid, + cephx_entity, + lockbox_entity, + ss); if (err < 0) { if (err == -ENOENT) { idempotent_auth = true; @@ -9826,21 +9828,28 @@ int OSDMonitor::prepare_command_osd_destroy( } } - auto svc = mon.kvmon(); - err = svc->validate_osd_destroy(id, uuid); + auto&& kvmon = mon.kvmon(); + err = kvmon->validate_osd_destroy(id, uuid); if (err < 0) { ceph_assert(err == -ENOENT); err = 0; idempotent_cks = true; } - if (!idempotent_auth) { - err = mon.authmon()->do_osd_destroy(cephx_entity, lockbox_entity); - ceph_assert(0 == err); + if (!idempotent_auth && !authmon->is_writeable()) { + authmon->wait_for_writeable(op, new C_RetryMessage(this, op)); + return -EAGAIN; + } + if (!idempotent_cks && !kvmon->is_writeable()) { + kvmon->wait_for_writeable(op, new C_RetryMessage(this, op)); + return -EAGAIN; } + if (!idempotent_auth) { + authmon->do_osd_destroy(cephx_entity, lockbox_entity); + } if (!idempotent_cks) { - svc->do_osd_destroy(id, uuid); + kvmon->do_osd_destroy(id, uuid); } pending_inc.new_state[id] = CEPH_OSD_DESTROYED; @@ -9855,6 +9864,7 @@ int OSDMonitor::prepare_command_osd_destroy( } int OSDMonitor::prepare_command_osd_purge( + MonOpRequestRef op, int32_t id, stringstream& ss) { @@ -9895,7 +9905,11 @@ int OSDMonitor::prepare_command_osd_purge( // no point destroying the osd again if it has already been marked destroyed if (!osdmap.is_destroyed(id)) { - err = prepare_command_osd_destroy(id, ss); + /* N.B.: up to this point, we've not changed pending at all. + * ::prepare_command_osd_destroy may return -EAGAIN if the kvmon/authmon is + * not writeable without changing pending. It will queue `op` if we should wait. + */ + err = prepare_command_osd_destroy(op, id, ss); if (err < 0) { if (err == -ENOENT) { err = 0; @@ -12669,11 +12683,17 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, paxos.plug(); if (is_destroy) { - err = prepare_command_osd_destroy(id, ss); + err = prepare_command_osd_destroy(op, id, ss); + if (err == EAGAIN) { + return false; + } // we checked above that it should exist. ceph_assert(err != -ENOENT); } else { - err = prepare_command_osd_purge(id, ss); + err = prepare_command_osd_purge(op, id, ss); + if (err == EAGAIN) { + return false; + } if (err == -ENOENT) { err = 0; ss << "osd." << id << " does not exist."; diff --git a/src/mon/OSDMonitor.h b/src/mon/OSDMonitor.h index 921a9d98b3f..3f6226c057a 100644 --- a/src/mon/OSDMonitor.h +++ b/src/mon/OSDMonitor.h @@ -699,8 +699,8 @@ public: void do_osd_create(const int32_t id, const uuid_d& uuid, const std::string& device_class, int32_t* new_id); - int prepare_command_osd_purge(int32_t id, std::stringstream& ss); - int prepare_command_osd_destroy(int32_t id, std::stringstream& ss); + int prepare_command_osd_purge(MonOpRequestRef op, int32_t id, std::stringstream& ss); + int prepare_command_osd_destroy(MonOpRequestRef op, int32_t id, std::stringstream& ss); int _prepare_command_osd_crush_remove( CrushWrapper &newcrush, int32_t id, -- 2.39.5