Closed anonymouse64 closed 4 years ago
Looks like a sane change to me. One thing I would note is that there's now four partitions in the image and, if they're all created as primary (I don't know if this is the case) then that potentially means that users can't create any more partitions (for whatever purposes).
It may be worth noting that the Pi's boot firmware gained the ability to handle GPT partition tables a while ago (primarily to support those booting off USB, but I understand it works on SD cards too). If the 4 partitions in the image are all primary (i.e. if there's no extended partition which users could create logical partitions within) then it might be worth switching the image to use GPT partitioning instead.
Anyway, overall it's a +1 from me.
that potentially means that users can't create any more partitions (for whatever purposes).
Well if users want additional partitions they will have to modify the gadget.yaml anyways and so they can probably weigh the benefits of keeping ubuntu-save or adding their own partition, but in light of this I will add a comment to the gadget.yaml explaining that ubuntu-save is optional for unencrypted devices and that with MBR there can no longer be any partitions.
It may be worth noting that the Pi's boot firmware gained the ability to handle GPT partition tables a while ago
This is great news actually, when we last looked at this, the firmware didn't support this, so if GPT is now supported across all the pi's we care about, then I think we should look into doing this. Would changing to GPT affect our u-boot support at all? I am fairly confident u-boot supports GPT, but the pi is special in numerous ways so I'm not sure if how we use it means it will be supported properly.
But not for this PR obviously since we want this before 1.0 :smile:
Re GPT partitions table opened https://github.com/snapcore/pi-gadget/issues/58
btw are our classic presintalled images not gpt already?
Fixes #54 for arm64
Note that this needs snapd 2.48 to build and install successfully.