From: John Spray Date: Tue, 16 Sep 2014 09:49:15 +0000 (+0100) Subject: mon: forbid tier changes when in use by FS X-Git-Tag: v0.86~65^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=681a49c412cc3ad251ec6080468ce92b486dc81e;p=ceph.git mon: forbid tier changes when in use by FS * Removing tiers from a base pool in use by CephFS is forbidden. * Using CephFS pools as tiers is forbidden. Signed-off-by: John Spray --- diff --git a/qa/workunits/cephtool/test.sh b/qa/workunits/cephtool/test.sh index fa2002316cb0..65f041adc9c1 100755 --- a/qa/workunits/cephtool/test.sh +++ b/qa/workunits/cephtool/test.sh @@ -452,6 +452,16 @@ function test_mon_mds() set -e ceph fs new cephfs fs_metadata mds-ec-pool + + # While a FS exists using the tiered pools, I should not be allowed + # to remove the tier + set +e + ceph osd tier remove-overlay mds-ec-pool 2>$TMPFILE + check_response 'in use by CephFS' $? 16 + ceph osd tier remove mds-ec-pool mds-tier 2>$TMPFILE + check_response 'in use by CephFS' $? 16 + set -e + ceph fs rm cephfs --yes-i-really-mean-it # ... but we should be forbidden from using the cache pool in the FS directly. @@ -473,6 +483,20 @@ function test_mon_mds() # Clean up tier + EC pools ceph osd tier remove-overlay mds-ec-pool ceph osd tier remove mds-ec-pool mds-tier + + # Create a FS using the 'cache' pool now that it's no longer a tier + ceph fs new cephfs fs_metadata mds-tier + + # We should be forbidden from using this pool as a tier now that + # it's in use for CephFS + set +e + ceph osd tier add mds-ec-pool mds-tier 2>$TMPFILE + check_response 'in use by CephFS' $? 16 + set -e + + ceph fs rm cephfs --yes-i-really-mean-it + + ceph osd pool delete mds-tier mds-tier --yes-i-really-really-mean-it ceph osd pool delete mds-ec-pool mds-ec-pool --yes-i-really-really-mean-it diff --git a/src/mds/MDSMap.h b/src/mds/MDSMap.h index 23620ea468fb..84ba4d42059d 100644 --- a/src/mds/MDSMap.h +++ b/src/mds/MDSMap.h @@ -271,6 +271,10 @@ public: return data_pools.count(poolid); } + bool pool_in_use(int64_t poolid) const { + return get_enabled() && (is_data_pool(poolid) || metadata_pool == poolid); + } + const std::map& get_mds_info() { return mds_info; } const mds_info_t& get_mds_info_gid(uint64_t gid) { assert(mds_info.count(gid)); diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc index 25a5bc7fc05c..a85979b51df6 100644 --- a/src/mon/OSDMonitor.cc +++ b/src/mon/OSDMonitor.cc @@ -5420,18 +5420,11 @@ done: assert(p); const pg_pool_t *tp = osdmap.get_pg_pool(tierpool_id); assert(tp); - if (p->tiers.count(tierpool_id)) { - assert(tp->tier_of == pool_id); - err = 0; - ss << "pool '" << tierpoolstr << "' is now (or already was) a tier of '" << poolstr << "'"; - goto reply; - } - if (tp->is_tier()) { - ss << "tier pool '" << tierpoolstr << "' is already a tier of '" - << osdmap.get_pool_name(tp->tier_of) << "'"; - err = -EINVAL; + + if (!_check_become_tier(tierpool_id, tp, pool_id, p, &err, &ss)) { goto reply; } + // make sure new tier is empty string force_nonempty; cmd_getval(g_ceph_context, cmdmap, "force_nonempty", force_nonempty); @@ -5478,6 +5471,11 @@ done: assert(p); const pg_pool_t *tp = osdmap.get_pg_pool(tierpool_id); assert(tp); + + if (!_check_remove_tier(pool_id, p, &err, &ss)) { + goto reply; + } + if (p->tiers.count(tierpool_id) == 0) { ss << "pool '" << tierpoolstr << "' is now (or already was) not a tier of '" << poolstr << "'"; err = 0; @@ -5552,6 +5550,11 @@ done: err = -EINVAL; goto reply; } + + if (!_check_remove_tier(pool_id, p, &err, &ss)) { + goto reply; + } + // go pg_pool_t *np = pending_inc.get_new_pool(pool_id, p); np->read_tier = overlaypool_id; @@ -5577,6 +5580,11 @@ done: ss << "there is now (or already was) no overlay for '" << poolstr << "'"; goto reply; } + + if (!_check_remove_tier(pool_id, p, &err, &ss)) { + goto reply; + } + // go pg_pool_t *np = pending_inc.get_new_pool(pool_id, p); np->clear_read_tier(); @@ -5716,18 +5724,11 @@ done: assert(p); const pg_pool_t *tp = osdmap.get_pg_pool(tierpool_id); assert(tp); - if (p->tiers.count(tierpool_id)) { - assert(tp->tier_of == pool_id); - err = 0; - ss << "pool '" << tierpoolstr << "' is now (or already was) a tier of '" << poolstr << "'"; - goto reply; - } - if (tp->is_tier()) { - ss << "tier pool '" << tierpoolstr << "' is already a tier of '" - << osdmap.get_pool_name(tp->tier_of) << "'"; - err = -EINVAL; + + if (!_check_become_tier(tierpool_id, tp, pool_id, p, &err, &ss)) { goto reply; } + int64_t size = 0; if (!cmd_getval(g_ceph_context, cmdmap, "size", size)) { ss << "unable to parse 'size' value '" @@ -6149,8 +6150,7 @@ int OSDMonitor::_check_remove_pool(int64_t pool, const pg_pool_t *p, // If the Pool is in use by CephFS, refuse to delete it MDSMap const &pending_mdsmap = mon->mdsmon()->pending_mdsmap; - if (pending_mdsmap.get_enabled() && (pending_mdsmap.is_data_pool(pool) || - pending_mdsmap.get_metadata_pool() == pool)) { + if (pending_mdsmap.pool_in_use(pool)) { *ss << "pool '" << poolstr << "' is in use by CephFS"; return -EBUSY; } @@ -6171,6 +6171,75 @@ int OSDMonitor::_check_remove_pool(int64_t pool, const pg_pool_t *p, return 0; } +/** + * Check if it is safe to add a tier to a base pool + * + * @return + * True if the operation should proceed, false if we should abort here + * (abort doesn't necessarily mean error, could be idempotency) + */ +bool OSDMonitor::_check_become_tier( + const int64_t tier_pool_id, const pg_pool_t *tier_pool, + const int64_t base_pool_id, const pg_pool_t *base_pool, + int *err, + ostream *ss) const +{ + const std::string &tier_pool_name = osdmap.get_pool_name(tier_pool_id); + const std::string &base_pool_name = osdmap.get_pool_name(base_pool_id); + + const MDSMap &pending_mdsmap = mon->mdsmon()->pending_mdsmap; + if (pending_mdsmap.pool_in_use(tier_pool_id)) { + *ss << "pool '" << tier_pool_name << "' is in use by CephFS"; + *err = -EBUSY; + return false; + } + + if (base_pool->tiers.count(tier_pool_id)) { + assert(tier_pool->tier_of == base_pool_id); + *err = 0; + *ss << "pool '" << tier_pool_name << "' is now (or already was) a tier of '" + << base_pool_name << "'"; + return false; + } + + if (tier_pool->is_tier()) { + *ss << "tier pool '" << tier_pool_name << "' is already a tier of '" + << osdmap.get_pool_name(tier_pool->tier_of) << "'"; + *err = -EINVAL; + return false; + } + + *err = 0; + return true; +} + + +/** + * Check if it is safe to remove a tier from this base pool + * + * @return + * True if the operation should proceed, false if we should abort here + * (abort doesn't necessarily mean error, could be idempotency) + */ +bool OSDMonitor::_check_remove_tier( + const int64_t base_pool_id, const pg_pool_t *base_pool, + int *err, ostream *ss) const +{ + const std::string &base_pool_name = osdmap.get_pool_name(base_pool_id); + + // If the pool is in use by CephFS, then refuse to remove its + // tier + const MDSMap &pending_mdsmap = mon->mdsmon()->pending_mdsmap; + if (pending_mdsmap.pool_in_use(base_pool_id)) { + *ss << "pool '" << base_pool_name << "' is in use by CephFS via its tier"; + *err = -EBUSY; + return false; + } + + *err = 0; + return true; +} + int OSDMonitor::_prepare_remove_pool(int64_t pool, ostream *ss) { dout(10) << "_prepare_remove_pool " << pool << dendl; diff --git a/src/mon/OSDMonitor.h b/src/mon/OSDMonitor.h index e28efba40229..61e51721859b 100644 --- a/src/mon/OSDMonitor.h +++ b/src/mon/OSDMonitor.h @@ -245,6 +245,14 @@ private: bool prepare_pgtemp(class MOSDPGTemp *m); int _check_remove_pool(int64_t pool, const pg_pool_t *pi, ostream *ss); + bool _check_become_tier( + int64_t tier_pool_id, const pg_pool_t *tier_pool, + int64_t base_pool_id, const pg_pool_t *base_pool, + int *err, ostream *ss) const; + bool _check_remove_tier( + int64_t base_pool_id, const pg_pool_t *base_pool, + int *err, ostream *ss) const; + int _prepare_remove_pool(int64_t pool, ostream *ss); int _prepare_rename_pool(int64_t pool, string newname);