s390x: do not write first boot args to BLS file

[tuan-hoang1]

The first boot arguments are just ephemeral. If we add them to BLS file,
then we have to remove them in ignition-firstboot-complete.service
during first boot, which would complicate it. Without adding them to BLS
file, an invoke of zipl without any parameters (assuming zipl.conf and
BLS files are present) is enough.

[cgwalters]

I see the problem but I don't quite understand how this fixes it. Won't we start without the firstboot flag? Or what am I missing?

[tuan-hoang1]

Without this change, the first boot arguments are written to both BLS files and bootloader segments/blocks of the disk ("burnt" by zipl). We only need the latter to start the image with first boot arguments. The write to BLS file is redundant. So basically this one is to fix my mistake :D

[tuan-hoang1]

Be reminded that, we only re-rewrite the BLS file atm is because of s390-tools issue 64 (aka. expect order of fields, and no blank lines). After it's fixed, we don't need to touch the BLS file at all. In fact Javier fixed it in ostree.

[jlebon]

Hmm, I'm confused, does zipl not apply the kernel cmdline options in the BLS config? Why do we need to pass it through --parmfile?

[cgwalters]

I don't quite understand this still, sorry. If you tested it and it has the full expected behavior that:

1. On first boot, the ignition.firstboot kernel parameter is provided
2. When ignition-firstboot-complete.service works, the kernel parameter is removed

IOW, Ignition only runs once - then OK by me!

But I thought s390x support for this was predicated on the patch to sed the BLS file that's...somewhere.

[cgwalters]

It may also be the case that while this patch doesn't fix everything, it makes things better - if so I have no objections to merging.

(Thinking about this a bit more, it seems to me that ignition-firstboot-complete.service would just need to rerun zipl with this patch - does that sound right?)

[tuan-hoang1]

@jlebon

Hmm, I'm confused, does zipl not apply the kernel cmdline options in the BLS config? Why do we need to pass it through --parmfile?

Yes, zipl does apply kcmdline options in BLS config, but if we just invoke zipl, it will write zipl records to the host's disk - not what we want. That's why we need --parmfile and --target to write on the rootfs/boot (aka, boot partition on target disk). There is a comment in the code about chroot into target disk which might help you.

[tuan-hoang1]

@cgwalters

I don't quite understand this still, sorry. If you tested it and it has the full expected behavior that:

1. On first boot, the `ignition.firstboot` kernel parameter is provided

Yes, zipl burns zipl records including ignition.firstboot on target disk (rootfs/boot mount point)

2. When `ignition-firstboot-complete.service` works, the kernel parameter is removed

Yes. I have not sent a patch yet but it would be as simple as this:

ignition-firstboot-complete.service
...
ExecStart=/usr/sbin/ignition-firstboot-complete.sh
...

ignition-firstboot-complete.sh
#!/bin/sh
mount -o remount,rw /boot && rm /boot/ignition.firstboot
if [ "$(uname -m)" = "s390x" ]; then
    # assuming /etc/zipl.conf (or /lib/s390utils/zipl.conf as Dan Horak suggested) and BLS files existed
    zipl
fi

IOW, Ignition only runs once - then OK by me!

Yes. It has to :)

But I thought s390x support for this was predicated on the patch to sed the BLS file that's...somewhere.

Maybe this one ? ibm-s390-linux/s390-tools#69

It may also be the case that while this patch doesn't fix everything, it makes things better - if so I have no objections to merging.

Correct. Without this patch, ignition-firstboot-complete.sh would look like this:

#!/bin/sh
mount -o remount,rw /boot && rm /boot/ignition.firstboot
if [ "$(uname -m)" = "s390x" ]; then
    sed -i -e '@firstboot.ignition@d' /boot/loader/entries/fedora.conf
    # assuming /etc/zipl.conf (or /lib/s390utils/zipl.conf as Dan Horak suggested) and BLS files existed
    zipl
fi

So there is no point to add ignition.firstboot to BLS file at all. Again, this is to fix my mistake.

(Thinking about this a bit more, it seems to me that ignition-firstboot-complete.service would just need to rerun zipl with this patch - does that sound right?)

Correct again!

[tuan-hoang1]

If you prefer, we can postpone this, and let us (from ibm/s390-tools and fedora/s390utils sides) fix the current 2 issues on with zipl first (expected to be done in the next 3-4 days) then we can merge this.

I say this because atm during ignition-firstboot-complete.sh, zipl cannot run cleanly because :

1. Missing zipl.conf
2. "Missing" /boot prefix in linux and initrd fields in BLS files, or zipl has to understand this "missing" and behave better.

I'm perfectly fine with this.

[tuan-hoang1]

The first issue got a patch, looks ok (s390-tools#70). Patch for second issue is ongoing (s390-tools#69).
I just got from Ian McLeod that even if those got landed, it might not get into RHEL/RHCOS in time ..... So, I guess we have to some not-so-pretty fixup in coreos-assembler then @martinezjavier

[tuan-hoang1]

Related ibm-s390-linux/s390-tools#71 and ibm-s390-linux/s390-tools#74

[tuan-hoang1]

Friendly ping. I think we have everything we needed.