From: Rishabh Dave Date: Mon, 17 Nov 2025 12:21:48 +0000 (+0530) Subject: mds: add MDS code to allow snap metadata mutations X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=a7617f354eb897ce86ba0b45eafa3005c3924d47;p=ceph.git mds: add MDS code to allow snap metadata mutations Fixes: https://tracker.ceph.com/issues/66293 Signed-off-by: Rishabh Dave --- diff --git a/src/common/ceph_strings.cc b/src/common/ceph_strings.cc index 01c613b60d4..63437bd2d91 100644 --- a/src/common/ceph_strings.cc +++ b/src/common/ceph_strings.cc @@ -318,6 +318,7 @@ const char *ceph_mds_op_name(int op) case CEPH_MDS_OP_MKSNAP: return "mksnap"; case CEPH_MDS_OP_RMSNAP: return "rmsnap"; case CEPH_MDS_OP_RENAMESNAP: return "renamesnap"; + case CEPH_MDS_OP_SNAP_METADATA: return "snap_md_op"; case CEPH_MDS_OP_READDIR_SNAPDIFF: return "readdir_snapdiff"; case CEPH_MDS_OP_SETFILELOCK: return "setfilelock"; case CEPH_MDS_OP_GETFILELOCK: return "getfilelock"; diff --git a/src/include/ceph_fs.h b/src/include/ceph_fs.h index b3aca907506..d9af11a7389 100644 --- a/src/include/ceph_fs.h +++ b/src/include/ceph_fs.h @@ -429,6 +429,7 @@ enum { CEPH_MDS_OP_RENAMESNAP = 0x01403, CEPH_MDS_OP_READDIR_SNAPDIFF = 0x01404, CEPH_MDS_OP_FILE_BLOCKDIFF = 0x01405, + CEPH_MDS_OP_SNAP_METADATA = 0x1406, // internal op CEPH_MDS_OP_FRAGMENTDIR= 0x01500, diff --git a/src/mds/Server.cc b/src/mds/Server.cc index 12636a0e363..2ad6a617160 100644 --- a/src/mds/Server.cc +++ b/src/mds/Server.cc @@ -268,6 +268,8 @@ void Server::create_logger() "Request type remove snapshot latency"); plb.add_time_avg(l_mdss_req_renamesnap_latency, "req_renamesnap_latency", "Request type rename snapshot latency"); + plb.add_time_avg(l_mdss_req_snap_md_op_latency, "req_snap_md_op_latency", + "Request type snapshot metadata op latency"); plb.add_time_avg(l_mdss_req_snapdiff_latency, "req_snapdiff_latency", "Request type snapshot difference latency"); plb.add_time_avg(l_mdss_req_file_blockdiff_latency, "req_blockdiff_latency", @@ -2273,6 +2275,9 @@ void Server::perf_gather_op_latency(const cref_t &req, utime_t l case CEPH_MDS_OP_RENAMESNAP: code = l_mdss_req_renamesnap_latency; break; + case CEPH_MDS_OP_SNAP_METADATA: + code = l_mdss_req_snap_md_op_latency; + break; case CEPH_MDS_OP_READDIR_SNAPDIFF: code = l_mdss_req_snapdiff_latency; break; @@ -2937,6 +2942,9 @@ void Server::dispatch_client_request(const MDRequestRef& mdr) case CEPH_MDS_OP_RENAMESNAP: handle_client_renamesnap(mdr); break; + case CEPH_MDS_OP_SNAP_METADATA: + handle_client_snap_md_op(mdr); + break; case CEPH_MDS_OP_READDIR_SNAPDIFF: handle_client_readdir_snapdiff(mdr); break; @@ -11919,6 +11927,122 @@ void Server::_snap_mutate_generic_finish(const MDRequestRef& mdr, CInode *diri, respond_to_request(mdr, 0); } +void Server::handle_client_snap_md_op(const MDRequestRef& mdr) +{ + const cref_t &req = mdr->client_request; + + CInode* diri = rdlock_path_pin_ref(mdr, false, true); + if (!diri) + return; + + if (!diri->is_dir()) { + respond_to_request(mdr, -ENOTDIR); + return; + } + + std::string_view snapname = req->get_filepath().last_dentry(); + + if (req->get_caller_uid() < g_conf()->mds_snap_min_uid || + req->get_caller_uid() > g_conf()->mds_snap_max_uid) { + dout(20) << __func__ << " " << snapname << " on " << *diri << + " denied to uid " << req->get_caller_uid() << dendl; + respond_to_request(mdr, -EPERM); + return; + } + + dout(10) << __func__ << " for " << snapname << " on " << *diri << dendl; + // does this snap exist? + if (snapname.length() == 0 || snapname[0] == '_') { + respond_to_request(mdr, -EINVAL); + return; + } + + if (!diri->snaprealm || !diri->snaprealm->exists(snapname)) { + respond_to_request(mdr, -ENOENT); + return; + } + + string md_key; + string md_val; + // setting initial value to an invalid mode to ensure no valid mode is + // assumed by default due to any unexpected errors from any code from the + // following try-catch blocks. the invalid mode would cause + // will_md_op_succeed() to reply negatively, preventing accidental changes + // to the snap MD. + unsigned int op_flag = 3; + if (req->get_data().length()) { + try { + auto iter = req->get_data().cbegin(); + decode(md_key, iter); + decode(md_val, iter); + decode(op_flag, iter); + } catch (const ceph::buffer::error &e) { + dout(20) << __func__ << " : no metadata in payload" << dendl; + respond_to_request(mdr, -EBADMSG); + return; + } + } + + snapid_t snapid = diri->snaprealm->resolve_snapname(snapname, diri->ino()); + dout(10) << __func__ << " snapid " << snapid << dendl; + + // NOTE: check if metadata op will succeed before intiating the transaction or + // projecting the inode so that there is not need to roll back. + bool will_succeed = diri->snaprealm->will_md_op_succeed(snapid, md_key, + md_val, op_flag); + if (!will_succeed) { + dout(10) << __func__ << " will_metadata_op_succeed() failed with md_key=" + << md_key << ", md_val=" << md_val << " and op_flag=" << op_flag + << dendl; + respond_to_request(mdr, -EINVAL); + return; + } + + // get stid + if (!mdr->more()->stid) { + mds->snapclient->prepare_update(diri->ino(), snapid, snapname, utime_t(), + &mdr->more()->stid, + new C_MDS_RetryRequest(mdcache, mdr)); + return; + } + version_t stid = mdr->more()->stid; + dout(10) << __func__ << " stid = " << stid << dendl; + + // project the inode. + auto pi = diri->project_inode(mdr, false, true); + pi.inode->ctime = mdr->get_op_stamp(); + if (mdr->get_op_stamp() > pi.inode->rstat.rctime) + pi.inode->rstat.rctime = mdr->get_op_stamp(); + pi.inode->version = diri->pre_dirty(); + + // update snap md. + auto& snapnode = *(pi.snapnode); + auto it = snapnode.snaps.find(snapid); + if (it == snapnode.snaps.end()) { + respond_to_request(mdr, -ENOENT); + return; + } else { + auto& snapinfo = (*it).second; + snapinfo.stamp = mdr->get_op_stamp(); + snapinfo.do_md_op(md_key, md_val, op_flag); + } + snapnode.last_modified = mdr->get_op_stamp(); + snapnode.change_attr++; + + // journal inode changes. + mdr->ls = mdlog->get_current_segment(); + EUpdate *le = new EUpdate(mdlog, "snap_metadata_op"); + le->metablob.add_client_req(req->get_reqid(), req->get_oldest_client_tid()); + le->metablob.add_table_transaction(TABLE_SNAP, stid); + mdcache->predirty_journal_parents(mdr, &le->metablob, diri, 0, PREDIRTY_PRIMARY, false); + mdcache->journal_dirty_inode(mdr.get(), &le->metablob, diri); + + // journal the snaprealm changes + auto finisher = new C_MDS_SnapMutateGeneric_finish(this, mdr, diri, snapid); + submit_mdlog_entry(le, finisher, mdr, __func__); + mdlog->flush(); +} + class C_MDS_file_blockdiff_finish : public ServerContext { public: C_MDS_file_blockdiff_finish(Server *server, const MDRequestRef& mdr, CInode *in, uint64_t scan_idx) diff --git a/src/mds/Server.h b/src/mds/Server.h index e9e9bf4ae25..016007db7b7 100644 --- a/src/mds/Server.h +++ b/src/mds/Server.h @@ -112,6 +112,7 @@ enum { l_mdss_req_snapdiff_latency, l_mdss_req_rmdir_latency, l_mdss_req_rmsnap_latency, + l_mdss_req_snap_md_op_latency, l_mdss_req_rmxattr_latency, l_mdss_req_setattr_latency, l_mdss_req_setdirlayout_latency, @@ -347,6 +348,7 @@ public: void handle_client_rmsnap(const MDRequestRef& mdr); void _rmsnap_finish(const MDRequestRef& mdr, CInode *diri, snapid_t snapid); void handle_client_renamesnap(const MDRequestRef& mdr); + void handle_client_snap_md_op(const MDRequestRef& mdr); void _snap_mutate_generic_finish(const MDRequestRef& mdr, CInode *diri, snapid_t snapid); void handle_client_readdir_snapdiff(const MDRequestRef& mdr); void handle_client_file_blockdiff(const MDRequestRef& mdr); diff --git a/src/mds/SnapRealm.cc b/src/mds/SnapRealm.cc index 6eed30da6d9..e9dd2f8bc31 100644 --- a/src/mds/SnapRealm.cc +++ b/src/mds/SnapRealm.cc @@ -21,6 +21,7 @@ #include "MDSRank.h" #include "SnapClient.h" #include "common/debug.h" +#include "include/ceph_assert.h" #include @@ -305,6 +306,14 @@ snapid_t SnapRealm::resolve_snapname(std::string_view n, inodeno_t atino, snapid } +bool SnapRealm::will_md_op_succeed(const snapid_t snap_id, const string& md_key, + const string& md_val, + const unsigned int op_flag) const +{ + return srnode.snaps.at(snap_id).will_md_op_succeed(md_key, md_val, op_flag); +} + + void SnapRealm::adjust_parent() { SnapRealm *newparent; diff --git a/src/mds/SnapRealm.h b/src/mds/SnapRealm.h index e723fb5c3e8..d5eebf5c42f 100644 --- a/src/mds/SnapRealm.h +++ b/src/mds/SnapRealm.h @@ -42,6 +42,10 @@ public: return false; } + bool will_md_op_succeed(const snapid_t snap_id, const std::string& md_key, + const std::string& md_val, + const unsigned int op_flag) const; + void prune_past_parent_snaps(); bool has_past_parent_snaps() const { return !srnode.past_parent_snaps.empty(); diff --git a/src/mds/snap.cc b/src/mds/snap.cc index 9fa77bcf997..89eb5a0b134 100644 --- a/src/mds/snap.cc +++ b/src/mds/snap.cc @@ -71,6 +71,69 @@ void SnapInfo::dump(Formatter *f) const f->close_section(); } +/* Will metadata operation succeed? + * + * NOTE: Call this method before projecting the inode to check if snap metadata + * will be mutated successfully. This avoids un-projecting the inode and rolling + * back the transaction in case the mutation fails. + */ +bool SnapInfo::will_md_op_succeed(const string& key, const string& val, + const unsigned int op_flag) const { + if (op_flag == CEPH_SNAP_MD_OP_CREATE) { + // CREATE flags adds k-v pair if absent and updates if k-v is present. + if (metadata.count(key) == 0) { + dout(10) << __func__ << " snapshot metadata op will pass: create is " + << "being attempted with CREATE flag. op_flag=" << op_flag + << dendl; + } else if (metadata.count(key) > 0) { + dout(10) << __func__ << " snapshot metadata op will pass: update is " + << "being attempted with CREATE flag. op_flag=" << op_flag + << dendl; + } + } else if (op_flag == (CEPH_SNAP_MD_OP_CREATE | CEPH_SNAP_MD_OP_EXCL)) { + if (metadata.count(key) > 0) { + dout(10) << __func__ << " snapshot metadata op will fail: update would " + << "be attempted in with EXCL flag. op_flag=" << op_flag + << dendl; + return false; + } + } else if (op_flag == CEPH_SNAP_MD_OP_REMOVE) { + if (metadata.count(key) == 0) { + dout(10) << __func__ << " snapshot metadata op will fail: remove would " + << "be attempted for a non-existing key. op_flag=" << op_flag + << dendl; + return false; + } + } else { + // NOTE: in case of wrong mode/op_flag value, client code exits with + // appropriate error number instead of contacting MDS with wrong + // mode/op_flag value. + // NOTE: Aborting on receiving wrong flag (via ceph_assert() or otherwise) + // is to be avoided since a corrupted client can be used to deliberately + // crash MDS by sending wrong op_mode flag. + return false; + } + + return true; +} + +// Perform metadata operation indicated by op_flag. +// +// NOTE: to avoid unprojecting the inode and rolling back the transaction for +// snap metadata mutation in case of failure, call this method only when success +// is guaranteed. whether success is guaranted or not, can be found out by +// will_md_op_succeed(), which is present here as well as in SnapRealm.cc. +void SnapInfo::do_md_op(const string& key, const string& val, + const unsigned int op_flag) { + if (op_flag == CEPH_SNAP_MD_OP_CREATE) { + metadata.insert_or_assign(key, val); + } else if (op_flag == (CEPH_SNAP_MD_OP_CREATE | CEPH_SNAP_MD_OP_EXCL)) { + metadata.emplace(key, val); + } else if (op_flag == CEPH_SNAP_MD_OP_REMOVE) { + metadata.erase(key); + } +} + std::list SnapInfo::generate_test_instances() { std::list ls; diff --git a/src/mds/snap.h b/src/mds/snap.h index fb8ccccc78d..41a52c8ba94 100644 --- a/src/mds/snap.h +++ b/src/mds/snap.h @@ -37,6 +37,12 @@ struct SnapInfo { void encode(ceph::buffer::list &bl) const; void decode(ceph::buffer::list::const_iterator &bl); void dump(ceph::Formatter *f) const; + + bool will_md_op_succeed(const std::string& key, const std::string& val, + const unsigned int op_flag) const; + void do_md_op(const std::string& key, const std::string& val, + const unsigned int op_flag); + static std::list generate_test_instances(); std::string_view get_long_name() const;