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

compose: Add boot-location: modules #1773

Closed
wants to merge 2 commits into from

Conversation

cgwalters
Copy link
Member

@cgwalters cgwalters commented Mar 7, 2019

The libostree support for handling /usr/lib/ostree-boot has
existed for over 4 years:

commit 37a059925f6b96d30190b65bee6bdde0ae1c6915
Commit:     Colin Walters <[email protected]>
CommitDate: Sun Nov 30 23:14:05 2014 -0500

    deploy: Ensure that we can deploy using only /usr/lib/ostree-boot

I think we assume now that no one is now making new treecomposes and needs
a newer rpm-ostree and that they expect people to be able to use as an
upgrade target from a libostree that predates that.

@@ -50,7 +50,6 @@
#include "rpmostree-rust.h"

typedef enum {
RPMOSTREE_POSTPROCESS_BOOT_LOCATION_BOTH,
RPMOSTREE_POSTPROCESS_BOOT_LOCATION_NEW
} RpmOstreePostprocessBootLocation;
Copy link
Member

Choose a reason for hiding this comment

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

We can just drop this enum completely now, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes but I have a followup PR to add boot-location: modules 😄

Copy link
Member

Choose a reason for hiding this comment

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

Ahh gotcha! Maybe just a one-liner in the commit msg this is prep for a new variant?

src/libpriv/rpmostree-postprocess.c Show resolved Hide resolved
I'd like to add a new `boot-location: modules`.  In prep
for that, let's remove the legacy `both` which drops into
`/boot`.

The libostree support for handling `/usr/lib/ostree-boot` has
existed for over 4 years:

```
commit 37a0599
Commit:     Colin Walters <[email protected]>
CommitDate: Sun Nov 30 23:14:05 2014 -0500

    deploy: Ensure that we can deploy using only /usr/lib/ostree-boot

```

I think we assume now that no one is now making *new* treecomposes and needs
a newer rpm-ostree and that they expect people to be able to use as an
upgrade target from a libostree that predates that.
@cgwalters cgwalters changed the title compose: Remove support for boot-location: both compose: Add boot-location: modules Mar 8, 2019
@vtolstov
Copy link

vtolstov commented Mar 8, 2019

sorry, but what is the benefits of using boot_location: modules vs new?
Not create additional dir or something else?

@cgwalters
Copy link
Member Author

sorry, but what is the benefits of using boot_location: modules vs new?

See the commit message -

There are a few reasons to do this; most prominent is that it avoids duplicating the content as the locations may have different SELinux labels.

@cgwalters
Copy link
Member Author

@vtolstov
Copy link

vtolstov commented Mar 8, 2019

@cgwalters ah thanks! i miss this commit message.

* "new": A misnomer, this value is no longer "new". Kernel data
goes in `/usr/lib/ostree-boot` in addition to `/usr/lib/modules`.
This is the default.
* "modules": Kernel data goes just in `/usr/lib/modules`
Copy link
Member

Choose a reason for hiding this comment

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

Nit: missing period.

EOF
diff -u bootls{-expected,}.txt
# Verify /usr/lib/ostree-boot
ostree --repo=${repobuild} ls -R ${treeref} /usr/lib/ostree-boot > bootls.txt
Copy link
Member

Choose a reason for hiding this comment

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

Minor: do we even need the directory to exist at all? It'd be cleaner to not even have it at all, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

For systems installed by Anaconda it's where the bootloader data lives.

For FCOS...we could actually inject it statically into /boot and drop it from the tree. That'd have to be a new config option...

Copy link
Member Author

Choose a reason for hiding this comment

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

Or really it'd have to be like compose tree --bootdir=$(pwd)/tmp/boot and then we change cosa to drop that stuff in the disk image's /boot - after we stop using Anaconda.

Copy link
Member Author

Choose a reason for hiding this comment

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

And this (for now at least) completes the epic journey of the
"where's the kernel"?  With this it's found solely in
`/usr/lib/modules/$kver`.

There are a few reasons to do this; most prominent is that
it avoids duplicating the content as the locations may have
different SELinux labels.
@jlebon
Copy link
Member

jlebon commented Mar 8, 2019

@rh-atomic-bot r+ 08acff6

@rh-atomic-bot
Copy link

⌛ Testing commit 08acff6 with merge adff1e9...

rh-atomic-bot pushed a commit that referenced this pull request Mar 8, 2019
And this (for now at least) completes the epic journey of the
"where's the kernel"?  With this it's found solely in
`/usr/lib/modules/$kver`.

There are a few reasons to do this; most prominent is that
it avoids duplicating the content as the locations may have
different SELinux labels.

Closes: #1773
Approved by: jlebon
@rh-atomic-bot
Copy link

☀️ Test successful - status-atomicjenkins
Approved by: jlebon
Pushing adff1e9 to master...

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

Successfully merging this pull request may close these issues.

4 participants