ROCm / ROCm/rocm-cli

[Issue]: No free-space check before multi-GB downloads and extractions

Open
#159 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
40
Forks
9
Avg merge
4d 20h
Merged PRs (30d)
59

Description

Problem Description

ROCm CLI starts multi-GB downloads and extractions without ever checking whether there is room for them. There is no free-space check anywhere in the codebase, and no handling of the resulting out-of-space error, so a user with a nearly-full disk gets a low-level write failure partway through an install rather than an upfront "you need N GB free".

Verified by searching apps/, crates/, engines/, and xtask/ for available_space, free_space, statvfs, ENOSPC, StorageFull, disk_space, fs2, and nix::sys::statvfs, plus prose variants (no space, out of space, insufficient disk, free disk) — zero hits. Nothing maps ErrorKind::StorageFull to a user-facing message.

sysinfo is already a dependency of crates/rocm-dash-collectors (used for CPU/memory metrics). sysinfo::Disks appears only in a TODO comment at crates/rocm-dash-collectors/src/host.rs:57, so the capability is available but unused.

This is made worse by the fact that a failed write currently leaks its partial file (see the companion issue on orphaned partial downloads), so hitting a full disk both fails the install and permanently consumes the space that was left.

Steps to Reproduce
  1. On a machine with less free space than the SDK requires, run an SDK install.
  2. The install begins downloading and fails partway through with a write error, rather than refusing upfront with a space requirement.
Suggested Fix

Check available space before starting a download or extraction and fail early with the required and available amounts. The natural checkpoints are the SDK install paths in apps/rocm/src/therock.rs (the tarball download at line 1062 and the extraction that follows it) and the shared helper download_file_to_path in crates/rocm-core/src/lib.rs:115.

Two details worth deciding:

  • Extraction needs headroom well beyond the compressed artifact, so the check should account for the extracted size, not just the download.
  • Content-Length gives the download size cheaply where the server provides it; the extracted size needs either a manifest value or a conservative multiplier.

It would also help to surface a clear message when a write does fail for lack of space, instead of the raw OS error.

Additional Information

Found while investigating unbounded disk growth from repeated installs.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the SDK install paths in apps/rocm/src/therock.rs, especially the tarball download at line 1062 and the following extraction, then inspect download_file_to_path in crates/rocm-core/src/lib.rs:115. Review the existing sysinfo usage and TODO in crates/rocm-dash-collectors/src/host.rs:57. Done means installs check required space before download and extraction and present a clear insufficient-space error, including handling for a write failure caused by no space.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
cli
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
50/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.