[Merge] ~gjolly/livecd-rootfs:ubuntu-cpc/mount_esp_on_boot into livecd-rootfs:ubuntu/master

Steve Langasek mp+433639 at code.launchpad.net
Wed Feb 1 16:15:05 UTC 2023


Review: Needs Fixing



Diff comments:

> diff --git a/live-build/ubuntu-cpc/hooks.d/base/disk-image-uefi.binary b/live-build/ubuntu-cpc/hooks.d/base/disk-image-uefi.binary
> index 8ef4fca..a653b7c 100755
> --- a/live-build/ubuntu-cpc/hooks.d/base/disk-image-uefi.binary
> +++ b/live-build/ubuntu-cpc/hooks.d/base/disk-image-uefi.binary
> @@ -51,7 +57,7 @@ create_partitions() {
>          arm64|armhf)
>              if [ "${SUBARCH:-}" = "generic" ]; then
>                  sgdisk "${disk_image}" \
> -                       --new=15:0:204800 \
> +                       --new=15:0:+1G \

How does this 1G limit line up with what our installers currently require as a minimum size for /boot?  It's ok if it's larger on the cloud; if it's smaller, we should make sure to bump it to at least match that minimum, which has been carefully worked out (but I don't know offhand what the current values are).

>                         --typecode=15:ef00 \
>                         --attributes=15:set:2 \
>                         --new=14::+4M \
> @@ -458,16 +475,34 @@ EOF
>  
>  disk_image=binary/boot/disk-uefi.ext4
>  
> +# create the disk
>  create_empty_disk_image "${disk_image}"
>  create_partitions "${disk_image}"
>  mount_image "${disk_image}" 1
>  
> -# Copy the chroot in to the disk
> +# create and mount the rootfs partition
>  make_ext4_partition "${rootfs_dev_mapper}"
>  mkdir mountpoint
>  mount "${rootfs_dev_mapper}" mountpoint
> -cp -a chroot/* mountpoint/
> -umount mountpoint
> +
> +# create and mount the ESP
> +create_and_mount_uefi_partition mountpoint
> +
> +# Copy the chroot in to the disk
> +# ...first copy the boot folder, this
> +# will return a non-0 exit code because boot
> +# contains symlinks that cannot be copied on
> +# the vfat partition
> +cp -a chroot/boot/* mountpoint/boot/ || true

But you configure $mountpoint/etc/kernel-img.conf to not create symlinks.  Would it perhaps be better to set set do_symlinks=no earlier so that there are no symlinks to be copied, and we don't have to ignore errors?  (This would be more robust against other symlinks being unexpectedly added to /boot later, failing to be copied, and causing runtime errors on the resulting image because of their absence.)

> +
> +# ...then copy everything else, we don't
> +# expect any error here
> +cp -a chroot/!(boot) mountpoint/
> +
> +configure_chroot mountpoint
> +
> +# cleanup the mount
> +umount -R mountpoint
>  rmdir mountpoint
>  
>  install_grub


-- 
https://code.launchpad.net/~gjolly/livecd-rootfs/+git/livecd-rootfs/+merge/433639
Your team Ubuntu Core Development Team is requested to review the proposed merge of ~gjolly/livecd-rootfs:ubuntu-cpc/mount_esp_on_boot into livecd-rootfs:ubuntu/master.




More information about the Ubuntu-reviews mailing list