Skip to content

Commit ed7bb4f

Browse files
batrickbatrick
committed
Merge PR ceph#62833 into main
* refs/pull/62833/head: qa: test charmap changes with dir and snaps mds: check for snapshots on parent snaprealms mds: use strict_strtobool for parsing bools common: take string_view for strict_tobool Reviewed-by: Greg Farnum <[email protected]>
2 parents ef76103 + 05ca22f commit ed7bb4f

File tree

5 files changed

+95
-36
lines changed

5 files changed

+95
-36
lines changed

‎qa/tasks/cephfs/test_dir_charmap.py‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,57 @@ def test_cs_feature_bit(self):
228228
stderr = p.stderr.getvalue()
229229
self.fail("command failed:\n%s", stderr)
230230

231+
def test_cs_snaps_set_insensitive(self):
232+
"""
233+
That setting a charmap fails for an empty directory with snaps.
234+
"""
235+
236+
attrs = {
237+
"ceph.dir.casesensitive": False,
238+
"ceph.dir.normalization": "nfc",
239+
"ceph.dir.encoding": "utf8",
240+
}
241+
242+
self.mount_a.run_shell_payload("mkdir -p foo/dir; mkdir foo/.snap/one; rmdir foo/dir")
243+
for attr, v in attrs.items():
244+
try:
245+
self.mount_a.setfattr("foo/", attr, v, helpfulexception=True)
246+
except DirectoryNotEmptyError:
247+
pass
248+
else:
249+
self.fail("should fail")
250+
try:
251+
self.check_cs("foo")
252+
except NoSuchAttributeError:
253+
pass
254+
else:
255+
self.fail("should fail")
256+
257+
def test_cs_parent_snaps_set_insensitive(self):
258+
"""
259+
That setting a charmap succeeds for an empty directory with parent snaps.
260+
"""
261+
262+
attrs = {
263+
"ceph.dir.casesensitive": False,
264+
"ceph.dir.normalization": "nfc",
265+
"ceph.dir.encoding": "utf8",
266+
}
267+
268+
self.mount_a.run_shell_payload("mkdir -p foo/{trash,bar}; mkdir foo/.snap/one; rmdir foo/trash;")
269+
for attr, v in attrs.items():
270+
try:
271+
self.mount_a.setfattr("foo/bar", attr, v, helpfulexception=True)
272+
except DirectoryNotEmptyError:
273+
pass
274+
else:
275+
self.fail("should fail")
276+
try:
277+
self.check_cs("foo/bar")
278+
except NoSuchAttributeError:
279+
pass
280+
else:
281+
self.fail("should fail")
231282

232283
def test_cs_remount(self):
233284
"""

‎src/common/strtol.cc‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,16 @@
2222
#include <strings.h>
2323
#include <string_view>
2424

25+
#include <boost/algorithm/string/predicate.hpp>
26+
2527
using std::ostringstream;
28+
using namespace std::literals::string_view_literals;
2629

27-
bool strict_strtob(const char* str, std::string *err)
30+
bool strict_strtob(std::string_view str, std::string *err)
2831
{
29-
if (strcasecmp(str, "false") == 0) {
32+
if (boost::iequals(str, "false"sv)) {
3033
return false;
31-
} else if (strcasecmp(str, "true") == 0) {
34+
} else if (boost::iequals(str, "true"sv)) {
3235
return true;
3336
} else {
3437
int b = strict_strtol(str, 10, err);

‎src/common/strtol.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ auto consume(std::string_view& s, int base = 10)
6969
}
7070
} // namespace ceph
7171

72-
bool strict_strtob(const char* str, std::string *err);
72+
bool strict_strtob(std::string_view str, std::string *err);
7373

7474
long long strict_strtoll(std::string_view str, int base, std::string *err);
7575

‎src/mds/Server.cc‎

Lines changed: 36 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -6354,11 +6354,10 @@ void Server::handle_client_setvxattr(const MDRequestRef& mdr, CInode *cur)
63546354
client_t exclude_ct = mdr->get_client();
63556355
mdcache->broadcast_quota_to_client(cur, exclude_ct, true);
63566356
} else if (name == "ceph.quiesce.block"sv) {
6357-
bool val;
6358-
try {
6359-
val = boost::lexical_cast<bool>(value);
6360-
} catch (boost::bad_lexical_cast const&) {
6361-
dout(10) << "bad vxattr value, unable to parse bool for " << name << dendl;
6357+
std::string errstr;
6358+
bool val = strict_strtob(value, &errstr);
6359+
if (!errstr.empty()) {
6360+
dout(10) << "bad vxattr value, unable to parse bool for " << name << ": " << errstr << dendl;
63626361
respond_to_request(mdr, -EINVAL);
63636362
return;
63646363
}
@@ -6416,7 +6415,13 @@ void Server::handle_client_setvxattr(const MDRequestRef& mdr, CInode *cur)
64166415
}
64176416
value = "0";
64186417
}
6419-
val = boost::lexical_cast<bool>(value);
6418+
std::string errstr;
6419+
val = strict_strtob(value, &errstr);
6420+
if (!errstr.empty()) {
6421+
dout(10) << "bad vxattr value, unable to parse bool for " << name << ": " << errstr << dendl;
6422+
respond_to_request(mdr, -EINVAL);
6423+
return;
6424+
}
64206425
} catch (boost::bad_lexical_cast const&) {
64216426
dout(10) << "bad vxattr value, unable to parse bool for " << name << dendl;
64226427
respond_to_request(mdr, -EINVAL);
@@ -6565,7 +6570,13 @@ void Server::handle_client_setvxattr(const MDRequestRef& mdr, CInode *cur)
65656570
}
65666571
value = "0";
65676572
}
6568-
val = boost::lexical_cast<bool>(value);
6573+
std::string errstr;
6574+
val = strict_strtob(value, &errstr);
6575+
if (!errstr.empty()) {
6576+
dout(10) << "bad vxattr value, unable to parse bool for " << name << ": " << errstr << dendl;
6577+
respond_to_request(mdr, -EINVAL);
6578+
return;
6579+
}
65696580
} catch (boost::bad_lexical_cast const&) {
65706581
dout(10) << "bad vxattr value, unable to parse bool for " << name << dendl;
65716582
respond_to_request(mdr, -EINVAL);
@@ -6592,11 +6603,7 @@ void Server::handle_client_setvxattr(const MDRequestRef& mdr, CInode *cur)
65926603
if (!xlock_policylock(mdr, cur, false, false, std::move(lov)))
65936604
return;
65946605

6595-
if (_dir_is_nonempty(mdr, cur)) {
6596-
respond_to_request(mdr, -ENOTEMPTY);
6597-
return;
6598-
}
6599-
if (cur->snaprealm && cur->snaprealm->srnode.snaps.size()) {
6606+
if (_dir_is_nonempty(mdr, cur) || _dir_has_snaps(mdr, cur)) {
66006607
respond_to_request(mdr, -ENOTEMPTY);
66016608
return;
66026609
}
@@ -6624,20 +6631,15 @@ void Server::handle_client_setvxattr(const MDRequestRef& mdr, CInode *cur)
66246631
if (!xlock_policylock(mdr, cur, false, false, std::move(lov)))
66256632
return;
66266633

6627-
if (_dir_is_nonempty(mdr, cur)) {
6628-
respond_to_request(mdr, -ENOTEMPTY);
6629-
return;
6630-
}
6631-
if (cur->snaprealm && cur->snaprealm->srnode.snaps.size()) {
6634+
if (_dir_is_nonempty(mdr, cur) || _dir_has_snaps(mdr, cur)) {
66326635
respond_to_request(mdr, -ENOTEMPTY);
66336636
return;
66346637
}
66356638

6636-
bool val;
6637-
try {
6638-
val = boost::lexical_cast<bool>(value);
6639-
} catch (boost::bad_lexical_cast const&) {
6640-
dout(10) << "bad vxattr value, unable to parse bool for " << name << dendl;
6639+
std::string errstr;
6640+
bool val = strict_strtob(value, &errstr);
6641+
if (!errstr.empty()) {
6642+
dout(10) << "bad vxattr value, unable to parse bool for " << name << ": " << errstr << dendl;
66416643
respond_to_request(mdr, -EINVAL);
66426644
return;
66436645
}
@@ -6662,11 +6664,7 @@ void Server::handle_client_setvxattr(const MDRequestRef& mdr, CInode *cur)
66626664
if (!xlock_policylock(mdr, cur, false, false, std::move(lov)))
66636665
return;
66646666

6665-
if (_dir_is_nonempty(mdr, cur)) {
6666-
respond_to_request(mdr, -ENOTEMPTY);
6667-
return;
6668-
}
6669-
if (cur->snaprealm && cur->snaprealm->srnode.snaps.size()) {
6667+
if (_dir_is_nonempty(mdr, cur) || _dir_has_snaps(mdr, cur)) {
66706668
respond_to_request(mdr, -ENOTEMPTY);
66716669
return;
66726670
}
@@ -6690,11 +6688,7 @@ void Server::handle_client_setvxattr(const MDRequestRef& mdr, CInode *cur)
66906688
if (!xlock_policylock(mdr, cur, false, false, std::move(lov)))
66916689
return;
66926690

6693-
if (_dir_is_nonempty(mdr, cur)) {
6694-
respond_to_request(mdr, -ENOTEMPTY);
6695-
return;
6696-
}
6697-
if (cur->snaprealm && cur->snaprealm->srnode.snaps.size()) {
6691+
if (_dir_is_nonempty(mdr, cur) || _dir_has_snaps(mdr, cur)) {
66986692
respond_to_request(mdr, -ENOTEMPTY);
66996693
return;
67006694
}
@@ -9223,6 +9217,16 @@ bool Server::_dir_is_nonempty_unlocked(const MDRequestRef& mdr, CInode *in)
92239217
return false;
92249218
}
92259219

9220+
bool Server::_dir_has_snaps(const MDRequestRef& mdr, CInode *diri)
9221+
{
9222+
dout(10) << __func__ << ": " << *diri << dendl;
9223+
ceph_assert(diri->is_auth());
9224+
ceph_assert(diri->snaplock.can_read(mdr->get_client()));
9225+
9226+
SnapRealm *realm = diri->find_snaprealm();
9227+
return !realm->get_snaps().empty();
9228+
}
9229+
92269230
bool Server::_dir_is_nonempty(const MDRequestRef& mdr, CInode *in)
92279231
{
92289232
dout(10) << "dir_is_nonempty " << *in << dendl;

‎src/mds/Server.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ class Server {
314314
void handle_client_unlink(const MDRequestRef& mdr);
315315
bool _dir_is_nonempty_unlocked(const MDRequestRef& mdr, CInode *rmdiri);
316316
bool _dir_is_nonempty(const MDRequestRef& mdr, CInode *rmdiri);
317+
bool _dir_has_snaps(const MDRequestRef& mdr, CInode *diri);
317318
void _unlink_local(const MDRequestRef& mdr, CDentry *dn, CDentry *straydn);
318319
void _unlink_local_finish(const MDRequestRef& mdr,
319320
CDentry *dn, CDentry *straydn,

0 commit comments

Comments
 (0)