Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not start pm in upgrade mode #39

Open
CODeRUS opened this issue Mar 19, 2019 · 9 comments
Open

Do not start pm in upgrade mode #39

CODeRUS opened this issue Mar 19, 2019 · 9 comments
Labels
backends daemon, systemd and dbus components enhancement this improves something

Comments

@CODeRUS
Copy link
Contributor

CODeRUS commented Mar 19, 2019

Add os-update.target to conflicts of pm service

@Olf0
Copy link
Contributor

Olf0 commented May 22, 2019

Thanks, this may resolve issue #33 as well.

Edit: No it won't, see #33 (comment).
Nevertheless, to implement this enhancement does make sense.

@nephros nephros added the enhancement this improves something label Sep 17, 2021
@nephros
Copy link
Contributor

nephros commented Sep 29, 2021

I'm sorry, I'd like to get some context on this.

Change proposed is to add a Conflicts=os-update.target entry to /usr/lib/systemd/system/dbus-org.SfietKonstantin.patchmanager.service, correct?

With the goal to not interfere with anything going on during a system update.

Qs:

  1. is this still relevant in the days of PM3 and SFOS 4.x?
  2. if yes, what creates os-update.target? Is it reliably* a thing that indicates a device is currently updating?
  3. if not, what else might be used to prevent PM to be active during updates?

[*] with definitions of reliable adapted to the reality of how SFOS/Jolla changes happen... ;)

@Olf0
Copy link
Contributor

Olf0 commented Sep 30, 2021

As:

  1. Yes, I believe so: It does not make any sense for PM's background services to start, when os-update.target is active.
  2. a. SailfishOS!
    b. Well, it is reliably indicating that a SailfishOS installation has booted into the "single user upgrade phase" of an OS upgrade via GUI.
    You know (if not upgrading SFOS at the command line), one looks for an SFOS-upgrade in the Settings app, downloads its RPMs there; then the device reboots (or switches?) to os-update.target, only a progress bar is shown while the RPMs are installed and finally the device switches (or reboots, I do not remember precisely) to the regular multiuser-with-UI target.
    Mind that this method of upgrading SFOS cannot be used on community ports and "Sailfish Free / Trial" installations (i.e., SFOS installations without a paid license from Jolla) and there are many other ways of upgrading SFOS (the "zypper dance" / per zypper --dup, per version dup / sfos-upgrade, per pkcon upgrade-system), which all do not boot / switch to a special target.
    TL;DR: os-update.target is a kind of "single user mode for upgrading SFOS", used when triggering an OS upgrade per Settings app.
    c. Well, this mechanism exists at least since SFOS 1.1.x (when I joined the club).
  3. Good question. I guess the answer is "nothing", because the upgrading per command line (whichever way) cannot be detected, as it is just downloading and installing RPMs.
    Actually, what all "alternative" methods do have in common, is that one has to set the SSU utility into release mode per ssu re <version-to-upgrade-to> while at the regular multiuser-with-UI target (the GUI upgrade likely does also set SSU to release mode, but only while at the os-update.target). The Seamless Software Update (SSU) utility was originally developed by Nokia for the N900 and Jolla's documentation for it is extremely terse, but their MDM documentation shows that there is an "int deviceMode() - The repository configuration mode e.g. Release, RnD, Upgrade. See ssu docs for further info.", which seems to be query-able (but I have no idea, where the mentioned documentation may be and if there is a hook / callback to be triggered).
    But you can dig deeper into SSU, its local infrastructure and how it is embedded into SFOS: https://github.com/search?q=org%3Asailfishos+ssu

HTH
If there are more questions, do ask, as the basics of this (local) low-level infrastructure is a field I was interested in for a long time (and hence have gathered some knowledge). But Andrey is likely an expert on this, because these things are part of his day job, AFAIK.

nephros added a commit to nephros/patchmanager that referenced this issue Oct 2, 2021
nephros added a commit to nephros/patchmanager that referenced this issue Oct 3, 2021
commit f68816c
Author: Peter G <[email protected]>
Date:   Sat Oct 2 16:48:23 2021 +0200

    Conflict with os-update.target

    Implements Issue sailfishos-patches#39
    sailfishos-patches#39
nephros added a commit to nephros/patchmanager that referenced this issue Oct 3, 2021
nephros added a commit to nephros/patchmanager that referenced this issue Oct 3, 2021
commit d14a3ee
Author: Peter G <[email protected]>
Date:   Sat Oct 2 16:48:23 2021 +0200

    Conflict with os-update.target

    Implements Issue sailfishos-patches#39
    sailfishos-patches#39

commit 03e4f18
Author: Peter G <[email protected]>
Date:   Sun Oct 3 14:04:44 2021 +0200

    Redesign About page, add License and Source links (sailfishos-patches#68)

    * [ui] Redesign About page, add License and Source links

    See Issue sailfishos-patches#12

    sailfishos-patches#12
nephros added a commit to nephros/patchmanager that referenced this issue Oct 3, 2021
commit d14a3ee
Author: Peter G <[email protected]>
Date:   Sat Oct 2 16:48:23 2021 +0200

    Conflict with os-update.target

    Implements Issue sailfishos-patches#39
    sailfishos-patches#39
nephros added a commit to nephros/patchmanager that referenced this issue Oct 4, 2021
commit d14a3ee
Author: Peter G <[email protected]>
Date:   Sat Oct 2 16:48:23 2021 +0200

    Conflict with os-update.target

    Implements Issue sailfishos-patches#39
    sailfishos-patches#39
nephros added a commit to nephros/patchmanager that referenced this issue Oct 4, 2021
commit d14a3ee
Author: Peter G <[email protected]>
Date:   Sat Oct 2 16:48:23 2021 +0200

    Conflict with os-update.target

    Implements Issue sailfishos-patches#39
    sailfishos-patches#39
nephros added a commit to nephros/patchmanager that referenced this issue Oct 4, 2021
commit d14a3ee
Author: Peter G <[email protected]>
Date:   Sat Oct 2 16:48:23 2021 +0200

    Conflict with os-update.target

    Implements Issue sailfishos-patches#39
    sailfishos-patches#39
@nephros
Copy link
Contributor

nephros commented Oct 5, 2021

Thank you. Much clearer now.

@Olf0 Can you please test nephros@d14a3ee somehow? (Well, it's just the single line added :) ).
I have confirmed adding the line doesn't affect restarting the service or rebooting, but I do not know how to test the actual "os-update.target exists" case.

nephros added a commit to nephros/patchmanager that referenced this issue Oct 5, 2021
@Olf0
Copy link
Contributor

Olf0 commented Oct 10, 2021

Thank you. Much clearer now.

Well, then it is time for me to report the things I do not understand WRT @CODeRUS' original issue description, "Add os-update.target to conflicts of pm service": See below.

@Olf0 Can you please test nephros@d14a3ee somehow? (Well, it's just the single line added :) ). I have confirmed adding the line doesn't affect restarting the service or rebooting, but I do not know how to test the actual "os-update.target exists" case.

Just adding Conflicts=os-update.target (to bin/patchmanager-daemon/systemd/dbus-org.SfietKonstantin.patchmanager.service) is definitely too simple:

  1. As with all dependency statements in systemd units, Conflicts= does not include a (temporal) order and does not make much sense (or results in unexpected behaviour) without one. See https://www.freedesktop.org/software/systemd/man/systemd.unit.html#Conflicts=
    Hence an After=os-update.target or Before=os-update.target should be added, but which? → See below.
  2. I tried research how this is supposed to work in practice (aside of the theoretical considerations I described above).
    • Searching the WWW for "os-update.target" reveals ... nothing.
    • But I remembered that systemd has some mechanisms Jolla might utilise:
    • So I tried to look at this in practice on an [email protected]
      But executing a systemctl -l -t target --all (or an equivalent systemctl -l -t target --all list-units) does not show an os-update.target!
      Neither does a systemctl -l -t target --all list-unit-files show anything indicating such a target!

But at least re-reading systemd's man-page for "offline updates", specifically "Recommendation 4" convinces me that Before=os-update.target is the better ordering statement.
Unfortunately system-update.target is completely missing on systemd's man-page "bootup".

@CODeRUS, can you please provide some background on os-update.target, how it works, where it can be found and analysed, and if simply adding Conflicts=os-update.target and Before=os-update.target to bin/patchmanager-daemon/systemd/dbus-org.SfietKonstantin.patchmanager.service is what you had in mind.

@nephros
Copy link
Contributor

nephros commented Oct 11, 2021

Good hints on where to look for some more! I conclude that os-upgrade.target was never meant literally, and it has always been about that systemd special.

I found that the packagekit package does contain some things related to OS-update:

For one there's a snippet of documentation on how to use the built-in feature: https://github.com/sailfishos/PackageKit/blob/master/docs/offline-updates.txt

And looking at the installed package on my device:

$ rpm -ql  PackageKit | grep systemd
/usr/lib/systemd/system/packagekit-offline-update.service
/usr/lib/systemd/system/packagekit.service
/usr/lib/systemd/system/system-update.target.wants/packagekit-offline-update.service

So this: https://github.com/sailfishos/PackageKit/blob/master/data/packagekit-offline-update.service.in

Aaand there's also sailfish-upgrade-ui which contains:

/usr/lib/systemd/system/osupdate-logging.service
/usr/lib/systemd/system/sailfish-upgrade-ui.service
/usr/lib/systemd/system/system-update-cleanup.service.d
/usr/lib/systemd/system/system-update-cleanup.service.d/os-update-running.conf
/usr/lib/systemd/system/system-update-pre.target.wants
/usr/lib/systemd/system/system-update-pre.target.wants/sailfish-upgrade-ui.service
/usr/libexec/sailfish-upgrade-ui

of which /usr/lib/systemd/system/system-update-cleanup.service.d/os-update-running.conf contains the very interesting:

[Unit]
ConditionPathExists=!/tmp/os-update-running

with /usr/lib/systemd/system/sailfish-upgrade-ui.service doing

ExecStartPre=-/bin/touch /tmp/os-update-running
ExecStopPost=-/bin/rm -f /tmp/os-update-running

So assuming that mechanism is used during OS updates, we should either

  • Conflicts= (and Before=/After=) one of those services
  • ConditionPathExists=!/tmp/os-update-running

It looks to me like just having that Condition is sufficient for the purpose of not starting if update is running.
And if I read the documentation you linked to correctly, we shouldn't have system-update.target anywhere in the Unit definition, or else we're actually pulled as a dep in that target which is the opposite of what we want. In fact, we shouldn't even be started for system-update.target so maybe this whole issue is actually moot -- unless our After=dbus.service somehow pulls us into the dep graph during updates.

@nephros
Copy link
Contributor

nephros commented Oct 11, 2021

One more thing:

Our RPM installs or changes /etc/ld.so.preload, which will of course be in effect always, even in upgrade mode. We could introduce a special service which Requires/Before=system-update-pre.target which moves that file out of the way during updates, so failure to load the library doesn't kill the update process.

Or we could set NO_PM_PRELOAD as an environment variable, but I don't know how to set that globally for everything and only in system-update mode.

nephros added a commit to nephros/patchmanager that referenced this issue Oct 11, 2021
@Olf0
Copy link
Contributor

Olf0 commented Oct 12, 2021

I conclude that os-upgrade.target was never meant literally, and it has always been about that systemd special.

system-update.target.
Yes, probably.

[...] of which /usr/lib/systemd/system/system-update-cleanup.service.d/os-update-running.conf contains the very interesting:

[Unit]
ConditionPathExists=!/tmp/os-update-running

Yes, now I remember to have seen this pattern a couple of times in systemd units created by Jolla.

with /usr/lib/systemd/system/sailfish-upgrade-ui.service doing

ExecStartPre=-/bin/touch /tmp/os-update-running
ExecStopPost=-/bin/rm -f /tmp/os-update-running

Thanks, while I have become a little bit of a systemd expert due to analysing SailfishOS, I never took the effort to track that down.

So assuming that mechanism is used during OS updates, we should either

* `Conflicts=` (and `Before=/After=`) one of those services

IMO, rather not, but ...

* `ConditionPathExists=!/tmp/os-update-running`

exactly that.

It looks to me like just having that Condition is sufficient for the purpose of not starting if update is running.

Well, sort of: But a ConditionPathExists=!/tmp/os-update-running will not stop our unit, when the condition becomes untrue, when it is already started.
While dependencies define when units are queued for starting, the Condition and Assert statements are evaluated, when a unit is started; but after that … it would take some research for me to see if anything can be done then.

I think the solution is not to have it enqueued too early.

And if I read the documentation you linked to correctly, we shouldn't have system-update.target anywhere in the Unit definition, or else we're actually pulled as a dep in that target which is the opposite of what we want.

No, I do not believe so.
But it is not necessary this way (with ConditionPathExists=!/tmp/os-update-running), any way.

In fact, we shouldn't even be started for system-update.target so maybe this whole issue is actually moot -- unless our After=dbus.service somehow pulls us into the dep graph during updates.

Wrong conclusion: It is systemd and its automatisms make it complicated to comprehend! 😜
After=dbus.service alone does nothing, it is just a directive for temporal ordering, if some dependency is defined.
Requires=dbus.service is such a dependency.
But both are unnecessary, because Type=dbus automatically sets both (Requires=dbus.service and After=dbus.service) any way, unless one sets DefaultDependencies=No. OTOH, it does no harm to duplicate statements, i.e. to express explicitly, what systemd already does implicitly.

And AFAIR (look at the boot chart) the dbus.target is reached before the units for the system-update.target are started.
Mind that targets are just defined steps in the boot process to group units.

The systemd man-pages are basically well written (except for the never properly defined, but often used phrase "to pull in"), but one has to read the most important ones thoroughly to gather an overall understanding. The issue is to find "the most important ones", because that is nowhere documented and depends a bit on the use case.

TL;DR: Solely use ConditionPathExists=!/tmp/os-update-running instead of the currently inserted Conflicts=os-update.target.

But please also take a look at the other units, they shall not be activated when /tmp/os-update-running exists, too.

P.S.: I will try to take a closer look at all these units some time, because at first sight a couple of things can and likely should be cleaned up. One issue with systemd is, that it often accepts superfluous and even contradicting statements silently, while trying to do the right thing (and even often succeeds with that), but sometimes such statements cause weird side effects.
In general systemd is a topic for which I should be able to assist well with guidance, know-how (just having studied all man-pages at least 3 times, some 10+) and practical experience. It is just very tedious to read and reread these man-pages often, but they seem to be exhausting, i.e. nothing is not defined in them. Trying things out is even more tedious with systemd, because everything is somewhat complicated (command syntax etc.), but feasible.

@Olf0
Copy link
Contributor

Olf0 commented Oct 12, 2021

One more thing:

Our RPM installs or changes /etc/ld.so.preload, which will of course be in effect always, even in upgrade mode.

But that behaves transparently (i.e., like its original version), when it loads things not on a whitelist, correct?
Or does nothing call it, when the PM-unit is not active?

Sorry, I never looked those things up and have not yet thought about them.

"installs or changes" (i.e., which of both) is the first thing we should know for sure!

We could introduce a special service which Requires/Before=system-update-pre.target which moves that file out of the way during updates, so failure to load the library doesn't kill the update process.

Which "the library"?
(Will ponder about that, some time.)

Or we could set NO_PM_PRELOAD as an environment variable, but I don't know how to set that globally for everything and only in system-update mode.

I do not think that is viable.

@nephros nephros added the backends daemon, systemd and dbus components label Nov 9, 2021
nephros added a commit to nephros/patchmanager that referenced this issue Jan 27, 2022
nephros added a commit to nephros/patchmanager that referenced this issue Feb 11, 2022
commit b23d86c478d648b3a6845f2734bd62499941d723
Author: Peter G <[email protected]>
Date:   Fri Dec 17 09:25:40 2021 +0100

    fix non-absolute paths in service files

commit 2b8799d918a65acda2bd3325be777f0c68b82c1d
Author: Peter G <[email protected]>
Date:   Mon Oct 11 14:35:16 2021 +0200

    fix and simplify Exec call

commit c3b53021efa2aaa37f10349f56d826292960dd9c
Author: Peter G <[email protected]>
Date:   Mon Oct 11 13:40:49 2021 +0200

    Add special systemd services for system updates

commit d851ae758862f0866461160b34c937224a020a73
Author: Peter G <[email protected]>
Date:   Mon Oct 11 13:40:02 2021 +0200

    conflict with the services, check for existance of os-update file

commit 007725c17c0776910be0bd456cff2a1c55f3503d
Author: Peter G <[email protected]>
Date:   Mon Oct 11 12:36:33 2021 +0200

    Revert "Conflict with os-update.target"

    This reverts commit 02013f0 which was
    the wrong solution.

commit 239495612550527c1130eb16fa1db38fad0e08d8
Author: Peter G <[email protected]>
Date:   Sat Oct 2 16:48:23 2021 +0200

    Conflict with os-update.target

    Implements Issue sailfishos-patches#39
    sailfishos-patches#39
nephros added a commit to nephros/patchmanager that referenced this issue Feb 25, 2022
nephros added a commit to nephros/patchmanager that referenced this issue Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backends daemon, systemd and dbus components enhancement this improves something
Projects
None yet
Development

No branches or pull requests

3 participants