New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mds: explicitly output error msg for dump cache asok command #15592

Merged
merged 1 commit into from Jun 20, 2017

Conversation

Projects
None yet
2 participants
@david-z
Member

david-z commented Jun 9, 2017

MDS asok command dump cache didn't output error msg in stderr even if something is wrong. It saved error msg in log file only. Some users try to use this command with the same path dump cache <file_path> every time and they didn't know no latest cache was dumped into this file if the path is already existed.

Within this change, dump cache to the existed file will output following error msg, or other error msg if something else gets wrong too.

sudo ceph --admin-daemon /var/run/ceph/ceph-mds.c167.asok dump cache /tmp/c167.cache
Failed to dump cache: (17) File exists

Signed-off-by: Zhi Zhang zhangz.david@outlook.com

@david-z

This comment has been minimized.

Member

david-z commented Jun 9, 2017

@jcsp Could you pls help to review this minor change? Thanks. Build failure is not related to this change. :)

@jcsp

Looks good apart from a couple of minor points

}
if (r != 0) {
ss << "Failed to dump cache: " << cpp_strerror(errno);

This comment has been minimized.

@jcsp

jcsp Jun 9, 2017

Contributor

This should be doing cpp_strerror(r), not errno

}
if (r != 0) {
ss << "Failed to dump tree: " << cpp_strerror(errno);

This comment has been minimized.

@jcsp

jcsp Jun 9, 2017

Contributor

As above, do cpp_strerror(r) instead of errno

cmd_getval(g_ceph_context, cmdmap, "root", root);
if (!cmd_getval(g_ceph_context, cmdmap, "depth", depth))
depth = -1;
{
Mutex::Locker l(mds_lock);
mdcache->dump_cache(root, depth, f);
r = mdcache->dump_cache(root, depth, f);

This comment has been minimized.

@jcsp

jcsp Jun 9, 2017

Contributor

if you move the ss <<... bit into this block, then you could do int r = mdcache->dump_cache... and thereby restrict the scope of the r variable.

Zhi Zhang
mds: explicitly output error msg for dump cache asok command
Signed-off-by: Zhi Zhang <zhangz.david@outlook.com>
@david-z

This comment has been minimized.

Member

david-z commented Jun 12, 2017

@jcsp Changes are done according to your requests. Pls help to review, thanks.

@jcsp

jcsp approved these changes Jun 12, 2017

@jcsp jcsp merged commit 412b397 into ceph:master Jun 20, 2017

2 of 3 checks passed

make check make check failed
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment