Introduce new disk partitioning scheme #478

unbel13ver · 2024-02-16T14:36:24Z

New disk configuration provides grounds for upcoming features, such as AB software updates and Storage VM and many more.

The Lenovo X1 config has two LVM pools, first one is fixed-size 250G "system" partition (which is going to be encrypted) and the rest of the disk is dedicated to the StorageVM.

Description of changes

Checklist for things done

Testing

Current upstream partition scheme is as follows:

[ghaf@ghaf-host:~]$ lsblk
NAME          MAJ:MIN RM   SIZE RO TYPE MOUNTPOINTS
sda             8:0    0 465.8G  0 disk 
├─sda1          8:1    0   236M  0 part 
└─sda2          8:2    0   8.6G  0 part 
nvme0n1       259:0    0 953.9G  0 disk 
├─nvme0n1p1   259:1    0     1M  0 part 
├─nvme0n1p2   259:2    0   500M  0 part /boot
└─nvme0n1p3   259:3    0 953.4G  0 part 
  └─pool-root 254:0    0 953.4G  0 lvm  /nix/store
                                        /

To test new partitioning scheme, run lenovo-x1-carbon-gen11-debug-installer image and install Ghaf into the internal SSD storage of Lenovo-X1.
After insallation is completed and the laptop is booted into the Ghaf system, check partitions with lsblk command.
It should be as follows:

[ghaf@ghaf-host:~]$ lsblk
NAME                  MAJ:MIN RM   SIZE RO TYPE MOUNTPOINTS
sda                     8:0    0 465.8G  0 disk 
├─sda1                  8:1    0   1.8G  0 part 
└─sda2                  8:2    0     3M  0 part 
nvme0n1               259:0    0 953.9G  0 disk 
├─nvme0n1p1           259:1    0     1M  0 part 
├─nvme0n1p2           259:2    0   500M  0 part /boot
├─nvme0n1p3           259:3    0   500M  0 part 
├─nvme0n1p4           259:4    0   250G  0 part 
│ ├─pool-vm_storage_b 254:1    0    30G  0 lvm  
│ ├─pool-vm_storage_a 254:2    0    30G  0 lvm  /vmstore
│ ├─pool-root_b       254:3    0    50G  0 lvm  
│ ├─pool-root_a       254:4    0    50G  0 lvm  /nix/store
│ │                                             /
│ ├─pool-reserved_b   254:5    0    10G  0 lvm  
│ ├─pool-reserved_a   254:6    0    10G  0 lvm  
│ ├─pool-gp_storage   254:7    0    50G  0 lvm  
│ └─pool-recovery     254:8    0    20G  0 lvm  
└─nvme0n1p5           259:5    0 702.9G  0 part 
  └─vmstore-storagevm 254:0    0 702.9G  0 lvm

templates/targets/x86_64/generic/disk-config.nix

vilvo · 2024-02-19T07:26:15Z

templates/targets/x86_64/generic/disk-config.nix

 # SPDX-License-Identifier: Apache-2.0
-# Example to create a bios compatible gpt partition
+#
+# The partitioning scheme is hardcoded for 500G disk.


With this comment we could actually make this specific to Lenovo X1 Carbon Gen11 SKU (stock keeping unit) with 500G of internal NVMe. I'm afraid we must build target specific installers as we have not yet described disk space extension in case the internal NVMe is larger. Optionally, the installer could resolve the size of the disk with lsblk and parametrize the size to generate disk config for generic use. I think all the other fixed sizes could remain the same and only the remaining space here https://github.com/tiiuae/ghaf/pull/478/files#diff-8a1fbff8658406e95003d28116ba21ced73c81b4fb8d1a5edad5fe5aeef1c427R154 would have to be calculated. Please check with @remimimimimi if this is feasible. The next iterations of installer must support other installer parameters - like user specific secrets - as well.

Optionally, the installer could resolve the size of the disk with lsblk and parametrize the size to generate disk config for generic use.

This exact approach assumes that the installer program changes disk-config.nix file before passing it into disko utility, which I am pretty sure we want to avoid.

I think all the other fixed sizes could remain the same and only the remaining space would have to be calculated.

Unfortunately, disko utility for some reason does not support size = "100%" parameter in that exact place. It fails with error while partitioning the disk. Probably I should have mentioned this in the comment inside the file itself.

The next iterations of installer must support other installer parameters - like user specific secrets - as well.

This is out of scope of this PR.

Optionally, the installer could resolve the size of the disk with lsblk and parametrize the size to generate disk config for generic use.

This exact approach assumes that the installer program changes disk-config.nix file before passing it into disko utility, which I am pretty sure we want to avoid.

Ok. I'm fine with these assumptions for this iteration so that we can start development of the other areas this enables but I think we've identified a few issues that affect the installer and target device support design moving forward. Given the constraints and your scoping, I propose changing the file location and naming because it's not template and generic anymore. It wasn't even in #340 but we kind of agreed to iterate this to proper for this device in the next iteration which is this.

My proposal for placement is <ghaf_repo_root>/targets/lenovo-x1-carbon-disk-config.nix because this is very specific to that device. I don't think that is a problem because the current iteration of installer only supports X1. If we want template (or rather disko-config.nix nix module which accepts arguments from installer) and generic x86_64 disko-config, we must change also the dependency in the installer. Feel free to disagree but imo the filename remains confusing if we don't change it.

I think all the other fixed sizes could remain the same and only the remaining space would have to be calculated.

Unfortunately, disko utility for some reason does not support size = "100%" parameter in that exact place. It fails with error while partitioning the disk. Probably I should have mentioned this in the comment inside the file itself.

Sure, never too late to add insight in comments.

The next iterations of installer must support other installer parameters - like user specific secrets - as well.

This is out of scope of this PR.

Please see my previous comment. Scoping and addressing dependencies (even if next iterations of development) are both important.

Unfortunately, disko utility for some reason does not support size = "100%" parameter in that exact place. It fails with error while partitioning the disk. Probably I should have mentioned this in the comment inside the file itself.

It seems that this should work @unbel13ver .

Co-Disko maintainer here. Regarding secrets one can use the diskoImagesScript to generate full systems that also contain secrets. The script is run outside of the NixOS sandbox. Documentation is here: https://github.com/nix-community/disko/blob/master/docs/reference.md#generating-disk-images-with-secrets-included-using-disko

My proposal for placement is <ghaf_repo_root>/targets/lenovo-x1-carbon-disk-config.nix because this is very specific to that device.

That is reasonable, implemented.

Also I managed to take the rest of the disk in use for StorageVM (should have added "100%FREE" instead of "100%").
@leivos-unikie could you retest this please? I've updated the expected partitioning in the header of this PR.

I spotted these deviations in log messages

at booting installer

in the beginning of installation process

after installation was finished and pressed Enter for reboot

Otherwise no issues found in testing.

Thanks for testing! Those are not related to the matter of this PR (those are not something worth to worry about, anyway).

vilvo

Please see the detailed comments.

leivos-unikie · 2024-02-19T11:43:39Z

@unbel13ver can you provide test instructions for this PR? What is the expected outcome of new partitioning scheme?

unbel13ver · 2024-02-19T13:11:24Z

@unbel13ver can you provide test instructions for this PR? What is the expected outcome of new partitioning scheme?

Testing instructions added.

leivos-unikie

lsblk gives expected output
ci-test-automation passed
performance ok
apps launch

Mic92 · 2024-02-26T20:26:43Z

targets/disc-configs/lenovo-x1-carbon-disk-config.nix

+              ];
+            };
+          };
+          vm_storage_a = {


How is the vm_storage_a and vm_storage_b used? For system updates it's clear to me that one would apply changes to alternating partitions, but how does this work for vm state?

There were some ideas that some VMs can be supplied as pre-built images or so, and the end user will not be able to change the contents of that images. Those VMs will be updated together with the main system and must correspond the specific version of the base system.

Mic92 · 2024-02-26T20:29:12Z

targets/disc-configs/lenovo-x1-carbon-disk-config.nix

+            name = "boot";
+            size = "1M";
+            type = "EF02";
+          };


The EFI standard allows only one boot loader to be registered, how would the second partition used, in case the other "fails"?

Currently there is only one bootloader partition. The duplication in this case require some external tooling to switch between A and B, which we decided to avoid.

Mic92 · 2024-02-26T20:30:30Z

targets/disc-configs/lenovo-x1-carbon-disk-config.nix

+              mountOptions = ["umask=0077"];
+            };
+          };
+          # LVM pool that is going to be encrypted


How is the encryption done here? Would it be not cryptsetup -> lvm?

There is some discussion going about that in the mail thread. I do not know at this moment how it is going to be implemented, and I decided to remove this comment to avoid the confusion.

Mic92 · 2024-02-26T20:31:06Z

targets/disc-configs/lenovo-x1-carbon-disk-config.nix

+            };
+          };
+          # LVM pool that is going to be passed to the Storage VM
+          other_2 = {


Are VMs not encrypted?

StorageVM partition is not going to be encrypted, since it is supposed that it stores encrypted data. In this case it allows us to backup and recover the stored data safely if needed.

Mic92 · 2024-02-26T20:31:31Z

targets/disc-configs/lenovo-x1-carbon-disk-config.nix

+              ];
+            };
+          };
+          recovery = {


Is the recover also some NixOS? How is it booted?

This is just a placeholder currently, but yes, it is going to contain some factory recovery image.

Mic92 · 2024-02-26T20:48:15Z

Brian asked me to compare using LVM vs ZFS for this setup. Here a comparison:

with LVM one has to plan in advance how much space needs to be used for what purpose. The A/B partitioning will take 50% of the space.
with zfs one would create a single pool and datasets can dynamically allocate space from that. Instead of A/B partitioning, one would use snapshots -> potentially also create boot loader entries for rollback.
Ext4 probably will provide best performance for most desktop workloads.
Zfs makes better use of storage. 500GB is not much and compression can help here. Checksums are also nice to be more confident about correct data.
LVM is accepted upstream so any Linux kernel will work. With zfs there is some delay usually until the latest kernel can be used. NixOS by default uses LTS version of the kernel, which will always support zfs. Apart from that there is zfs.latestCompatibleLinuxPackages that will give the latest linux kernel supported
I'd recommend having a swap for zfs because the page eviction in ARC is not as quick as the normal page cache - to normal users it appears that zfs uses way more memory but this is mostly because ARC is not accounted as page cache
Encryption: There some open issues with zfs's native encryption. Cryptsetup is recommended if one wants to use zfs send at some point.
Impermanence: It's possible to implement automatic state reset with both LVM/Zfs. For LVM one would probably recreate/reformat a volatile state partitions. With zfs it would be a of a dataset to some blank snapshot (exceptionally a bit easier)

I currently do not have the full picture of all how this a/b partitioning will be implemented in the end. Especially around VM storage.

Mic92 · 2024-02-26T21:03:18Z

targets/disc-configs/lenovo-x1-carbon-disk-config.nix

+              format = "ext4";
+              mountpoint = "/";
+              mountOptions = [
+                "defaults"


Maybe add noatime for all of the ext4 mountpoints. It's not useful for most applications.

Thanks for the hint, done

Mic92 · 2024-02-26T21:06:35Z

targets/disc-configs/lenovo-x1-carbon-disk-config.nix

+              type = "filesystem";
+              format = "vfat";
+              mountpoint = "/boot";
+              mountOptions = ["umask=0077"];


I like adding "nofail" for non-root mounts. It makes recovering easier in case there is something wrong with mountpoints. Otherwise systemd will go into "emergency" mode. This can even happen on nixos-rebuild.

Suggested change

mountOptions = ["umask=0077"];

mountOptions = ["umask=0077" "nofail"];

Thanks for the hint, done

Mic92 · 2024-03-26T08:50:34Z

targets/disc-configs/lenovo-x1-carbon-disk-config.nix

+              ];
+            };
+          };
+          reserved_a = {


Maybe some comment what this is for would be helpful.
If this is just a spaceholder do we need a filesystem on it?

I have removed the filesystem from the reserved partitions and the recovery partition. I have also added a comment section in the beginning of the file to explain some of its parts.

vilvo · 2024-04-03T12:54:42Z

Brian asked me to compare using LVM vs ZFS for this setup. Here a comparison:
...

LVM is accepted upstream so any Linux kernel will work. With zfs there is some delay usually until the latest kernel can be used. NixOS by default uses LTS version of the kernel, which will always support zfs. Apart from that there is zfs.latestCompatibleLinuxPackages that will give the latest linux kernel supported

ghaf hardened requires a ZFS kernel config fragment. It's not an issue but it's a dependency to keep in mind.

I currently do not have the full picture of all how this a/b partitioning will be implemented in the end. Especially around VM storage.

Yeah, the A/B partitioning was specified and documented before the latest arch documentation process change. It would probably help to collect the prev documented designs and LVM/ZFS comparisons together according to the new process. With such many contributors I'd like to call it a collaborative documentation process to ensure a better full picture for contributors.

vunnyso · 2024-04-08T09:14:25Z

Is it possible to use smaller partitioning size and resizing mechanism later with current implementation? As final image generated will be 260GB which will take more time to generate and flash.

If not whether following postboot commands will be valid to execute still?

New disk configuration provides grounds for upcoming features, such as AB software updates and Storage VM and many more. Signed-off-by: Ivan Nikolaenko <ivan.nikolaenko@unikie.com>

Signed-off-by: Ivan Nikolaenko <ivan.nikolaenko@unikie.com>

unbel13ver · 2024-05-15T13:10:20Z

I have added a second commit with ZFS configuration so it is easier to compare what has changed.
ZFS also requires an option boot.kernelPackages = config.boot.zfs.package.latestCompatibleLinuxPackages; which I think changes the host's kernel version.

Known issues: For some reason, disko now fails with OOM during the nix store copying process if I give it's VM less than 16G of RAM.

brianmcgillion · 2024-05-22T14:17:43Z

Known issues: For some reason, disko now fails with OOM during the nix store copying process if I give it's VM less than 16G of RAM.

that should be fixed if you rebase against main. that was fixed in disko and the main has been bumped to use the newer version.

lheckemann · 2024-05-23T12:49:18Z

ZFS also requires an option boot.kernelPackages = config.boot.zfs.package.latestCompatibleLinuxPackages; which I think changes the host's kernel version.

This shouldn't be the case since as Mic92 mentioned NixOS uses an LTS kernel by default, which should generally be supported by ZFS. But I don't see the line in the diff anyway, so it's fine I guess :)

lheckemann · 2024-05-23T12:18:09Z

modules/disko/disko-ab-partitions-zfs.nix

+# Copyright 2022-2024 TII (SSRC) and the Ghaf contributors
+# SPDX-License-Identifier: Apache-2.0
+#
+# !!! To utilize this partition scheme, the disk size must be >252G !!!


Is this necessary? I suspect the first pool can be a fair bit smaller.

The quota on a zfs filesystem limits its maximum size but doesn't guarantee availability of the given amount of space. If you really want to ensure the availability, you should set a reservation or refreservation on the filesystems.

lheckemann · 2024-05-23T12:22:00Z

modules/disko/disko-ab-partitions-zfs.nix

+# - ESP-B : (500M)
+#
+# First ZFS pool contains next partitions:
+# - root-A : (50G) Root FS


I think if we're using ZFS it doesn't make that much sense to have exactly two root filesystems. It would make more sense to:

Allow more than two, since zfs doesn't limit us to allocating the space ahead of time

Not set a quota on them, since they should (AFAIU) be immutable and therefore fixed-size anyway

Depending on our threat model, it may also be undesirable to use zfs (for the root filesystem(s)) at all, since this mixes them up with the mutable pieces of the filesystem and doesn't allow us to use something like dm-verity to authenticate the whole partition (guarding against malicious filesystem images, for instance).

I've also personally experienced some unreliability from zfs, where the zfs kernel driver would crash and any further access to files on zfs filesystems would hang until reboot. Using ext4 (with dm-verity to counteract both malicious modification and bitrot) would probably be more reliable.

lheckemann · 2024-05-23T12:23:01Z

modules/disko/disko-ab-partitions-zfs.nix

+# - root-B : (50G)
+# - vm-storage-A : (30G) Possible standalone pre-built VM images are stored here
+# - vm-storage-B : (30G)
+# - reserved-A : (10G) Reserved partition, no use


This shouldn't be necessary. ZFS allows us the flexibility to add or remove filesystems from a pool after the fact if we end up needing them.

lheckemann · 2024-05-23T12:34:42Z

modules/disko/disko-ab-partitions-zfs.nix

+          };
+        };
+      };
+      zroot_2 = {


We should probably avoid importing this pool in the host at all, but I'm not sure that's currently feasible with disko. @Mic92?

lheckemann · 2024-05-23T12:38:43Z

modules/disko/disko-ab-partitions.nix

+# - root-B : (50G)
+# - vm-storage-A : (30G) Possible standalone pre-built VM images are stored here
+# - vm-storage-B : (30G)
+# - reserved-A : (10G) Reserved partition, no use


Like with zfs, LVM affords us some flexibility here (though indeed not as much). Why were you thinking to allocate this? I could see it being more necessary with LVM than with zfs, but I'm still skeptical about its usefulness.

lheckemann · 2024-05-23T12:39:39Z

modules/disko/disko-ab-partitions-zfs.nix

+# - reserved-A : (10G) Reserved partition, no use
+# - reserved-B : (10G)
+# - gp-storage : (50G) General purpose storage for some common insecure cases
+# - recovery : (rest of the LVM pool) Recovery factory image is stored here


Suggested change

# - recovery : (rest of the LVM pool) Recovery factory image is stored here

# - recovery : (rest of the ZFS pool) Recovery factory image is stored here

lheckemann · 2024-05-23T12:41:07Z

modules/disko/disko-ab-partitions.nix

@@ -0,0 +1,181 @@
+# Copyright 2022-2024 TII (SSRC) and the Ghaf contributors


Should we actually have this file if we're not referencing it anywhere?

unbel13ver temporarily deployed to internal-build-workflow February 16, 2024 14:36 — with GitHub Actions Inactive

unbel13ver requested review from remimimimimi, vilvo, brianmcgillion, vadika and leivos-unikie February 16, 2024 14:37

unbel13ver temporarily deployed to internal-build-workflow February 19, 2024 05:49 — with GitHub Actions Inactive

unbel13ver temporarily deployed to internal-build-workflow February 19, 2024 05:59 — with GitHub Actions Inactive

vilvo reviewed Feb 19, 2024

View reviewed changes

templates/targets/x86_64/generic/disk-config.nix Outdated Show resolved Hide resolved

vilvo reviewed Feb 19, 2024

View reviewed changes

templates/targets/x86_64/generic/disk-config.nix Outdated Show resolved Hide resolved

vilvo reviewed Feb 19, 2024

View reviewed changes

templates/targets/x86_64/generic/disk-config.nix Outdated Show resolved Hide resolved

vilvo reviewed Feb 19, 2024

View reviewed changes

vilvo suggested changes Feb 19, 2024

View reviewed changes

unbel13ver force-pushed the disco-config branch from 68c2e84 to 00b7412 Compare February 19, 2024 12:08

unbel13ver temporarily deployed to internal-build-workflow February 19, 2024 12:08 — with GitHub Actions Inactive

leivos-unikie approved these changes Feb 20, 2024

View reviewed changes

leivos-unikie added the Tested on Lenovo X1 Carbon This PR has been tested on Lenovo X1 Carbon label Feb 20, 2024

unbel13ver temporarily deployed to internal-build-workflow February 21, 2024 06:40 — with GitHub Actions Inactive

unbel13ver force-pushed the disco-config branch from 00b7412 to af1d8c7 Compare February 21, 2024 15:00

unbel13ver temporarily deployed to internal-build-workflow February 21, 2024 15:00 — with GitHub Actions Inactive

unbel13ver force-pushed the disco-config branch from af1d8c7 to 1794271 Compare February 22, 2024 12:37

unbel13ver temporarily deployed to internal-build-workflow February 22, 2024 12:37 — with GitHub Actions Inactive

unbel13ver removed the Tested on Lenovo X1 Carbon This PR has been tested on Lenovo X1 Carbon label Feb 22, 2024

leivos-unikie added the Tested on Lenovo X1 Carbon This PR has been tested on Lenovo X1 Carbon label Feb 23, 2024

Mic92 reviewed Feb 26, 2024

View reviewed changes

Mic92 reviewed Mar 26, 2024

View reviewed changes

unbel13ver force-pushed the disco-config branch from 1794271 to eb54ea7 Compare March 28, 2024 14:59

unbel13ver temporarily deployed to internal-build-workflow March 28, 2024 14:59 — with GitHub Actions Inactive

unbel13ver force-pushed the disco-config branch from eb54ea7 to 607200d Compare April 2, 2024 12:20

unbel13ver temporarily deployed to internal-build-workflow April 2, 2024 12:20 — with GitHub Actions Inactive

unbel13ver force-pushed the disco-config branch from 607200d to d5a2868 Compare April 3, 2024 12:19

unbel13ver temporarily deployed to internal-build-workflow April 3, 2024 12:19 — with GitHub Actions Inactive

unbel13ver force-pushed the disco-config branch from d5a2868 to 5c35203 Compare April 3, 2024 12:22

unbel13ver temporarily deployed to internal-build-workflow April 3, 2024 12:22 — with GitHub Actions Inactive

unbel13ver force-pushed the disco-config branch from 5c35203 to a15b8db Compare April 3, 2024 12:26

unbel13ver temporarily deployed to internal-build-workflow April 3, 2024 12:26 — with GitHub Actions Inactive

unbel13ver added 2 commits May 15, 2024 15:48

lenovo-x1: new disk partitioning scheme

8bbf2c9

New disk configuration provides grounds for upcoming features, such as AB software updates and Storage VM and many more. Signed-off-by: Ivan Nikolaenko <ivan.nikolaenko@unikie.com>

Add ZFS config for disko

ac41cca

Signed-off-by: Ivan Nikolaenko <ivan.nikolaenko@unikie.com>

unbel13ver force-pushed the disco-config branch from a15b8db to ac41cca Compare May 15, 2024 13:00

unbel13ver temporarily deployed to internal-build-workflow May 15, 2024 13:00 — with GitHub Actions Inactive

clayhill66 requested a review from lheckemann May 22, 2024 07:05

lheckemann reviewed May 23, 2024

View reviewed changes

	mountOptions = ["umask=0077"];
	mountOptions = ["umask=0077" "nofail"];

	# - recovery : (rest of the LVM pool) Recovery factory image is stored here
	# - recovery : (rest of the ZFS pool) Recovery factory image is stored here

		@@ -0,0 +1,181 @@
		# Copyright 2022-2024 TII (SSRC) and the Ghaf contributors

Introduce new disk partitioning scheme #478

Are you sure you want to change the base?

Introduce new disk partitioning scheme #478

Conversation

unbel13ver commented Feb 16, 2024 • edited

Description of changes

Checklist for things done

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 Feb 21, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vilvo left a comment

Choose a reason for hiding this comment

leivos-unikie commented Feb 19, 2024

unbel13ver commented Feb 19, 2024

leivos-unikie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 Feb 26, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 commented Feb 26, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vilvo commented Apr 3, 2024

vunnyso commented Apr 8, 2024

unbel13ver commented May 15, 2024 • edited

brianmcgillion commented May 22, 2024

lheckemann commented May 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unbel13ver commented Feb 16, 2024 •

edited

Mic92 Feb 21, 2024 •

edited

Mic92 Feb 26, 2024 •

edited

Mic92 commented Feb 26, 2024 •

edited

unbel13ver commented May 15, 2024 •

edited