Subject: Re: [bug#41785] [PATCH] DRAFT services: Add 'hurd-in-vm service-type'.

From: Ludovic Courtès <[email-masked]>
Date: Fri, 12 Jun 2020 16:45:04 +0200

Hi!

Jan Nieuwenhuizen <[email-masked]> skribis:

>> It’s a volatile VM, due to the use of ‘-snapshot’, right?
>
> By default: Yes.  That seemed more ready-to-use.  A stateful VM image
> would need to an out-of-store, writable copy.  You can actually do that
> and modify the hurd-vm-configuration.

It’s maybe worth mentioning in the manual.

>> (The Hurd actually has “sub-Hurds”¹ and “neighborhurds”².  I wonder if
>> it’s our duty to coin another term… a guesthurd? a visithurd?)
>>
>> ¹ https://www.gnu.org/software/hurd/hurd/subhurd.html
>> ² https://www.gnu.org/software/hurd/hurd/neighborhurd.html
>
> Oh, that's cool!  Associating along from the neighborhurd pun, what
> about a "childhurd" (as a pun on childhood -- only needed while the Hurd
> is growing up)?

“Childhurd”, LOVE IT!

> "herd start childhurd" -- hmm?  In the updated patch, I still have
> hurd-vm.  If we do our duty and coin "childhurd", should I just
> s/hurd-vm/childhurd/g ?

Shepherd services can have more than one name, so I propose to have both!

>> So, assuming ‘find-image’ is non-monadic, the code above becomes
>> something like:
>>
>>   (system-image
>>     (image (inherit base-image)
>>            (size image-size)
>>            (operating-system
>>             (with-parameters ((%current-target-system "i586-pc-gnu"))
>>               os))))
>
> Hmm...I don't think that I understand.  This
>
> (define* (disk-image os #:key (image-size 'guess) target)
>   "Return a disk-image for OS with size IMAGE-SIZE, built for TARGET."
>   (let ((base-image (find-image "ext2")))
>     (system-image
>      (image (inherit base-image)
>             (size image-size)
>             (operating-system
>               (with-parameters ((%current-target-system target))
>                 os))))))
>
>
> gives
>
> $ ~/src/guix/master/pre-inst-env guix system build dundal.scm
> %default-substitute-urls:("https://ci.guix.gnu.org")
> Backtrace:
> In ice-9/boot-9.scm:
>   1736:10  4 (with-exception-handler _ _ #:unwind? _ #:unwind-for-type _)
> In unknown file:
>            3 (apply-smob/0 #<thunk 7f4ce92e3980>)
> In ice-9/boot-9.scm:
>     718:2  2 (call-with-prompt _ _ #<procedure default-prompt-handler (k proc)>)
> In ice-9/eval.scm:
>     619:8  1 (_ #(#(#<directory (guile-user) 7f4ce8f05f00>)))
> In guix/ui.scm:
>   1945:12  0 (run-guix-command _ . _)
>
> guix/ui.scm:1945:12: In procedure run-guix-command:
> In procedure operating-system-file-systems: Wrong type argument: #<<parameterized> bindings: ((#<<parameter> 7f4ce7c23740 proc: #<procedure 7f4ce7c28200 at ice-9/boot-9.scm:1299:5 () | (x)>> #<procedure 7f4cd32f83c0 at gnu/services/virtualization.scm:806:14 ()>)) thunk: #<procedure 7f4cd32f8340 at gnu/services/virtualization.scm:806:14 ()>>
>
> ...I could do with some help here.

Ooh, sadness.  We can’t do that because the image machinery really
expects an <operating-system>.

What if you move ‘with-parameters’ around (system-image …) instead?

>>> +(define %hurd-in-vm-operating-system
> [..]
>>> +  (operating-system
>>> +               (service openssh-service-type
>>> +                        (openssh-configuration
>>> +                         (openssh openssh-sans-x)
> [..]
>>> +               %base-services/hurd))))
>>
>> I understand the need to factorize useful configs, but IMO it doesn’t
>> belong here.  So I’d just leave it out.  There’s already
>> ‘%hurd-default-operating-system’ that does the heavy lifting anyway.
>
> Sure, removed!  Users will most probably want to add an openssh server
> using openssh-sans-x; but I guess that's something for a blog post or
> cookbook then.

Yeah.

Hmm, come to think about it, we need a default value here anyway, so
after all, we have a good reason to have it here.  (Says the guy who
changes his mind.)

> From b01b8d2a46a6a04cb8f09d74c06cbbc82878f070 Mon Sep 17 00:00:00 2001
> From: "Jan (janneke) Nieuwenhuizen" <[email-masked]>
> Date: Thu, 11 Jun 2020 22:52:12 +0200
> Subject: [PATCH v2 1/2] image: Make 'find-image' non-monadic.
>
> * gnu/system/image.scm (find-image): Make non-monadic.
> * gnu/tests/install.scm (run-install): Update caller.
> * guix/scripts/system.scm (perform-action): Likewise.

[...]

>    "Find and return an image that could match the given FILE-SYSTEM-TYPE.  This
>  is useful to adapt to interfaces written before the addition of the <image>
>  record."
> -  (mlet %store-monad ((target (current-target-system)))
> -    (mbegin %store-monad
> -      (return
> -       (match file-system-type
> -         ("iso9660" iso9660-image)
> -         (_ (cond
> -             ((and target
> -                   (hurd-triplet? target))
> -              hurd-disk-image)
> -             (else
> -              efi-disk-image))))))))
> +  (let ((target (%current-target-system)))
> +    (match file-system-type
> +      ("iso9660" iso9660-image)
> +      (_ (cond
> +          ((and target
> +                (hurd-triplet? target))
> +           hurd-disk-image)
> +          (else
> +           efi-disk-image))))))

I’d prefer:

  (define* (find-image #:optional (system (%current-system)))
    …)

In your case, you’d need to make this call:

  (find-image "i586-gnu")

(Beware of the triplet/system type distinction!)

Perhaps the other call sites need to be adjusted.

> From e5bdf050f628cc7ea1b6bc4ccdcfeb757429820f Mon Sep 17 00:00:00 2001
> From: "Jan (janneke) Nieuwenhuizen" <[email-masked]>
> Date: Wed, 10 Jun 2020 00:10:28 +0200
> Subject: [PATCH v2 2/2] services: Add 'hurd-vm service-type'.
>
> * gnu/services/virtualization.scm (disk-image, hurd-in-vm-shepherd-service,
> hurd-vm-disk-image): New procedures.
> (hurd-in-vm-service-type): New variable.
> (<hurd-in-vm-configuration>): New record type.
> * doc/guix.texi (Virtualization Services): Document it.

s/hurd-in-vm/hurd-vm/ in the commit log.

Otherwise LGTM, thank you!

Ludo’.