Comment 19 for bug 1790901

Revision history for this message
Robie Basak (racb) wrote :

> There could be cases where former PXE setups using the old technique would have to be slightly adapted to work with the new one. But all partners and peers related to this expressed that the current one is almost unusable and they'd want the new one.

I appreciate Christian pointing this out directly rather than having me spend time discovering it during review. The point itself concerned me. After speaking to Christian we concluded (and he tested) that the former method still works without modification. So previous behaviour is preserved, and therefore I don't need to consider whether breaking it is acceptable for SRU (and we don't have to risk that some users exists who is relying on it). To ensure this, please add a test for previous behaviour to SRU verification.

My understanding then is:

Provide a filename, like before, and it is treated as a bundled blob containing what is needed. Provide no filename, or a filename that is not found, and whereas previously boot would have failed, now syslinux's pxelinux emulation is enabled and boot will proceed using the emulation (fetching pxelinux configuration and using that, etc).

Does everyone realise that once this proposed change lands it'll be "locked in", and you won't necessarily be able to revise the interface again, for example changing behaviour in the syslinux pxelinux emulation? Since by the SRU justification itself it'll be useful this time, so any future changes may violate the "intrusive" part of the policy.

Review notes:

This is the effective proposed change to the source tree in Bionic:

 hw/s390x/ipl.c | 18 +-
 hw/s390x/ipl.h | 25 ++-
 pc-bios/s390-ccw/Makefile | 3 +-
 pc-bios/s390-ccw/bootmap.c | 60 +-----
 pc-bios/s390-ccw/bootmap.h | 20 +-
 pc-bios/s390-ccw/iplb.h | 34 ++-
 pc-bios/s390-ccw/jump2ipl.c | 91 ++++++++
 pc-bios/s390-ccw/libc.c | 88 ++++++++
 pc-bios/s390-ccw/libc.h | 37 +++-
 pc-bios/s390-ccw/main.c | 23 +-
 pc-bios/s390-ccw/netboot.mak | 16 +-
 pc-bios/s390-ccw/netmain.c | 384 +++++++++++++++++++++++++--------
 pc-bios/s390-ccw/s390-ccw.h | 4 +
 pc-bios/s390-ccw/sclp.c | 10 +-
 roms/SLOF/lib/libc/include/assert.h | 36 ++++
 roms/SLOF/lib/libc/include/stdio.h | 1 +
 roms/SLOF/lib/libc/stdio/Makefile.inc | 2 +-
 roms/SLOF/lib/libc/stdio/snprintf.c | 28 +++
 roms/SLOF/lib/libc/stdlib/free.c | 4 +-
 roms/SLOF/lib/libc/string/Makefile.inc | 2 +-
 roms/SLOF/lib/libc/string/strrchr.c | 28 +++
 roms/SLOF/lib/libnet/Makefile | 2 +-
 roms/SLOF/lib/libnet/bootp.c | 2 +-
 roms/SLOF/lib/libnet/dhcp.c | 76 +++++--
 roms/SLOF/lib/libnet/dhcpv6.c | 3 +-
 roms/SLOF/lib/libnet/ipv6.c | 2 +-
 roms/SLOF/lib/libnet/libnet.code | 6 +-
 roms/SLOF/lib/libnet/netapps.h | 3 +-
 roms/SLOF/lib/libnet/netload.c | 252 ++++++++++++++--------
 roms/SLOF/lib/libnet/ping.c | 14 +-
 roms/SLOF/lib/libnet/pxelinux.c | 251 +++++++++++++++++++++
 roms/SLOF/lib/libnet/pxelinux.h | 33 +++
 roms/SLOF/lib/libnet/tftp.c | 132 ++++++++++--
 roms/SLOF/lib/libnet/tftp.h | 16 +-
 34 files changed, 1335 insertions(+), 371 deletions(-)

Changes are clearly s390x-specific. AFAICT, roms/SLOF/lib/libnet is only used by pc-bios/s390/ccw/, and Christian confirmed that roms/SLOF in this (packaged) source tree is only used by s390x.

The backport from Cosmic to Bionic:

lp-1790901-partial-SLOF-for-s390x-netboot-2.11-to-3.0.patch: went from 2.12 to 2.11. The patch touches files roms/SLOF/lib/libnet, and after patch application this directory is identical between Cosmic and Bionic, so this is a wholesale backport "version bump" of this part of the subtree. roms/SLOF/lib/libc is identical as specified (ctype, string, stdlib, include, stdio; Makefile and getopt differ).

These are entirely new patches in Bionic:

ubuntu/lp-1790901-0001-ccw-update-libc.patch (changes are to pc-bioc/s390-ccw only)
ubuntu/lp-1790901-0002-s390-ccw-move-auxiliary-IPL-data-to-separate-locatio.patch (changes to hw/s390x/ and pc-bios/s390-ccw/ only)
ubuntu/lp-1790901-0101-s390-Do-not-pass-inofficial-IPL-type-to-the-guest.patch (changes to pc-bios/s390-ccw only)

These are identical:

lp-1790901-0102-s390-ccw-force-diag-308-subcode-to-unsigned-long.patch
lp-1790901-0201-pc-bios-s390-ccw-net-Split-up-net_load-into-init-loa.patch

lp-1790901-0202-pc-bios-s390-ccw-net-Use-diag308-to-reset-machine-be.patch has been changed for the backport as noted in the patch header (thanks for the note!) and I see no functional changes.

The remaining patches are identical:
lp-1790901-0203-pc-bios-s390-ccw-net-Add-support-for-.INS-config-fil.patch
lp-1790901-0204-pc-bios-s390-ccw-net-Update-code-for-the-latest-chan.patch
lp-1790901-0205-pc-bios-s390-ccw-net-Add-support-for-pxelinux-style-.patch
lp-1790901-0206-pc-bios-s390-ccw-net-Try-to-load-pxelinux.cfg-file-a.patch
lp-1790901-0207-pc-bios-s390-ccw-Optimize-the-s390-netboot.img-for-s.patch

Conclusion: all changes look correct and as described and intended.