-
Notifications
You must be signed in to change notification settings - Fork 135
stages/coreos.live-artifacts: add erofs support #2002
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
Conversation
595cd28 to
961497b
Compare
We discussed this in a meeting last week with @mvo5. What we (CoreOS team) would really like to do here is actually define these settings inside of the payload that is the CoreOS (Fedora CoreOS or RHEL CoreOS) filesystem/target image. The question we have here is: can we inspect and do things based on the target image? If yes then what's preferred? Should we base our behavior on:
Our preference at this point is to base it on some configuration intentionally applied inside the target image. |
Right, i've opened this PR to start the discussion. Reading configuration file(s) is in progress right now. |
mvo5
left a comment
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.
Some quick drive-by code-reivew
As for the questions how to enable erofs/squashfs: in general our policy is that this should come via configuration (like this PR). However AIUI this is not the desired behavior for you so I'm fine with a different approach that looks at e.g. some configuration inside the target and determine based on that if erofs should be used or not. While not something we generally do or encourage this stage here is already unusual so doing seems okay to me.
5fff0ea to
3400e9a
Compare
|
@mvo5, @dustymabe , @jlebon as discussed, 2nd patch here adds ability to get options from file |
3400e9a to
8db5ed0
Compare
|
I'll review and test this today. Thanks @nikita-dubrovskii |
dustymabe
left a comment
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.
Thank you for working on this @nikita-dubrovskii. This mostly LGTM. a few comments!
8db5ed0 to
7488785
Compare
7488785 to
2f26bef
Compare
dustymabe
left a comment
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
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.
Thanks. Other than Dusty's comments, this all LGTM. Feel free to address those comments in a separate PR if you'd rather have this merged sonner.
This adds possibility to specify filesystem of
rootfs.img.Issue: coreos/fedora-coreos-tracker#1852