[libvirt PATCH 0/8] rpm: Fix handling of systemd units

Andrea Bolognani posted 8 patches 9 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20230714143942.125179-1-abologna@redhat.com
libvirt.spec.in | 453 +++++++++++++++++++++++++-----------------------
1 file changed, 232 insertions(+), 221 deletions(-)
[libvirt PATCH 0/8] rpm: Fix handling of systemd units
Posted by Andrea Bolognani 9 months, 3 weeks ago
Plus some extras I'm throwing in for free :)

To understand why these changes are needed, see the original bug
report[1] as well as the discussion triggered by Martin's initial
attempt at addressing it[2].

Getting this right is quite tricky, so in order to convince myself
that I'm not just going to break everyone's deployment I've tested
things fairly extensively.

In particular, I've verified that things work as expected when
upgrading from libvirt 9.0.0 (e.g. pre-split) on AlmaLinux 8 when the
initial configuration is

  * a default one (socket-activated monolithic daemon);

  * monolithic daemon with --listen;

  * modular daemons;

  * modular daemons with virtproxyd-tcp.socket enabled;

as well as when installing from scratch with

  * no particular configuration;

  * local systemd preset overrides that result in the modular daemons
    being preferred to the monolithic one.

Everything seems to work fine, but further testing would certainly be
more than welcome!

[1] https://bugzilla.redhat.com/show_bug.cgi?id=2210058
[2] https://listman.redhat.com/archives/libvir-list/2023-June/240226.html

Andrea Bolognani (8):
  rpm: Bump min_fedora
  rpm: Style/alignment tweaks
  rpm: Reorder scriptlets
  rpm: Reduce use of with_modular_daemons
  rpm: Remove custom libvirtd restart logic
  rpm: Introduce new macros for handling of systemd units
  rpm: Switch to new macros for handling of systemd units
  rpm: Delete unused macros

 libvirt.spec.in | 453 +++++++++++++++++++++++++-----------------------
 1 file changed, 232 insertions(+), 221 deletions(-)

-- 
2.41.0
Re: [libvirt PATCH 0/8] rpm: Fix handling of systemd units
Posted by Daniel P. Berrangé 9 months, 3 weeks ago
On Fri, Jul 14, 2023 at 04:39:34PM +0200, Andrea Bolognani wrote:
> Plus some extras I'm throwing in for free :)
> 
> To understand why these changes are needed, see the original bug
> report[1] as well as the discussion triggered by Martin's initial
> attempt at addressing it[2].
> 
> Getting this right is quite tricky, so in order to convince myself
> that I'm not just going to break everyone's deployment I've tested
> things fairly extensively.
> 
> In particular, I've verified that things work as expected when
> upgrading from libvirt 9.0.0 (e.g. pre-split) on AlmaLinux 8 when the
> initial configuration is
> 
>   * a default one (socket-activated monolithic daemon);
> 
>   * monolithic daemon with --listen;
> 
>   * modular daemons;
> 
>   * modular daemons with virtproxyd-tcp.socket enabled;
> 
> as well as when installing from scratch with
> 
>   * no particular configuration;
> 
>   * local systemd preset overrides that result in the modular daemons
>     being preferred to the monolithic one.
> 
> Everything seems to work fine, but further testing would certainly be
> more than welcome!

I don't have an objection to the conceptual approach.

My concern is about where the solution is applied and divergance from
Fedora guidelines.

The long term direction of Fedora / RPM has been to reduce the number
of scriptlets required to be explicitly listed in package specfiles, by
having RPM globally apply script logic for all content in certain given
directories. If we're using standard Fedora macros, then we'd not expect
to see problems if the macros get changed to adapt to new approaches,
but if we're going our own way all bets are off.

The current macros we're using are specified in the Fedora packaging
guidelines:

  https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_systemd

and their impl is provided by systemd upstream itself:

  https://github.com/systemd/systemd/blob/main/src/rpm/macros.systemd.in

The problems described do not appear to be things unique to libvirt.

IOW, I think the problem needs to be raised & addressed in context of
the Fedora and systemd communities, rather than having libvirt diverge
from normal Feora packaging practice.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 0/8] rpm: Fix handling of systemd units
Posted by Andrea Bolognani 9 months, 3 weeks ago
On Fri, Jul 14, 2023 at 04:02:30PM +0100, Daniel P. Berrangé wrote:
> I don't have an objection to the conceptual approach.
>
> My concern is about where the solution is applied and divergance from
> Fedora guidelines.
>
> The long term direction of Fedora / RPM has been to reduce the number
> of scriptlets required to be explicitly listed in package specfiles, by
> having RPM globally apply script logic for all content in certain given
> directories. If we're using standard Fedora macros, then we'd not expect
> to see problems if the macros get changed to adapt to new approaches,
> but if we're going our own way all bets are off.
>
> The current macros we're using are specified in the Fedora packaging
> guidelines:
>
>   https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_systemd

That's *mostly* correct, but as noted in one of the commit messages
we have already been forced to implement a bunch of additional custom
logic because the standard macros simply don't cover all the
scenarios we need.

> and their impl is provided by systemd upstream itself:
>
>   https://github.com/systemd/systemd/blob/main/src/rpm/macros.systemd.in
>
> The problems described do not appear to be things unique to libvirt.

They're not! That said, somehow libvirt frequently manages to push
boundaries in ways that are fairly uncomfortable :)

> IOW, I think the problem needs to be raised & addressed in context of
> the Fedora and systemd communities, rather than having libvirt diverge
> from normal Feora packaging practice.

I absolutely agree with you, and I fully intend to push for these
changes (or comparable ones) to be implemented in systemd, where they
belong and from where they can benefit more than just us.

That said, timing is a concern. Fedora 38 and RHEL 9.2 are both on
libvirt 9.0.0 at the moment, so they haven't been hit by the issue
yet, but new releases for both are just around the corner and I have
little confidence that we'd be able to push the necessary changes
through in time. So a local solution seems like the only plausible
way that we can avoid breaking user's deployments.

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 0/8] rpm: Fix handling of systemd units
Posted by Daniel P. Berrangé 9 months, 3 weeks ago
On Fri, Jul 14, 2023 at 08:25:57AM -0700, Andrea Bolognani wrote:
> On Fri, Jul 14, 2023 at 04:02:30PM +0100, Daniel P. Berrangé wrote:
> > I don't have an objection to the conceptual approach.
> >
> > My concern is about where the solution is applied and divergance from
> > Fedora guidelines.
> >
> > The long term direction of Fedora / RPM has been to reduce the number
> > of scriptlets required to be explicitly listed in package specfiles, by
> > having RPM globally apply script logic for all content in certain given
> > directories. If we're using standard Fedora macros, then we'd not expect
> > to see problems if the macros get changed to adapt to new approaches,
> > but if we're going our own way all bets are off.
> >
> > The current macros we're using are specified in the Fedora packaging
> > guidelines:
> >
> >   https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_systemd
> 
> That's *mostly* correct, but as noted in one of the commit messages
> we have already been forced to implement a bunch of additional custom
> logic because the standard macros simply don't cover all the
> scenarios we need.
> 
> > and their impl is provided by systemd upstream itself:
> >
> >   https://github.com/systemd/systemd/blob/main/src/rpm/macros.systemd.in
> >
> > The problems described do not appear to be things unique to libvirt.
> 
> They're not! That said, somehow libvirt frequently manages to push
> boundaries in ways that are fairly uncomfortable :)
> 
> > IOW, I think the problem needs to be raised & addressed in context of
> > the Fedora and systemd communities, rather than having libvirt diverge
> > from normal Feora packaging practice.
> 
> I absolutely agree with you, and I fully intend to push for these
> changes (or comparable ones) to be implemented in systemd, where they
> belong and from where they can benefit more than just us.
> 
> That said, timing is a concern. Fedora 38 and RHEL 9.2 are both on
> libvirt 9.0.0 at the moment, so they haven't been hit by the issue
> yet, but new releases for both are just around the corner and I have
> little confidence that we'd be able to push the necessary changes
> through in time. So a local solution seems like the only plausible
> way that we can avoid breaking user's deployments.

If we at least start the discussion, we can get feedback on whether the
idea is likely to gain traction, or there are other things we have
overlooked

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [libvirt PATCH 0/8] rpm: Fix handling of systemd units
Posted by Andrea Bolognani 9 months, 3 weeks ago
On Fri, Jul 14, 2023 at 04:28:11PM +0100, Daniel P. Berrangé wrote:
> > > IOW, I think the problem needs to be raised & addressed in context of
> > > the Fedora and systemd communities, rather than having libvirt diverge
> > > from normal Feora packaging practice.
> >
> > I absolutely agree with you, and I fully intend to push for these
> > changes (or comparable ones) to be implemented in systemd, where they
> > belong and from where they can benefit more than just us.
> >
> > That said, timing is a concern. Fedora 38 and RHEL 9.2 are both on
> > libvirt 9.0.0 at the moment, so they haven't been hit by the issue
> > yet, but new releases for both are just around the corner and I have
> > little confidence that we'd be able to push the necessary changes
> > through in time. So a local solution seems like the only plausible
> > way that we can avoid breaking user's deployments.
>
> If we at least start the discussion, we can get feedback on whether the
> idea is likely to gain traction, or there are other things we have
> overlooked

I can open an issue on the systemd side pointing to these patches,
but polishing things up to the level of a proper PR is off the table
right now because I simply can't allocate enough time for it. Would
you consider that good enough to move forward with the libvirt
changes?

On the Fedora side, I wouldn't know what forum would be the most
appropriate to get the discussion started. Any pointers?

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 0/8] rpm: Fix handling of systemd units
Posted by Daniel P. Berrangé 9 months, 3 weeks ago
On Fri, Jul 14, 2023 at 08:52:15AM -0700, Andrea Bolognani wrote:
> On Fri, Jul 14, 2023 at 04:28:11PM +0100, Daniel P. Berrangé wrote:
> > > > IOW, I think the problem needs to be raised & addressed in context of
> > > > the Fedora and systemd communities, rather than having libvirt diverge
> > > > from normal Feora packaging practice.
> > >
> > > I absolutely agree with you, and I fully intend to push for these
> > > changes (or comparable ones) to be implemented in systemd, where they
> > > belong and from where they can benefit more than just us.
> > >
> > > That said, timing is a concern. Fedora 38 and RHEL 9.2 are both on
> > > libvirt 9.0.0 at the moment, so they haven't been hit by the issue
> > > yet, but new releases for both are just around the corner and I have
> > > little confidence that we'd be able to push the necessary changes
> > > through in time. So a local solution seems like the only plausible
> > > way that we can avoid breaking user's deployments.
> >
> > If we at least start the discussion, we can get feedback on whether the
> > idea is likely to gain traction, or there are other things we have
> > overlooked
> 
> I can open an issue on the systemd side pointing to these patches,
> but polishing things up to the level of a proper PR is off the table
> right now because I simply can't allocate enough time for it. Would
> you consider that good enough to move forward with the libvirt
> changes?
> 
> On the Fedora side, I wouldn't know what forum would be the most
> appropriate to get the discussion started. Any pointers?

Fedora devel list is probably the catch all place to raise it

https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/

There is a packaging list, which is technically the most on-topic place,
but it is mostly a ghost town as people tend to use the devel list for
everything

https://lists.fedoraproject.org/archives/list/packaging@lists.fedoraproject.org/

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [libvirt PATCH 0/8] rpm: Fix handling of systemd units
Posted by Andrea Bolognani 9 months, 2 weeks ago
On Fri, Jul 14, 2023 at 05:20:16PM +0100, Daniel P. Berrangé wrote:
> > > If we at least start the discussion, we can get feedback on whether the
> > > idea is likely to gain traction, or there are other things we have
> > > overlooked
> >
> > I can open an issue on the systemd side pointing to these patches,
> > but polishing things up to the level of a proper PR is off the table
> > right now because I simply can't allocate enough time for it. Would
> > you consider that good enough to move forward with the libvirt
> > changes?
> >
> > On the Fedora side, I wouldn't know what forum would be the most
> > appropriate to get the discussion started. Any pointers?
>
> Fedora devel list is probably the catch all place to raise it
>
> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/

Done.

https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/DLS73XUFYRXRFWHUQHE53ATAUDKHCMJK/

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 0/8] rpm: Fix handling of systemd units
Posted by Andrea Bolognani 9 months, 1 week ago
On Thu, Jul 20, 2023 at 07:42:19AM -0700, Andrea Bolognani wrote:
> On Fri, Jul 14, 2023 at 05:20:16PM +0100, Daniel P. Berrangé wrote:
> > > > If we at least start the discussion, we can get feedback on whether the
> > > > idea is likely to gain traction, or there are other things we have
> > > > overlooked
> > >
> > > I can open an issue on the systemd side pointing to these patches,
> > > but polishing things up to the level of a proper PR is off the table
> > > right now because I simply can't allocate enough time for it. Would
> > > you consider that good enough to move forward with the libvirt
> > > changes?
> > >
> > > On the Fedora side, I wouldn't know what forum would be the most
> > > appropriate to get the discussion started. Any pointers?
> >
> > Fedora devel list is probably the catch all place to raise it
> >
> > https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/
>
> Done.
>
> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/DLS73XUFYRXRFWHUQHE53ATAUDKHCMJK/

Summing up the discussion that happened in that thread over the last
week:

  * while the proper way to address the issue at hand is to change or
    replace the macros shipped with systemd;

  * given that such a process would require more time than we have
    available before Fedora and RHEL users are affected;

  * there is no strong objection to libvirt adopting the solution
    that I've implemented as a temporary measure.

Based on the above and the fact that the series is now fully ACKed, I
will push it before the end of the day so that it can be part of rc2.

Please shout if you have a problem with this plan!

-- 
Andrea Bolognani / Red Hat / Virtualization