Skip to content

Conversation

@matiasinsaurralde
Copy link
Contributor

Same as #3223 but for the shim package.

Had to update the Backend interface to take the context 👍

Copy link
Collaborator

@r4victor r4victor left a comment

Choose a reason for hiding this comment

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

Cancelling some operations like "umount" can be problematic. Not sure it's an improvement.

Signed-off-by: Matías Insaurralde <[email protected]>
Prevents umount command cancellation which could lead to data corruption
and filesystem inconsistencies. Addresses review feedback about cancelling
critical filesystem operations.

Signed-off-by: Matías Insaurralde <[email protected]>
@matiasinsaurralde
Copy link
Contributor Author

Cancelling some operations like "umount" can be problematic. Not sure it's an improvement.

Makes sense - I've refactored the PR a bit, still passing context to unmountVolumes for logging, etc. But added context.TODO in the specific call tu umount:

exec.CommandContext(context.TODO(), "umount", "-qf", mountPoint)

I've inspected the other operations (there should be no impact if cancelled, they return errors and/or can be retried):

  • mountpoint - just checks if the path is mounted.
  • lsblk - query device info
  • mkfs.ext4 - formatting can be retried
  • mount - can be retried

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants