grml-debootstrap icon indicating copy to clipboard operation
grml-debootstrap copied to clipboard

Initial arm64 support

Open GavinPacini opened this issue 2 years ago • 5 comments

Hi there, I finally decided to tackle #169, about time. :)

This is an initial implementation. I've tested it with:

sudo UPGRADE_SYSTEM='no' ./grml-debootstrap \
        	--debopt "--verbose" \
        	--arch arm64 \
        	--filesystem ext4 \
        	--hostname host \
        	--nopassword \
        	--nopackages \
        	--release bullseye \
        	--keep_src_list \
        	--verbose \
        	--vmfile \
        	--vmsize 15G \
        	--target ../debian-grml.raw

and I've been able to boot this image with qemu on an Apple Silicon based Mac (through UTM).

Trouble is that I cannot really make sure I didn't break the amd64 builds, albeit I made every effort not to.

I plan to test it a bit more but would appreciate an initial review at this stage. Thanks!

GavinPacini avatar Mar 24 '22 03:03 GavinPacini

Hi @GavinPacini oh wow, kudos! :partying_face: I'm currently quite busy and won't have time to review it right now, but on first glance it looks rather reasonable, will try to go through it and give it a try ASAP! :)

mika avatar Mar 25 '22 13:03 mika

Thanks @mika! No problem, appreciate any feedback once you have the chance.

GavinPacini avatar Mar 25 '22 16:03 GavinPacini

@GavinPacini finally had some time to take a look at it. Overall the PR looks good to me, there are some minor details we could improve, but let's polish this once we know it's working as expected. :)

I tried installing it into a VM, running:

CONFFILES=$(pwd) ./grml-debootstrap --vm --vmfile --target /dev/shm/debian-arm64.img -r bullseye --arch arm64 --password grml

But this failed for me with:

However the following packages replace it:
  grub2-common grub-common

E: Package 'grub-pc' has no installation candidate

This is related to the usage of --nopackages (adding that option then built the VM as expected), so I suppose we need to further adjust chroot-script to properly handle the grub situation also for arm64? :)

mika avatar May 06 '22 13:05 mika

Thanks for reviewing @mika!

I just reproduced that myself, thanks for bringing it up. After some digging, grub-pc seems to be the standard way to setup MBR / BIOS systems. There is no real support for MBR / BIOS on arm64 architectures (they use UEFI only). For this reason, we use grub2-common and grub-efi-arm64 packages (as shown in the diff here).

Basically, arm64 images should never have grub-pc installed. However grub-pc is in the packages file. Once I removed this from the packages file it created the VM fine. I see three options:

  1. Create a separate packages file for arm64 images (e.g. packages-arm64) which we can switch to based on the --arch flag.
  2. Add some flags into the packages file which allows us to denote if a package should be skipped for certain architectures.
  3. Hardcode in ignoring grub-pc for arm64 images.

I would probably pick number 1 as the most future proof, but there would be a lot of duplication for now.

What do you think?

GavinPacini avatar May 24 '22 22:05 GavinPacini

1. seems also the most reliable, least surprising to me. 2. seems rather complex to implement, not very intuitive. 3. would be the most surprising.

So From the three options, I like 1. most.

adrelanos avatar Jun 01 '22 22:06 adrelanos

@GavinPacini Thanks for looking into it. Are you still on it? 😊

fosple avatar Jan 12 '23 09:01 fosple

Which of these options would be acceptable? @mika

adrelanos avatar Jan 13 '23 15:01 adrelanos

Hey @fosple, once @mika provides some direction on the preferred options mentioned above, I am happy to finish it out.

GavinPacini avatar Jan 17 '23 01:01 GavinPacini

Sorry for the delay, I also think that the first option (Create a separate packages file for arm64 images (e.g. packages-arm64) which we can switch to based on the --arch flag.) seems preferable given our current situation!

mika avatar Feb 02 '23 14:02 mika

Hi again, I've just pushed something that I think solves the outstanding issues. @mika I would appreciate another review when you have a chance. Thanks!

GavinPacini avatar Feb 27 '23 15:02 GavinPacini

@GavinPacini great, thanks! Your changes look good to me on a quick sight, though I need a more quiet time to give it a more detailed look. Also in Debian we're in freeze stage for bookworm (see https://release.debian.org/testing/freeze_policy.html), so it might make sense to apply those changes once we can upload towards Debian/unstable with changes like new features again. :)

mika avatar Mar 15 '23 10:03 mika

@mika Hey, just wondering what else is needed to get this merged in? I just want to make sure we don't miss the next release window. Thanks!

GavinPacini avatar Sep 29 '23 12:09 GavinPacini

@GavinPacini I'll try to look into this in the next few days to get this merged ASAP! Thanks for the ping

mika avatar Sep 29 '23 12:09 mika

@GavinPacini let's try to get this merged before more changes pile up and cause further merge conflicts :) I took your PR and tried to get it ready for submission, please see https://github.com/grml/grml-debootstrap/pull/214 - feel free to take it over and force-push it here or so - all credits should be toally yours of course! :) (When reviewing, ignore all the indention related changes, e.g. via git show --ignore-all-space on the cmdline)

Disclaimer: not yet tested/verified on my side. BTW, what's your environment to test this? An actual arm64 system, or a VM?

I've only two things in mind now:

  1. why do we skip the Adjusting grub.cfg for successful boot sequence. in the arm64 situation? (Only trying to understand the situation we're in)
  2. now also having EFI support for VMs (see f4d3906aae588d8dac1a146f05151211073cbb80), maybe there is something we could do to simplify code and unify code, see e.g.:
parted -s "${TARGET}" 'mkpart ESP fat32 1MiB 101MiB'

vs

parted -s "${TARGET}" 'mkpart EFI fat32 1MiB 10MiB'

But perfect is the enemy of good, I think we can further improve/iterate of anyway, though I'd appreciate your feedback before submitting this :)

mika avatar Oct 13 '23 14:10 mika

I also think it's better to merge imperfect stuff fast for the purpose of avoiding merge conflicts.

You can withhold it from user documentation and/or label it an alpha/unsupported feature meanwhile if not (yet) tested. Since this is primarily a sysadmin / developer tool, the facts can just be documented and/or mentioned by the scripts. By facts I mean mentioning "community supported architecture, untested".

With increasing platform support it won't be possible for the main developer(s) to keep all tested. If github actions supported arm64 (not at time of writing but planned according to the tickets), we could use that. Perhaps in the future.

adrelanos avatar Oct 13 '23 17:10 adrelanos

FYI: if anyone of you can give https://github.com/grml/grml-debootstrap/pull/214 a try and let me know whether this works for you, I'd submit it ASAP.

mika avatar Oct 13 '23 18:10 mika

I tested it but a bit too slow. Before today's rebase.

I am not an expert for qemu but this is my quick way to test boot it.

#!/bin/bash

set -x
set -e

## We might get away with --no-install-recommends, fewer packages.
sudo apt-get install qemu-system-arm qemu-system-aarch64 qemu-efi-aarch64 qemu-utils

qemu-system-aarch64 \
    -M virt \
    -cpu cortex-a57 \
    -m 1024 \
    -bios /usr/share/qemu-efi-aarch64/QEMU_EFI.fd \
    -drive format=raw,file=/home/user/grml-debootstraptestbin/test.img \
    -nographic

Happy to report that build + boot was success.

adrelanos avatar Oct 16 '23 13:10 adrelanos

Continued testing here: https://github.com/grml/grml-debootstrap/pull/214#issuecomment-1771021365

adrelanos avatar Oct 19 '23 14:10 adrelanos

@GavinPacini we took care of your PR via https://github.com/grml/grml-debootstrap/pull/214, thanks for your work!

mika avatar Oct 25 '23 16:10 mika