fix(deploy): align shrunk guest images to pmem boundary

[fslongjin]

Summary

• Round shrunk one-click guest image sizes up to a 2MiB boundary before the final truncate/resize2fs.
• Add a defensive alignment check so future changes fail clearly if the generated cube-guest-image.img is not pmem aligned.
• This is needed because the hypervisor maps cube-guest-image.img as a pmem device and requires the image size to be 2MiB aligned; otherwise template launch can fail with PmemSizeNotAligned.

Test plan

• bash -n deploy/one-click/build-vm-assets.sh
• Cursor lints for deploy/one-click/build-vm-assets.sh

Assisted-by: Cursor:GPT-5.5

Made with Cursor

[cubesandboxbot]

Claude finished @fslongjin's task in 46s —— View job

---

Code Review: fix(deploy): align shrunk guest images to pmem boundary

Overview

This PR modifies deploy/one-click/build-vm-assets.sh to round the shrunk guest image size up to a 2 MiB boundary, ensuring compatibility with the hypervisor's pmem device alignment requirement (PmemSizeNotAligned error). It also adds defensive guards so misalignment fails early with a clear diagnostic.

---

What the changes do

Change	Location	Purpose
PMEM_ALIGN_BYTES constant	L293 (after patch)	2 MiB alignment constant derived from the hypervisor's hugepage granularity
Defensive % block_size == 0 check	L314–316	Verifies alignment is a multiple of the ext4 block size (always true today, catches exotic configs)
target_blocks ceiling rounding	L322	Rounds up so final_bytes = target_blocks * block_size is 2 MiB aligned
Post-compute alignment assertion	L336–338	Sanity-check that final_bytes is truly 2 MiB aligned before truncating
Log message update	L352	Makes the build log self-documenting about pmem alignment

---

Correctness analysis

Rounding logic: The ceiling-division expression (( (target_blocks + pmem_align_blocks - 1) / pmem_align_blocks) * pmem_align_blocks) is the standard formula and produces correct results for all positive integers. ✅

Block size invariance: PMEM_ALIGN_BYTES (2 MiB = 2,097,152) is divisible by 1024, 2048, and 4096 — all legal ext4 block sizes. The defensive check catches any future exotic block size. ✅

Safety guarantee: Rounding up only enlarges target_blocks, which means final_bytes >= min_blocks + reserved_blocks. The existing final_bytes < min_bytes assertion remains vacuously true. ✅

Initial image alignment: The pre-shrink image created by calculate_guest_image_size_bytes aligns to 256 MiB, which is itself a multiple of 2 MiB. So even if shrink_ext4_image were skipped or failed, the initial image would still satisfy the pmem constraint. ✅

---

Observations

• No integer overflow risk: Bash arithmetic is 64-bit signed; target_blocks * block_size would overflow only at petabyte-scale images.
• Consistent coding style: Uses the same (( ... )) arithmetic syntax and run_as_root patterns as the surrounding code. No quoting issues in arithmetic context.
• Self-documenting comments: The inline comments explaining why 2 MiB, referencing the hypervisor source path, and noting that rounding only grows headroom are all helpful for future maintainers.
• LGTM — these are the right changes for the stated problem, implemented correctly.

---

Nitpick (non-blocking)

• PMEM_ALIGN_BYTES is defined as a global constant (L293) but without readonly. Adding readonly PMEM_ALIGN_BYTES=... would defend against accidental reassignment. Minor and optional.

---

Verdict

Approved. Clean, minimal, well-justified change with no bugs or regressions.

[cubesandboxbot]

Claude finished @fslongjin's task in 1m 39s —— View job

---

Code Review: fix(deploy): align shrunk guest images to pmem boundary

Changes overview

The PR modifies deploy/one-click/build-vm-assets.sh to round the shrunk guest image size up to the nearest 2 MiB boundary, ensuring compatibility with the hypervisor's pmem device alignment requirement. It adds both a proactive alignment check and a post-computation assertion.

What the code does

Change	Lines	Purpose
PMEM_ALIGN_BYTES=$((2 * 1024 * 1024))	290	2 MiB alignment constant matching the hypervisor's pmem constraint
Defensive % block_size == 0 check	315–317	Ensures alignment is compatible with the ext4 block size (always true today, but fails clearly if an exotic config appears)
Ceiling rounding of target_blocks	326	Round up so final_bytes is 2 MiB-aligned
Post-compute alignment assertion	336–338	Sanity-check before truncating the image
Updated log message	349–352	Documents pmem alignment in build output

Correctness

Ceiling-division rounding: ((target_blocks + pmem_align_blocks - 1) / pmem_align_blocks) * pmem_align_blocks is the standard ceiling-division formula and is correct for all positive integers. No overflow concern — bash arithmetic is 64-bit signed, and overflow would require petabyte-scale images.

Alignment invariant: 2 MiB (2,097,152) is divisible by 1,024, 2,048, and 4,096 — all legal ext4 block sizes. The defensive % check catches exotic future block sizes gracefully.

Safety monotonicity: Rounding up only increases target_blocks, so final_bytes >= min_bytes remains trivially satisfied.

set -e safety: The if (( ... )) guards (lines 315, 336) are correctly immune to set -e since they're condition expressions. The $(( ... )) expansion on line 326 is safe because block_size is validated to be non-empty before this point.

Edge case: initial image alignment

The pre-shrink image created by calculate_guest_image_size_bytes aligns to 256 MiB, which is itself a multiple of 2 MiB. If shrink_ext4_image were ever skipped, the initial image would still satisfy the pmem constraint.

Consistency

The ceiling-division pattern on line 320 ((reserved_bytes + block_size - 1) / block_size) and line 326 ((target_blocks + pmem_align_blocks - 1) / pmem_align_blocks) are consistent with each other.

Minor observations (non-blocking)

• PMEM_ALIGN_BYTES without readonly: As the previous review noted, adding readonly PMEM_ALIGN_BYTES would prevent accidental reassignment. Consistent with the rest of the file (which doesn't use readonly for other globals either), so this is purely stylistic.

• No environment variable override: Unlike SHRINK_RESERVED_BYTES (which can be overridden via ONE_CLICK_GUEST_IMAGE_RESERVED_BYTES), there's no ONE_CLICK_PMEM_ALIGN_BYTES override. This is likely intentional since 2 MiB is a hard hypervisor constraint, but worth noting for testing/debugging scenarios.

• Division by zero guard: If block_size were somehow 0, the $(( PMEM_ALIGN_BYTES / block_size )) on line 318 would cause a division-by-zero error and abort the script. This is fine since block_size is validated to be non-empty and ext4 block sizes are always >= 1024, but a zero-check guard could make this more explicit:

  if (( block_size == 0 )); then
    die "ext4 block size is zero"
  fi

Verdict

LGTM. The changes are minimal, well-documented, and correctly address the PmemSizeNotAligned error. The defensive assertions make future regressions fail with clear diagnostics.

---