From 681e993626f685440daf50d6111d76e607fe41fc Mon Sep 17 00:00:00 2001 From: atlas Date: Sun, 21 Jun 2026 13:22:00 +0200 Subject: [PATCH] hive-c0re: address review on btrfs qgroup usage parsing Select the level-0 (`0/`) leaf qgroup row explicitly instead of taking the last data line, so usage parsing is unambiguous even if an operator has assigned the subvolume to a higher-level aggregate qgroup. `btrfs qgroup show -f ` already scopes the listing to qgroups impacting the given path (excluding ancestral qgroups, per btrfs-qgroup-show(8)); selecting the `0/` leaf among them pins it to the subvolume's own automatic usage qgroup. Also: case-insensitive match on the stable "quota not enabled" error fragment (wording varies across btrfs-progs versions), `# Errors` doc sections on the three public priv_client quota functions, and precise doc comments on the `-f` flag semantics. --- hive-c0re/src/bin/hivectl.rs | 6 +++++- hive-c0re/src/priv_client.rs | 33 ++++++++++++++++++++++++++------- hive-priv/src/main.rs | 8 +++++--- 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/hive-c0re/src/bin/hivectl.rs b/hive-c0re/src/bin/hivectl.rs index 15bfd274..bc24d2de 100644 --- a/hive-c0re/src/bin/hivectl.rs +++ b/hive-c0re/src/bin/hivectl.rs @@ -715,7 +715,11 @@ async fn quota_show(name: Option<&str>) -> Result<()> { } Err(e) => { let msg = format!("{e:#}"); - if msg.contains("not enabled") { + // btrfs-progs prints "ERROR: ... quota not enabled" to stderr + // when qgroups are off; match the stable fragment case- + // insensitively rather than an exact line (the surrounding + // wording varies across btrfs-progs versions). + if msg.to_ascii_lowercase().contains("quota not enabled") { bail!("btrfs quota not enabled — run `hivectl quota enable` first"); } // A plain-dir agent (no subvolume) has no qgroup; report it diff --git a/hive-c0re/src/priv_client.rs b/hive-c0re/src/priv_client.rs index 09ae5b76..25582347 100644 --- a/hive-c0re/src/priv_client.rs +++ b/hive-c0re/src/priv_client.rs @@ -329,13 +329,21 @@ pub async fn delete_agent_subvolume(agent_name: &str) -> Result<()> { /// Enable btrfs qgroup accounting on the agent-state filesystem (operator /// opt-in; prerequisite for usage reads + quotas). Idempotent; no-op off /// btrfs. Via hive-priv (root). +/// +/// # Errors +/// Returns an error if the hive-priv call fails or `btrfs quota enable` +/// reports a non-zero exit. pub async fn ensure_btrfs_quota() -> Result<()> { ok(call(&PrivRequest::EnsureBtrfsQuota).await?) } /// Read an agent state subvolume's btrfs qgroup usage, returning -/// `(referenced_bytes, exclusive_bytes)`. Errors propagate (e.g. quota not -/// enabled) so the caller can surface them. Via hive-priv (root). +/// `(referenced_bytes, exclusive_bytes)`. Via hive-priv (root). +/// +/// # Errors +/// Returns an error if the hive-priv call fails, `btrfs qgroup show` exits +/// non-zero (e.g. quota not enabled — the message propagates so the caller +/// can surface it), or the output has no level-0 qgroup row to parse. pub async fn read_subvolume_usage(agent_name: &str) -> Result<(u64, u64)> { let (stdout, _) = check( call(&PrivRequest::ReadSubvolumeUsage { @@ -348,6 +356,10 @@ pub async fn read_subvolume_usage(agent_name: &str) -> Result<(u64, u64)> { /// Set or clear a btrfs qgroup size limit on an agent's state subvolume. /// `limit_bytes = None` clears it. Requires quota enabled. Via hive-priv. +/// +/// # Errors +/// Returns an error if the hive-priv call fails or `btrfs qgroup limit` +/// reports a non-zero exit (e.g. quota not enabled). pub async fn set_subvolume_quota(agent_name: &str, limit_bytes: Option) -> Result<()> { ok(call(&PrivRequest::SetSubvolumeQuota { agent_name: agent_name.to_owned(), @@ -357,19 +369,26 @@ pub async fn set_subvolume_quota(agent_name: &str, limit_bytes: Option) -> } /// Parse `(referenced, exclusive)` bytes from `btrfs qgroup show -f --raw` -/// output. Skips the header rows and reads the last data row (a qgroup row -/// is ` …`). +/// output (a qgroup row is ` …`). +/// +/// `-f ` already restricts the listing to qgroups impacting that path +/// (excluding ancestral qgroups — see btrfs-qgroup-show(8)), so it never +/// mixes in other agents' subvolumes. Among the rows it returns we select +/// the **level-0** qgroup (`0/`) — the subvolume's own automatic +/// usage qgroup — rather than blindly taking the last line. Picking the +/// `0/` leaf is unambiguous even if an operator has assigned the subvolume +/// to a higher-level aggregate qgroup (`1/`, …) that `-F` would surface. fn parse_qgroup_usage(out: &str) -> Result<(u64, u64)> { - for line in out.lines().rev() { + for line in out.lines() { let cols: Vec<&str> = line.split_whitespace().collect(); if cols.len() >= 3 - && cols[0].contains('/') + && cols[0].starts_with("0/") && let (Ok(rfer), Ok(excl)) = (cols[1].parse::(), cols[2].parse::()) { return Ok((rfer, excl)); } } - bail!("no qgroup data row in `btrfs qgroup show` output: {out:?}") + bail!("no level-0 qgroup data row in `btrfs qgroup show` output: {out:?}") } fn check(resp: PrivResponse) -> Result<(String, String)> { diff --git a/hive-priv/src/main.rs b/hive-priv/src/main.rs index ac9500ac..4d5fffd0 100644 --- a/hive-priv/src/main.rs +++ b/hive-priv/src/main.rs @@ -671,10 +671,12 @@ async fn ensure_btrfs_quota() -> Result<(String, String)> { Ok((String::new(), String::new())) } -/// `ReadSubvolumeUsage` — return an agent subvolume's qgroup row +/// `ReadSubvolumeUsage` — return an agent subvolume's qgroup rows /// (`btrfs qgroup show -f --raw <…/agent_name>`) verbatim in stdout for -/// hive-c0re to parse. `-f` filters to the subvolume the path lives in, so -/// the output is the single relevant row. See the wire doc. +/// hive-c0re to parse. `-f` lists the qgroups impacting the given path, +/// excluding ancestral qgroups (per btrfs-qgroup-show(8)) — so it scopes +/// to this subvolume and never mixes in other agents'. hive-c0re then +/// selects the level-0 (`0/`) leaf row. See the wire doc. async fn read_subvolume_usage(agent_name: &str) -> Result<(String, String)> { let agent_root = PathBuf::from(AGENT_STATE_ROOT).join(agent_name); let out = Command::new("btrfs")