[libvirt PATCH] Make systemd unit ordering more robust

[Michal Prívozník]

On 2/15/22 12:12, Martin Kletzander wrote:
> Since libvirt-guests script/service can operate on various URIs and we do
> support both socket activation and traditional services, the ordering should be
> specified for all the possible sockets and services.
> 
> Also remove the Wants= dependency since do not want to start any service.  We
> cannot know which one libvirt-guests is configured, so we'd have to start all
> the daemons which would break if unused colliding services are not
> masked (libvirtd.service in the modular case and all the modular daemon service
> units in the monolithic scenario).  Fortunately we can assume that the system is
> configured properly to start services/sockets that are of interest to the user.
> That also works with the setup described in https://libvirt.org/daemons.html .
> 
> To make it even more robust we add the daemon service into the machine units
> created for individual domains as it was missing there.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1868537
> 
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
>  src/util/virsystemd.c           |  4 ++++
>  tools/libvirt-guests.service.in | 12 +++++++++++-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
> index a86d4c6bb905..2d9711ad41b6 100644
> --- a/src/util/virsystemd.c
> +++ b/src/util/virsystemd.c
> @@ -441,7 +441,9 @@ int virSystemdCreateMachine(const char *name,
>                                                  nicindexes, nnicindexes, sizeof(int));
>          gprops = g_variant_new_parsed("[('Slice', <%s>),"
>                                        " ('After', <['libvirtd.service']>),"
> +                                      " ('After', <['virt%sd.service']>),"
>                                        " ('Before', <['virt-guest-shutdown.target']>)]",
> +                                      drivername,
>                                        slicename);

Surely these two variables need to be swapped.

>          message = g_variant_new("(s at ayssus@ai at a(sv))",
>                                  name,
> @@ -489,7 +491,9 @@ int virSystemdCreateMachine(const char *name,
>                                            uuid, 16, sizeof(unsigned char));
>          gprops = g_variant_new_parsed("[('Slice', <%s>),"
>                                        " ('After', <['libvirtd.service']>),"
> +                                      " ('After', <['virt%sd.service']>),"
>                                        " ('Before', <['virt-guest-shutdown.target']>)]",
> +                                      drivername,
>                                        slicename);

And here too.

>          message = g_variant_new("(s at ayssus@a(sv))",
>                                  name,
> diff --git a/tools/libvirt-guests.service.in b/tools/libvirt-guests.service.in
> index 1a9b233e1177..3cf647619612 100644
> --- a/tools/libvirt-guests.service.in
> +++ b/tools/libvirt-guests.service.in
> @@ -1,10 +1,20 @@
>  [Unit]
>  Description=Suspend/Resume Running libvirt Guests
> -Wants=libvirtd.service
>  Requires=virt-guest-shutdown.target
>  After=network.target
>  After=time-sync.target
> +After=libvirtd.socket
> +After=virtqemud.socket
> +After=virtlxcd.socket
> +After=virtvboxd.socket
> +After=virtvzd.socket
> +After=virtxend.socket
>  After=libvirtd.service
> +After=virtqemud.service
> +After=virtlxcd.service
> +After=virtvboxd.service
> +After=virtvzd.service
> +After=virtxend.service
>  After=virt-guest-shutdown.target
>  Documentation=man:libvirt-guests(8)
>  Documentation=https://libvirt.org

Otherwise looking good.

Michal