[libvirt] [PATCH] spec: If installing default network, reload libvirtd (bz 867546)
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 -- 2.3.5
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). Michal
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. I also meant to respond to this patch and again got distracted - when I tried to manually do that this patch supposedly does, I got an error: I copied a new XML file into /etc/libvirt/qemu/networks and a link for it into /etc/libvirt/qemu/networks/autostart, then called systemctl reload libvirtd.service, I ended up with a bridge device created with the correct IP address, but libvirt showed the new network as inactive and this error log: networkCheckRouteCollision:121 : internal error: Network is already in use by interface virbr3 (virbr3 is the bridge created for this network; it didn't exist prior to telling libvirtd to reload, and there is no other route in conflict with it.) I didn't have time to investigate, but am wondering if the patch is really doing what is intended.
On 04/21/2015 10:34 AM, Laine Stump 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
On 04/21/2015 09:48 AM, Michal Privoznik wrote: 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. (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?)
On 04/21/2015 10:43 AM, Laine Stump wrote:
On 04/21/2015 10:34 AM, Laine Stump 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
On 04/21/2015 09:48 AM, Michal Privoznik wrote: 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
participants (3)
-
Cole Robinson -
Laine Stump -
Michal Privoznik