sysroot: Handle ro /boot but rw /sysroot #2261
Conversation
cgwalters
force-pushed
the
boot-ro-more
branch
2 times, most recently
from
a1a9cae to
adac37c
Compare
I was being very conservative initially here, but I think it's really safe to just unconditionally set up the mount namespace. This avoids having to check twice for a read-only `/sysroot` (once in the binary and once in the library).
Just like we hold a fd for `/sysroot`, also do so for `/boot` instead of opening and closing it in a few places. This is a preparatory cleanup for further work.
The recent change in coreos/fedora-coreos-config#659 broke some of our tests that do `mount -o remount,rw /sysroot` but leave `/boot` read-only. We had code for having `/boot` read-only before `/sysroot` but in practice we had a file descriptor for `/sysroot` that we opened before the remount that would happen later on. Clean things up here so that in the library, we also remount `/boot` at the same time we remount `/sysroot` if either are readonly. Delete the legacy code for remounting `/boot` rw if we're not in a mount namespace. I am fairly confident most users are either using the `ostree` CLI, or they're using the mount namespace.
cgwalters
force-pushed
the
boot-ro-more
branch
from
adac37c to
9a526bb
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, lucab 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 |
|
/override continuous-integration/travis-ci/pr |
|
@cgwalters: Overrode contexts on behalf of cgwalters: continuous-integration/travis-ci/pr In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
The recent change in coreos/fedora-coreos-config#659
broke some of our tests that do
mount -o remount,rw /sysrootbutleave
/bootread-only.We had code for having
/bootread-only before/sysrootbutin practice we had a file descriptor for
/sysrootthat we openedbefore the remount that would happen later on.
Clean things up here so that in the library, we also remount
/bootat the same time we remount/sysrootif either are readonly.Second, adapt the client side code to check for either being
readonly to enable the mount namespace. Now honestly we could
almost certainly just set this unconditionally - the original
client code here is just being excessively conservative. But
I'd like to make that cleanup independently of this fix.