Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

app: Only remount /sysroot if needed #2486

Merged
merged 1 commit into from
Nov 22, 2021

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Nov 19, 2021

We should only try to remount /sysroot if we're actually handling the
sysroot repo and the repo isn't writable. We already have public APIs to
check each of those, so let's use them.

Closes: #2485

We should only try to remount `/sysroot` if we're actually handling the
sysroot repo and the repo isn't writable. We already have public APIs to
check each of those, so let's use them.

Closes: ostreedev#2485
@kjbracey
Copy link

I can confirm this patch fixes my script, applied to 2021.1.

@lucab
Copy link
Member

lucab commented Nov 29, 2021

I think this resulted in a change of behavior for a very specific CLI usecase.
rpm-ostree CI does some hackish manipulation via ostree --repo /ostree/foo/bar, which apparently was relying on the previous logic here to remount sysroot as RW.
I'm tying to amend the CI hacks in coreos/rpm-ostree#3246, but I guess it would be nice to special case --repo /ostree/... and --repo /sysroot/ostree/... so that ostree automatically remounts /sysroot.
Or maybe there is a smarter way to do the detection via fstatvfs()?

@jlebon
Copy link
Member Author

jlebon commented Nov 29, 2021

I'm tying to amend the CI hacks in coreos/rpm-ostree#3246, but I guess it would be nice to special case --repo /ostree/... and --repo /sysroot/... so that ostree automatically remounts /sysroot. Or maybe there is a smarter way to do the detection via vstatvfs()?

Hmm, maybe. I think this is somewhat specific to the rpm-ostree CI which likes to keep test repos in /ostree/repo/ to ensure hardlinks with the sysroot repo. Maybe simplest is to have a hidden stamp file in /run somewhere and we just always remount rw in that case in this code? That way in the rpm-ostree CI, we'd just need a touch /run/... upfront.

Though if the only fallout is coreos/rpm-ostree#3246, then I'd vote for not doing anything at all for now. :)

@lucab
Copy link
Member

lucab commented Nov 30, 2021

Yes I haven't seen any other fallout, at least yet.
I'm not super happy with the proliferation of magic stamp files as reliable interfaces for consumers.
Thinking a bit more about my previous copmment, the statfs approach would probably be to broad.
The rpm-ostree got easily fixed already.
Overall, I agree on:

  • keeping things as is going forward
  • if we notice further breakage for similar cases, just special-casing the /ostree path-prefixes

@jlebon jlebon deleted the pr/remount-ostree branch April 24, 2023 02:50
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.

/run/ostree-booted present in initramfs after ostree-prepare-root?
4 participants