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

os/filestore/FileJournal: FileJournal::open() close journal file before return error #16120

Merged
merged 1 commit into from Jul 7, 2017

Conversation

Projects
None yet
3 participants
@yanghonggang
Contributor

yanghonggang commented Jul 5, 2017

After successfully opened journal in FileJournal::open(), the following
error returns will cause journal->open() in JournalingObjectStore::journal_replay()
failed and delete journal. In ~FileJournal(), we assert fd == -1. As we not cleanup
journal fd in FileJournal::open(), fd is not -1. This will generate a core.

Fixes: http://tracker.ceph.com/issues/20504
Signed-off-by: Yang Honggang joseph.yang@xtaotech.com

assert(fd >= 0);
_close(fd);
fd = -1;

This comment has been minimized.

@tchaikov

tchaikov Jul 5, 2017

Contributor

please remove this empty line.

@@ -493,6 +500,13 @@ int FileJournal::open(uint64_t fs_op_seq)
}
return 0;
out:
assert(fd >= 0);

This comment has been minimized.

@tchaikov

tchaikov Jul 5, 2017

Contributor

nit, could reuse FileJournal::close() here.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 5, 2017

@yanghonggang could you prefix the title of your commit message with the subcomponent your are changing ? see https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#3-describe-your-changes, in this case, it's "os/filestore/FileJournal".

could you add a "Signed-off-by" line at the end of your commit message? "git commit -s" will do the trick for you. see https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#1-sign-your-work. you are missing the brackets around your mail address.

os/filestore/FileJournal: FileJournal::open() close journal file befo…
…re return error

After successfully opened journal in FileJournal::open(), the following
error returns will cause journal->open() in JournalingObjectStore::journal_replay()
failed and delete journal. In ~FileJournal(), we assert fd == -1. As we not cleanup
journal fd in FileJournal::open(), fd is not -1. This will generate a core.

Fixes: http://tracker.ceph.com/issues/20504
Signed-off-by: Yang Honggang <joseph.yang@xtaotech.com>
@yanghonggang

This comment has been minimized.

Contributor

yanghonggang commented Jul 5, 2017

@tchaikov Done, thank you for your suggestions

@tchaikov tchaikov changed the title from FileJournal: FileJournal::open() close journal file before return error to os/filestore/FileJournal: FileJournal::open() close journal file before return error Jul 5, 2017

@tchaikov tchaikov added the needs-qa label Jul 5, 2017

@liewegas liewegas merged commit 7a923dc into ceph:master Jul 7, 2017

3 of 4 checks passed

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