grow ssd partition and filesystem at boot (one-shot) #44
Conversation
WalkthroughThe changes add an automated SSD growth service to Void Linux installations. The base installation script now installs cloud-guest-utils and configures a one-shot runit service ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
files/etc/sv/grow-ssd/run (1)
11-11: Consider preserving error output for debugging.Suppressing all output from the dry-run check (
>/dev/null 2>&1) makes troubleshooting difficult ifgrowpartfails for unexpected reasons. Consider redirecting stderr to a log file or allowing it to reach the service logs whilst still exiting gracefully when growth is unnecessary.🔎 Proposed adjustment to preserve error messages
-growpart --dry-run "$DISK" "$PART" >/dev/null 2>&1 || exit 0 +if ! growpart --dry-run "$DISK" "$PART" >/dev/null; then + echo "Partition does not need growing or cannot be grown (this is normal)" + exit 0 +fibase/voidlinux.sh (1)
36-44: Consider whether logging setup is necessary.A
logdirectory is created (line 39) but nolog/runscript is provided. For a one-shot boot service with minimal output, this may be acceptable as output will go to runit's default catchlog. However, if you need persistent logging, add alog/runscript.Additionally, note that the run script is only copied if it doesn't already exist (line 40). This means updates to the script won't be automatically applied on subsequent runs, which may be intentional to preserve local modifications.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
base/voidlinux.shfiles/etc/sv/grow-ssd/finishfiles/etc/sv/grow-ssd/run
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
📚 Learning: 2025-11-27T08:38:05.963Z
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
Applied to files:
files/etc/sv/grow-ssd/runfiles/etc/sv/grow-ssd/finish
🔇 Additional comments (4)
files/etc/sv/grow-ssd/finish (1)
1-2: LGTM!The finish script correctly returns success status for the one-shot service, preventing unnecessary restarts by runit.
base/voidlinux.sh (1)
28-34: LGTM!The addition of
cloud-guest-utilsprovides the necessarygrowparttool for the SSD growth functionality. The multi-line package installation format improves readability.files/etc/sv/grow-ssd/run (2)
6-8: No action required—this hardcoded device path is appropriate for this deployment.This script is deployment-specific and targets a known hardware configuration. The codebase already depends on environment-specific paths (e.g.,
/ssdmust be mounted for execution). The README explicitly notes that supporting multiple platforms is a future TODO, so the hardcoded/dev/sdais intentional for the current "native" Void Linux deployment target.Likely an incorrect or invalid review comment.
16-17: Add filesystem type detection before resizing.The
resize2fscommand only supports ext2/ext3/ext4 filesystems. Add filesystem type detection before attempting the resize operation, or use conditional logic to apply the appropriate resize tool (e.g.xfs_growfsfor XFS,btrfs filesystem resizefor Btrfs).
There was a problem hiding this comment.
Pull request overview
This PR adds automatic SSD partition and filesystem growth at system boot using a new one-shot runit service. The implementation aims to expand storage capacity when the system detects additional space available on the disk.
Key Changes
- Adds a new runit service
grow-ssdthat runsgrowpartandresize2fsto expand partition and filesystem - Installs the
cloud-guest-utilspackage which provides thegrowpartutility - Configures the service to run at boot via runit's service management
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| files/etc/sv/grow-ssd/run | Main service script that performs partition and filesystem growth operations |
| files/etc/sv/grow-ssd/finish | Finish script for the runit service (exits with status 0) |
| base/voidlinux.sh | Installation logic to set up the grow-ssd service and install cloud-guest-utils package |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
base/voidlinux.shfiles/etc/sv/grow-ssd/finishfiles/etc/sv/grow-ssd/run
🚧 Files skipped from review as they are similar to previous changes (2)
- files/etc/sv/grow-ssd/run
- files/etc/sv/grow-ssd/finish
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
📚 Learning: 2025-11-27T08:38:05.963Z
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
Applied to files:
base/voidlinux.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (1)
base/voidlinux.sh (1)
46-46: LGTM!The symlink correctly enables the grow-ssd service in the default runsvdir, following standard runit service activation patterns.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
base/voidlinux.sh (2)
28-34: Critical: cloud-guest-utils is not available in Void Linux official repositories.This issue was previously flagged. The
cloud-guest-utilspackage (which providesgrowpart) is not available in Void Linux official binary repositories and must be built locally using void-packages/xbps-src or installed from upstream sources. Thexbps-installcommand will fail, blocking the entire installation.You must either:
- Build cloud-guest-utils locally via xbps-src and document the requirement, or
- Remove the dependency and implement partition growth using alternative tools available in Void repos (e.g.,
parted,sgdisk), or- Make this step conditional with clear documentation for manual installation
44-47: Critical: Wrong source file copied to finish script location.This issue was previously flagged. Line 45 copies the
runscript to thefinishlocation instead of copying the actualfinishscript. The finish script will have incorrect content, breaking the one-shot service behaviour.🔎 Proposed fix
if [ ! -f /$GROW_SSD_SVDIR/finish ]; then - cp "$SYSUPDATES_ROOTDIR/files/$GROW_SSD_SVDIR/run" /$GROW_SSD_SVDIR/finish + cp "$SYSUPDATES_ROOTDIR/files/$GROW_SSD_SVDIR/finish" /$GROW_SSD_SVDIR/finish chmod +x /$GROW_SSD_SVDIR/finish fi
🧹 Nitpick comments (1)
files/etc/sv/grow-ssd/run (1)
36-37: Filesystem resize assumes ext4.The use of
resize2fsassumes an ext2/ext3/ext4 filesystem. Based on previous discussions, ext4 is the expected filesystem type. Note thatresize2fssupports online resizing for ext4, so operating on a mounted filesystem is safe.If other filesystem types might be used in the future, consider adding filesystem type detection and using appropriate tools (e.g.,
xfs_growfsfor XFS).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
base/voidlinux.shfiles/etc/sv/grow-ssd/finishfiles/etc/sv/grow-ssd/run
🚧 Files skipped from review as they are similar to previous changes (1)
- files/etc/sv/grow-ssd/finish
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
📚 Learning: 2025-11-27T08:38:05.963Z
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
Applied to files:
base/voidlinux.shfiles/etc/sv/grow-ssd/run
🔇 Additional comments (1)
files/etc/sv/grow-ssd/run (1)
15-28: Robust device and partition detection logic.The implementation correctly handles various disk naming conventions (sd*, vd*, nvme*) using
lsblkwith asedfallback, and includes proper validation of the partition number. This addresses previous concerns about hardcoded device paths.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
files/etc/sv/grow-ssd/run (1)
30-37: Partition growth achieves idempotency but error handling leaves a gap.The dry-run check (line 31) provides idempotency—after successful growth, subsequent runs exit early. However, with
set -e(line 4), ifresize2fsfails aftergrowpartsucceeds, the partition will be grown but the filesystem will not be resized, leaving an inconsistent state with no opportunity for rollback.Consider explicit error checking:
🔎 Suggested improvement
# Only run if partition can be grown growpart --dry-run "$DISK" "$PART" >/dev/null 2>&1 || exit 0 echo "Growing partition $SSD_DEV" -growpart "$DISK" "$PART" +if ! growpart "$DISK" "$PART"; then + echo "grow-ssd: growpart failed for $DISK partition $PART" + exit 1 +fi echo "Resizing filesystem on $SSD_DEV" -resize2fs "$SSD_DEV" +if ! resize2fs "$SSD_DEV"; then + echo "grow-ssd: resize2fs failed for $SSD_DEV - partition grown but filesystem not resized" + echo "grow-ssd: manual intervention required - run: resize2fs $SSD_DEV" + exit 1 +fiThis makes failures explicit and provides clear recovery instructions.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
base/voidlinux.shfiles/etc/sv/grow-ssd/finishfiles/etc/sv/grow-ssd/run
🚧 Files skipped from review as they are similar to previous changes (1)
- files/etc/sv/grow-ssd/finish
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
📚 Learning: 2025-11-27T08:38:05.963Z
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
Applied to files:
base/voidlinux.shfiles/etc/sv/grow-ssd/run
🔇 Additional comments (4)
base/voidlinux.sh (2)
28-34: LGTM!The package installation is correctly formatted and includes all necessary dependencies. The user has confirmed that
cloud-guest-utilsis available in the Void Linux repositories.
36-48: Previous issues resolved; service setup looks correct.The copy-paste error on line 45 and incomplete symlink on line 48 from previous reviews have been fixed. The service setup correctly:
- Creates necessary directories
- Copies both run and finish scripts
- Sets executable permissions
- Creates the symlink in
/var/service/grow-ssdNote: Line 39 creates a
logdirectory but no log service script is provided. This is acceptable as runit services can operate without a dedicated log service.files/etc/sv/grow-ssd/run (2)
6-13: LGTM!The device detection logic using
findmntis appropriate and robust. The validation ensures the detected source is a device path, and the early exit with status 0 is correct for cases where/ssdis not mounted.
15-28: LGTM!The disk and partition detection logic is robust:
- Uses
lsblk -no pknameto reliably determine the parent disk for all device types (SCSI, NVMe, virtio)- Falls back to regex-based parsing when pkname is unavailable
- Validates partition number is numeric before proceeding
- Handles diverse device naming conventions (/dev/sdX, /dev/nvmeXnYpZ, /dev/vdX)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
files/etc/sv/grow-ssd/run (1)
56-57: Document the ext4 filesystem assumption.The script uses
resize2fs, which only supports ext2/ext3/ext4 filesystems. Based on past feedback, this assumption is intentional for the current use case.Consider adding a comment in the script to document this limitation for future maintainers.
Based on past feedback acknowledging the ext4 assumption.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
base/voidlinux.shfiles/etc/sv/grow-ssd/run
🚧 Files skipped from review as they are similar to previous changes (1)
- base/voidlinux.sh
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
📚 Learning: 2025-11-27T08:38:05.963Z
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
Applied to files:
files/etc/sv/grow-ssd/run
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (2)
files/etc/sv/grow-ssd/run (2)
1-7: One-shot semantics correctly implemented.The trap at line 7 ensures the service is marked down on exit, preventing continuous restarts. This addresses the one-shot requirement effectively.
18-40: Robust device name handling.The fallback logic correctly handles various device naming conventions including NVMe devices with single-digit controllers and namespaces, and regular block devices. The use of
lsblkwithPKNAMEas the primary method is appropriate.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @files/etc/sv/grow-ssd/run:
- Around line 18-44: The case patterns for NVMe devices only match single
digits; update the matching arms for "/dev/nvme[0-9]n[0-9])" and
"/dev/nvme[0-9]n[0-9]p[0-9]*)" to allow multi-digit controller/namespace numbers
(e.g., use patterns like "/dev/nvme[0-9]*n[0-9]*" and
"/dev/nvme[0-9]*n[0-9]*p[0-9]*") so the DISK and PART extraction logic (the
DISK=... and PART=... assignments that follow those arms) still runs for names
like /dev/nvme10n1 or /dev/nvme0n10; keep the existing sed extraction lines
intact since they already handle multi-digit values.
🧹 Nitpick comments (1)
files/etc/sv/grow-ssd/run (1)
9-16: Consider adding an explicit mount dependency or dependency check to ensure/ssdis mounted before this service starts.The script exits gracefully if
/ssdis not yet mounted, which prevents errors. However, runit has no native dependency system to guarantee/ssdis mounted first. If a boot-time race condition occurs where this service starts before the/ssdmount completes, the partition will never be grown (the service marks itself down permanently via the trap).Whilst
/ssdis critical infrastructure for the system (required for hourly cron jobs), adding an explicit check—such as waiting for the mount point to exist, or documenting an assumption that/ssdis mounted by the initramfs or kernel before runit services start—would improve robustness.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
base/voidlinux.shfiles/etc/sv/grow-ssd/run
🚧 Files skipped from review as they are similar to previous changes (1)
- base/voidlinux.sh
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
📚 Learning: 2025-11-27T08:38:05.963Z
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
Applied to files:
files/etc/sv/grow-ssd/run
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (4)
files/etc/sv/grow-ssd/run (4)
1-7: LGTM!The trap mechanism correctly ensures one-shot behaviour by marking the service down on any exit, preventing unwanted restarts whilst gracefully handling failures of the
svcommand itself.
46-52: LGTM!The partition validation correctly ensures that only devices with parseable numeric partition numbers proceed to growth operations. Exiting gracefully for bare or unparseable devices is appropriate.
54-61: Growth logic is sound.The dry-run check (line 55) efficiently prevents unnecessary operations when the partition is already at maximum size. The use of
growpartfollowed byresize2fsis appropriate for online ext4 filesystem growth.Note: The potential inconsistency if
growpartsucceeds butresize2fsfails has been flagged in previous reviews and is a known consideration.
63-63: LGTM!The completion message correctly matches the service name.
|
@kristapsk I've opened a new pull request, #45, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@kristapsk I've opened a new pull request, #46, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @files/etc/sv/grow-ssd/run:
- Around line 14-35: The wait loop can detect SSD_DEV but then unconditionally
prints the failure message and exits; modify the logic so successful detection
skips the failure path: set a marker (e.g., FOUND=1) when you detect /dev/*
inside the inner case (before the break), then after the while check that marker
(or test SSD_DEV) and only print "unable to detect" and exit if not found;
otherwise continue to the growth logic. Reference: SSD_DEV, WAITED, MAX_WAIT and
the while ... do / case ... in block where you currently call break.
- Around line 37-63: PKNAME path sets DISK but never initializes PART, causing
an unbound variable when set -u is used; fix by assigning PART in that branch
too: after setting DISK="/dev/$PKNAME" derive PART from SSD_DEV by extracting
the trailing numeric partition suffix (the same way the other branches do) and
set PART to the empty string if no trailing digits are present so subsequent
references to PART are always defined.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
base/voidlinux.shfiles/etc/sv/grow-ssd/logfiles/etc/sv/grow-ssd/run
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
📚 Learning: 2025-11-27T08:38:05.963Z
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
Applied to files:
base/voidlinux.shfiles/etc/sv/grow-ssd/run
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (7)
files/etc/sv/grow-ssd/log (1)
1-2: LGTM!Standard runit log script that correctly delegates logging to vlogger with appropriate tagging.
base/voidlinux.sh (2)
28-34: LGTM!The package list correctly includes
cloud-guest-utilswhich provides thegrowpartutility required by the grow-ssd service.
36-48: LGTM!The service setup correctly:
- Creates the service directory structure with log subdirectory
- Copies the run and log scripts conditionally
- Creates the symlink to enable the service
The one-shot behaviour is handled via the trap in the run script (
sv down), so no separate finish script is required.files/etc/sv/grow-ssd/run (4)
1-7: LGTM!The strict mode (
set -eu) and trap pattern correctly implement one-shot semantics by marking the service down on any exit.
65-71: LGTM!The validation correctly handles both empty and non-numeric partition values, providing a graceful exit path for unsupported device configurations.
73-80: LGTM!The dry-run check ensures idempotency by gracefully skipping when no growth is possible. The
resize2fsoperation on mounted ext4 filesystems is supported by the kernel.
82-82: LGTM!Completion message correctly matches the service name.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@kristapsk I've opened a new pull request, #47, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @files/etc/sv/grow-ssd/run:
- Around line 98-102: The script currently enables the service while growpart
and resize2fs happen separately, which can leave the partition grown but the
filesystem unresized if resize2fs fails; change the control flow so the service
is not enabled until resize2fs completes successfully: run growpart and then
immediately run resize2fs, check resize2fs exit status, and only enable or leave
enabled the service after resize2fs returns success; if resize2fs fails, exit
non‑zero (and/or sv down the service if already started) and emit a clear error
so the system does not stay enabled with an inconsistent partition state.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
files/etc/sv/grow-ssd/run
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
📚 Learning: 2025-11-27T08:38:05.963Z
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
Applied to files:
files/etc/sv/grow-ssd/run
🔇 Additional comments (3)
files/etc/sv/grow-ssd/run (3)
6-41: Wait logic for /ssd mount is well-implemented.The 60-second wait with proper detection and fallback ensures the service handles both immediate and delayed mounts gracefully. The post-loop verification (lines 29-39) correctly distinguishes between successful detection and timeout, addressing the earlier critical logic bug.
43-81: Device and partition detection logic is comprehensive.The dual-path approach (primary
lsblkwith fallback pattern matching) handles diverse device naming schemes. The partition number extraction in the PKNAME branch (lines 46-57) correctly addresses the earlier initialisation issue.
83-96: Partition validation and dry-run check implement proper one-shot semantics.The numeric partition validation (lines 84-90) prevents operation on bare devices, and the
growpart --dry-runcheck (line 93) ensures idempotency by downing the service when growth is unnecessary. This correctly implements the one-shot pattern for a boot-time service.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@kristapsk I've opened a new pull request, #48, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @files/etc/sv/grow-ssd/log/run:
- Around line 1-2: The base installation script is missing the socklog package
required for vlogger used by the grow-ssd log script (and lnd/bitcoind
services); update the xbps-install invocation in base/voidlinux.sh to include
"socklog" in the package list so vlogger is installed by default, ensuring the
run script (#!/bin/sh exec vlogger -t grow-ssd -p daemon) can run without
runtime failures.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
base/voidlinux.shfiles/etc/sv/grow-ssd/log/runfiles/etc/sv/grow-ssd/run
🚧 Files skipped from review as they are similar to previous changes (2)
- files/etc/sv/grow-ssd/run
- base/voidlinux.sh
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
📚 Learning: 2025-11-27T08:38:05.963Z
Learnt from: kristapsk
Repo: nakamochi/sysupdates PR: 43
File: update.sh:18-18
Timestamp: 2025-11-27T08:38:05.963Z
Learning: In the nakamochi/sysupdates repository, the update.sh script is deployed to and executed from /ssd/sysupdates/ (via cron at /etc/cron.hourly/sysupdate). If /ssd is not mounted, the script cannot execute at all, making additional validation of /ssd availability unnecessary.
Applied to files:
files/etc/sv/grow-ssd/log/run
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@kristapsk I've opened a new pull request, #49, to work on those changes. Once the pull request is ready, I'll request review from you. |
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.