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

osd: improve error message when FileStore op fails due to EPERM #12181

Merged
merged 1 commit into from Jan 4, 2017

Conversation

Projects
None yet
3 participants
@smithfarm
Contributor

smithfarm commented Nov 24, 2016

References: http://tracker.ceph.com/issues/18037
Signed-off-by: Nathan Cutler ncutler@suse.com

@smithfarm smithfarm added the core label Nov 24, 2016

@@ -2997,6 +2997,10 @@ void FileStore::_do_transaction(
msg = "ENOTEMPTY suggests garbage data in osd data dir";
}
if (r == -EPERM) {

This comment has been minimized.

@jecluis

jecluis Nov 25, 2016

Member

personally, i'd go with an else if branching off after the previous if. otherwise it looks good to me.

@athanatos

This comment has been minimized.

Contributor

athanatos commented Nov 29, 2016

This generally looks ok to me (I support @jecluis 's comment though).

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Nov 29, 2016

@jecluis @athanatos Please have another look. I got access to the cluster where the EPERM was manifesting and was very surprised to find that "chown -R ceph.ceph /var/lib/ceph/osd/..." did not fix it. The leveldb was corrupted, though, and repairing it made the error go away (though the OSD still refused to start, but for a different reason).

Based on the above, I revised the message and also converted all the if clauses into else-ifs hanging off the first if in the series.

Because I made the changes requested.

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Dec 9, 2016

@jecluis Could you re-review?

// For now, if we hit _any_ ENOSPC, crash, before we do any damage
// by partially applying transactions.
msg = "ENOSPC handling not implemented";
if (r == -ENOTEMPTY) {
else if (r == -ENOTEMPTY) {
msg = "ENOTEMPTY suggests garbage data in osd data dir";
}

This comment has been minimized.

@athanatos

athanatos Dec 9, 2016

Contributor

} else if {

@@ -2994,15 +2994,16 @@ void FileStore::_do_transaction(
op->op == Transaction::OP_CLONE ||
op->op == Transaction::OP_CLONERANGE2))
msg = "ENOENT on clone suggests osd bug";
if (r == -ENOSPC)
else if (r == -ENOSPC)

This comment has been minimized.

@athanatos

athanatos Dec 9, 2016

Contributor

add braces around the first if and make it all consistent.

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Dec 27, 2016

@athanatos Done!

@athanatos

This comment has been minimized.

Contributor

athanatos commented Jan 3, 2017

Just needs to be tested.

osd: improve error message when FileStore op fails due to EPERM
References: http://tracker.ceph.com/issues/18037
Signed-off-by: Nathan Cutler <ncutler@suse.com>
@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jan 3, 2017

Rebased and pushed wip-18037 to shaman.

teuthology-suite --priority 101 --suite rados --subset $(expr $RANDOM % 2000)/2000 --email ncutler@suse.cz --ceph wip-18037 --machine-type smithi -k distro

pass http://pulpito.ceph.com:80/smithfarm-2017-01-04_09:02:46-rados-wip-18037-distro-basic-smithi/

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jan 4, 2017

@athanatos Is the above run sufficient?

@athanatos

This comment has been minimized.

Contributor

athanatos commented Jan 4, 2017

Assuming it was a rados suite run, sure.

@smithfarm smithfarm merged commit cc45514 into ceph:master Jan 4, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@smithfarm smithfarm deleted the SUSE:wip-18037 branch Jan 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment