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

osutil/mkfs: disable orphan_file feature for ext4 #13373

Closed

Conversation

upils
Copy link
Contributor

@upils upils commented Nov 20, 2023

The SRU to fix LP #2025339 has been rejected. So I would like to find a solution in snapd to fix https://bugs.launchpad.net/ubuntu-image/+bug/2028564.

Currently I disabled the orphan_file feature in every case but this will not work for series older than lunar because this feature was not present (mkfs.ext4 fails with a Invalid filesystem option set: ^orphan_file error). An ideal solution would be to do it only when running on Lunar. I am looking into it but do you know if there is already an easy way to check the series in snapd?

Currently, calling mkfs.ext4 without any option will result in a filesystem with the orphan_file feature disabled on mantic and up, so we do not loose anything setting it up explicitly for now.

@Meulengracht
Copy link
Member

So just FTR, I believe the way to get the version of the host ubuntu is to utilize the information from /etc/os-release. This information is readily available in the following string: release.ReleaseInfo.VersionID, which contains for instance "22.04" or whatever version. You'd need to parse the string, or create a helper to do this.

@upils upils force-pushed the prevent-ext4-compatibility-issues branch from d016e63 to ad6a371 Compare November 21, 2023 10:31
@upils
Copy link
Contributor Author

upils commented Nov 21, 2023

The current strategy is to check if we are running on a Lunar version. I saw we already do this kind of checks in other places in the code.

What we really would like to do is check if mkfs.ext4 supports the orphan_file feature and disable it if so. But this is more complex change, probably needing to either execute mkfs.ext4 a first time or/and properly parse /etc/mk2fs.conf. So let me know if you think that would be a cleaner solution.

@upils upils marked this pull request as ready for review November 21, 2023 10:34
@upils upils changed the title Disable orphan_file feature for ext4 osutil/mkfs: disable orphan_file feature for ext4 Nov 21, 2023
@upils upils force-pushed the prevent-ext4-compatibility-issues branch from ad6a371 to 752744a Compare November 27, 2023 08:37
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (89cdb0c) 78.82% compared to head (752744a) 78.82%.
Report is 5 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #13373      +/-   ##
==========================================
- Coverage   78.82%   78.82%   -0.01%     
==========================================
  Files        1022     1022              
  Lines      127437   127447      +10     
==========================================
+ Hits       100458   100460       +2     
- Misses      20698    20703       +5     
- Partials     6281     6284       +3     
Flag Coverage Δ
unittests 78.82% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alfonsosanchezbeato
Copy link
Member

alfonsosanchezbeato commented Nov 28, 2023

I think that the alternative would be to include mkfs.ext4 in the ubuntu-image snap (this would solve other issues like a crash that happens when running in xenial). ubuntu-image could also ship all the different versions of /etc/mke2fs.conf from the different ubuntu releases (this files comes in e2fsprogs deb package).

According to the mkfs.ext4 man page, there is an env var called MKE2FS_CONFIG that can be used to determine the path of the configuration files. So ubuntu-image would do

  1. Check UC version in the model assertion
  2. Set MKE2FS_CONFIG accordingly (something like $SNAP/<some_path>/<LTS_release>/mke2fs.conf)
  3. Then call snapd as usual

Doing things this way would make sure that mkfs.ext4 uses always the default options for a given Ubuntu release, regardless of where it is running.

cc @sil2100

@upils
Copy link
Contributor Author

upils commented Nov 28, 2023

@alfonsosanchezbeato thank you for the suggestion. I thought solving this in snapd would also prevent other use cases you may have from breaking. But if you think this is fine as is in snapd we can fix it in ubuntu-image.

Quick question regarding your suggestion: It means we would need to maintain a list of mke2fs.conf files, yes? I suspect this might be a problem because we would need to closely follow modifications made in the file when the e2fsprogs package is updated. Is there a recommended way on this?

@alfonsosanchezbeato
Copy link
Member

@alfonsosanchezbeato thank you for the suggestion. I thought solving this in snapd would also prevent other use cases you may have from breaking. But if you think this is fine as is in snapd we can fix it in ubuntu-image.

Solving it in ubuntu-image would solve this kind of issues in the long run, not only this concrete one. Note that in any case including mkfs.ext4 is necessary as I've seen crashes due to mismatch of libraries shipped in the snap with the ones expected by the rootfs mkfs.ext4. Which is a common problem with classic snaps.

Quick question regarding your suggestion: It means we would need to maintain a list of mke2fs.conf files, yes? I suspect this might be a problem because we would need to closely follow modifications made in the file when the e2fsprogs package is updated. Is there a recommended way on this?

Yes, it will be necessary to have such a list. In principle it should not be a big deal to download the latest version of e2fsprogs from the xxxxx-proposed pocket and extract that concrete file while building the `ubuntu-image´ snap - I certainly would not include them in the git repo, but instead gather them at build time. Also, I do not really expect modifications to this configuration files once a release is out.

@upils
Copy link
Contributor Author

upils commented Nov 28, 2023

OK, thanks. I was already convinced adding e2fsprogs in the snap was needed, it will kill two birds with one stone.

Let's close this then.

@upils upils closed this Nov 28, 2023
@upils upils deleted the prevent-ext4-compatibility-issues branch November 30, 2023 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants