vmm: Use HashSet for VmConfig::preserved_fds

Each VM reboot re-entered the VFIO/virtio-net add paths and re-appended
the same originating fds, leaving duplicates in preserved_fds and a
double-close hazard at final teardown. Switching to HashSet makes
add_preserved_fds idempotent.

Signed-off-by: Bo Chen <bchen@crusoe.ai>
Assisted-by: Claude:Opus-4.7
This commit is contained in:
Bo Chen
2026-05-06 22:03:45 +00:00
committed by Rob Bradford
parent 5c25d82f59
commit 8fd8224ee3
3 changed files with 9 additions and 10 deletions
+6 -8
View File
@@ -3,7 +3,7 @@
// SPDX-License-Identifier: Apache-2.0 // SPDX-License-Identifier: Apache-2.0
// //
use std::collections::{BTreeSet, HashMap}; use std::collections::{BTreeSet, HashMap, HashSet};
#[cfg(feature = "ivshmem")] #[cfg(feature = "ivshmem")]
use std::fs; use std::fs;
use std::path::PathBuf; use std::path::PathBuf;
@@ -3560,16 +3560,14 @@ impl VmConfig {
/// # Safety /// # Safety
/// To use this safely, the caller must guarantee that the input /// To use this safely, the caller must guarantee that the input
/// fds are all valid. /// fds are all valid.
pub unsafe fn add_preserved_fds(&mut self, mut fds: Vec<i32>) { pub unsafe fn add_preserved_fds(&mut self, fds: Vec<i32>) {
if fds.is_empty() { if fds.is_empty() {
return; return;
} }
if let Some(preserved_fds) = &self.preserved_fds { self.preserved_fds
fds.append(&mut preserved_fds.clone()); .get_or_insert_with(HashSet::new)
} .extend(fds);
self.preserved_fds = Some(fds);
} }
#[cfg(feature = "tdx")] #[cfg(feature = "tdx")]
@@ -3627,7 +3625,7 @@ impl Clone for VmConfig {
impl Drop for VmConfig { impl Drop for VmConfig {
fn drop(&mut self) { fn drop(&mut self) {
if let Some(mut fds) = self.preserved_fds.take() { if let Some(mut fds) = self.preserved_fds.take() {
for fd in fds.drain(..) { for fd in fds.drain() {
// SAFETY: FFI call with valid FDs // SAFETY: FFI call with valid FDs
unsafe { libc::close(fd) }; unsafe { libc::close(fd) };
} }
+1 -1
View File
@@ -4797,7 +4797,7 @@ impl DeviceManager {
debug!("Closing preserved FDs from virtio-net device: id={id}, fds={fds:?}"); debug!("Closing preserved FDs from virtio-net device: id={id}, fds={fds:?}");
for fd in fds { for fd in fds {
config.preserved_fds.as_mut().unwrap().retain(|x| *x != fd); config.preserved_fds.as_mut().unwrap().remove(&fd);
// SAFETY: We are closing the only remaining instance of this FD. // SAFETY: We are closing the only remaining instance of this FD.
unsafe { unsafe {
libc::close(fd); libc::close(fd);
+2 -1
View File
@@ -2,6 +2,7 @@
// //
// SPDX-License-Identifier: Apache-2.0 // SPDX-License-Identifier: Apache-2.0
// //
use std::collections::HashSet;
use std::net::IpAddr; use std::net::IpAddr;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
#[cfg(feature = "fw_cfg")] #[cfg(feature = "fw_cfg")]
@@ -1065,7 +1066,7 @@ pub struct VmConfig {
// causes the FDs to be closed early. This allows management software to // causes the FDs to be closed early. This allows management software to
// gracefully clean up resources (e.g., libvirt closes tap devices). // gracefully clean up resources (e.g., libvirt closes tap devices).
#[serde(skip)] #[serde(skip)]
pub preserved_fds: Option<Vec<i32>>, pub preserved_fds: Option<HashSet<i32>>,
#[serde(default)] #[serde(default)]
pub landlock_enable: bool, pub landlock_enable: bool,
pub landlock_rules: Option<Box<[LandlockConfig]>>, pub landlock_rules: Option<Box<[LandlockConfig]>>,