Subject: Re: [bug#45692] [PATCH v4 3/3] gnu: Add ZFS service type.

From: Xinglu Chen <[email-masked]>
Date: Mon, 06 Sep 2021 16:22:15 +0200

On Mon, Sep 06 2021, raid5atemyhomework via Guix-patches via wrote:

> Hello Xinglu Chen,
>
> Thank you for your interest.

You are welcome!  Thank you for working on ZFS support!

>> You might want to bring up the topic of subsystem maintainers on the
>> guix-devel mailing list to get some more attention.
>
> Not personally interested.
>
>
>> > +The above will keep 8 @code{frequent} snapshots and 12 @code{hourly} snapshots.
>> > +@code{daily}, @code{weekly}, and @code{monthly} snapshots will keep their
>> > +defaults (31 @code{daily}, 8 @code{weekly}, and 12 @code{monthly}).
>> > +
>> > +@end table
>> > +@end deftp
>>
>> IIUC, there is not way specify ZFS pools in the ‘file-systems’ field of
>> an <operating-system> record. Does this mean that ZFS as the root file
>> system is not supported, and if so, is there a particular reason for
>> this?
>
> No, and as you saw, that requires some more work.
>
> In particular, older versions of this patchset included the ability to
> add ZFS pools / datasets `file-system` objects, but the exact
> implementation was objected to, with no suggestion for what
> alternative to use instead.  Since I thought it was contentious, I
> removed it entirely instead.
>
> Note that ZFS-on-Root on Guix is even harder because of the need to
> put loading in `initrd`, and a lot of coding as well, not just
> `file-system`s support.
>
> Given the sheer lack of review and etc etc, I am not encouraged to
> write more code that will remain unreviewed and unmerged.  Maybe if
> this gets merged as-is, I will, but otherwise, I don't see the point.

I understand your feeling; hopefully we can get the patch merged soon!
:-)

>> The field name usually just contains the package name, so ‘zfs’ and
>> ‘zfs-auto-snapshot’ instead of ‘base-zfs’ and ‘base-zfs-auto-snapshot’.
>
>
> The point is that the service does **NOT** use `base-zfs` directly ---
> it creates a new version of that package, targeted towards the
> specific kernel you provided.  This is necessary since internal kernel
> APIs and ABIs may change between versions, even minor v==ersions
> (Linux has no commitment to keeping kernel interfaces stable, it only
> has a commitment to keeping userspace interfaces stable, and OpenZFS
> **requires** the kernel interfaces, so it is safest to compile
> specifically to the kernel version that is used).
>
> Thus the ***`base-`*** prefixes: the `zfs-service-type` does not use
> the `base-zfs` and `base-zfs-autosnapshot` packages as-is, they are
> instead used as the basis for the actual packages that are compiled
> and installed into the system.  I thought this would be sufficiently
> different from other services, which use package names as-is (but use
> the packages as-is, without inheriting from them, unlike this service)
> that the `base-` prefix was justified.

Ah, that makes sense, then I am fine with keeping it as-is.

>> > -                       ;; Output to current-error-port, otherwise the
>> >
>> >
>> > -                       ;; user will not see any prompts for passwords
>> >
>> >
>> > -                       ;; of encrypted datasets.
>> >
>> >
>> > -                       ;; XXX Maybe better to explicitly open /dev/console ?
>> >
>> >
>>
>> Seeing this comment, I assume that encrypted pools are supported, right?
>
> Encrypted datasets are supported, yes.  If you set
> `keylocation=prompt` then the `init` process will pause and ask for
> the password on the console.  You can even use
> `keylocation=file:///some/keyfile`, I also tested that in a VM.  My
> own production setup (which doesn't use this service, but includes
> some code copy-pasted from this service) uses
> `keylocation=file:///*elided*`.

Cool, hopefully I will get to test this soon.  :-)

>> > -        '(frequent hourly daily weekly monthly))))
>> >
>> >
>>
>> Maybe use something like
>>
>> (map first %default-auto-snapshot-keep)
>>
>> to avoid having to changing it if things change in the future.
>
> Good idea, thank you.
>
> Thanks
> raid5atemyhomework