On Fri, Jul 21, 2023 at 08:20:21AM +0000, Zbigniew Jędrzejewski-Szmek wrote:
> On Thu, Jul 20, 2023 at 07:40:08AM -0700, Andrea Bolognani wrote:
> > Now, since this is clearly not a libvirt-specific issue, I believe
> > this approach should be adopted across Fedora by way of these macros
> > (or comparable ones) being added to systemd. Packages would have to
> > migrate from the old macros over time, and at some point it should be
> > possible to get rid of them entirely.
>
> Systemd macros are maintained upstream. So if you have
> patches/suggestions/etc., they should be filed upstream.
>
> I looked briefly at your links [4,5] but it seems very libvirt-specific and
> it's hard to see the full plan without context. Please file a pull request
> upstream and it'll be easier to discuss there. There're also people from
> other distros there.
Making this generic enough that it would work with arbitrary
services, and polished enough that I would consider submitting it as
a systemd change, would require a non-trivial amount of additional
work.
That's work that I'm absolutely willing and intending to do[1], but
even under the most optimistic circumstances I see no chance of it
being completed in time for Fedora 39 and RHEL 9.3, which is when we
would need it in order to avoid breaking users' deployments.
So what I'm looking for here is validation that the overall approach
is reasonably sound, the current implementation is not completely
broken, and in general Fedora would not reject a libvirt.spec file
that contained those changes.
Does that seem fair?
> One immediate comment is that systemd macros have been reworked to restart
> everything in %posttrans using a single operation. I.e. various per-package
> macros mark services with "needs-restart", and then in %posttrans
> a single 'systemctl reload-or-restart --marked' does the deed.
> It seems your macros reimplement the previous behaviour of individual
> restarts, and we wouldn't want to go back to that.
Each service is restarted/reloaded only once, but yeah there's a
separate call to systemctl for each of those reloads/restarts which I
agree is suboptimal.
For the short-term implementation that lives in libvirt.spec I
thought it would be best to rely as little as possible on external
bits of machinery, such as the call to reload-or-restart, in order to
keep things self-contained and easy to inspect. Especially since the
reload-or-restart bit is part of systemd.spec's %transfiletriggerin,
which at least AFAICT is kind of opaque when looking at an installed
system.
Note that systemd-update-helper also doesn't currently support
marking services as needs-reload, only needs-restart, so that would
have to be added.
> > +%post -n kdump-tools
> > # Initial installation
> > %systemd_post kdump.service
>
> This one is tricky. %systemd_post presets the service on "first installation",
> which is actually the first the time package is installed. I.e. it unfortunately
> also would execute the preset on upgrades that split out a subpackage, because
> as far as rpm is concerned, this is the initial installation of that subpackage.
>
> The righteous way to solve this would be something like this:
>
> %triggerprein -n kdump-tools -- kexec-tools < 2.0.26-8
> touch %{_localstatedir}/lib/rpm-state/kexec-tools.no-preset
>
> %post -n kdump-tools
> rm %{_localstatedir}/lib/rpm-state/kexec-tools.no-preset 2>/dev/null && return 0
> # Initial installation
> %systemd_post kdump.service
>
> A bit of a bother, but at least nobody will be suprised by
> kdump.service changing state.
This obviously works, but also requires a lot more involvement from
the package maintainer: specifying the last version number that's not
affected by the split[2] making sure that each bit of logic is in the
scriptlet for the correct package...
The beauty of the "check at %pre time whether the unit already exists
on disk" approach is that it should work out of the box in all
scenarios, with no further information needed. Do you see a problem
with it that I have failed to consider?
[1] I definitely don't want libvirt.spec to keep carrying around its
own systemd macros forever!
[2] Or is that the first one that is? O:-)
--
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
devel mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx
Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
