Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

nixos/hot-resize: init #357523

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

nixos/hot-resize: init #357523

wants to merge 1 commit into from

Conversation

liberodark
Copy link
Contributor

@liberodark liberodark commented Nov 20, 2024 •
edited
Loading

Add ability to resize your fs ext4 or xfs or btrfs in vm or vma without reboot.
Have created dedicated module for this because is more agnostic to use it in various situations.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Nov 20, 2024
@liberodark
Copy link
Contributor Author

liberodark commented Nov 20, 2024 •
edited
Loading

How to use :

services.hotResize = {
  enable = true;
  device = "/dev/vda1";
  fsType = "ext4";  # or "xfs" or "btrfs"
  mountPoint = "/";
};

After you resize your disk just run nixos-rebuild switch to apply without reboot.

@liberodark liberodark force-pushed the resize branch 2 times, most recently from 30113a0 to 70cc946 Compare November 20, 2024 14:07
@h7x4 h7x4 added the 8.has: module (new) This PR adds a module in `nixos/` label Nov 20, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Nov 21, 2024
@makuru-org makuru-org removed the 8.has: module (update) This PR changes an existing module in `nixos/` label Dec 4, 2024
@liberodark
Copy link
Contributor Author

Also you can backport this PR if is possible.

@makuru-org
Copy link
Contributor

Also you can backport this PR if is possible.

You mean me?
I cant and as a new feature it probably makes more sense to push it only to master.

@liberodark
Copy link
Contributor Author

Has you want im just asking.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 5, 2024
@github-actions github-actions bot added the 8.has: module (update) This PR changes an existing module in `nixos/` label Dec 10, 2024
@liberodark
Copy link
Contributor Author

liberodark commented Dec 10, 2024 •
edited
Loading

Hi , @JohnRTitor can you review this PR ?
For me all is good but if you see anyone is missing ?

One question from @GaetanLepage is that :

What is the best :
writeShellScript + {lib.getExe foo}
writeShellApplication + runtimeInputs

Best Regards

SigmaSquadron
SigmaSquadron suggested changes Dec 10, 2024
View reviewed changes
Copy link
Contributor

@SigmaSquadron SigmaSquadron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I question the need for such a module. Disk resizing - even if it's just expanding disks - is a dangerous operation; one that I would certainly not trust an automatic bash script to do.

What issue does this solve? Is this issue worth maintaining more templated bash that is probably not very extensible?

nixos/modules/services/system/hot-resize.nix Outdated Show resolved Hide resolved
nixos/modules/services/system/hot-resize.nix Outdated Show resolved Hide resolved
nixos/modules/services/system/hot-resize.nix Outdated Show resolved Hide resolved
nixos/modules/services/system/hot-resize.nix Outdated Show resolved Hide resolved
nixos/modules/services/system/hot-resize.nix Outdated Show resolved Hide resolved
@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 10, 2024
@liberodark
Copy link
Contributor Author

I question the need for such a module. Disk resizing - even if it's just expanding disks - is a dangerous operation; one that I would certainly not trust an automatic bash script to do.

What issue does this solve? Is this issue worth maintaining more templated bash that is probably not very extensible?

Hi, @SigmaSquadron

The module specifically solves the use case of VMs (especially on platforms like Proxmox) where:

Adding disk space is a common and safe operation
The VM continues to run but cannot use the added space without intervention
The resizing process is standardized and well tested

Best Regards

@liberodark
Copy link
Contributor Author

New Usage :

services.hotResize = {
  enable = true;
  devices = [
    {
      device = "/dev/vda1";
      fsType = "ext4";
      mountPoint = "/";
    }
    {
      device = "/dev/vdb1";
      fsType = "xfs";
      mountPoint = "/data";
    }
  ];
};

@liberodark liberodark force-pushed the resize branch 2 times, most recently from 6c2dd55 to 30c909c Compare December 12, 2024 08:59
@SigmaSquadron
Copy link
Contributor

@ofborg eval

SigmaSquadron
SigmaSquadron approved these changes Dec 14, 2024
View reviewed changes
Copy link
Contributor

@SigmaSquadron SigmaSquadron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. The CI failure is unrelated.

nixos/modules/services/system/hot-resize.nix Outdated
Comment on lines 23 to 78
text = ''
process_device() {
local device="$1"
local fstype="$2"
local mountpoint="$3"

echo "Processing $device ($fstype) mounted at $mountpoint..."

REAL_DEVICE=$(readlink -f "$device")
DISK=$(lsblk -ndo pkname "$REAL_DEVICE")
PART_NUM=$(lsblk -ndo part "$REAL_DEVICE")

if [ -z "$DISK" ] || [ -z "$PART_NUM" ]; then
echo "Failed to determine disk and partition number for $device"
return 1
fi

echo "Growing partition /dev/$DISK partition $PART_NUM..."
GROWPART_OUTPUT=$(growpart "/dev/$DISK" "$PART_NUM" 2>&1) || {
EXIT_CODE=$?
if [ $EXIT_CODE -eq 2 ]; then
echo "Partition is already at maximum size, continuing with filesystem resize..."
else
echo "Failed to grow partition: $GROWPART_OUTPUT"
return $EXIT_CODE
fi
}

echo "Resizing filesystem..."
case "$fstype" in
ext4)
resize2fs "$device"
;;
xfs)
xfs_growfs "$mountpoint"
;;
btrfs)
btrfs filesystem resize max "$mountpoint"
;;
*)
echo "Unsupported filesystem type: $fstype"
return 1
;;
esac

echo "Current size:"
df -h "$mountpoint"
echo "---"
}

process_device "$@"
'';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is small for now, but in the future, as this module gets bigger, you may want to consider splitting the bash part into a separate file.

Copy link
Contributor Author

@liberodark liberodark Dec 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

Yes i can if you want ?
If you think is really needed.

Best Regards

SigmaSquadron
SigmaSquadron suggested changes Dec 14, 2024
View reviewed changes
Copy link
Contributor

@SigmaSquadron SigmaSquadron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, wait, how does the script get the devices? You haven't added them as inputs anywhere.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 15, 2024
Aleksanaa
Aleksanaa reviewed Dec 15, 2024
View reviewed changes
nixos/modules/services/system/hot-resize.nix
description = "Filesystem type (supported: ext4, xfs, btrfs)";
};

mountPoint = lib.mkOption {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a good idea to imitate hardware-configuration.nix, making mount point the name of attributes instead of list

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this module may be more appropriate in tools like disko, rather than a standalone NixOS module

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal was to not use disko that why have create this module.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what your intention is, having a dedicated program is much better than throwing random solutions everywhere, especially when you are trying to do a one shot job with a NixOS module thats gonna be persistent...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disko is the ideal place for something like this, especially since Disko is also written in bash.

@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 16, 2024
@liberodark
Copy link
Contributor Author

Actually, wait, how does the script get the devices? You haven't added them as inputs anywhere.

That have been fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SigmaSquadron SigmaSquadron SigmaSquadron requested changes

@Aleksanaa Aleksanaa Aleksanaa left review comments

@nim65s nim65s nim65s left review comments

@makuru-org makuru-org makuru-org approved these changes

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (new) This PR adds a module in `nixos/` 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@liberodark @makuru-org @SigmaSquadron @nim65s @Aleksanaa @wegank @h7x4