On Thu, Dec 12, 2019 at 7:04 PM Michal Privoznik <mprivozn@redhat.com> wrote:
On 12/3/19 9:56 AM, Christian Ehrhardt wrote:
> The most common operation of libvirt-guests is to manage the local
> libvirtd. But users might have disabled that and while we are
> After=libvirtd for ordering we are not Requiring it..
> OTOH adding that or any harder dependency might affect our ordering.
>
> But if people have disabled libvirt they will do a full retry loop
> until timeout. Lets check if the local service is active at all and skip
> fast if it is not.
>
> Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1854653
>
> Reported-by: Doug Smythies <dsmythies@telus.net>
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> ---
>   tools/libvirt-guests.sh.in | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in
> index 4bc6e866f0..5a9930ee2f 100644
> --- a/tools/libvirt-guests.sh.in
> +++ b/tools/libvirt-guests.sh.in
> @@ -90,6 +90,14 @@ test_connect()
>   {
>       uri=$1
>   
> +    if [ "x$uri" = xdefault ]; then
> +        # Default config is most common and for the local libvirtd
> +        # Check if it is active before wasting time in connect loop
> +        if ! systemctl -q is-active libvirtd; then

Systemd is still not the only init out there:

moe ~ # systemctl -q is-active libvirtd
-su: systemctl: command not found

However, this will make libvirt-guests unusable when using split daemons
(which are not enabled yet). So I guess we need to check whether either
libvirtd or any of the hypervisor daemons is running (we can't assume
the default URI is qemu:///system).

Just for completeness - we can assume that because that is what the ["x$uri" = xdefault] check is for.
For any non default cases it would have not tried to fast path it.

But I like Daniels suggestion of maybe just dropping the retry logic on this one.
Therefore consider this patch obsolete and I'll send a very different v2 somewhen later.

Also, it would be nice to print a
message why we tested connection unusable. Something like:

   eval_gettext "Libvirtd is not running. Skipping."
   return 1

Michal



--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd