msg/async: reset fd to -1 after closing it

[shangfufei]

After listen_socket was closed, it entered the start() process magically. so I think we should make '_fd = -1' when listen_socket was closed. And in the start process, determine the value of listen_socket, if it equal to -1, return the process.
Fixes: https://tracker.ceph.com/issues/23600
Signed-off-by: shangfufei shangfufei@inspur.com

[tchaikov]

@shangfufei please summarize the change in the title of your commit message, see https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#title-of-pull-requests-and-title-of-commits, and use the notation of "Fixes: " in the commit message, see https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#tag-the-commit

[yuyuyu101]

when listen_socket abort, the socket will be reset. what happen if server_socket exists and fd==-1?

[shangfufei]

this time will run the Processor::stop function,just delete listen socket fd from epoll event.

[tchaikov]

• http://pulpito.ceph.com/kchai-2018-04-16_09:06:16-rados-wip-kefu-testing-2018-04-16-1515-distro-basic-smithi/
• http://pulpito.ceph.com/kchai-2018-04-15_23:41:27-rados-wip-kefu-testing-2018-04-15-2317-distro-basic-smithi/

the failed tests are caused by #21338

[tchaikov]

@shangfufei again, please summarize the change in the title of your commit message. and please remove "bug fix 23600" from the title.

[liewegas]

I'm definitely on board with setting fd to -1 on close; not doing so can lead to all kinds of problems with data written to the wrong socket or file (!!!), and writing to -1 at least will error out instead of causing more damage.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you are a maintainer or core committer, please follow-up on this issue to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

[tchaikov]

@shangfufei please note the commit message and title of the PR are different. also please rebase. modulo these nits, lgtm.

[shangfufei]

get it, i'm doing now,

[tchaikov]

@shangfufei please remove the merge commit from the PR. you might want to rebase onto the master or just cherry-pick your change.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you are a maintainer or core committer, please follow-up on this issue to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

[liewegas]

@shangfufei can you rebase this on top of current master so it is a single git commit?

[shangfufei]

@shangfufei can you rebase this on top of current master so it is a single git commit?

Do you mean I need to submit it directly in the master branch?

[shangfufei]

This modification is currently in the wip-async-bugfix-23600 branch, which needs to be merged into the master trunk branch if detection is not a problem with conventional operations.

[smithfarm]

@shangfufei I think what @liewegas is asking for is to squash the three commits in this PR [1] (using git rebase -i) into a single commit so the PR has only one commit instead of three.

[1] https://github.com/ceph/ceph/pull/21296/commits

(Note, you may need to do a simple git fetch upstream ; git rebase upstream/master, where upstream is the name of your ceph/ceph remote, first, just to get rid of the merge commit. Then squash using git rebase -i.)

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you are a maintainer or core committer, please follow-up on this issue to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

[stale]

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

[xiexingguo]

@liewegas Already merged by #27599