Skip to content

Commit 517eec9

Browse files
committed
fix(volume): share duplicate roots across mount targets
1 parent 5ffb0f3 commit 517eec9

4 files changed

Lines changed: 83 additions & 22 deletions

File tree

docs/verity-volumes.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ A required volume that is missing, malformed, or fails verification stops guest
7373
preparation. dm-verity continues verifying blocks lazily as the application
7474
reads them.
7575

76+
The same `verity_root` may be declared more than once with different targets.
77+
The VMM attaches one disk for that root, and the guest mounts the verified
78+
content at each requested target.
79+
7680
For diagnostics, `dstack-volume scan` lists recognized disks and
7781
`dstack-volume status app-compose.json` compares requested roots with attached
7882
and active devices.

dstack/crates/dstack-volume/src/bin/dstack-volume.rs

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
//! table. Everything read from a disk is untrusted: kind handlers use the
1010
//! measured app compose as their source of policy and cryptographic identity.
1111
12+
use std::collections::HashMap;
1213
use std::io::Read;
1314
use std::os::unix::ffi::OsStrExt;
1415
use std::os::unix::fs::MetadataExt;
@@ -96,13 +97,24 @@ fn mount_all(compose_path: PathBuf) -> Result<()> {
9697
requested = compose.verity_volumes.len(),
9798
"discovered dstack volumes"
9899
);
100+
let mut active_roots: HashMap<[u8; 32], PathBuf> = HashMap::new();
99101
for (index, requested) in compose.verity_volumes.iter().enumerate() {
100-
activate_requested(index, requested, &volumes).with_context(|| {
102+
if let Some(mapped) = active_roots.get(&requested.verity_root) {
103+
mount_volume(requested, mapped).with_context(|| {
104+
format!(
105+
"failed to mount required volume {index} at {}",
106+
requested.target.display()
107+
)
108+
})?;
109+
continue;
110+
}
111+
let mapped = activate_requested(index, requested, &volumes).with_context(|| {
101112
format!(
102113
"failed to activate required volume {index} at {}",
103114
requested.target.display()
104115
)
105116
})?;
117+
active_roots.insert(requested.verity_root, mapped);
106118
}
107119
Ok(())
108120
}
@@ -126,10 +138,17 @@ fn status(compose_path: PathBuf) -> Result<()> {
126138
let attached = volumes
127139
.iter()
128140
.any(|volume| volume.root_hash == requested.verity_root);
129-
let mapper_name = format!("dstack-verity{index}");
130-
let active = Path::new("/dev/mapper").join(&mapper_name).exists()
131-
&& mapping_root(&mapper_name)?
132-
.eq_ignore_ascii_case(&hex::encode(requested.verity_root));
141+
let expected_root = hex::encode(requested.verity_root);
142+
let mut active = false;
143+
for mapper_index in 0..compose.verity_volumes.len() {
144+
let mapper_name = format!("dstack-verity{mapper_index}");
145+
if Path::new("/dev/mapper").join(&mapper_name).exists()
146+
&& mapping_root(&mapper_name)?.eq_ignore_ascii_case(&expected_root)
147+
{
148+
active = true;
149+
break;
150+
}
151+
}
133152
println!(
134153
"{index}\troot={}\ttarget={}\tattached={attached}\tactive={active}",
135154
hex::encode(requested.verity_root),
@@ -275,7 +294,7 @@ fn activate_requested(
275294
index: usize,
276295
requested: &RequestedVolume,
277296
volumes: &[VerityVolume],
278-
) -> Result<()> {
297+
) -> Result<PathBuf> {
279298
let candidate = volumes
280299
.iter()
281300
.find(|volume| volume.root_hash == requested.verity_root)
@@ -293,7 +312,7 @@ fn activate_requested(
293312
return Err(err);
294313
}
295314
info!(mapper = mapper_name, "reused active verity mapping");
296-
return Ok(());
315+
return Ok(mapped);
297316
}
298317
run_cmd!(veritysetup close $mapper_name).context("closing stale verity mapping")?;
299318
}
@@ -308,7 +327,7 @@ fn activate_requested(
308327
let _ = run_cmd!(veritysetup close $mapper_name);
309328
return Err(err);
310329
}
311-
Ok(())
330+
Ok(mapped)
312331
}
313332

314333
fn verify_first_block(path: &Path) -> Result<()> {

dstack/dstack-types/src/lib.rs

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -141,19 +141,12 @@ pub struct VerityVolume {
141141
pub target: std::path::PathBuf,
142142
}
143143

144-
/// Reject ambiguous volume declarations before any disk is attached or
145-
/// activated. A root identifies one logical volume, and a mount target can
146-
/// only be owned by one volume.
144+
/// Reject ambiguous mount declarations before any disk is attached or
145+
/// activated. The same root may intentionally be mounted at multiple targets,
146+
/// but a target can only be owned by one volume.
147147
pub fn validate_verity_volumes(volumes: &[VerityVolume]) -> Result<(), String> {
148-
let mut roots = std::collections::HashSet::new();
149148
let mut targets = std::collections::HashSet::new();
150149
for volume in volumes {
151-
if !roots.insert(volume.verity_root) {
152-
return Err(format!(
153-
"duplicate verity root {}",
154-
hex::encode(volume.verity_root)
155-
));
156-
}
157150
if !targets.insert(&volume.target) {
158151
return Err(format!(
159152
"duplicate verity volume target {}",
@@ -191,10 +184,8 @@ mod verity_volume_tests {
191184
}
192185

193186
#[test]
194-
fn rejects_duplicate_roots_and_targets() {
195-
assert!(validate_verity_volumes(&[volume(1, "/a"), volume(1, "/b")])
196-
.unwrap_err()
197-
.contains("duplicate verity root"));
187+
fn allows_duplicate_roots_but_rejects_duplicate_targets() {
188+
validate_verity_volumes(&[volume(1, "/a"), volume(1, "/b")]).unwrap();
198189
assert!(validate_verity_volumes(&[volume(1, "/a"), volume(2, "/a")])
199190
.unwrap_err()
200191
.contains("duplicate verity volume target"));

dstack/vmm/src/main_service.rs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
//
33
// SPDX-License-Identifier: Apache-2.0
44

5+
use std::collections::HashSet;
56
use std::ops::Deref;
67
use std::path::{Component, Path, PathBuf};
78
use std::time::{SystemTime, UNIX_EPOCH};
@@ -237,7 +238,19 @@ fn resolve_volumes(
237238
bail!("volumes requested but cvm.volumes_dir is not configured");
238239
}
239240
let base = fs::canonicalize(dir)?;
241+
let mut roots = HashSet::new();
240242
reqs.iter()
243+
.filter(|volume| {
244+
let first = roots.insert(volume.verity_root);
245+
if !first {
246+
warn!(
247+
root = %hex::encode(volume.verity_root),
248+
source = volume.source,
249+
"not attaching duplicate verity root"
250+
);
251+
}
252+
first
253+
})
241254
.map(|v| {
242255
let real = resolve_volume_source(&base, &v.source)?;
243256
Ok(crate::app::VmVolume {
@@ -1057,4 +1070,38 @@ mod tests {
10571070
);
10581071
Ok(())
10591072
}
1073+
1074+
#[test]
1075+
fn resolve_volumes_attaches_duplicate_root_once() -> Result<()> {
1076+
let tmp = tempfile::tempdir()?;
1077+
fs::write(tmp.path().join("first.img"), b"volume")?;
1078+
let mut cvm_config = test_cvm_config();
1079+
cvm_config.volumes_dir = tmp.path().to_string_lossy().into_owned();
1080+
let root = [7; 32];
1081+
1082+
let volumes = resolve_volumes(
1083+
&[
1084+
dstack_types::VerityVolume {
1085+
source: "first.img".into(),
1086+
verity_root: root,
1087+
target: "/run/first".into(),
1088+
},
1089+
dstack_types::VerityVolume {
1090+
// This source deliberately does not exist: the first entry
1091+
// owns the single attachment for this content root.
1092+
source: "duplicate.img".into(),
1093+
verity_root: root,
1094+
target: "/run/second".into(),
1095+
},
1096+
],
1097+
&cvm_config,
1098+
)?;
1099+
1100+
assert_eq!(volumes.len(), 1);
1101+
assert_eq!(
1102+
volumes[0].source,
1103+
tmp.path().join("first.img").display().to_string()
1104+
);
1105+
Ok(())
1106+
}
10601107
}

0 commit comments

Comments
 (0)