mds: use parse_filesystem in parse_role to handle exceptions and reuse parsing code#11357
mds: use parse_filesystem in parse_role to handle exceptions and reuse parsing code#11357jcsp merged 4 commits intoceph:masterfrom
Conversation
|
iirc it was intentional that this would throw if the filesystem didn't exist (otherwise we open the door to segfaults if the callers don't rigorously check nullness of the return value) -- I'd rather have the caller check for existence (and get an exception if they forget to do it) than have the caller check for null returns (and get a segfault if they forget to do it). I don't claim all the existing code necessarily sticks to that principle though! Clearly this is a bit inconsistent wrt the name-based get_filesystem call. The number-based one is for use internally whereas the name based one is always used for handling user input. I wonder if the name based one should get renamed to "resolve_filesystem" or something to make it less weird. |
|
@jcsp please take another look. |
jcsp
left a comment
There was a problem hiding this comment.
This is nice, just need to back out the use of refs for output arguments (one of the more debatable parts of the style guide but nevertheless it's in there)
src/mds/FSMap.cc
Outdated
| int FSMap::parse_filesystem( | ||
| std::string const &ns_str, | ||
| std::shared_ptr<const Filesystem> *result | ||
| std::shared_ptr<const Filesystem> &result |
There was a problem hiding this comment.
Using pointers for output arguments is a style guide thing (https://github.com/ceph/ceph/blob/master/CodingStyle#L127)
|
@jcsp, thanks for pointing that out. I've removed that commit. |
|
retest |
|
retest this please |
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
This allows us to reuse code in parse_filesystem and avoid get_filesystem which may fail if the fscid does not exist. This would result in the program (mon) aborting due to the uncaught exception. Fixes: http://tracker.ceph.com/issues/17518 Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
get_filesystem may fail in parse_role if the fscid does not exist. This
would result in the program (mon) aborting due to the uncaught
exception.
Fixes: http://tracker.ceph.com/issues/17518
Signed-off-by: Patrick Donnelly pdonnell@redhat.com