Fix context usage in shim package#3224
Fix context usage in shim package#3224matiasinsaurralde wants to merge 2 commits intodstackai:masterfrom
Conversation
r4victor
left a comment
r4victor
left a comment
There was a problem hiding this comment.
Cancelling some operations like "umount" can be problematic. Not sure it's an improvement.
Signed-off-by: Matías Insaurralde <matias@insaurral.de>
matiasinsaurralde
force-pushed
the
internal-shim-fixes
branch
from
d3b0e0a to
15d30a9
Compare
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 <matias@insaurral.de>
Makes sense - I've refactored the PR a bit, still passing context to 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):
|
| continue | ||
| } | ||
| cmd = exec.Command("umount", "-qf", mountPoint) | ||
| cmd = exec.CommandContext(context.TODO(), "umount", "-qf", mountPoint) |
There was a problem hiding this comment.
So what's the benefit of CommandContext(context.TODO()?
|
This PR is stale because it has been open for 14 days with no activity. |
|
This PR was closed because it has been inactive for 7 days since being marked as stale. Please reopen the PR if it is still relevant. |
2 participants
Same as #3223 but for the shim package.
Had to update the
Backendinterface to take the context 👍