mon/MDSMonitor: rectify errmsg for "mds fail" cmd#57380
mon/MDSMonitor: rectify errmsg for "mds fail" cmd#57380rishabh-d-dave wants to merge 1 commit intoceph:mainfrom
Conversation
bf98e12 to
51d59be
Compare
chrisphoffman
left a comment
There was a problem hiding this comment.
In the case you mention, that errmsg message does look weird, nice catch.
If you remove this output, what about the other instances when gid_from_arg() is called and fails in the same way, it now loses this errmsg.
See:
./bin/ceph mds freeze zzz yes
batrick
left a comment
There was a problem hiding this comment.
After this change, ss would be an unused argument. I think it does make sense to keep the error message here but perhaps the caller should be a little smarter about clearing ss if it doesn't care if the mds does not exist. For example:
Line 1046 in b494674
should just either not pass ss (make it a pointer) or clear it before continue;.
51d59be to
c0cc424
Compare
|
This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days. |
Rectify the error message printed when a non-existent MDS's name is passed to "ceph mds fail" command. Fixes: https://tracker.ceph.com/issues/65875 Signed-off-by: Rishabh Dave <ridave@redhat.com>
c0cc424 to
53901f4
Compare
joscollin
left a comment
There was a problem hiding this comment.
In the case you mention, that errmsg message does look weird, nice catch.
If you remove this output, what about the other instances when
gid_from_arg()is called and fails in the same way, it now loses this errmsg.See:
./bin/ceph mds freeze zzz yes
diff --git a/src/mon/MDSMonitor.cc b/src/mon/MDSMonitor.cc
index 76a57ac443d..7899a2146c6 100644
--- a/src/mon/MDSMonitor.cc
+++ b/src/mon/MDSMonitor.cc
@@ -1410,7 +1410,7 @@ mds_gid_t MDSMonitor::gid_from_arg(const FSMap &fsmap, const string &arg, ostrea
const MDSMap::mds_info_t *mds_info = fsmap.find_by_name(arg);
if (!mds_info) {
ss << "MDS named '" << arg
- << "' does not exist, or is not up";
+ << "' does not exist, or is not up or you lack the permission to see.";
return MDS_GID_NONE;
}
dout(10) << __func__ << ": resolved MDS name '" << arg
@@ -1600,8 +1600,6 @@ int MDSMonitor::filesystem_command(
MDSMap::mds_info_t failed_info;
mds_gid_t gid = gid_from_arg(fsmap, who, ss);
if (gid == MDS_GID_NONE) {
- ss << "MDS named '" << who << "' does not exist, is not up or you "
- << "lack the permission to see.";
return 0;
}
if(!fsmap.gid_exists(gid, op->get_session()->get_allowed_fs_names())) {
This^ fixes the issue @chrisphoffman mentioned.
|
@rishabh-d-dave ping? |
|
(waiting for @rishabh-d-dave to update) |
|
This one is on comparativley lower priority compared to mgr/vol PRs and certain BZs. I'll get back to it as soon my work on some of these is complete. |
|
@rishabh-d-dave ping |
|
This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days. |
|
This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days. |
|
This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution! |
|
This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days. |
|
This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution! |
|
This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days. |
|
Closing it since its a minor cleanup and nothing urgent or super important. |
Rectify the error message printed when a non-existent MDS's name is
passed to "ceph mds fail" command.
Fixes: https://tracker.ceph.com/issues/65875
Signed-off-by: Rishabh Dave ridave@redhat.com
Contribution Guidelines
To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an
xbetween the brackets:[x]. Spaces and capitalization matter when checking off items this way.Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard cephadmjenkins test apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume toxjenkins test windowsjenkins test rook e2e