Skip to content

Commit 37e88da

Browse files
committed
fix(vmm): make runtime networking state consistent
Reject configurations with multiple forwarding-enabled bridges so DHCP lease updates cannot redirect the VM-wide forwarding target between interfaces.\n\nPersist runtime network snapshots atomically before launching QEMU, publish the snapshot to in-memory state before DHCP can arrive, and clear both copies when process startup fails. Add regression coverage for forwarding validation and atomic snapshot replacement.
1 parent c728dcd commit 37e88da

3 files changed

Lines changed: 81 additions & 14 deletions

File tree

vmm/src/app.rs

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -378,20 +378,33 @@ impl App {
378378

379379
let devices = self.try_allocate_gpus(&vm_config.manifest)?;
380380
let processes = vm_config.config_qemu(&work_dir, &self.config.cvm, &devices)?;
381-
for process in processes {
382-
self.supervisor
383-
.deploy(&process)
384-
.await
385-
.with_context(|| format!("Failed to start process {}", process.id))?;
386-
}
387-
388381
let runtime_networks =
389382
crate::app::qemu::resolved_networks(&vm_config.manifest, &self.config.cvm);
390383
work_dir.set_runtime_networks(&runtime_networks)?;
384+
{
385+
let mut state = self.lock();
386+
let vm_state = state.get_mut(id).context("VM not found")?;
387+
vm_state.state.runtime_networks = runtime_networks;
388+
}
389+
for process in processes {
390+
if let Err(err) = self.supervisor.deploy(&process).await {
391+
if let Err(clear_err) = work_dir.clear_runtime_networks() {
392+
warn!(
393+
id,
394+
"failed to clear runtime networks after start failure: {clear_err}"
395+
);
396+
}
397+
if let Some(vm_state) = self.lock().get_mut(id) {
398+
vm_state.state.runtime_networks.clear();
399+
}
400+
return Err(err)
401+
.with_context(|| format!("failed to start process {}", process.id));
402+
}
403+
}
404+
391405
let mut state = self.lock();
392406
let vm_state = state.get_mut(id).context("VM not found")?;
393407
vm_state.state.devices = devices;
394-
vm_state.state.runtime_networks = runtime_networks;
395408
}
396409
Ok(())
397410
}

vmm/src/app/qemu.rs

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,13 @@ pub(crate) fn validate_resolved_network(networking: &Networking) -> Result<()> {
152152
}
153153

154154
pub(crate) fn validate_resolved_networks(networks: &[Networking]) -> Result<()> {
155+
let forwarding_bridges = networks
156+
.iter()
157+
.filter(|networking| networking.is_bridge() && networking.forward_service_enabled)
158+
.count();
159+
if forwarding_bridges > 1 {
160+
bail!("built-in port forwarding supports only one bridge");
161+
}
155162
for networking in networks {
156163
validate_resolved_network(networking)?;
157164
}
@@ -599,7 +606,12 @@ impl VmState {
599606
mod tests {
600607
use super::{
601608
amd_sev_snp_memory_backend_arg, mac_address_for_vm, mac_address_for_vm_index,
602-
parse_amd_sev_snp_qmp_capabilities, sanitize_optional, virtio_pci_device,
609+
parse_amd_sev_snp_qmp_capabilities, sanitize_optional, virtio_pci_device, VmWorkDir,
610+
};
611+
use std::{
612+
fs,
613+
os::unix::fs::symlink,
614+
time::{SystemTime, UNIX_EPOCH},
603615
};
604616

605617
#[test]
@@ -635,6 +647,27 @@ mod tests {
635647
);
636648
}
637649

650+
#[test]
651+
fn runtime_networks_snapshot_replaces_target_instead_of_following_it() -> anyhow::Result<()> {
652+
let temp = std::env::temp_dir().join(format!(
653+
"dstack-vmm-runtime-networks-test-{}",
654+
SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos()
655+
));
656+
fs::create_dir_all(&temp)?;
657+
let external = temp.join("external.json");
658+
fs::write(&external, "sentinel")?;
659+
let workdir = VmWorkDir::new(temp.join("vm"));
660+
fs::create_dir_all(workdir.path())?;
661+
symlink(&external, workdir.runtime_networks_path())?;
662+
663+
workdir.set_runtime_networks(&[])?;
664+
665+
assert_eq!(fs::read_to_string(&external)?, "sentinel");
666+
assert_eq!(fs::read_to_string(workdir.runtime_networks_path())?, "[]");
667+
fs::remove_dir_all(temp)?;
668+
Ok(())
669+
}
670+
638671
#[test]
639672
fn amd_sev_snp_memory_backend_arg_uses_passed_final_memory_size() {
640673
assert_eq!(
@@ -1431,11 +1464,9 @@ impl VmWorkDir {
14311464
}
14321465

14331466
pub fn set_runtime_networks(&self, networks: &[Networking]) -> Result<()> {
1434-
fs::write(
1435-
self.runtime_networks_path(),
1436-
serde_json::to_string(networks)?,
1437-
)
1438-
.context("failed to write runtime networks")
1467+
let serialized = serde_json::to_vec(networks)?;
1468+
safe_write::safe_write(self.runtime_networks_path(), serialized)
1469+
.context("failed to write runtime networks")
14391470
}
14401471

14411472
pub fn clear_runtime_networks(&self) -> Result<()> {

vmm/src/main_service.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -919,6 +919,29 @@ mod tests {
919919

920920
assert!(err.to_string().contains("custom networking mode"));
921921
}
922+
923+
#[test]
924+
fn multiple_bridges_are_rejected_when_builtin_forwarding_is_enabled() {
925+
let mut cvm_config = test_cvm_config();
926+
cvm_config.networking.forward_service_enabled = true;
927+
let mut request = test_vm_configuration();
928+
request.networks = vec![
929+
rpc::NetworkingConfig {
930+
mode: "bridge".to_string(),
931+
bridge_name: "lo".to_string(),
932+
},
933+
rpc::NetworkingConfig {
934+
mode: "bridge".to_string(),
935+
bridge_name: "lo".to_string(),
936+
},
937+
];
938+
939+
let err = create_manifest_from_vm_config(request, &cvm_config).unwrap_err();
940+
941+
assert!(err
942+
.to_string()
943+
.contains("built-in port forwarding supports only one bridge"));
944+
}
922945
}
923946

924947
impl RpcCall<App> for RpcHandler {

0 commit comments

Comments
 (0)