Tails

#1 Updated by intrigeri 2018-11-26 14:26:08

• related to Bug #15988: Set unique UUID on first boot & adapt bootloader added

#3 Updated by intrigeri 2018-11-26 14:27:29

• related to deleted (Bug #15988: Set unique UUID on first boot & adapt bootloader)

#4 Updated by segfault 2018-11-26 21:31:10

• Status changed from Confirmed to In Progress

Applied in changeset commit:tails|5c84a362968e731211adc87f487679fb53359c05.

#5 Updated by segfault 2018-11-26 21:35:28

• Assignee changed from segfault to intrigeri
• QA Check set to Ready for QA

Fixed on the branch. The issue was that when the partition is resized, it is actually deleted and a new partition is created, for which we did not specify the type, so the default for VFAT was used.

My first suspicion was that the partition type was changed by the create-usb-image-from-iso script, when the partition is formatted after being created - and indeed the update-partition-type argument had an incorrect type (string instead of bool). I’m surprised that this does not change the type. Note this code was copied from Tails Installer.

#6 Updated by intrigeri 2018-11-27 07:59:33

• % Done changed from 0 to 60

Thanks! Code review passes, will now test.

#7 Updated by intrigeri 2018-11-27 08:44:27

• Status changed from In Progress to Resolved
• Assignee deleted (intrigeri)
• % Done changed from 60 to 100

Works for me :)

#8 Updated by intrigeri 2018-11-27 20:00:13

• Status changed from Resolved to In Progress
• Assignee set to segfault
• % Done changed from 100 to 50
• QA Check deleted (Ready for QA)
• Feature Branch changed from feature/15292-usb-image to feature/15292-generate-usb-image

segfault wrote:
> Fixed on the branch. The issue was that when the partition is resized, it is actually deleted and a new partition is created, for which we did not specify the type, so the default for VFAT was used.

I think I only tested this part this morning, so I think that one works.

> My first suspicion was that the partition type was changed by the create-usb-image-from-iso script, when the partition is formatted after being created - and indeed the update-partition-type argument had an incorrect type (string instead of bool). I’m surprised that this does not change the type.

Sadly, that’s not sufficient: the system partition is still not an ESP in the image generated by create-usb-image-from-iso (so even if we successfully fix that on first boot, that won’t help on an EFI system that won’t boot at all). Thanks kibi for spotting this with his work on Bug #16003: automated tests are useful :)

> Note this code was copied from Tails Installer.

That’s correct, but later on, Tails Installer does this:

        # _set_partition_flags resets the partition type with udisks2 2.7.3-4,
        # so let's set the right one again
        # XXX: sometimes fails (https://labs.riseup.net/code/issues/10987)
        system_partition.call_set_type_sync(ESP_GUID, GLib.Variant('a{sv}', None))

… due to https://github.com/storaged-project/udisks/issues/418, which I’ve reported upstream last year and that got fixed there, but not in the version that’s in Stretch. How about we do the same on feature/15292-generate-usb-image, then merge it into feature/buster, and finally revert this workaround there?

FTR, in the future I’ll try to force myself to validate code changes in create-usb-image-from-iso using the udisks2 version that’s in our current build VMs: as we can see here, testing them on my sid system will lead to different results. And I encourage everyone involved to do the same, for the same reason :)

#10 Updated by intrigeri 2018-11-28 08:43:25

• % Done changed from 50 to 60
• QA Check set to Ready for QA

That bug must affect Stretch as well because applying the same workaround as in Tails Installer fixed the problem for me. Please review and if happy, merge into master→stable→devel and file a ticket so we revert this once our Vagrant build box runs Buster.

#11 Updated by segfault 2018-11-28 11:16:50

• QA Check changed from Ready for QA to Pass

intrigeri wrote:
> Sadly, that’s not sufficient: the system partition is still not an ESP in the image generated by create-usb-image-from-iso

I see. Sorry I missed this. The description actually says that it’s already the image created by create-usb-image-from-iso which doesn’t have the ESP partition, but when I couldn’t reproduce this on my Sid, and then found the issue in the partitioning script, I thought that that must be the reason.

I was now able to reproduce the issue in a Stretch VM and confirm that your commit fixes it.

> FTR, in the future I’ll try to force myself to validate code changes in create-usb-image-from-iso using the udisks2 version that’s in our current build VMs: as we can see here, testing them on my sid system will lead to different results. And I encourage everyone involved to do the same, for the same reason :)

Yes, that seems reasonable, I’ll try to do the same.

#12 Updated by segfault 2018-11-28 11:19:03

• Status changed from In Progress to Fix committed
• % Done changed from 60 to 100

Applied in changeset commit:tails|0de4461484a4fa37f73e63236b2e129f26b26846.

#13 Updated by intrigeri 2018-11-28 13:57:48

• Status changed from Fix committed to Resolved

#14 Updated by intrigeri 2018-11-28 13:58:27

• Assignee deleted (segfault)