-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
When refreshing after a reboot, force lock allocation #3073
When refreshing after a reboot, force lock allocation #3073
Conversation
Got some lint unhappiness @mheon |
After a reboot, when we refresh Podman's state, we retrieved the lock from the fresh SHM instance, but we did not mark it as allocated to prevent it being handed out to other containers and pods. Provide a method for marking locks as in-use, and use it when we refresh Podman state after a reboot. Fixes containers#2900 Signed-off-by: Matthew Heon <matthew.heon@pm.me>
Refresh tests are failing, which I suppose is expected given that the old locks still exist... We already deprecated |
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
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.
LGTM
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.
LGTM
Do I smell a v1.3.-1 in the near future?
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.
LGTM. Small nits with C code vs C standard. type differences would only be an issue where errno value > 32bit.
// Returns an error if the semaphore with this ID does not exist, or has already | ||
// been allocated. | ||
// Returns 0 on success, or negative errno values on failure. | ||
int32_t allocate_given_semaphore(shm_struct_t *shm, uint32_t sem_index) { |
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.
errno's are more correctly defined as int by the standard.
|
||
return 0; | ||
} | ||
|
||
// Deallocate a given semaphore | ||
// Returns 0 on success, negative ERRNO values on failure | ||
int32_t deallocate_semaphore(shm_struct_t *shm, uint32_t sem_index) { |
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.
errno type is int in standard.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jwhonce, mheon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/lgtm |
After a reboot, when we refresh Podman's state, we retrieved the lock from the fresh SHM instance, but we did not mark it as allocated to prevent it being handed out to other containers and pods.
Provide a method for marking locks as in-use, and use it when we refresh Podman state after a reboot.
Fixes #2900