ubuntu-phone team mailing list archive

[Barry Warsaw <barry@xxxxxxxxxx>]

For a while now I've been struggling with LP: #1217098 in system-image.  The
bug captures the feature where system-image can avoid re-downloading all the
data files in some situations.  Here's a typical use case:

The user checks for an update via the u/i on a phone.  system-image finds an
update and downloads everything, say because automatic downloads are enabled.
Then the u/i presents the user with a button to apply the update and reboot,
but here the user doesn't push the button immediately.

A long while passes, long enough for system-image-dbus to timeout and exit.
Then the user pushes the "apply and reboot" button.

Previously, this would force s-i-dbus to re-download everything, because no
state persists between separate invocations.  This is still true now, but in
fixing this bug, s-i-dbus is able to look at what files have already been
downloaded and avoid re-downloading them again if possible.

Note that only downloading data files can be skipped.  s-i-dbus will still
always attempt to download the blacklist, channels.json, and index.json files,
along with any other keyrings that need to be downloaded in order to verify
and calculate the upgrade path.  If after all this, the winning upgrade path
names some data files which are already downloaded, and those files still have
valid checksums and signatures, then s-i-dbus won't download them again.

In the common use case above, let's say the user pushes the "apply and reboot"
button again after, say 20 minutes.  The previously calculated and downloaded
update will likely still be exactly the same, so while it will have to get all
the preliminary files again, they are usually quite small in relation to the
much bigger data files, so the subsequent upgrade will be much more efficient.

Re-downloading the preliminary files is IMO necessary to ensure that the
upgrader can't be subverted by pushing bogus files to the cache partition
which look like new data files.  Doing the upgrade path calculation and
verification ensures that such bogus files would fail checksum and/or gpg
signatures and thus would be ignored.

There's a catch though: I have to change the D-Bus API slightly.  The current
API is documented here:

https://wiki.ubuntu.com/ImageBasedUpgrades/Client

The affected method is ApplyUpdate().  Currently this is defined as a
synchronous method returning a string, but I need to change this into an
asynchronous method not returning anything, but issuing a `Rebooting` signal.

Why?  Because when the u/i "apply and reboot" button is clicked, this is the
method that it calls.  However, if s-i-dbus has exited and been reactivated
via D-Bus, it will have to download the 'preliminary' files mentioned above.
Now that downloading is provided by the ubuntu-download-manager service, this
involves D-Bus calls to that service, and reactions to its signals.

If ApplyUpdate() is synchronous, we don't return to the D-Bus mainloop so when
s-i-dbus calls into u-d-m, it never receives its signals.  Thus while the
files do actually get downloaded, s-i-dbus will timeout waiting for signal
reception blocked by the upstream synchronous call.  By making ApplyUpdate()
asynchronous, we return to the D-Bus mainloop, and properly react to the u-d-m
signals, and everything is happy.

I don't think the u/i has to actually change here, or at least not much,
although it could if it wants to handle some corner cases.  In the majority of
situations, ApplyUpdate() will issue a system reboot, so who cares what it
returns or what signals it emits?

Where it matters is if the reboot fails for some reason.  Previously
ApplyUpdate() would return an error string in this case, but with the proposed
change, a Rebooting(False) signal will be emitted instead.  Technically
speaking, even a successful call of the method will issue a Rebooting(True)
signal but in normal operation the u/i probably won't see it because the
device reboots.  So again, who cares?

This signal is actually needed most in system-image's test suite where
ApplyUpdate() is mocked to not actually issue the reboot, which would be, um,
inconvenient ;).  In this case, the test suite receives and checks the
Rebooting(True) signal and carries on.

If you've read this far, I intend to make this change in system-image 2.0,
which I am nearly ready to upload to Trusty.  If you think this change is a
problem, please let me know.  The code is available in the system-image trunk.

Cheers,
-Barry

Attachment: signature.asc
Description: PGP signature