[libvirt] [PATCH 0/4] systemd-related fixes and improvements

Make libvirt on systemd nicer for the user, by getting rid of some confusing behavior, and overall more solid. More details in each specific patch. Andrea Bolognani (4): virtlogd.socket: Tie lifecycle to libvirtd.service libvirt-guests.service: Improve description libvirt-guests.service: Split After= relationship libvirt-guests.service: Add Requires=libvirtd.service daemon/libvirtd.service.in | 1 + src/logging/virtlogd.service.in | 2 ++ src/logging/virtlogd.socket.in | 2 ++ tools/libvirt-guests.service.in | 7 +++++-- 4 files changed, 10 insertions(+), 2 deletions(-) -- 2.7.4

We already guarantee that virtlogd.socket is enabled/disabled along with libvirtd.service, but if libvirtd.service has just been installed and is started before rebooting, then virtlogd.socket will not be running and guest startup will fail. Add Requires=virtlogd.socket to libvirtd.service to make sure virtlogd.socket is always started along with libvirtd.service, and add Before=libvirtd.service to both virtlogd.socket and virtlogd.service so that virtlogd never disappears before libvirtd has exited. Also add PartOf=libvirtd.service to both virtlogd.socket and virtlogd.service, so that virtlogd can be shut down when not needed. Resolves: https://bugzilla.redhat.com/1372576 --- daemon/libvirtd.service.in | 1 + src/logging/virtlogd.service.in | 2 ++ src/logging/virtlogd.socket.in | 2 ++ 3 files changed, 5 insertions(+) diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in index 1616e7a..bbf27da 100644 --- a/daemon/libvirtd.service.in +++ b/daemon/libvirtd.service.in @@ -5,6 +5,7 @@ [Unit] Description=Virtualization daemon +Requires=virtlogd.socket Before=libvirt-guests.service After=network.target After=dbus.service diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in index a264d3a..8287994 100644 --- a/src/logging/virtlogd.service.in +++ b/src/logging/virtlogd.service.in @@ -1,6 +1,8 @@ [Unit] Description=Virtual machine log manager Requires=virtlogd.socket +Before=libvirtd.service +PartOf=libvirtd.service Documentation=man:virtlogd(8) Documentation=http://libvirt.org diff --git a/src/logging/virtlogd.socket.in b/src/logging/virtlogd.socket.in index 724976d..efb6504 100644 --- a/src/logging/virtlogd.socket.in +++ b/src/logging/virtlogd.socket.in @@ -1,5 +1,7 @@ [Unit] Description=Virtual machine log manager socket +Before=libvirtd.service +PartOf=libvirtd.service [Socket] ListenStream=@localstatedir@/run/libvirt/virtlogd-sock -- 2.7.4

libvirt-guests.service does both suspend *and* resume guests, depending on whether it's being started or stopped: the description should reflect this, to avoid confusing messages during startup. Replace "active" with "running" (to match virsh list's output) and don't capitalize libvirt. --- tools/libvirt-guests.service.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/libvirt-guests.service.in b/tools/libvirt-guests.service.in index c31f663..66f6cc2 100644 --- a/tools/libvirt-guests.service.in +++ b/tools/libvirt-guests.service.in @@ -1,5 +1,5 @@ [Unit] -Description=Suspend Active Libvirt Guests +Description=Suspend/Resume Running libvirt Guests After=network.target libvirtd.service time-sync.target Documentation=man:libvirtd(8) Documentation=http://libvirt.org -- 2.7.4

We use a separate line for each After= relationship in other unit files: do the same here for consistency's sake, and also to make future changes nicer to diff --- tools/libvirt-guests.service.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/libvirt-guests.service.in b/tools/libvirt-guests.service.in index 66f6cc2..3c901b9 100644 --- a/tools/libvirt-guests.service.in +++ b/tools/libvirt-guests.service.in @@ -1,6 +1,8 @@ [Unit] Description=Suspend/Resume Running libvirt Guests -After=network.target libvirtd.service time-sync.target +After=network.target +After=time-sync.target +After=libvirtd.service Documentation=man:libvirtd(8) Documentation=http://libvirt.org -- 2.7.4

Having After=libvirtd.service merely ensures that, if both services are asked to start, libvirtd.service will start first. What we really want is for libvirtd.service to be started whenever libvirt-guests.service is asked to start. Adding a Requires= relationship guarantees that will happen. --- tools/libvirt-guests.service.in | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/libvirt-guests.service.in b/tools/libvirt-guests.service.in index 3c901b9..b4f54f2 100644 --- a/tools/libvirt-guests.service.in +++ b/tools/libvirt-guests.service.in @@ -1,5 +1,6 @@ [Unit] Description=Suspend/Resume Running libvirt Guests +Requires=libvirtd.service After=network.target After=time-sync.target After=libvirtd.service -- 2.7.4

On Tue, Sep 06, 2016 at 03:55:20PM +0200, Andrea Bolognani wrote:
Make libvirt on systemd nicer for the user, by getting rid of some confusing behavior, and overall more solid.
More details in each specific patch.
ACK to all Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Tue, 2016-09-06 at 14:58 +0100, Daniel P. Berrange wrote:
On Tue, Sep 06, 2016 at 03:55:20PM +0200, Andrea Bolognani wrote:
Make libvirt on systemd nicer for the user, by getting rid of some confusing behavior, and overall more solid.
More details in each specific patch.
ACK to all
Pushed, thanks :) -- Andrea Bolognani / Red Hat / Virtualization

Daniel, Andrea, have you looked into the change in behaviour caused by patch 4 of this series? When libvirtd is stopped I get all my running domains suspended. Restarting libvirtd now also causes a disruption since all domains are suspended and than resumed. Before the commit of this patch the domains used to remain running without these disruptions and as a user this is what I expect to happen. I propose to revert commit fb2025ede9ecde1aa04ba6e01ce0c82c9e42dc66. On 09/06/2016 03:58 PM, Daniel P. Berrange wrote:
On Tue, Sep 06, 2016 at 03:55:20PM +0200, Andrea Bolognani wrote:
Make libvirt on systemd nicer for the user, by getting rid of some confusing behavior, and overall more solid.
More details in each specific patch.
ACK to all
Regards, Daniel
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On Fri, 2016-10-07 at 09:12 +0200, Boris Fiuczynski wrote:
Daniel, Andrea, have you looked into the change in behaviour caused by patch 4 of this series? When libvirtd is stopped I get all my running domains suspended. Restarting libvirtd now also causes a disruption since all domains are suspended and than resumed. Before the commit of this patch the domains used to remain running without these disruptions and as a user this is what I expect to happen. I propose to revert commit fb2025ede9ecde1aa04ba6e01ce0c82c9e42dc66.
Oh, you're right! Thank you for reporting this issue. We don't want to revert the change though, merely to downgrade the Requires to a Wants. That way, libvirtd stopping (or restarting) will not cause libvirt-guests to stop, but when starting libvirt-guests an attempt will still be made to start up libvirtd if it's not running already. I'll have patches on the list shortly. -- Andrea Bolognani / Red Hat / Virtualization

On 10/07/2016 09:31 AM, Andrea Bolognani wrote:
On Fri, 2016-10-07 at 09:12 +0200, Boris Fiuczynski wrote:
Daniel, Andrea, have you looked into the change in behaviour caused by patch 4 of this series? When libvirtd is stopped I get all my running domains suspended. Restarting libvirtd now also causes a disruption since all domains are suspended and than resumed. Before the commit of this patch the domains used to remain running without these disruptions and as a user this is what I expect to happen. I propose to revert commit fb2025ede9ecde1aa04ba6e01ce0c82c9e42dc66.
Oh, you're right! Thank you for reporting this issue.
We don't want to revert the change though, merely to downgrade the Requires to a Wants.
That way, libvirtd stopping (or restarting) will not cause libvirt-guests to stop, but when starting libvirt-guests an attempt will still be made to start up libvirtd if it's not running already.
I'll have patches on the list shortly.
-- Andrea Bolognani / Red Hat / Virtualization
The directive Wants works for me. Just tried it out. -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On 10/07/2016 09:31 AM, Andrea Bolognani wrote:
On Fri, 2016-10-07 at 09:12 +0200, Boris Fiuczynski wrote:
Daniel, Andrea, have you looked into the change in behaviour caused by patch 4 of this series? When libvirtd is stopped I get all my running domains suspended. Restarting libvirtd now also causes a disruption since all domains are suspended and than resumed. Before the commit of this patch the domains used to remain running without these disruptions and as a user this is what I expect to happen. I propose to revert commit fb2025ede9ecde1aa04ba6e01ce0c82c9e42dc66.
Oh, you're right! Thank you for reporting this issue.
We don't want to revert the change though, merely to downgrade the Requires to a Wants.
That way, libvirtd stopping (or restarting) will not cause libvirt-guests to stop, but when starting libvirt-guests an attempt will still be made to start up libvirtd if it's not running already.
I'll have patches on the list shortly.
-- Andrea Bolognani / Red Hat / Virtualization
Andrea, there is another "side effect" of the Requires directive. libvirt-guests gets automatically started when libvirt is updated to v2.3.0. This has some rather nasty implications for the end users. -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On Fri, 2016-10-07 at 11:19 +0200, Boris Fiuczynski wrote:
Andrea, there is another "side effect" of the Requires directive. libvirt-guests gets automatically started when libvirt is updated to v2.3.0. This has some rather nasty implications for the end users.
Mh, I don't see why that would happen. The Requires relationship goes in the opposite direction, so if libvirt-guests was not running before the upgrade there should be no reason for it to be started, whether that relationship is there or not. I'll try to reproduce this on my machine and get back to you in a while. -- Andrea Bolognani / Red Hat / Virtualization

On Fri, 2016-10-07 at 11:42 +0200, Andrea Bolognani wrote:
Andrea, there is another "side effect" of the Requires directive. libvirt-guests gets automatically started when libvirt is updated to v2.3.0. This has some rather nasty implications for the end users. Mh, I don't see why that would happen. The Requires relationship goes in the opposite direction, so if libvirt-guests was not running before the upgrade
On Fri, 2016-10-07 at 11:19 +0200, Boris Fiuczynski wrote: there should be no reason for it to be started, whether that relationship is there or not. I'll try to reproduce this on my machine and get back to you in a while.
I tried upgrading from 2.2.0 to 2.3.0 a bunch of times, but I haven't been able to reproduce the failure you're reporting: libvirt-guests is never started automatically. Can you provide more information? What distribution are you using? Are you building from source, or using your distribution's packages? Are you sure libvirt-guests was not running even before upgrade? -- Andrea Bolognani / Red Hat / Virtualization

On 10/07/2016 05:04 PM, Andrea Bolognani wrote:
On Fri, 2016-10-07 at 11:42 +0200, Andrea Bolognani wrote:
On Fri, 2016-10-07 at 11:19 +0200, Boris Fiuczynski wrote:
Andrea, there is another "side effect" of the Requires directive. libvirt-guests gets automatically started when libvirt is updated to v2.3.0. This has some rather nasty implications for the end users.
Mh, I don't see why that would happen.
The Requires relationship goes in the opposite direction, so if libvirt-guests was not running before the upgrade there should be no reason for it to be started, whether that relationship is there or not.
I'll try to reproduce this on my machine and get back to you in a while.
I tried upgrading from 2.2.0 to 2.3.0 a bunch of times, but I haven't been able to reproduce the failure you're reporting: libvirt-guests is never started automatically.
Can you provide more information? What distribution are you using? Are you building from source, or using your distribution's packages? Are you sure libvirt-guests was not running even before upgrade?
-- Andrea Bolognani / Red Hat / Virtualization
I used fc20 and built the libvirt rpms from source. libvirt-guests was not running when updating from 2.2.0 to libvirt 2.3.0. I switched systems and retried on fc23 after rebuilding libvirt 2.2.0 and 2.3.0 from source. On this system I cannot reproduce the behaviour. You are correct that the behaviour I got does not match with the requires directive. So my guess is that my old fc20 might still have some systemd problem which has been fixed in newer fc versions. -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On Tue, 2016-10-11 at 14:59 +0200, Boris Fiuczynski wrote:
I tried upgrading from 2.2.0 to 2.3.0 a bunch of times, but I haven't been able to reproduce the failure you're reporting: libvirt-guests is never started automatically. Can you provide more information? What distribution are you using? Are you building from source, or using your distribution's packages? Are you sure libvirt-guests was not running even before upgrade? I used fc20 and built the libvirt rpms from source. libvirt-guests was not running when updating from 2.2.0 to libvirt 2.3.0. I switched systems and retried on fc23 after rebuilding libvirt 2.2.0 and 2.3.0 from source. On this system I cannot reproduce the behaviour. You are correct that the behaviour I got does not match with the requires directive. So my guess is that my old fc20 might still have some systemd problem which has been fixed in newer fc versions.
That's great to hear! Thanks for providing feedback :) -- Andrea Bolognani / Red Hat / Virtualization
participants (3)
-
Andrea Bolognani
-
Boris Fiuczynski
-
Daniel P. Berrange