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.

Sign up for GitHub

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

Jump to bottom

common: include <signal.h> in Preforker.h #36194

Merged
merged 1 commit into from Jul 22, 2020
Merged

common: include <signal.h> in Preforker.h #36194

merged 1 commit into from Jul 22, 2020

Conversation

wjwithagen
Copy link
Contributor

Since it does use functions declared by signal.h

/home/jenkins/workspace/ceph-master/src/common/Preforker.h:50:5: error: use of undeclared identifier 'sigemptyset'
sigemptyset(&sa.sa_mask);
^
/home/jenkins/workspace/ceph-master/src/common/Preforker.h:52:9: error: no matching constructor for initialization of 'sigaction'
if (sigaction(SIGHUP, &sa, nullptr) != 0) {
^ ~~~~~~~~~~~~~~~~~~~~
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit copy constructor) not via ble: requires 1 argument, but 3 were provided
struct sigaction {
^
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit move constructor) not via ble: requires 1 argument, but 3 were provided
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit default constructor) not viable: requires 0 arguments, but 3 were provided
2 errors generated.

fixes: #35844

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard backend
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@wjwithagen wjwithagen requested a review from tchaikov July 18, 2020 21:50
@batrick batrick mentioned this pull request Jul 18, 2020
Merged
2 tasks
@wjwithagen
common: include <signal.h> in Preforker.h
93e2ad6 
Since it does use functions declared by signal.h

/home/jenkins/workspace/ceph-master/src/common/Preforker.h:50:5: error: use of undeclared identifier                                             'sigemptyset'
    sigemptyset(&sa.sa_mask);
    ^
/home/jenkins/workspace/ceph-master/src/common/Preforker.h:52:9: error: no matching constructor for                                             initialization of 'sigaction'
    if (sigaction(SIGHUP, &sa, nullptr) != 0) {
        ^         ~~~~~~~~~~~~~~~~~~~~
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit copy constructor) not via                                            ble: requires 1 argument, but 3 were provided
struct sigaction {
       ^
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit move constructor) not via                                            ble: requires 1 argument, but 3 were provided
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit default constructor) not                                             viable: requires 0 arguments, but 3 were provided
2 errors generated.

fixes: ceph#35844
Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>
@smithfarm smithfarm mentioned this pull request Jul 19, 2020
Merged
2 tasks
@smithfarm
Copy link
Contributor

Note: tracker ticket may not be needed, because I have already staged the backport of #35844 to both octopus and nautilus, and the need for this fix has been mentioned in both of those PRs.

@smithfarm
Copy link
Contributor 

Test failures: 190 - unittest_rbd_mirror (Failed)

@smithfarm
Copy link
Contributor

jenkins test make check

@wjwithagen
Copy link
Contributor Author

Note: tracker ticket may not be needed, because I have already staged the backport of #35844 to both octopus and nautilus, and the need for this fix has been mentioned in both of those PRs.

Thanx, I had planned to look into exactly this today. But now I kan hunt for other stuff as well.

@smithfarm
Copy link
Contributor

Thanx, I had planned to look into exactly this today. But now I kan hunt for other stuff as well.

Thanks, Willem. As a follow-on fix for something that is already in the process of being backported, this one is a special case where the fix has to be included in the backport PRs of the original change that introduced the issue.

@tchaikov
Copy link
Contributor

@tchaikov tchaikov merged commit dd79679 into ceph:master Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tchaikov tchaikov tchaikov approved these changes

@batrick batrick batrick approved these changes

Assignees
No one assigned
Labels
bug-fix common needs-qa wip-kefu-testing
Projects
None yet
Milestone
No milestone
4 participants
@wjwithagen @smithfarm @tchaikov @batrick