ZFS as a module #3107

rouming · 2023-03-17T16:17:14Z

This partially reverts commit 17dd29b.

With this revert we build ZFS again as a module. Why?
The linux kernel is licensed GPL, but OpenZFS is licensed CDDL.
These two are incompatible. So we compile both (kernel and ZFS)
and ship them separately.

This patch breaks collecting of the kernel dumps for the /persist
volume formatted as ZFS. We still jump into the kdump and output
the dmesg to the console, but not dump or logs are stored in the
/persist volume. This will be fixed in future releases of the
EVE-OS

cc: @deitch
cc: @eriknordmark

pkg/storage-init/storage-init.sh

eriknordmark

One question about storage-init but kicking off the tests in parallel.

rene · 2023-03-20T13:39:39Z

@rouming , the riscv64 build is broken: https://github.com/lf-edge/eve/actions/runs/4449491006/jobs/7818263989?pr=3107#step:8:827

rene · 2023-03-20T13:53:17Z

pkg/dom0-ztools/rootfs/etc/init.d/000-mod-params

@@ -34,4 +34,6 @@ zfs_set_default_parameters() {
    set_module_parameter zfs zfs_smoothing_write 5
 }

+# Load ZFS and set parameters
+modprobe zfs


Now that the ZFS module is loadable again, I'm wondering if doesn't make sense to pass all parameters during the loading instead of use /sys/module, that was set because module was built-in....

What is the difference? All the options start acting only when you do the mount. From kernel perspective I do not see a big difference when to update the static option variables: on modprobe or just after.

well, there isn't too much difference in terms of execution, but I don't see any reason to keep two functions (set_module_parameter and zfs_set_default_parameters), that were created exclusively because the module was built-in, while we can just load the module and set the parameters at once. By the way, the right way to load the module and pass these parameters is actually by creating a file zfs.conf at /etc/modprobe.d and use the format: option <module_name> <parameter(s)>, we do this, for instance, in /hostfs/etc/modprobe.d/kms.conf. And to load zfs module, you can add the module name to /hostfs/etc/modules, unless I'm missing something that justifies this script for the module loading...

So I did an attempt but it is not that simple with our container model. The description is here: #3107 (comment) I revoke the patch and leave as it is supposed to be - just a revert in order to fix the licensing problem. I assume we need to revisit this and fix how we call modprobe and make it proper way.

eriknordmark

Looks like the riscv build is failing for some reason.

rouming · 2023-03-21T06:17:06Z

Looks like the riscv build is failing for some reason.

Can be this line: diff -cw .config .config.new, which silently fails. Will take a look.

rouming · 2023-03-21T11:06:58Z

Difference to the previous version:

Fixed riscv64 build by adding ZLIB_DEFFLATE=m
Rebased on latest master

eriknordmark

LGTM

rouming · 2023-03-22T04:49:17Z

Difference to the previous version:

Get rid of a custom mechanism of setting parameters for zfs built-in module through the sysfs in a favor of a standard modprobe.d configuration file.

rouming · 2023-03-22T09:33:09Z

Difference to the previous version:

Have to revoke the patch which does the modprobe of the zfs instead of the custom script. The problem is that we call modprobe from a container (surprise surprise) which does not see the original rootfs, -C option does not work (modprobe version is not a GNU one). So fixing this requires time and more changes, which is not the intention of this PR (supposed to be a quick revert fix). cc: @rene

eriknordmark

Run eden

rene · 2023-03-22T11:48:53Z

Difference to the previous version:

* Have to revoke the patch which does the modprobe of the zfs instead of the custom script. The problem is that we call modprobe from a container (surprise surprise) which does not see the original rootfs, -C option does not work (modprobe version is not a GNU one). So fixing this requires time and more changes, which is not the intention of this PR (supposed to be a quick revert fix). cc: @rene

Ok, thanks @rouming . Let's discuss about it and create the proper issue to handle modprobe in a better (and unified) way.

LGTM.

eriknordmark

@rouming I've re-run the eden tests about 7 times and I don't think it has passed for any of the zfs variants at any time.
(But it does work if I use the rootfs to update a device running with a ZFS /persist).
So these failures needs some investigation to make sure installation and onboarding works correctly with a ZFS /persist.

rouming · 2023-03-27T14:17:31Z

Difference to the previous version:

Rebased on latest master

@eriknordmark according to my understanding profile server test cases fail for some reason, for example:

time: 2023-03-24T14:24:55.433386041Z out: 	appName local-manager state changed to RUNNING
time: 2023-03-24T14:24:55.433409642Z out: 	appName app-profile-1 state changed to HALTED
time: 2023-03-24T14:24:55.433413542Z out: 	appName app-profile-2 state changed to HALTED
time: 2023-03-24T14:24:55.433416543Z out: 	appName app-profile-1-2 state changed to HALTED
time: 2023-03-24T14:29:04.224650206Z out: 	appName app-profile-1 state changed to BOOTING
time: 2023-03-24T14:29:09.227216468Z out: 	appName app-profile-1 state changed to RUNNING
time: 2023-03-24T14:29:50.265640796Z out: 	appName app-profile-2 state changed to BOOTING
time: 2023-03-24T14:29:54.269975876Z out: 	appName app-profile-2 state changed to RUNNING

so some of the vms are running, which means that the error not zfs related (I assume if /persist is readonly, could not be mounted, has errors, etc, we could not boot and run vms at all), but maybe memory/time issue? I will check more precisely.

Update: also

2023-03-24T12:32:57.9577308Z [pool_name:"persist" storage_type:STORAGE_TYPE_INFO_ZFS zfs_version:"zfs-kmod-2.1.2-1" current_raid:STORAGE_RAID_TYPE_NORAID compression_ratio:1 zpool_size:3489660928 storage_state:STORAGE_STATUS_ONLINE disks:{disk_name:{name:"/dev/sda9" serial:"QM00001"} status:STORAGE_STATUS_ONLINE state:"No error."} pool_status_msg:"OK"]

no errors, all good

eriknordmark · 2023-03-28T01:27:24Z

no errors, all good

Seems like you're correct that this PR doesn't make it any worse. But glancing at eden results for other PRs it seems like 100% of the eden runs for zfs are failing since a week or two (unrelated to this PR). I have no idea whether this is an issue with flaky test(s) which somehow are more brittle when using zfs compared to ext4, or if there is an EVE-OS issue affecting ZFS. It feels scary to continue to accept PR until we know which case it is. So someone needs to root cause this failures before we proceed with any PRs.

eriknordmark

Kick off tests again

Don't do that! kdump kernel is not for re-formatting the FS, but for attempting to collect a dump. Even the FS is corrupted we don't re-format the FS. Even ZFS modules are not loaded we don't re-format the FS. Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>

This partially reverts commit 17dd29b. With this revert we build ZFS again as a module. Why? The linux kernel is licensed GPL, but OpenZFS is licensed CDDL. These two are incompatible. So we compile both (kernel and ZFS) and ship them separately. This patch breaks collecting of the kernel dumps for the /persist volume formatted as ZFS. We still jump into the kdump and output the dmesg to the console, but not dump or logs are stored in the /persist volume. This will be fixed in future releases of the EVE-OS Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>

eriknordmark · 2023-04-02T04:42:11Z

@rouming note that this never passes any of the Eden tests with zfs (I think 2x(6+8)=28 tries all failed). So I don't know what the impact will be on neither product quality nor our ability to use the eden tests to test product quaity. Can we please prioritize getting the failing tests fixed? I really hate driving blind.

rouming · 2023-04-04T07:32:09Z

@eriknordmark I'm running the following locally on the latest master:

./eden config set default --key=eve.disks --value=4
./eden config set default --key=eve.disk --value=4096
./eden config set default --key=eve.ram --value=4096
./eden config set default --key=eve.cpu --value=4
./eden config set default --key eve.tag --value ${EVE_TAG}
./eden config set default --key eve.log-level --value debug

./eden setup -v debug --grub-options='set_global dom0_extra_args "$dom0_extra_args eve_install_zfs_with_raid_level "'
EDEN_TEST_STOP=n ./eden test ./tests/workflow -v debug

(copied from the zfs run from github actions)

All tests successfully pass. From what I see on github, all failures have random nature, like waited for the VM state and timeout expired. My simple assumption is that zfs is greedy to resources and since we don't control the testing environment this is just a matter of unluck to get these failures.

@dautovri Can we have more resources on github actions and somehow guarantee dedicated hardware for our tests?

dautovri · 2023-04-04T09:04:13Z

@rouming I will create a support ticket to increase a size of GH runners, plus we can try to make own self hosted GH runners.

rouming · 2023-04-04T12:16:09Z

@dautovri Thanks, Ruslan! That what we definitely need. Let's start from the increasing number of runners. And then self hosted GH.

rouming requested review from eriknordmark and rvs as code owners March 17, 2023 16:17

eriknordmark reviewed Mar 17, 2023

View reviewed changes

pkg/storage-init/storage-init.sh Show resolved Hide resolved

eriknordmark approved these changes Mar 17, 2023

View reviewed changes

eriknordmark requested a review from rene March 17, 2023 21:03

rene reviewed Mar 20, 2023

View reviewed changes

eriknordmark requested changes Mar 21, 2023

View reviewed changes

rouming force-pushed the zfs-as-a-module branch from 0cd1283 to 37f4061 Compare March 21, 2023 11:05

eriknordmark approved these changes Mar 21, 2023

View reviewed changes

rouming force-pushed the zfs-as-a-module branch from 37f4061 to d815eb5 Compare March 22, 2023 04:47

rouming force-pushed the zfs-as-a-module branch from d815eb5 to 37e53fc Compare March 22, 2023 09:28

eriknordmark approved these changes Mar 22, 2023

View reviewed changes

eriknordmark requested changes Mar 27, 2023

View reviewed changes

rouming force-pushed the zfs-as-a-module branch from 37e53fc to f27a06a Compare March 27, 2023 14:11

eriknordmark approved these changes Mar 28, 2023

View reviewed changes

rouming added 2 commits March 31, 2023 17:10

rouming force-pushed the zfs-as-a-module branch from f27a06a to c395d5c Compare March 31, 2023 15:11

rouming merged commit 37039cd into lf-edge:master Mar 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ZFS as a module #3107

ZFS as a module #3107

rouming commented Mar 17, 2023

eriknordmark left a comment

rene commented Mar 20, 2023

rene Mar 20, 2023

rouming Mar 21, 2023

rene Mar 21, 2023

rouming Mar 22, 2023

eriknordmark left a comment

rouming commented Mar 21, 2023

rouming commented Mar 21, 2023 •

edited

Loading

eriknordmark left a comment

rouming commented Mar 22, 2023

rouming commented Mar 22, 2023 •

edited

Loading

eriknordmark left a comment

rene commented Mar 22, 2023 •

edited

Loading

eriknordmark left a comment

rouming commented Mar 27, 2023 •

edited

Loading

eriknordmark commented Mar 28, 2023

eriknordmark left a comment

eriknordmark commented Apr 2, 2023

rouming commented Apr 4, 2023

dautovri commented Apr 4, 2023

rouming commented Apr 4, 2023

ZFS as a module #3107

ZFS as a module #3107

Conversation

rouming commented Mar 17, 2023

eriknordmark left a comment

Choose a reason for hiding this comment

rene commented Mar 20, 2023

rene Mar 20, 2023

Choose a reason for hiding this comment

rouming Mar 21, 2023

Choose a reason for hiding this comment

rene Mar 21, 2023

Choose a reason for hiding this comment

rouming Mar 22, 2023

Choose a reason for hiding this comment

eriknordmark left a comment

Choose a reason for hiding this comment

rouming commented Mar 21, 2023

rouming commented Mar 21, 2023 • edited Loading

eriknordmark left a comment

Choose a reason for hiding this comment

rouming commented Mar 22, 2023

rouming commented Mar 22, 2023 • edited Loading

eriknordmark left a comment

Choose a reason for hiding this comment

rene commented Mar 22, 2023 • edited Loading

eriknordmark left a comment

Choose a reason for hiding this comment

rouming commented Mar 27, 2023 • edited Loading

eriknordmark commented Mar 28, 2023

eriknordmark left a comment

Choose a reason for hiding this comment

eriknordmark commented Apr 2, 2023

rouming commented Apr 4, 2023

dautovri commented Apr 4, 2023

rouming commented Apr 4, 2023

rouming commented Mar 21, 2023 •

edited

Loading

rouming commented Mar 22, 2023 •

edited

Loading

rene commented Mar 22, 2023 •

edited

Loading

rouming commented Mar 27, 2023 •

edited

Loading