On Tue, 2015-07-21 at 03:27 +0000, Zbigniew Jędrzejewski-Szmek wrote: > fedup-system-upgrade.service uses an additional flag file which is > checked with ConditionPathExists so it will not run if 'dnf fedup > reboot' > did not create the flag, even if we go into system-upgrade.target. > > packagekit-offline-update.service does not have anything like this, > and > is always run in system-upgrade.target.
Strange - from what I can tell from the source, pk-offline-update is supposed to bail out unless the update was prepared by PackageKit itself[1]? Perhaps your system was also trying to install new updates via PackageKit at the time? Maybe packagekit-offline-update.service also needs a ConditionPathExists.. > Running two upgrade mechanisms in parallel does not seem like a good > idea. (Even if they use a lock file to prevent concurrent access to > the rpm database, they are bound to interfere with one another: the > first finishes and decides to reboot, or the first updates some > packages and messes up the state for the second one...) It seems that > *some* mechanism to run only one upgrade mechanism is wanted. The > approach > that dnf-plugin-fedup uses seems reasonable: it creates the file only > when > a reboot with 'dnf fedup reboot' is requested. > As an alternative we could allow only one upgrade mechanism to be > enabled. > Dunno. How would that be enforced, though? Special handling for system -updates.target? Or: does systemd need some convention/support for handling the general problem of choosing one of multiple (mutually-exclusive) services that provide the same functionality? > Also, which is a minor thing, but related: OnFailure=reboot.target > seems inferior to FailureAction=reboot. IIRC, the second one uses > irreversible transaction and should be more robust. It also is a > higher level setting in some sense. OnFailure=reboot.target is taken > directly from the spec, so should be changed there first. I'll add a note about this to the fedup-system-upgrade.service, I guess, and if the spec changes I'll change it there too. > So maybe tmpfiles snippet should be used to remove the link. Such a > change would mean that the update services should not depend on the > symlink being present, and should instead look for their > installation data in their own state directory. Right - this is what dnf-plugin-fedup does, since pk-offline-update might run first and remove the symlink before we get there. > To summarize, following changes to the spec are proposed: > - use Condition* or similar to conditionalize whether a specific > upgrade mechanism should run > - use Action=reboot FailureAction=reboot, I guess? > - use Type=oneshot This is the default, correct? Should it be explicitly listed in the unit file, or should I rely on the default behavior? > - check that logind.Reboot() is not called on failure by the service > - services should not look for /systemd-update symlink, > and the symlink should be removed by tmpfiles before we even get to > the upgrade. These all seem reasonable to me. I'll happily make changes to dnf -plugin-fedup to follow any changes to the spec, once consensus is reached and the spec is changed. Thanks for testing this! -w [1] https://github.com/hughsie/PackageKit/blob/PACKAGEKIT_1_0_7/client/ pk-offline-update.c#L404 _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel