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

ceph-fuse: start up log on parent process before shutdown #12347

Merged
merged 1 commit into from Dec 7, 2016

Conversation

gregsfortytwo
Copy link
Member

Otherwise, we hit an assert in the Ceph context and logging teardown.

Fixes: http://tracker.ceph.com/issues/18157

Signed-off-by: Greg Farnum gfarnum@redhat.com

Otherwise, we hit an assert in the Ceph context and logging teardown.

Fixes: http://tracker.ceph.com/issues/18157

Signed-off-by: Greg Farnum <gfarnum@redhat.com>
@gregsfortytwo
Copy link
Member Author

This is the obvious quick fix — we stop the logging, fork, and then the parent shuts down without restarting the log, then asserts. I'm missing some context here, though. Why do we stop the log @liewegas?

@gregsfortytwo
Copy link
Member Author

@tchaikov, this is from one of your PRs, any comments?

@gregsfortytwo
Copy link
Member Author

I tested this by, you know, running ceph-fuse and observing no more crashes. ;)

@liewegas
Copy link
Member

liewegas commented Dec 6, 2016

It's a bit silly to open the log again before shutdown, but I think it's safe. We don't log anything from the parent thread.

@liewegas liewegas added this to the kraken milestone Dec 6, 2016
@@ -295,6 +295,8 @@ int main(int argc, const char **argv, const char *envp[]) {
//cout << "child done" << std::endl;
return r;
} else {
if (restart_log)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we'd better use global_init_prefork() before the fork() and call global_init_postfork_start() and global_init_postfork_finish() after fork(). also, why don't we use the Preforker here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to try and put together something better that'd be welcome. But I think we need a fix pretty much now and I'm not familiar enough with the init code to make those kinds of changes, @tchaikov.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gregsfortytwo sure, i am on it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see #12358

@liewegas
Copy link
Member

liewegas commented Dec 7, 2016 via email

@liewegas liewegas closed this Dec 7, 2016
@liewegas liewegas reopened this Dec 7, 2016
@liewegas liewegas merged commit cd56396 into ceph:master Dec 7, 2016
@gregsfortytwo gregsfortytwo deleted the wip-18157-fix branch December 14, 2016 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants