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

s390x: skip grub.cfg #706

Merged
merged 1 commit into from
Aug 23, 2019
Merged

s390x: skip grub.cfg #706

merged 1 commit into from
Aug 23, 2019

Conversation

alicefr
Copy link

@alicefr alicefr commented Aug 13, 2019

Grub is not used on s390x architecture

Signed-off-by: Alice Frosi [email protected]

@alicefr
Copy link
Author

alicefr commented Aug 13, 2019

@jcajka @ausil

alicefr pushed a commit to alicefr/coreos-assembler that referenced this pull request Aug 13, 2019
Grub is not used on s390x architecture

Backport of: coreos#706

Signed-off-by: Alice Frosi <[email protected]>
@ajeddeloh
Copy link
Contributor

This fixes an error, but I don't think it'll be sufficient to make gf-platformid work right now. Won't we need to run zipl to regen the bootloader config until we get two stage bootloading working?

@alicefr
Copy link
Author

alicefr commented Aug 14, 2019

@ajeddeloh we need to rerun zipl during firstboot to remove the ignition kargs. We're setting the ignition arg during build (see https://github.com/tuan-hoang1/coreos-assembler/blob/7542ca45cf9007d9d0dbf91b340b6650574afaee/src/create_disk.sh#L141 part of Tuan's PR). We plan to add that part in ignition-dracut. Without coreOS is always booted in firstboot mode

alicefr pushed a commit to alicefr/coreos-assembler that referenced this pull request Aug 14, 2019
Grub is not used on s390x architecture

Backport of: coreos#706

Signed-off-by: Alice Frosi <[email protected]>
alicefr pushed a commit to alicefr/coreos-assembler that referenced this pull request Aug 15, 2019
Grub is not used on s390x architecture

Backport of: coreos#706

Signed-off-by: Alice Frosi <[email protected]>
@dustymabe
Copy link
Member

We're setting the ignition arg during build (see https://github.com/tuan-hoang1/coreos-assembler/blob/7542ca45cf9007d9d0dbf91b340b6650574afaee/src/create_disk.sh#L141 part of Tuan's PR).

This looks kind of like what I suggested we do for all architectures. @ajeddeloh @alicefr could I get you to comment over there?

@@ -41,7 +41,7 @@ coreos_gf_run_mount "${tmp_dest}"
# * grub config
# * BLS config (for subsequent config regeneration)
# First, the grub config.
if [ "$arch" != "x86_64" ]; then
if [ "$arch" != "x86_64" ] && [ "$arch" != "s390x" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

It would be great if we explained in the comment above why we don't need to do this on these two platforms

Copy link
Contributor

Choose a reason for hiding this comment

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

actually arm should be in this list too since we landed anaconda-less arm.

Copy link
Author

Choose a reason for hiding this comment

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

@ajeddeloh should I add arm too? I don't have the possibility to test it on arm

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah

Copy link
Author

Choose a reason for hiding this comment

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

@ajeddeloh so I guess, I can simply state if arch == ppc64le. Is it still required for power because they use anaconda?

Copy link
Contributor

Choose a reason for hiding this comment

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

Right. On non-anaconda the grub config uses BLS and thus doesn't have the entries in the grub config, which is what this path is handling. The anaconda path is doing a more traditional grub2-mkconfig.

$arch == ppc64le sgtm.

Copy link
Author

Choose a reason for hiding this comment

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

@ajeddeloh I updated the PR

@alicefr
Copy link
Author

alicefr commented Aug 20, 2019

@barthy1 @tuan-hoang1

@alicefr
Copy link
Author

alicefr commented Aug 20, 2019

We're setting the ignition arg during build (see https://github.com/tuan-hoang1/coreos-assembler/blob/7542ca45cf9007d9d0dbf91b340b6650574afaee/src/create_disk.sh#L141 part of Tuan's PR).

This looks kind of like what I suggested we do for all architectures. @ajeddeloh @alicefr could I get you to comment over there?

@dustymabe on s390x every time you want to overwrite the kernel param you need to rerun zipl. So I think s390x needs always to be handled in a special way.

Set grub.cfg only for ppc64le (using anaconda)

Signed-off-by: Alice Frosi <[email protected]>
@cgwalters
Copy link
Member

This LGTM as is - an improvement though we can make later (or now) is to have a centralized ARCH_USES_ANACONDA variable in e.g. cmdlib.sh and test against that, then we could do the same in the

    case "$arch" in
        "x86_64"|"aarch64"|"s390x")

bit in cmd-build.

@cgwalters
Copy link
Member

(But eh, it's not like new architectures come along frequently, we're just slightly optimizing I guess for the person who will try to add RISC-V)

@cgwalters cgwalters merged commit 68bd29a into coreos:master Aug 23, 2019
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.

4 participants