[libvirt] [PATCH] spec: Restart libvirtd in posttrans

Jiri Denemark posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/fefece4edec5aaa12f24eb8d58bb7576245e1073.1508767257.git.jdenemar@redhat.com
There is a newer version of this series
libvirt.spec.in | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
[libvirt] [PATCH] spec: Restart libvirtd in posttrans
Posted by Jiri Denemark 6 years, 6 months ago
When upgrading libvirt packages, there's no strict ordering for the
installation or removal of the individual libvirt sub packages. Thus
libvirt-daemon may be upgraded (and its %postun scriptlet) started
before all sub packages with driver libraries are upgraded. When
libvirt-daemon's %postun scriptlet restarts the daemon old drivers may
still be laying around and the daemon may crash when it tries to use
them.

Let's restart the daemon in %posttrans to make sure libvirtd is
restarted only after all sub packages are at the same version.

https://bugzilla.redhat.com/show_bug.cgi?id=1464300

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 libvirt.spec.in | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 0123d0655..73e647cb5 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1554,13 +1554,11 @@ fi
 if [ $1 -ge 1 ] ; then
     /bin/systemctl reload-or-try-restart virtlockd.service >/dev/null 2>&1 || :
     /bin/systemctl reload-or-try-restart virtlogd.service >/dev/null 2>&1 || :
-    /bin/systemctl try-restart libvirtd.service >/dev/null 2>&1 || :
 fi
 %else
 if [ $1 -ge 1 ]; then
     /sbin/service virtlockd reload > /dev/null 2>&1 || :
     /sbin/service virtlogd reload > /dev/null 2>&1 || :
-    /sbin/service libvirtd condrestart > /dev/null 2>&1
 fi
 %endif
 
@@ -1570,10 +1568,16 @@ fi
 if [ "$1" -ge "1" ]; then
     /sbin/service virtlockd reload > /dev/null 2>&1 || :
     /sbin/service virtlogd reload > /dev/null 2>&1 || :
-    /sbin/service libvirtd condrestart > /dev/null 2>&1
 fi
 %endif
 
+%posttrans daemon
+%if %{with_systemd}
+/bin/systemctl try-restart libvirtd.service >/dev/null 2>&1 || :
+%else
+/sbin/service libvirtd condrestart > /dev/null 2>&1 || :
+%endif
+
 # In upgrade scenario we must explicitly enable virtlockd/virtlogd
 # sockets, if libvirtd is already enabled and start them if
 # libvirtd is running, otherwise you'll get failures to start
@@ -1642,11 +1646,13 @@ fi
 
 %post daemon-config-nwfilter
 cp %{_datadir}/libvirt/nwfilter/*.xml %{_sysconfdir}/libvirt/nwfilter/
+
+%posttrans daemon-config-nwfilter
 # Make sure libvirt picks up the new nwfilter defininitons
 %if %{with_systemd}
-    /bin/systemctl try-restart libvirtd.service >/dev/null 2>&1 ||:
+/bin/systemctl try-restart libvirtd.service >/dev/null 2>&1 || :
 %else
-    /sbin/service libvirtd condrestart > /dev/null 2>&1 || :
+/sbin/service libvirtd condrestart > /dev/null 2>&1 || :
 %endif
 
 
-- 
2.14.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: Restart libvirtd in posttrans
Posted by Pavel Hrdina 6 years, 6 months ago
On Mon, Oct 23, 2017 at 04:00:57PM +0200, Jiri Denemark wrote:
> When upgrading libvirt packages, there's no strict ordering for the
> installation or removal of the individual libvirt sub packages. Thus
> libvirt-daemon may be upgraded (and its %postun scriptlet) started
> before all sub packages with driver libraries are upgraded. When
> libvirt-daemon's %postun scriptlet restarts the daemon old drivers may
> still be laying around and the daemon may crash when it tries to use
> them.
> 
> Let's restart the daemon in %posttrans to make sure libvirtd is
> restarted only after all sub packages are at the same version.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1464300
> 
> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> ---
>  libvirt.spec.in | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: Restart libvirtd in posttrans
Posted by Pavel Hrdina 6 years, 6 months ago
On Mon, Oct 23, 2017 at 06:07:49PM +0200, Pavel Hrdina wrote:
> On Mon, Oct 23, 2017 at 04:00:57PM +0200, Jiri Denemark wrote:
> > When upgrading libvirt packages, there's no strict ordering for the
> > installation or removal of the individual libvirt sub packages. Thus
> > libvirt-daemon may be upgraded (and its %postun scriptlet) started
> > before all sub packages with driver libraries are upgraded. When
> > libvirt-daemon's %postun scriptlet restarts the daemon old drivers may
> > still be laying around and the daemon may crash when it tries to use
> > them.
> > 
> > Let's restart the daemon in %posttrans to make sure libvirtd is
> > restarted only after all sub packages are at the same version.
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1464300
> > 
> > Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> > ---
> >  libvirt.spec.in | 16 +++++++++++-----
> >  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

After some discussion in person we might want to improve the spec file
to restart libvirtd only once.  In order to do that you can follow this
guide [1].  The idea is to create temporary file in %post which will
indicate that we should restart libvirtd and in %posttrans we will
restart libvirtd only if the file exists and remove it.  This will
ensure that only one %posttrans will actually restart libvirtd.

Pavel

[1] <https://fedoraproject.org/wiki/Packaging:Scriptlets?rd=Packaging:ScriptletSnippets#Writing_Scriptlets>

> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list