-
Notifications
You must be signed in to change notification settings - Fork 880
stage0/gc: try to avoid double overlay mounts #3806
Conversation
4.13 hasn't yet hit the core repo. |
rkt/gc.go
Outdated
| // This behavior was introduced in Linux 4.13, double-mounts were allowed | ||
| // in older kernels. | ||
| if err == syscall.EBUSY { | ||
| return false, 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.
Shouldn't the double-mount be detected above by the if m.MountPoint == stage1Dir? In this case, the EBUSY could be a real error since the code above would have detected it.
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.
True, this is probably a tautological check which won't be reached in normal cases. However I trust the kernel mount return code more than our mountinfo parser, so this helps avoiding GC being aborted by mistake in situations were it could have succeeded.
In the best cases this let the kernel catch corner-cases we didn't see, in the worst case we will error our in the next step trying to call stage1 GC.
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 there's a good argument for at least logging it even if we don't fail on it.
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've verified this fixes the issue on my system, and the code change LGTM as well.
rkt/gc.go
Outdated
| // This behavior was introduced in Linux 4.13, double-mounts were allowed | ||
| // in older kernels. | ||
| if err == syscall.EBUSY { | ||
| return false, 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.
I think there's a good argument for at least logging it even if we don't fail on it.
Before Linux 4.13, it used to be possible to perform double overlayfs mounts in place. This now fails at mount() with EBUSY. We were abusing this feature to unconditionally ensure that the stage1 image overlay mount was present before calling gc entrypoint. This improves stage0 logic to perform the stage1 image overlay mount only if it doesn't already exist.
566f9d2 to
a778cd1
Compare
|
I've added a log statement, but I've left the keep-going on |
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
Before Linux 4.13, it used to be possible to perform double overlayfs
mounts in place. This now fails at mount() with EBUSY.
We were abusing this feature to unconditionally ensure that the stage1
image overlay mount was present before calling gc entrypoint.
This improves stage0 logic to perform the stage1 image overlay mount
only if it doesn't already exist.
Fixes #3805