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
daemon: make activation optional #2627
Conversation
return nil, err | ||
} | ||
|
||
if err := os.Chmod(socketPath, 0666); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chipaca wondering, is this a bit racy? is there a best/better practice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is. And I was about to say no, but yes. Pushing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot!
return nil, err | ||
} | ||
|
||
oldmask := unix.Umask(0111) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you want runtime.LockOSThread and friend then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chipaca thanks for the change to use umask, I think we need to protect it with (Un)LockOSThread though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chipaca thank you
// the listeners, so check it before using it. | ||
d.snapListener = listener | ||
} else { | ||
logger.Debugf("cannot get listener for %q: %v", dirs.SnapSocket, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we turn this into a real error now ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe leave that for after the cut?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as you prefer, think is best
offered as-is. Could also add spread tests, after the standup, or spread tests can be added in a later branch.