mount: setupLoop() doesn't work with with Autoclear#4991
mount: setupLoop() doesn't work with with Autoclear#4991dmcgowan merged 1 commit intocontainerd:masterfrom
Conversation
|
Skipping CI for Draft Pull Request. |
|
If my theory is right, we may need to fix |
|
Build succeeded.
|
Should we return a file handle from |
|
It seems broken and I think we should return a file handle. Autoclear translated to LO_FLAGS_AUTOCLEAR, which destructs a loopback device on its last close. However, since setupLoop() is returning a file name, the function should close all file handles at the end. Otherwise, a file descriptor will be leaked. But that means the loopback device will be destructed at the end of the function. |
|
I've added a new commit. I would squash them together tomorrow PST if others don't mind. |
|
Build succeeded.
|
kzys
force-pushed
the
no-auto-clear
branch
2 times, most recently
from
483c234 to
cd9e32f
Compare
|
Build succeeded.
|
There was a problem hiding this comment.
Are the devmapper tests which use this still going to run into issues using the loop device?
There was a problem hiding this comment.
Since LoopParams doesn't have Autoclear here, closing the file won't destruct the loopback device.
There was a problem hiding this comment.
OK, I guess we can separate that some. I thought we were still seeing some related issues on devmapper. In this case, could/should it just return the file with autoclear to avoid detach? If there are no leaks there though that might not help the running out of loop devices issues there.
There was a problem hiding this comment.
nit: typo (fo instead of for)
not a blocker, just in case other changes are needed before merge
|
Build succeeded.
|
setupLoop()'s Autoclear (LO_FLAGS_AUTOCLEAR) will destruct the loopback device when all associated file descriptors are closed. However this behavior didn't work before since setupLoop() was returning a file name. The looppack device was destructed at the end of the function when LoopParams had Autoclear = true. Fixes containerd#4969. Signed-off-by: Kazuyoshi Kato <[email protected]>
|
Build succeeded.
|
|
Note that this change only affects Linux and the failing test is on Windows. |
6 participants
setupLoop()'s Autoclear (LO_FLAGS_AUTOCLEAR) will destruct the
loopback device when all FDs are closed.
However since setupLoop() returns a file name, not a file handle,
the looppack device will be destructed at the end of the function all the time.
Signed-off-by: Kazuyoshi Kato [email protected]