This patchset addresses some caveats that were present in my first btrfs
patchset; and makes it possible to switch between snapshots taken of
"/".
Changes:
- rename all subvols to follow the common @* btrfs subvol naming scheme.
- add subvol @root, because roots home directory shouldn't be rolled
back.
- make subvol @var not Copy-on-Write (nodatacow) to avoid write
multiplication on logs, VMs, databases, containers and flatpaks.
- add subvol @snapshots because that lets us change the root subvol to a
read-write snapshot of @ without affecting snapshots.
- add subvol @srv because it contains data for Web and FTP servers,
which shouldn't roll back together with root subvol.
Jacob Ludvigsen (4):
install: add more subdivision of btrfs subvols, to filter out
directories that shouldn't be snapshotted or rolled back
install/format: mount btrfs subvols using labels rather than
subvolids, to increase robustness
install/format: fix nodataCoW on /var by mounting btrfs @var subvol
before doing chattr +C
install: improve comment regarding create_home_from_skel under btrfs
pmb/install/_install.py | 15 ++++++-----
pmb/install/format.py | 55 ++++++++++++++++++++++++++++++++++++-----
2 files changed, 58 insertions(+), 12 deletions(-)
--
2.38.5
Hi Jacob, here are a few review comments, so I didn't directly apply it yet. Note that pmbootstrap.git has been moved as explained here: https://postmarketos.org/blog/2024/01/17/moving-pmbootstrap/ If you could post the next version to gitlab.com it would make it easier, however if you strongly prefer using git send e-mail then we can also finish this series up over e-mail. Some general comments: * The commit message titles are too long, should be 50 characters max (see also: https://cbea.ms/git-commit/) * It looks like the first three patches of this serious should be squashed, as they just fix up the previous patches. * There is a patch that removes /tmp from fstab again. Since recently we do mount /tmp as tmpfs, but it depends on deviceinfo and the amount of RAM the device has: https://gitlab.com/postmarketOS/pmaports/-/merge_requests/4588 So I guess we should just keep the fstab entryp for /tmp you made? As I understand, the service would mount /tmp to tmpfs on top of it (if the conditions like RAM are met). But please test this.
This looks like an unrelated change. Thank you very much for the patches, I highly appreciate how you are improving btrfs support! \o/ Best regards, Oliver
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~postmarketos/pmbootstrap-devel/patches/48577/mbox | git am -3Learn more about email & git
From: Jacob Ludvigsen <jacob.ludvigsen@protonmail.com> --- pmb/install/_install.py | 14 ++++++---- pmb/install/format.py | 58 ++++++++++++++++++++++++++++++++++++----- 2 files changed, 61 insertions(+), 11 deletions(-) diff --git a/pmb/install/_install.py b/pmb/install/_install.py index 696e99da..a8ab829f 100644 --- a/pmb/install/_install.py +++ b/pmb/install/_install.py @@ -768,20 +768,24 @@ def create_fstab(args, layout, suffix): boot_dev = f"/dev/installp{layout['boot']}" root_dev = f"/dev/installp{layout['root']}" + root_filesystem = pmb.install.get_root_filesystem(args) + boot_filesystem = args.deviceinfo["boot_filesystem"] or "ext2" + boot_mount_point = f"UUID={get_uuid(args, boot_dev)}" root_mount_point = "/dev/mapper/root" if args.full_disk_encryption \ else f"UUID={get_uuid(args, root_dev)}" - boot_filesystem = args.deviceinfo["boot_filesystem"] or "ext2" - root_filesystem = pmb.install.get_root_filesystem(args) - if root_filesystem == "btrfs": # btrfs gets separate subvolumes for root, var and home fstab = f""" # <file system> <mount point> <type> <options> <dump> <pass> {root_mount_point} / {root_filesystem} subvol=@,compress=zstd:2,ssd 0 0 -{root_mount_point} /home {root_filesystem} subvol=home,compress=zstd:2,ssd 0 0 -{root_mount_point} /var {root_filesystem} subvol=var,compress=zstd:2,ssd 0 0 +{root_mount_point} /home {root_filesystem} subvol=@home,compress=zstd:2,ssd 0 0 +{root_mount_point} /root {root_filesystem} subvol=@root,compress=zstd:2,ssd 0 0 +{root_mount_point} /var {root_filesystem} subvol=@var,ssd 0 0 +{root_mount_point} /tmp {root_filesystem} subvol=@tmp,compress=zstd:2,ssd 0 0 +{root_mount_point} /srv {root_filesystem} subvol=@srv,compress=zstd:2,ssd 0 0 +{root_mount_point} /.snapshots {root_filesystem} subvol=@snapshots,compress=zstd:2,ssd 0 0 {boot_mount_point} /boot {boot_filesystem} defaults 0 0 """.lstrip() diff --git a/pmb/install/format.py b/pmb/install/format.py index a1cd78fb..445a5d4c 100644 --- a/pmb/install/format.py +++ b/pmb/install/format.py @@ -124,21 +124,67 @@ def format_and_mount_root(args, device, root_label, disk): pmb.chroot.root(args, ["mkdir", "-p", mountpoint]) pmb.chroot.root(args, ["mount", device, mountpoint]) - # Create separate subvolumes if root filesystem is btrfs + """ + Create separate subvolumes if root filesystem is btrfs + This lets us do snapshots and rollbacks of relevant parts + of the filesystem. + /var contains logs, VMs, containers, flatpaks; and shouldn't roll back, + root's home directory /root shouldn't roll back, + snapshotting temporary files in /tmp is unnecessary, + /srv contains data for web and FTP servers, and shouldn't roll back, + snapshots should be a separate subvol so that changing the root subvol + doesn't affect snapshots + """ if filesystem == "btrfs": pmb.chroot.root(args, - ["btrfs", "subvol", "create", mountpoint + "/@", \ - mountpoint + "/var", mountpoint + "/home"]) + ["btrfs", "subvol", "create", + mountpoint + "/@", mountpoint + "/@var", + mountpoint + "/@home", mountpoint + "/@root", + mountpoint + "/@tmp", mountpoint + "/@srv", + mountpoint + "/@snapshots"]) + + # Set the default root subvolume to be separate from top level btrfs + # subvol. This lets us easily swap out current root subvol with an + # earlier snapshot. pmb.chroot.root(args, ["btrfs", "subvol", "set-default", mountpoint + "/@"]) + + # Make directories to mount subvols onto. + # snapshots contain sensitive information, + # and should only be readable by root. pmb.chroot.root(args, ["umount", mountpoint]) pmb.chroot.root(args, ["mount", device, mountpoint]) + pmb.chroot.root(args, ["mkdir", + mountpoint + "/var", + mountpoint + "/home", + mountpoint + "/srv", + "-m", "700", mountpoint + "/root", + "-m", "766", mountpoint + "/tmp", + "-m", "750", mountpoint + "/.snapshots"]) + + # Disable CoW for /var, to avoid write multiplication + # and slowdown on databases, containers and VM images. + pmb.chroot.root(args, ["chattr", "+C", mountpoint + "/var"]) + pmb.chroot.root(args, - ["mkdir", mountpoint+"/home", mountpoint+"/var"]) + ["mount", "-o", "subvolid=257", + device, mountpoint+"/var"]) pmb.chroot.root(args, - ["mount", "-o", "subvolid=257", device, mountpoint+"/var"]) + ["mount", "-o", "subvolid=258", + device, mountpoint+"/home"]) pmb.chroot.root(args, - ["mount", "-o", "subvolid=258", device, mountpoint+"/home"]) + ["mount", "-o", "subvolid=259", + device, mountpoint+"/root"]) + pmb.chroot.root(args, + ["mount", "-o", "subvolid=260", + device, mountpoint+"/tmp"]) + pmb.chroot.root(args, + ["mount", "-o", "subvolid=261", + device, mountpoint+"/srv"]) + pmb.chroot.root(args, + ["mount", "-o", "subvolid=262", + device, mountpoint+"/.snapshots"]) + -- 2.38.5
Hi Jacob, here are a few review comments, so I didn't directly apply it yet. Note that pmbootstrap.git has been moved as explained here: https://postmarketos.org/blog/2024/01/17/moving-pmbootstrap/ If you could post the next version to gitlab.com it would make it easier, however if you strongly prefer using git send e-mail then we can also finish this series up over e-mail. Some general comments: * The commit message titles are too long, should be 50 characters max (see also: https://cbea.ms/git-commit/) * It looks like the first three patches of this serious should be squashed, as they just fix up the previous patches. * There is a patch that removes /tmp from fstab again. Since recently we do mount /tmp as tmpfs, but it depends on deviceinfo and the amount of RAM the device has: https://gitlab.com/postmarketOS/pmaports/-/merge_requests/4588 So I guess we should just keep the fstab entryp for /tmp you made? As I understand, the service would mount /tmp to tmpfs on top of it (if the conditions like RAM are met). But please test this.
This looks like an unrelated change. Thank you very much for the patches, I highly appreciate how you are improving btrfs support! \o/ Best regards, Oliver
From: Jacob Ludvigsen <jacob.ludvigsen@protonmail.com> don't create btrfs subvolume for /tmp, since it is tmpfs anyway --- pmb/install/_install.py | 1 - pmb/install/format.py | 27 ++++++++++++--------------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/pmb/install/_install.py b/pmb/install/_install.py index a8ab829f..aba64392 100644 --- a/pmb/install/_install.py +++ b/pmb/install/_install.py @@ -783,7 +783,6 @@ def create_fstab(args, layout, suffix): {root_mount_point} /home {root_filesystem} subvol=@home,compress=zstd:2,ssd 0 0 {root_mount_point} /root {root_filesystem} subvol=@root,compress=zstd:2,ssd 0 0 {root_mount_point} /var {root_filesystem} subvol=@var,ssd 0 0 -{root_mount_point} /tmp {root_filesystem} subvol=@tmp,compress=zstd:2,ssd 0 0 {root_mount_point} /srv {root_filesystem} subvol=@srv,compress=zstd:2,ssd 0 0 {root_mount_point} /.snapshots {root_filesystem} subvol=@snapshots,compress=zstd:2,ssd 0 0 diff --git a/pmb/install/format.py b/pmb/install/format.py index 445a5d4c..33c4ebdb 100644 --- a/pmb/install/format.py +++ b/pmb/install/format.py @@ -125,7 +125,7 @@ def format_and_mount_root(args, device, root_label, disk): pmb.chroot.root(args, ["mount", device, mountpoint]) """ - Create separate subvolumes if root filesystem is btrfs + Create separate subvolumes if root filesystem is btrfs. This lets us do snapshots and rollbacks of relevant parts of the filesystem. /var contains logs, VMs, containers, flatpaks; and shouldn't roll back, @@ -138,9 +138,11 @@ def format_and_mount_root(args, device, root_label, disk): if filesystem == "btrfs": pmb.chroot.root(args, ["btrfs", "subvol", "create", - mountpoint + "/@", mountpoint + "/@var", - mountpoint + "/@home", mountpoint + "/@root", - mountpoint + "/@tmp", mountpoint + "/@srv", + mountpoint + "/@", + mountpoint + "/@var", + mountpoint + "/@home", + mountpoint + "/@root", + mountpoint + "/@srv", mountpoint + "/@snapshots"]) # Set the default root subvolume to be separate from top level btrfs @@ -159,35 +161,30 @@ def format_and_mount_root(args, device, root_label, disk): mountpoint + "/home", mountpoint + "/srv", "-m", "700", mountpoint + "/root", - "-m", "766", mountpoint + "/tmp", "-m", "750", mountpoint + "/.snapshots"]) # Disable CoW for /var, to avoid write multiplication # and slowdown on databases, containers and VM images. pmb.chroot.root(args, ["chattr", "+C", mountpoint + "/var"]) + # Mount subvols pmb.chroot.root(args, - ["mount", "-o", "subvolid=257", + ["mount", "-o", "subvol=@var", device, mountpoint+"/var"]) pmb.chroot.root(args, - ["mount", "-o", "subvolid=258", + ["mount", "-o", "subvol=@home", device, mountpoint+"/home"]) pmb.chroot.root(args, - ["mount", "-o", "subvolid=259", + ["mount", "-o", "subvol=@root", device, mountpoint+"/root"]) pmb.chroot.root(args, - ["mount", "-o", "subvolid=260", - device, mountpoint+"/tmp"]) - pmb.chroot.root(args, - ["mount", "-o", "subvolid=261", + ["mount", "-o", "subvol=@srv", device, mountpoint+"/srv"]) pmb.chroot.root(args, - ["mount", "-o", "subvolid=262", + ["mount", "-o", "subvol=@snapshots", device, mountpoint+"/.snapshots"]) - - def format(args, layout, boot_label, root_label, disk): """ :param layout: partition layout from get_partition_layout() -- 2.38.5
From: Jacob Ludvigsen <jacob.ludvigsen@protonmail.com> --- pmb/install/format.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pmb/install/format.py b/pmb/install/format.py index 33c4ebdb..83f785bb 100644 --- a/pmb/install/format.py +++ b/pmb/install/format.py @@ -163,10 +163,6 @@ def format_and_mount_root(args, device, root_label, disk): "-m", "700", mountpoint + "/root", "-m", "750", mountpoint + "/.snapshots"]) - # Disable CoW for /var, to avoid write multiplication - # and slowdown on databases, containers and VM images. - pmb.chroot.root(args, ["chattr", "+C", mountpoint + "/var"]) - # Mount subvols pmb.chroot.root(args, ["mount", "-o", "subvol=@var", @@ -184,6 +180,10 @@ def format_and_mount_root(args, device, root_label, disk): ["mount", "-o", "subvol=@snapshots", device, mountpoint+"/.snapshots"]) + # Disable CoW for /var, to avoid write multiplication + # and slowdown on databases, containers and VM images. + pmb.chroot.root(args, ["chattr", "+C", mountpoint + "/var"]) + def format(args, layout, boot_label, root_label, disk): """ -- 2.38.5
From: Jacob Ludvigsen <jacob.ludvigsen@protonmail.com> --- pmb/install/_install.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pmb/install/_install.py b/pmb/install/_install.py index aba64392..2f9032df 100644 --- a/pmb/install/_install.py +++ b/pmb/install/_install.py @@ -153,7 +153,7 @@ def create_home_from_skel(args): Create /home/{user} from /etc/skel """ rootfs = args.work + "/chroot_native/mnt/install" - # btrfs' home subvol + dir is created in format.py + # In btrfs, home subvol & home dir is created in format.py if args.filesystem == "btrfs": pass else: -- 2.38.5