Skip to content
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/PGBackend: build_push_op assert() before dereferencing iter #9357

Merged
merged 1 commit into from Nov 13, 2016

Conversation

Aran85
Copy link
Contributor

@Aran85 Aran85 commented May 27, 2016

here i meet a segment fault, maybe assert better

Signed-off-by: Zengran Zhang zhangzengran@h3c.com

here i meet a segment fault, maybe assert better

Signed-off-by: Zengran Zhang <zhangzengran@h3c.com>
@Aran85
Copy link
Contributor Author

Aran85 commented May 27, 2016

steps to reproduce:
./vstart.sh -n
./ceph osd pool set rbd size 2
./rados put testfile /etc/hosts -p rbd
ps: the testfile map to pg0.6 whose upset is [0,1]
rm ./dev/osd0/current/0.6_head/xxxtestfilexxx_0
./ceph osd out osd.1
./ceph osd down osd.1
then osd.0 crash.

bc the fd of testifle is in filestore's fdcache, so it could get the object context of it. so let the osd.0 think testfile is not missing, but when it prepare to push it to a new upset osd, it fail to lfn_open it.

does this situation need to worry about in actual production environment?

@yuyuyu101
Copy link
Member

I think assert is fine now. Because ceph doesn't have a rule to handle missing file in local filesystem...

@yuyuyu101
Copy link
Member

And I think this isn't allowed

@yuyuyu101
Copy link
Member

I don't think this change makes sense, remove this line will result into later segment fault. If you want to have a good error handle. return ENOENT may better.

But since we will hit assert(r == 0) by checking return value. There is no difference in real env. Anyway, return errno give a good sharp to let others handle this case later.

@Aran85
Copy link
Contributor Author

Aran85 commented Jun 18, 2016

@yuyuyu101 thanks for review. the assert line added by me,not remove. do you mean we should return errno here?

@yuyuyu101
Copy link
Member

oh, sorry. my mistake. yes, I think we can return errno.

@tchaikov tchaikov changed the title osd/PGBackend: build_push_op segment fault osd/PGBackend: build_push_op assert() before dereferencing iter Nov 13, 2016
@tchaikov tchaikov merged commit c462427 into ceph:master Nov 13, 2016
@Aran85 Aran85 deleted the wip-assert-build-push branch November 15, 2016 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants