Merge lp:~morphis/goget-ubuntu-touch/add-turbo-bootstrap-support into lp:goget-ubuntu-touch

Proposed by Simon Fels on 2016-04-12
Status: Merged
Approved by: Ɓukasz Zemczak on 2016-08-03
Approved revision: 239
Merged at revision: 229
Proposed branch: lp:~morphis/goget-ubuntu-touch/add-turbo-bootstrap-support
Merge into: lp:goget-ubuntu-touch
Diff against target: 129 lines (+74/-9)
2 files modified
devices/fastboot.go (+7/-0)
ubuntu-device-flash/touch.go (+67/-9)
To merge this branch: bzr merge lp:~morphis/goget-ubuntu-touch/add-turbo-bootstrap-support
Reviewer Review Type Date Requested Status
Alfonso Sanchez-Beato Approve on 2016-07-12
Sergio Schvezov 2016-04-12 Approve on 2016-07-11
Alex Tu (community) Needs Fixing on 2016-05-03
Review via email: mp+291583@code.launchpad.net

Commit message

Add bootstrap support for turbo/frieza/cooler devices

The bootloader on turbo/frieza/cooler does not have support for fastboot format <partition> or fastboot boot <image>. Therefor we have to workaround this by using a OEM specific bootloader command to reboot into recovery and adjusting the cleanup of the /cache partition before we push any files.

Also this adds an alias for turbo which is named "smdk" from the bootloader which can't be changed anymore on our production devices.

Description of the change

Corresponding MP for turbo to add the new "format cache" command can be found at https://code.launchpad.net/~morphis/zhongshan/+git/platform_bootable_recovery/+merge/291582

Opened this for a first discussion if we want to add device specific quirks this way.

To post a comment you must log in.
Alex Tu (alextu) wrote :

verified the prebuilt one [1] and it works fine on latest engineer mode recovery which enabled adb[2] by command:
$ ubuntu-device-flash touch --channel ubuntu-touch/rc-proposed/meizu.en --recovery-image=${the-recovery[2]}.img --bootstrap --device turbo

[1] https://private-fileshare.canonical.com/~morphis/ubuntu-device-flash
[2] https://git.launchpad.net/~device-release/turbo/+git/recovery/plain/recovery.img

Alex Tu (alextu) wrote :

LGTM.

review: Approve
Sergio Schvezov (sergiusens) wrote :

Thanks for your contribution, interesting to see what happened when bootloaders are not taken care of early ;-)

The only thing that worries me here is the fact that we only really format cache because there could be files from a failed "flash" there, by wiping we make sure there is enough space. Maybe a message about this when using turbo would be good and make for an easier troubleshoot.

Alex Tu (alextu) :
review: Needs Fixing
Alex Tu (alextu) wrote :

refer the conversation on irc, we might need to do some more operations to simulate fastboot -w then do update, so the proposed way might be 1. reboot recovery to erase data and cache partition 2. reboot recovery again to do update.

<ondra> alextu as for MP for recovery, so I still believe format cache is needed, but it should do proper cache format, or at least delete all from cache, not skip files under /cache/recovery
<ondra> alextu to fix cases when cache is running out of space and there are some old hanging files
<alextu> ondra, hmm ... agree, it seems no reason to keep updates which in /cache/recovery .
* alextu going to upload another mp for that.
<ondra> alextu problem is that then it's not gonna solve problem morphis|off is addressing with that change, when he wants to emulate fastboot -w
<ondra> alextu which should format userdata and cache
<ondra> alextu but then we wipe tarballs which we need to do install update, so it's kinda broken
<ondra> alextu only thing I can think of is to a) move format chache to the end, so it's formatted at the end of update, so it's safe to wipe it all, or we will need two runs, something like "format cache and userdata and reboot back to recovery" so u-d-f can start pushing files
<alextu> ondra, hmm ... sounds option b) is more like what we want to simulate
<ondra> alextu yep
<alextu> ondra, option a) will also remove cache/device-build which is the version of device tarball which updated by u-d-f
<ondra> alextu true

Bin Li (binli) wrote :

After talked with abeato and morphis, we prepare to add 'rm -rf /cache/*' in u-d-f.

Bin Li (binli) wrote :

And I read the code, I found the u-d-f will download the tarballs first, then fastboot.format /cache, and reboot into recovery, then call bitPusher to push these tarballs, and before push it will rm previous tarball like below. After that u-d-f push the tarballs into recovery.

adb.Shell("rm -rf /cache/recovery/*.xz /cache/recovery/*.xz.asc")

So for simple I think we just change above line. Is it ok? Thanks!

adb.Shell("rm -rf /cache/*")

Bin Li (binli) wrote :
review: Approve

Sam behaviour has been observed in frieza and cooler. Can we simply try the "rm -rf /cache/*" if format cache has failed?

review: Needs Fixing

Tested on cooler now, works fine.

review: Approve

Tested on frieza too. Worked as expected.

Snappy Tarmac (snappydevtarmac) wrote :

The attempt to merge lp:~morphis/goget-ubuntu-touch/add-turbo-bootstrap-support into lp:goget-ubuntu-touch failed. Below is the output from the failed tests.

Checking formatting
Formatting wrong in following files
ubuntu-device-flash/touch.go

# we always run in a fresh dir in tarmac
export GOPATH=$(mktemp -d)
trap 'rm -rf "$GOPATH"' EXIT

# this is a hack, but not sure tarmac is golang friendly
mkdir -p $GOPATH/src/launchpad.net/goget-ubuntu-touch
cp -a . $GOPATH/src/launchpad.net/goget-ubuntu-touch
cd $GOPATH/src/launchpad.net/goget-ubuntu-touch

./run-checks

Snappy Tarmac (snappydevtarmac) wrote :

The attempt to merge lp:~morphis/goget-ubuntu-touch/add-turbo-bootstrap-support into lp:goget-ubuntu-touch failed. Below is the output from the failed tests.

Checking formatting
Formatting wrong in following files
ubuntu-device-flash/touch.go

# we always run in a fresh dir in tarmac
export GOPATH=$(mktemp -d)
trap 'rm -rf "$GOPATH"' EXIT

# this is a hack, but not sure tarmac is golang friendly
mkdir -p $GOPATH/src/launchpad.net/goget-ubuntu-touch
cp -a . $GOPATH/src/launchpad.net/goget-ubuntu-touch
cd $GOPATH/src/launchpad.net/goget-ubuntu-touch

./run-checks

239. By Simon Fels on 2016-07-21

Correct formatting

Robert Bruce Park (robru) wrote :

This MP was released through bileto, but then tarmac merged it prematurely, confusing bileto. If you intend to continue using goget-ubuntu-touch in bileto you need to disable tarmac.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'devices/fastboot.go'
2--- devices/fastboot.go 2015-04-16 22:59:18 +0000
3+++ devices/fastboot.go 2016-07-21 15:57:39 +0000
4@@ -85,3 +85,10 @@
5 }
6 return device, err
7 }
8+
9+// Send OEM specific commands to bootloader
10+func (fastboot Fastboot) SendOemCommand(command string) (err error) {
11+ cmd := append(fastboot.params, []string{"oem", command}...)
12+ err = exec.Command(fastbootCommand, cmd...).Run()
13+ return err
14+}
15
16=== modified file 'ubuntu-device-flash/touch.go'
17--- ubuntu-device-flash/touch.go 2016-06-13 14:19:39 +0000
18+++ ubuntu-device-flash/touch.go 2016-07-21 15:57:39 +0000
19@@ -2,7 +2,7 @@
20 // ubuntu-device-flash - Tool to download and flash devices with an Ubuntu Image
21 // based system
22 //
23-// Copyright (c) 2013-2014 Canonical Ltd.
24+// Copyright (c) 2013-2016 Canonical Ltd.
25 //
26 // Written by Sergio Schvezov <sergio.schvezov@canonical.com>
27 //
28@@ -35,6 +35,7 @@
29 Bootstrap bool `long:"bootstrap" description:"bootstrap the system, do this from the bootloader"`
30 Wipe bool `long:"wipe" description:"Clear all data after flashing"`
31 Serial string `long:"serial" description:"Serial of the device to operate"`
32+ AdbSerial string `long:"adb-serial" description:"Serial for ADB to specify the device to operate"`
33 DeveloperMode bool `long:"developer-mode" description:"Enables developer mode after the factory reset, this is meant for automation and makes the device insecure by default (requires --password)"`
34 AdbKeys string `long:"adb-keys" description:"Specify a local adb keys files, instead of using default ~/.android/adbkey.pub (requires --developer-mode)"`
35 DeviceTarball string `long:"device-tarball" description:"Specify a local device tarball to override the one from the server (using official Ubuntu images with different device tarballs)"`
36@@ -234,17 +235,60 @@
37 if err := touchCmd.fastboot.Flash("recovery", recovery); err != nil {
38 return errors.New("can't flash recovery image")
39 }
40- if err := touchCmd.fastboot.Format("cache"); err != nil {
41- log.Print("Cache formatting was not successful, flashing may fail, " +
42- "check your partitions on device")
43- }
44-
45- if err := touchCmd.fastboot.BootImage(recovery); err != nil {
46- return errors.New("Can't boot recovery image")
47- }
48+
49+ var deviceSupportsCacheFormat bool = true
50+ var deviceSupportsBootImage bool = true
51+ var deviceSupportsOemRebootRecovery bool = false
52+
53+ if touchCmd.Device == "turbo" {
54+ deviceSupportsCacheFormat = false
55+ deviceSupportsBootImage = false
56+ deviceSupportsOemRebootRecovery = true
57+ }
58+ if touchCmd.Device == "frieza" || touchCmd.Device == "cooler" {
59+ deviceSupportsCacheFormat = false
60+ }
61+
62+ if deviceSupportsCacheFormat {
63+ if err := touchCmd.fastboot.Format("cache"); err != nil {
64+ log.Print("Cache formatting was not successful, flashing may fail, " +
65+ "check your partitions on device")
66+ }
67+ }
68+
69+ if deviceSupportsBootImage {
70+ if err := touchCmd.fastboot.BootImage(recovery); err != nil {
71+ return errors.New("Can't boot recovery image")
72+ }
73+ } else if deviceSupportsOemRebootRecovery {
74+ // Some bootloaders does not support the BootImage command so we have
75+ // to flash the recovery first and then reboot through a OEM specific
76+ // command the bootloader offers.
77+ if err := touchCmd.fastboot.SendOemCommand("reboot recovery"); err != nil {
78+ return errors.New("Can't reboot device")
79+ }
80+ } else {
81+ log.Print("We can't reboot your device automatically. Please reboot " +
82+ "your device manually to recovery mode.")
83+ }
84+
85+ log.Print("Waiting for device to enter recovery mode ...")
86 if err := touchCmd.adb.WaitForRecovery(); err != nil {
87 return err
88 }
89+
90+ if !deviceSupportsCacheFormat {
91+ // On some devices we can't run `fastboot format cache` as the bootloader
92+ // does not support this command. We're erasing all bits from the
93+ // cache partition manually once we've booted into the recovery.
94+ if _, err := touchCmd.adb.Shell("rm -rf /cache/*"); err != nil {
95+ log.Fatal("Cannot cleanup /cache/ to ensure clean deployment", err)
96+ }
97+
98+ if _, err := touchCmd.adb.Shell("mkdir /cache/recovery"); err != nil {
99+ log.Fatal("Failed to recreate filesystem structure on cache partition", err)
100+ }
101+ }
102 }
103 go bitPusher(touchCmd.adb, files, done)
104 for i := 0; i < totalFiles; i++ {
105@@ -309,10 +353,24 @@
106 touchCmd.fastboot.SetSerial(touchCmd.Serial)
107 }
108
109+ if touchCmd.AdbSerial != "" {
110+ touchCmd.adb.SetSerial(touchCmd.AdbSerial)
111+ }
112+
113 if touchCmd.Device == "" {
114 if touchCmd.Bootstrap {
115 log.Print("Expecting the device to be in the bootloader... waiting")
116 touchCmd.Device, err = touchCmd.fastboot.GetDevice()
117+
118+ // For turbo it was missed to put the proper name into the bootloader
119+ // and this can't be changed anymore after production already started.
120+ // Reflashing the bootloader through an OTA update to fix this is also
121+ // very unlikely to happen so we have to work around here and make sure
122+ // this never happens again.
123+ if touchCmd.Device == "smdk" {
124+ touchCmd.Device = "turbo"
125+ }
126+
127 return err
128 } else {
129 log.Print("Expecting the device to expose an adb interface...")

Subscribers

People subscribed via source and target branches