[libvirt] [PATCH] spec: If installing default network, reload libvirtd (bz 867546)

[Cole Robinson]

On 04/21/2015 10:43 AM, Laine Stump wrote:
> On 04/21/2015 10:34 AM, Laine Stump wrote:
>> On 04/21/2015 09:48 AM, Michal Privoznik wrote:
>>> On 16.04.2015 21:42, Cole Robinson wrote:
>>>> If libvirt-daemon-config-network is installed while libvirtd is already
>>>> running, the daemon needs to be restarted to pick up the change.
>>>>
>>>> Instead let's trigger a daemon reload when the package is first installed.
>>>> Then the default network is available immediately if libvirtd was already
>>>> running.
>>>>
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=867546
>>>> ---
>>>>  libvirt.spec.in | 8 ++++++++
>>>>  1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/libvirt.spec.in b/libvirt.spec.in
>>>> index e08c9e7..ada0257 100644
>>>> --- a/libvirt.spec.in
>>>> +++ b/libvirt.spec.in
>>>> @@ -1770,6 +1770,14 @@ if test $1 -eq 1 && test ! -f %{_sysconfdir}/libvirt/qemu/networks/default.xml ;
>>>>           < %{_datadir}/libvirt/networks/default.xml \
>>>>           > %{_sysconfdir}/libvirt/qemu/networks/default.xml
>>>>      ln -s ../default.xml %{_sysconfdir}/libvirt/qemu/networks/autostart/default.xml
>>>> +
>>>> +    # Make sure libvirt picks up the new network defininiton
>>>> +      %if %{with_systemd}
>>>> +    /bin/systemctl reload libvirtd.service >/dev/null 2>&1 ||:
>>>> +      %else
>>>> +    /sbin/service libvirtd reload > /dev/null 2>&1 || :
>>>> +      %endif
>>>> +
>>>>  fi
>>>>      %endif
>>>>  
>>>>
>>> There's already a 'systemctl try-restart libvirtd.service' call just a
>>> few lines below. They were added in 4789fb2e. I think we can use them -
>>> also, I'm not sure why it doesn't work since we are restarting daemon
>>> even now (without this patch).
>> I think the issue may be that the default network is in a different
>> sub-package than libvirtd, so libvirtd is already started/restart by the
>> time the file is installed. Or something like that. I do recall that
>> this is a real problem though.
> 
> Heh. And if I had taken the time to go read the BZ that Cole referenced
> in his commit log message, I would have seen that I came to that same
> conclusion all the way back in 2012 :-/ It's amazing just how much I
> forget...
> 
> So I think this patch will be okay if we can just verify that it doesn't
> experience the same problem that I experienced when performing the
> reload manually.

I'll see if I can reproduce, I didn't hit that issue in my testing. Maybe
try-restart is fine, but not sure if people will care

 (Or, what about the idea I posted in the BZ about
> checking if libvirtd is running, and sending the file to "virsh
> net-define" instead of copying it when that's the case?)
> 

Interacting with virsh during RPM install scares me. Would need some testing
to make sure there isn't polkit weirdness, how we deal with
LIBVIRT_DEFAULT_URI, possibly other strange issues.

- Cole