Skip to content

Restore dnf4 restart-safety semantics for the restart candidate list - #13

Merged
ThatStasGuy merged 1 commit into
amazonlinux:mainfrom
VinzSpring:feat/dnf5-service-output-filtering
Aug 25, 2026
Merged

Restore dnf4 restart-safety semantics for the restart candidate list#13
ThatStasGuy merged 1 commit into
amazonlinux:mainfrom
VinzSpring:feat/dnf5-service-output-filtering

Conversation

@VinzSpring

Copy link
Copy Markdown
Contributor

dnf5's 'needs-restarting -s' lost two safety properties of dnf4's process-based implementation, and smart-restart pipes that output straight into 'systemctl restart':

  1. dnf5 reports completed oneshot units ("active (exited)", e.g. cloud-config.service after boot). Those have no processes that could be running outdated code, and "restarting" them re-executes their one-time boot action instead of reloading anything; re-running cloud-config.service deadlocks under cloud-init >= 25.3's single-process architecture. Since smart-restart runs from the libdnf5 actions plugin hook, the blocking systemctl call then wedges the whole parent dnf transaction until it is killed. dnf4 never reported process-less units.

  2. dnf4 refuses to recommend restarting services defined by reboot-class packages: units whose unit file belongs to an installed package providing one of the NEED_REBOOT names (kernel, glibc, systemd, dbus, dbus-broker, ...) are withheld and printed to stderr under 'Warning: The following services should not be restarted but require a reboot:'. dnf5 dropped that guard, so smart-restart would blindly restart e.g. dbus-broker.service after a dbus update - which can sever the session driving the update, mid-transaction.

Enforce both rules on the smart-restart side so behavior no longer depends on which dnf generation produced the list:

  • Keep only services whose SubState is "running" when assembling the restart list.
  • Withhold services whose unit file is owned by a package providing a reboot-class name (the dnf4 NEED_REBOOT list plus dnf5's CORE_PACKAGE_NAMES additions, matched by provides exactly like dnf4), printing dnf4's warning. Only the rpmdb is read (rpm -q --whatprovides / rpm -qf), no dnf metadata and no locks, so this is safe from inside the transaction hook.
  • Apply the same guards to the post-restart staleness check, so a deliberately untouched oneshot or reboot-class service does not fake a failed restart into a reboot hint. The reboot recommendation for reboot-class packages remains the --reboothint path's responsibility, unchanged.

The guards only activate on dnf5 stacks (detected from NEEDS_RESTARTING_COMMAND, overridable via
ENFORCE_RESTART_SAFETY_GUARDS): dnf4's and yum's needs-restarting derive services from running processes and withhold reboot-class services themselves, so their output never needs the guards, and the AL2/AL2023 code paths remain byte-identical. systemctl is queried without --value (systemd >= 230; AL2 ships 219) so the helpers are portable even when enabled manually.

Testing: rpm is overridable via RPM_COMMAND; add an rpm mock, SubState/FragmentPath support in the systemctl mock, and six new tests covering the oneshot filter, the reboot-class withholding, the stderr warning, both reboot-hint exclusions, and the legacy-stack passthrough (guards off restores verbatim dnf4 behavior).

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

dnf5's 'needs-restarting -s' lost two safety properties of dnf4's
process-based implementation, and smart-restart pipes that output
straight into 'systemctl restart':

1. dnf5 reports completed oneshot units ("active (exited)", e.g.
cloud-config.service after boot). Those have no processes that could
be running outdated code, and "restarting" them re-executes their
one-time boot action instead of reloading anything; re-running
cloud-config.service deadlocks under cloud-init >= 25.3's
single-process architecture. Since smart-restart runs from the libdnf5
actions plugin hook, the blocking systemctl call then wedges the whole
parent dnf transaction until it is killed. dnf4 never reported
process-less units.

2. dnf4 refuses to recommend restarting services defined by
reboot-class packages: units whose unit file belongs to an installed
package providing one of the NEED_REBOOT names (kernel, glibc,
systemd, dbus, dbus-broker, ...) are withheld and printed to stderr
under 'Warning: The following services should not be restarted but
require a reboot:'. dnf5 dropped that guard, so smart-restart would
blindly restart e.g. dbus-broker.service after a dbus update - which
can sever the session driving the update, mid-transaction.

Enforce both rules on the smart-restart side so behavior no longer
depends on which dnf generation produced the list:

- Keep only services whose SubState is "running" when assembling the
  restart list.
- Withhold services whose unit file is owned by a package providing a
  reboot-class name (the dnf4 NEED_REBOOT list plus dnf5's
  CORE_PACKAGE_NAMES additions, matched by provides exactly like
  dnf4), printing dnf4's warning. Only the rpmdb is read (rpm -q
  --whatprovides / rpm -qf), no dnf metadata and no locks, so this is
  safe from inside the transaction hook.
- Apply the same guards to the post-restart staleness check, so a
  deliberately untouched oneshot or reboot-class service does not fake
  a failed restart into a reboot hint. The reboot recommendation for
  reboot-class packages remains the --reboothint path's
  responsibility, unchanged.

The guards only activate on dnf5 stacks (detected from
NEEDS_RESTARTING_COMMAND, overridable via
ENFORCE_RESTART_SAFETY_GUARDS): dnf4's and yum's needs-restarting
derive services from running processes and withhold reboot-class
services themselves, so their output never needs the guards, and the
AL2/AL2023 code paths remain byte-identical. systemctl is queried
without --value (systemd >= 230; AL2 ships 219) so the helpers are
portable even when enabled manually.

Testing: rpm is overridable via RPM_COMMAND; add an rpm mock,
SubState/FragmentPath support in the systemctl mock, and six new
tests covering the oneshot filter, the reboot-class withholding, the
stderr warning, both reboot-hint exclusions, and the legacy-stack
passthrough (guards off restores verbatim dnf4 behavior).
@ThatStasGuy
ThatStasGuy merged commit b3f1070 into amazonlinux:main Aug 25, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants