Ludovic Courtès writes: > "Jan (janneke) Nieuwenhuizen" skribis: > >> * gnu/system/examples/bare-hurd.tmpl (%hurd-os)[services]: Add secret-service. >> * gnu/services/virtualization.scm (%hurd-vm-operating-system): Likewise. >> (hurd-vm-shepherd-service): Use it to install secrets. >> * doc/guix.texi (The Hurd in a Virtual Machine): Document it. > > Yay, minor issues, but overall LGTM! \o/ >> (services (cons* >> + ;; Receive secret keys on port 1004, TCP. >> + (service secret-service-type 1004) > > > [...] > >> + (start >> + (with-imported-modules >> + (source-module-closure '((gnu build secret-service) >> + (guix build utils))) >> + #~(let ((spawn (make-forkexec-constructor #$vm-command))) >> + (lambda _ >> + (let ((pid (spawn)) >> + (port #$(hurd-vm-port config %hurd-vm-secrets-port)) >> + (root #$(hurd-vm-configuration-secret-root config))) >> + (and root (directory-exists? root) >> + (catch #t >> + (lambda _ >> + (secret-service-send-secrets port root)) > In any case, we should assume that the VM is always running the secret > service server, and thus call ‘secret-service-send-secrets’ > unconditionally (‘secret-service-send-secrets’ does (find-files root), > which returns the empty list when ROOT doesn’t exist, Yeah I was struggling a bit with this; the hurd-vm-service and the childhurd must agree on the usage of secret-service. That's why I came up with this root-dir #f switch...but it's certainly simpler if we say that it must always be there. Let's see if we can get away with that! So, I removed the root-dir checks and we always call 'secret-service-send-secrets', and changed the default from #f to (secret-root hurd-vm-configuration-secret-root ;string (default "/etc/childhurd"))) where "/etc/childhurd" does not need to exist. > Perhaps ‘hurd-vm-service-type’ should unconditionally extend (via > ‘service-extension’) ‘secret-service-type’, just to ensure that Hurd VMs > always include the secret service. Eh, hurd-vm-service lives in the host, the secret-services lives in the client; am I missing something? ;-) We could add a check for secret-service, possibly here (define (hurd-vm-disk-image config) "Return a disk-image for the Hurd according to CONFIG." (let ((os (hurd-vm-configuration-os config)) (disk-size (hurd-vm-configuration-disk-size config))) (system-image (image (inherit hurd-disk-image) (size disk-size) (operating-system os))))) and/or insert if it it's missing...seems a bit over the top to me? > I think.) Yes, it does, but then the default cannot be #f, it must be a string. I'm picking "/etc/childurd" as a default that need not exist. >> + (lambda (keys . args) > > Should be “key” (singular). Oops :-) >> + (format (current-error-port) >> + "failed to send secrets: ~a ~s\n" key args) >> + (kill pid) > > (kill (- pid)) to kill the whole process group (just in case). > > I’d remove the ‘format’ call and just re-throw the exception: shepherd > should report it correctly. Done! Changed to unconditionally run (catch #t (lambda _ (secret-service-send-secrets port root)) (lambda (key . args) (kill (- pid) SIGTERM) (apply throw key args))) pid))))) >> + (service (@@ (gnu services virtualization) >> + secret-service-type) 5999)) > > This is useful for testing but I wouldn’t commit it (in particular > because the example would no longer work for people who’re just spawning > the VM and not trying to feed it secrets over TCP). Right, removed. > That’s it, thanks a lot! You too! Janneke -- Jan Nieuwenhuizen | GNU LilyPond http://lilypond.org Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com