Re: [libvirt] [PATCH] rpm: fix incorrect expansion of %systemd_preun macro
On 03/20/2018 01:00 PM, Daniel P. Berrangé wrote:
Macros in RPMs are expanded before line continuations, so when we
write
%systemd_preun foo \
bar
What happens is that it expands to
if [ $1 -eq 0 ] ; then
# Package removal, not upgrade
systemctl --no-reload disable --now foo \ > /dev/null 2>&1 || :
fi
bar
which is obviously complete garbage and not what we expected. It is
simply not safe to ever use line continuations in combination with
macros.
Introduced in commit bffdd6c3034164127b1543ffd2e9ed599baf4838, present
in released libvirt-4.1.0.
This is going to be problematic for any rpm-based distro that has a
4.1.0 rpm, e.g. Fedora rawhide and F28 - if someone has updated to the
broken rpm, they won't be able to get rid of it with a plain update, and
dnf has no command that passes through the necessary --nopreun command
to rpm. Instead, they'll need to run rpm manually - "rpm --nopreun blah
blah".
If there is already a 4.1.0-maint branch, we should pull this patch back
to there, and think about how to notify the poor F28/rawhide users of
their predicament (hopefully there aren't too many, as F28 isn't yet
released)
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
Reviewed-by: Laine Stump <laine(a)laine.org>
---
libvirt.spec.in | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index bc8257f34b..6bf8368476 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1512,9 +1512,9 @@ exit 0
%if %{with_systemd}
%if %{with_systemd_macros}
- %systemd_post virtlockd.socket virtlockd-admin.socket \
- virtlogd.socket virtlogd-admin.socket \
- libvirtd.service
+ %systemd_post virtlockd.socket virtlockd-admin.socket
+ %systemd_post virtlogd.socket virtlogd-admin.socket
+ %systemd_post libvirtd.service
It ends up being a bit inefficient in the shell script that's created,
since it puts each line in its own conditional, but that's not visible
to the user anywhere, and the performance impact is effectively 0 :-)
%else
if [ $1 -eq 1 ] ; then
# Initial installation
@@ -1549,9 +1549,9 @@ touch %{_localstatedir}/lib/rpm-state/libvirt/restart || :
%preun daemon
%if %{with_systemd}
%if %{with_systemd_macros}
- %systemd_preun libvirtd.service \
- virtlogd.socket virtlogd-admin.socket virtlogd.service \
- virtlockd.socket virtlockd-admin.socket virtlockd.service
+ %systemd_preun libvirtd.service
+ %systemd_preun virtlogd.socket virtlogd-admin.socket virtlogd.service
+ %systemd_preun virtlockd.socket virtlockd-admin.socket virtlockd.service
%else
if [ $1 -eq 0 ] ; then
# Package removal, not upgrade