Subject: Re: [bug#27887] [PATCH] services: Add libvirt services

From: Christopher Baines <[email-masked]>
Date: Sat, 19 Aug 2017 20:11:43 +0100

On Sat, 19 Aug 2017 10:57:59 -0700
Ryan Moe <[email-masked]> wrote:

> > I think running outside of the shepherd works, as the environment
> > will be different. I think you can look at the environment for the
> > running process with:
> >
> >   sudo cat /proc/$(pgrep -f libvirtd | head -n1)/environ
> >
> > What I see when running the service through the shepherd is very
> > minimal, and doesn't have a value for PATH.
> >
> > As I understand it, often in Guix, having a minimal runtime
> > environment might not be a problem, as packages retain references
> > to their dependencies from when they were built. If you look at the
> > libvirt package though, qemu is an input, but the output doesn't
> > reference it. 
> 
> This is because qemu isn't listed in propagated-inputs?

My understanding of propagated-inputs isn't great, and as far as I
know, it doesn't affect the environment in which the service runs, so I
don't think changing qemu to a propagated input would fix this.

> >> My initial thought was that it had something to do with how I was
> >> installing two packages into the system profile. qemu and libvirt
> >> both end up there so I don't see why it doesn't work though.  
> >
> > My guess is that the system profile isn't relevant here, as the bin
> > directory of the system profile isn't on the PATH.
> >
> > I've written a basic system test for the libvirt service (a patch is
> > attached), and with that I tested what happens if set the PATH for
> > the service to explicitly include the QEMU package bin directory,
> > and the test passes for me with this change.
> >  
> 
> Setting the PATH does indeed work, thank you for figuring that out.
> 
> > Back to getting this working though. Either wrapping libvirtd during
> > the package build to have a PATH including qemu, or including qemu
> > in the service will work, and I guess the deciding factor might be
> > how many things libvirt may need to access? Are there lots of other
> > bits of software that libvirt needs at runtime, where the package
> > doesn't currently reference them?  
> 
> Libvirt can work with other hypervisors like lxc, openvz, and xen and
> it supports lots of storage backends like NFS, LVM, and RBD.

Hmm, ok. If you don't have any strong opinions on how to fix this,
wrapping the libvirtd binary in the libvirt package is straightforward
and common. I've attached a patch that does this.
From 559c259fc641ab0e944e439816e95370e6878854 Mon Sep 17 00:00:00 2001
From: Christopher Baines <[email-masked]>
Date: Sat, 19 Aug 2017 20:07:47 +0100
Subject: [PATCH] packages: virtualization: Wrap libvirtd with iproute and
 qemu.

libvirtd runs qemu if its configured to use it, and also uses the ip command.

* gnu/packages/virtualization.scm (libvirt)[arguments]: Add wrap-libvirtd
  phase.
---
 gnu/packages/virtualization.scm | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/virtualization.scm b/gnu/packages/virtualization.scm
index ab364cd1f..d76a6dfd8 100644
--- a/gnu/packages/virtualization.scm
+++ b/gnu/packages/virtualization.scm
@@ -361,7 +361,16 @@ manage system or application containers.")
            (lambda _
              (zero? (system* "make" "install"
                              "sysconfdir=/tmp/etc"
-                             "localstatedir=/tmp/var")))))))
+                             "localstatedir=/tmp/var"))))
+         (add-after 'install 'wrap-libvirtd
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+             (let ((out (assoc-ref outputs "out")))
+               (wrap-program (string-append out "/sbin/libvirtd")
+                 `("PATH" = (,(string-append (assoc-ref inputs "iproute")
+                                             "/sbin")
+                             ,(string-append (assoc-ref inputs "qemu")
+                                             "/bin"))))
+               #t))))))
     (inputs
      `(("libxml2" ,libxml2)
        ("gnutls" ,gnutls)
-- 
2.14.1