Skip to content

Conversation

champtar
Copy link
Collaborator

@champtar champtar commented Sep 18, 2025

- ostree-remount: improve do_remount(), use warn()

When we remount read only, we only want to change the vfs read only
flag, not the fs one, so use MS_BIND.
On the contrary when we remount read write, we want to change both (already ok).
To check that we are allowed to write, we now use 'access'.

We should never need do_remount(), as everything should have been
properly setup by prepare-root and ostree-system-generator,
so start to warn() when we actually remounted something.

- prepare-root: remove $stateroot/var rw bind mount

Now that we have mount.ostreebindrw helper, we can safely
remove the $stateroot/var rw bind mount.

This allow to have /sysroot fully read-only.

- generator: add mount.ostreebindrw helper

Recent versions of mount that use the new mount API (util-linux 2.39)
will honor the 'bind,rw' mount option and create a writable bind mount
even if the source is read-only.
For older mount versions, install a simple mount helper that create
the bind mount then remount bind,rw it.
This avoids having /var read-only before ostree-remount runs.

- prepare-root/generator: drop /run/ostree/initramfs-mount-var support

Ignition stopped using it in 2019 (https://github.com/coreos/ignition/commit/852008ffcc205c15e6b6a0d1b2deaf33fb2bad79),
and it has been broken for 2 years (https://github.com/ostreedev/ostree/commit/64afbcdeb0cb101ce302ae28f1c0852a9d73b907)
(mkdirat(/run/ostree) fails if /run/ostree already exists)

Note: This requires a small change on CoreOS side: coreos/fedora-coreos-config#3773

Fixes #3519

@champtar champtar requested a review from cgwalters September 18, 2025 20:44
@github-actions github-actions bot added the area/prepare-root Issue relates to ostree-prepare-root label Sep 18, 2025
Copy link

openshift-ci bot commented Sep 18, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@champtar champtar force-pushed the remove-sysroot-rw-bind-mount branch 3 times, most recently from 542e525 to 09ba26c Compare September 19, 2025 13:41
@champtar
Copy link
Collaborator Author

Blocked by coreos/fedora-coreos-config#3773

@jlebon
Copy link
Member

jlebon commented Sep 22, 2025

Would be good to sanity-check el9-based SCOS before merging this.

Ignition stopped using it in 2019 (coreos/ignition@852008f),
and it has been broken for 2 years (ostreedev@64afbcd)
(mkdirat(/run/ostree) fails if /run/ostree already exists)
@champtar champtar force-pushed the remove-sysroot-rw-bind-mount branch 2 times, most recently from a489241 to 12133f1 Compare September 22, 2025 16:42
@champtar

This comment was marked as resolved.

@champtar champtar marked this pull request as ready for review September 22, 2025 19:41
@champtar
Copy link
Collaborator Author

Would be good to sanity-check el9-based SCOS before merging this.

Done, it boots fine

[core@cosa-devsh ~]$ journalctl -u ostree-remount.service 
Sep 23 15:37:17 localhost systemd[1]: Starting OSTree Remount OS/ Bind Mounts...
Sep 23 15:37:17 localhost ostree-remount[1543]: Remounted rw: /var
Sep 23 15:37:17 localhost systemd[1]: Finished OSTree Remount OS/ Bind Mounts.

[core@cosa-devsh ~]$ grep '/sysroot' /proc/1/mountinfo 
75 1 0:33 / / ro,relatime shared:1 - overlay composefs ro,seclabel,lowerdir+=/run/ostree/.private/cfsroot-lower,datadir+=/sysroot/ostree/repo/objects,redirect_dir=on,metacopy=on
76 75 252:4 / /sysroot ro,relatime shared:3 - xfs /dev/vda4 rw,seclabel,attr2,inode64,logbufs=8,logbsize=32k,prjquota

[core@cosa-devsh ~]$ systemctl cat var.mount 
# /run/systemd/generator/var.mount
##
# Automatically generated by ostree-system-generator
##

[Unit]
Documentation=man:ostree(1)
ConditionKernelCommandLine=!systemd.volatile
Before=local-fs.target

[Mount]
Where=/var
What=/sysroot/ostree/deploy/scos/var
Options=bind,rw,slave,shared

Recent versions of mount that use the new mount API (util-linux 2.39)
will honor the 'bind,rw' mount option and create a writable bind mount
even if the source is read-only.
For older mount versions, install a simple mount helper that create
the bind mount then remount bind,rw it.
This avoids having /var read-only before ostree-remount runs.
Now that we have mount.ostreebindrw helper, we can safely
remove the $stateroot/var rw bind mount.

This allow to have /sysroot fully read-only.
When we remount read only, we only want to change the vfs read only
flag, not the fs one, so use MS_BIND.
On the contrary when we remount read write, we want to change both (already ok).
To check that we are allowed to write, we now use 'access'.

We should never need do_remount(), as everything should have been
properly setup by prepare-root and ostree-system-generator,
so start to warn() when we actually remounted something.
@champtar champtar force-pushed the remove-sysroot-rw-bind-mount branch from 12133f1 to 5070049 Compare September 24, 2025 03:27
@champtar
Copy link
Collaborator Author

I've now added a small mount helper so var.mount is writable right away.

AC_DEFINE([BUILDOPT_LIBSYSTEMD_AND_LIBMOUNT], 1, [Define if systemd and libmount]))
if test x$with_libsystemd = xyes; then OSTREE_FEATURES="$OSTREE_FEATURES systemd"; fi

AC_ARG_WITH(mounthelper,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the work on this!

I am really not excited about the idea of installing a globally accessible binary just to expose Type=ostreebindrw with .mount units.

I think instead couldn't we generate a .mount unit with a Requires+After on a .service which did a pre-mount or something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've played a bit more with this, if we mount something at /var before var.mount then var.mount will be enabled but it'll not run, and most importantly it'll print a confusing message (/var was skipped because of an unmet condition check (ConditionKernelCommandLine=!systemd.volatile).)

We can't use mount move without changing the mount propagation (moving a mount residing under a shared mount is unsupported.), so we could make /sysroot private, but this might have side effects.

Last solution is to have a first .service that prepare a temp bind mount already rw (at $stateroot/var), have var.mount bind that, then have a second .service to cleanup the temporary bind mount

Adding mount.ostreebindrw would only impact EL9 and is way simpler / cleaner than 2 extra .service IMO

@cgwalters what do you prefer ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another solution is also to keep thing as is for distro with old mount (EL9)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well...yes, we could probably have a rhel9 branch here.

I guess bigger picture from my PoV we're trying really hard to work on the new bootc+composefs backend, and I'm hesitant on the risk/reward of further deeper changes here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for a separate rhel9 branch, just a configure option

Will send the first cleanup commit in a separate PR and rework this one to remove the mount just for recent system, the changes should be smaller.

Will the new backend be usable with rpm-ostree ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/prepare-root Issue relates to ostree-prepare-root

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing /sysroot/ostree/deploy/fedora/var bind mount after soft-reboot (cosmetic)

3 participants