Skip to content

Conversation

@marmarek
Copy link
Member

They all have Alias=display-manager.service in the unit file, and
having more of them enabled at the same time causes conflicts (for
example systemctl preset call during installation fails, same as
systemctl preset-all on release upgrade.

They all have `Alias=display-manager.service` in the unit file, and
having more of them enabled at the same time causes conflicts (for
example `systemctl preset` call during installation fails, same as
`systemctl preset-all` on release upgrade.
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.98%. Comparing base (845aa9f) to head (dced872).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #203   +/-   ##
=======================================
  Coverage   72.98%   72.98%           
=======================================
  Files          10       10           
  Lines        1155     1155           
=======================================
  Hits          843      843           
  Misses        312      312           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@marmarek
Copy link
Member Author

marmarek commented Nov 5, 2025

@fepitre @HW42 @ben-grande is this change a good idea? I made it just to avoid systemctl preset-all failing, but there is also an alternative solution: QubesOS/qubes-dist-upgrade#22
On the other hand, this might break cases when the user uninstalls lightdm to use something else. What do you think?

@ben-grande
Copy link
Contributor

@fepitre @HW42 @ben-grande is this change a good idea? I made it just to avoid systemctl preset-all failing, but there is also an alternative solution: QubesOS/qubes-dist-upgrade#22 On the other hand, this might break cases when the user uninstalls lightdm to use something else. What do you think?

I think that not enabling via preset the login managers by default is better than hiding errors of preset-all.

If they uninstall lightdm to use something else, they need to enable something else as per Fedora's policy of services being disabled by default. It is true that it might fail in cases the user relies on the other display/login manager being enabled by default, but Qubes' KDE documentation already documents disabling lightdm and enabling sddm.

If we don't want to break user upgrades, makes sense to handle the preset-all failure, but at least OpenQA should test without the handling, which would create a non-user tested case, which is not nice for finding bugs...

@marmarek
Copy link
Member Author

marmarek commented Nov 5, 2025

The problem is that release upgrade calls preset-all, which is important to enable new services (we can't really enforce new preset file to be installed before all the other packages, too many dependency cycles...). This applies both to qubes services (for example change to libvirt services structure - now they are modular), and to fedora services (not sure if there are significant changes fc37 -> fc41, but for example before it was needed for dbus-daemon -> dbus-broker migration, otherwise you end up without dbus daemon at all and everything breaks...).

So, if calling preset-all on release upgrade, with this PR it would disable other display managers, that user might have enabled. Maybe upgrade should record which one was enabled before and re-enable it later?
Alternatively, instead of calling preset-all, the upgrade script could preset just selected units we know about (based on diff in preset files between R4.2 and R4.3?). But that feels fragile...

@ben-grande
Copy link
Contributor

So, if calling preset-all on release upgrade, with this PR it would disable other display managers, that user might have enabled. Maybe upgrade should record which one was enabled before and re-enable it later?

I see now... yes, it should record, but doing that for every release upgrade for every relevant package? What about sshd not being enabled via preset.... It should be documented that user must add preset policies in dom0.

For the units that we do know, use ignore directive?

@ben-grande
Copy link
Contributor

For the units that we do know, use ignore directive?

Mostly ok except it will enable lightdm and keep enabled sddm? In that case, something else need to detect that there is a display manager enabled to not enable lightdm....

@ben-grande
Copy link
Contributor

For the units that we do know, use ignore directive?

Yeah, this still requires handling errors on preset-all because it will fail to enabled lightdm if another display manager is enabled. Maybe also ignore lightdm?

For the units that we do know, use ignore directive?

Mostly ok except it will enable lightdm and keep enabled sddm? In that case, something else need to detect that there is a display manager enabled to not enable lightdm....

systemctl disable lightdm; systemctl enable sddm
systemctl is-enabled lightdm sddm
> disabled
> enabled
# preset
ignore lightdm.service
ignore sddm.service

No errors on systemctl preset-all.

systemctl preset-all
systemctl is-enabled lightdm sddm
> disabled
> enabled

@marmarek
Copy link
Member Author

marmarek commented Nov 5, 2025

Ok, then ignore looks like a good option - including for lightdm. And then have something else enable it on the initial installation.

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.

3 participants