Create Net devices during configuration instead of boot-time

src/devices/src/virtio/mmio.rs

@@ -53,16 +53,13 @@ pub struct MmioTransport {
 
 impl MmioTransport {
     /// Constructs a new MMIO transport for the given virtio device.
-    pub fn new(
-        mem: GuestMemoryMmap,
-        device: Arc<Mutex<dyn VirtioDevice>>,
-    ) -> std::io::Result<MmioTransport> {
+    pub fn new(mem: GuestMemoryMmap, device: Arc<Mutex<dyn VirtioDevice>>) -> MmioTransport {
         let interrupt_status = device
             .lock()
             .expect("Poisoned device lock")
             .interrupt_status();
 
-        Ok(MmioTransport {
+        MmioTransport {
             device,
             features_select: 0,
             acked_features_select: 0,
@@ -71,7 +68,7 @@ impl MmioTransport {
             config_generation: 0,
             mem,
             interrupt_status,
-        })
+        }
     }
 
     pub fn locked_device(&self) -> MutexGuard<dyn VirtioDevice + 'static> {
@@ -441,7 +438,7 @@ mod tests {
         let mut dummy = DummyDevice::new();
         // Validate reset is no-op.
         assert!(dummy.reset().is_none());
-        let mut d = MmioTransport::new(m, Arc::new(Mutex::new(dummy))).unwrap();
+        let mut d = MmioTransport::new(m, Arc::new(Mutex::new(dummy)));
 
         // We just make sure here that the implementation of a mmio device behaves as we expect,
         // given a known virtio device implementation (the dummy device).
@@ -470,7 +467,7 @@ mod tests {
     #[test]
     fn test_bus_device_read() {
         let m = GuestMemoryMmap::from_ranges(&[(GuestAddress(0), 0x1000)]).unwrap();
-        let mut d = MmioTransport::new(m, Arc::new(Mutex::new(DummyDevice::new()))).unwrap();
+        let mut d = MmioTransport::new(m, Arc::new(Mutex::new(DummyDevice::new())));
 
         let mut buf = vec![0xff, 0, 0xfe, 0];
         let buf_copy = buf.to_vec();
@@ -549,7 +546,7 @@ mod tests {
     fn test_bus_device_write() {
         let m = GuestMemoryMmap::from_ranges(&[(GuestAddress(0), 0x1000)]).unwrap();
         let dummy_dev = Arc::new(Mutex::new(DummyDevice::new()));
-        let mut d = MmioTransport::new(m, dummy_dev.clone()).unwrap();
+        let mut d = MmioTransport::new(m, dummy_dev.clone());
         let mut buf = vec![0; 5];
         write_le_u32(&mut buf[..4], 1);
 
@@ -706,7 +703,7 @@ mod tests {
     #[test]
     fn test_bus_device_activate() {
         let m = GuestMemoryMmap::from_ranges(&[(GuestAddress(0), 0x1000)]).unwrap();
-        let mut d = MmioTransport::new(m, Arc::new(Mutex::new(DummyDevice::new()))).unwrap();
+        let mut d = MmioTransport::new(m, Arc::new(Mutex::new(DummyDevice::new())));
 
         assert!(!d.are_queues_valid());
         assert!(!d.locked_device().is_activated());
@@ -824,7 +821,7 @@ mod tests {
     #[test]
     fn test_bus_device_reset() {
         let m = GuestMemoryMmap::from_ranges(&[(GuestAddress(0), 0x1000)]).unwrap();
-        let mut d = MmioTransport::new(m, Arc::new(Mutex::new(DummyDevice::new()))).unwrap();
+        let mut d = MmioTransport::new(m, Arc::new(Mutex::new(DummyDevice::new())));
         let mut buf = vec![0; 4];
 
         assert!(!d.are_queues_valid());

src/devices/src/virtio/net/device.rs

@@ -57,6 +57,8 @@ fn init_vnet_hdr(buf: &mut [u8]) {
 }
 
 pub struct Net {
+    id: String,
+
     pub(crate) tap: Tap,
     avail_features: u64,
     acked_features: u64,
@@ -94,6 +96,7 @@ pub struct Net {
 impl Net {
     /// Create a new virtio network device with the given TAP interface.
     pub fn new_with_tap(
+        id: String,
         tap: Tap,
         guest_mac: Option<&MacAddr>,
         rx_rate_limiter: RateLimiter,
@@ -145,6 +148,7 @@ impl Net {
         };
 
         Ok(Net {
+            id,
             tap,
             avail_features,
             acked_features: 0u64,
@@ -171,6 +175,16 @@ impl Net {
         })
     }
 
+    /// Provides the ID of this net device.
+    pub fn id(&self) -> &String {
+        &self.id
+    }
+
+    /// Provides the MAC of this net device.
+    pub fn guest_mac(&self) -> Option<&MacAddr> {
+        self.guest_mac.as_ref()
+    }
+
     fn signal_used_queue(&self) -> result::Result<(), DeviceError> {
         self.interrupt_status
             .fetch_or(VIRTIO_MMIO_INT_VRING as usize, Ordering::SeqCst);
@@ -784,6 +798,7 @@ pub(crate) mod tests {
             let guest_mac = Net::default_guest_mac();
 
             let mut net = Net::new_with_tap(
+                format!("net-device{}", next_tap),
                 tap,
                 Some(&guest_mac),
                 RateLimiter::default(),

src/utils/src/net/tap.rs

@@ -366,6 +366,13 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_tap_exclusive_open() {
+        let _tap1 = Tap::open_named("exclusivetap").unwrap();
+        // Opening same tap device a second time should not be permitted.
+        Tap::open_named("exclusivetap").unwrap_err();
+    }
+
     #[test]
     fn test_tap_partial_eq() {
         assert_ne!(Tap::new().unwrap(), Tap::new().unwrap());

src/vmm/src/builder.rs

@@ -24,11 +24,11 @@ use utils::eventfd::EventFd;
 use utils::terminal::Terminal;
 use utils::time::TimestampUs;
 use vm_memory::{Bytes, GuestAddress, GuestMemoryMmap};
-use vmm_config;
 use vmm_config::boot_source::BootConfig;
 use vmm_config::drive::BlockDeviceConfigs;
-use vmm_config::net::NetworkInterfaceConfigs;
+use vmm_config::net::NetBuilder;
 use vmm_config::vsock::VsockDeviceConfig;
+use vmm_config::RateLimiterConfig;
 use vstate::{KvmContext, Vcpu, VcpuConfig, Vm};
 use {device_manager, VmmEventsObserver};
 
@@ -702,7 +702,7 @@ fn attach_block_devices(
 
         let rate_limiter = drive_config
             .rate_limiter
-            .map(vmm_config::RateLimiterConfig::try_into)
+            .map(RateLimiterConfig::try_into)
             .transpose()
             .map_err(CreateRateLimiter)?;
 
@@ -722,8 +722,7 @@ fn attach_block_devices(
         attach_mmio_device(
             vmm,
             drive_config.drive_id.clone(),
-            MmioTransport::new(vmm.guest_memory().clone(), block_device.clone())
-                .map_err(CreateBlockDevice)?,
+            MmioTransport::new(vmm.guest_memory().clone(), block_device.clone()),
         )
         .map_err(RegisterBlockDevice)?;
     }
@@ -733,47 +732,22 @@ fn attach_block_devices(
 
 fn attach_net_devices(
     vmm: &mut Vmm,
-    network_ifaces: &NetworkInterfaceConfigs,
+    network_ifaces: &NetBuilder,
     event_manager: &mut EventManager,
 ) -> std::result::Result<(), StartMicrovmError> {
     use self::StartMicrovmError::*;
 
-    for cfg in network_ifaces.iter() {
-        let allow_mmds_requests = cfg.allow_mmds_requests();
-
-        let rx_rate_limiter = cfg
-            .rx_rate_limiter
-            .map(vmm_config::RateLimiterConfig::try_into)
-            .transpose()
-            .map_err(CreateRateLimiter)?;
-
-        let tx_rate_limiter = cfg
-            .tx_rate_limiter
-            .map(vmm_config::RateLimiterConfig::try_into)
-            .transpose()
-            .map_err(CreateRateLimiter)?;
-
-        let tap = cfg.open_tap().map_err(|_| NetDeviceNotConfigured)?;
-        let net_device = Arc::new(Mutex::new(
-            devices::virtio::net::Net::new_with_tap(
-                tap,
-                cfg.guest_mac(),
-                rx_rate_limiter.unwrap_or_default(),
-                tx_rate_limiter.unwrap_or_default(),
-                allow_mmds_requests,
-            )
-            .map_err(CreateNetDevice)?,
-        ));
+    for net_device in network_ifaces.iter() {
         event_manager
             .add_subscriber(net_device.clone())
-            .map_err(StartMicrovmError::RegisterEvent)?;
+            .map_err(RegisterEvent)?;
 
+        let id = net_device.lock().unwrap().id().clone();
+        // The device mutex mustn't be locked here otherwise it will deadlock.
         attach_mmio_device(
             vmm,
-            cfg.iface_id.clone(),
-            MmioTransport::new(vmm.guest_memory().clone(), net_device).map_err(|e| {
-                RegisterNetDevice(super::device_manager::mmio::Error::CreateMmioDevice(e))
-            })?,
+            id,
+            MmioTransport::new(vmm.guest_memory().clone(), net_device.clone()),
         )
         .map_err(RegisterNetDevice)?;
     }
@@ -805,9 +779,7 @@ fn attach_vsock_device(
     attach_mmio_device(
         vmm,
         vsock.vsock_id.clone(),
-        MmioTransport::new(vmm.guest_memory().clone(), vsock_device)
-            .map_err(device_manager::mmio::Error::CreateMmioDevice)
-            .map_err(RegisterVsockDevice)?,
+        MmioTransport::new(vmm.guest_memory().clone(), vsock_device),
     )
     .map_err(RegisterVsockDevice)?;
 
@@ -1105,7 +1077,7 @@ pub mod tests {
             allow_mmds_requests: false,
         };
 
-        let mut network_interface_configs = NetworkInterfaceConfigs::new();
+        let mut network_interface_configs = NetBuilder::new();
         network_interface_configs.insert(network_interface).unwrap();
 
         assert!(

src/vmm/src/device_manager/mmio.rs

@@ -25,8 +25,6 @@ use utils::eventfd::EventFd;
 pub enum Error {
     /// Failed to perform an operation on the bus.
     BusError(devices::BusError),
-    /// Could not create the mmio device to wrap a VirtioDevice.
-    CreateMmioDevice(io::Error),
     /// Appending to kernel command line failed.
     Cmdline(kernel_cmdline::Error),
     /// Failure in creating or cloning an event fd.
@@ -47,7 +45,6 @@ impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {
             Error::BusError(ref e) => write!(f, "failed to perform bus operation: {}", e),
-            Error::CreateMmioDevice(ref e) => write!(f, "failed to create mmio device: {}", e),
             Error::Cmdline(ref e) => {
                 write!(f, "unable to add device to kernel command line: {}", e)
             }
@@ -300,9 +297,7 @@ mod tests {
             type_id: u32,
             device_id: &str,
         ) -> Result<u64> {
-            let mmio_device = devices::virtio::MmioTransport::new(guest_mem, device)
-                .map_err(Error::CreateMmioDevice)?;
-
+            let mmio_device = devices::virtio::MmioTransport::new(guest_mem, device);
             self.register_mmio_device(vm, mmio_device, cmdline, type_id, device_id)
         }
 
@@ -509,16 +504,6 @@ mod tests {
                 devices::BusError::Overlap
             )
         );
-        assert_eq!(
-            format!(
-                "{}",
-                Error::CreateMmioDevice(io::Error::from_raw_os_error(0))
-            ),
-            format!(
-                "failed to create mmio device: {}",
-                io::Error::from_raw_os_error(0)
-            )
-        );
         assert_eq!(
             format!("{}", Error::IrqsExhausted),
             "no more IRQs are available"

src/vmm/src/resources.rs

@@ -67,7 +67,7 @@ pub struct VmResources {
     /// The configurations for block devices.
     pub block: BlockDeviceConfigs,
     /// The configurations for network interface devices.
-    pub network_interface: NetworkInterfaceConfigs,
+    pub network_interface: NetBuilder,
     /// The configurations for vsock devices.
     pub vsock: Option<VsockDeviceConfig>,
 }
@@ -244,29 +244,31 @@ mod tests {
     use vmm_config::boot_source::{BootConfig, BootSourceConfig, DEFAULT_KERNEL_CMDLINE};
     use vmm_config::drive::{BlockDeviceConfig, BlockDeviceConfigs, DriveError};
     use vmm_config::machine_config::{CpuFeaturesTemplate, VmConfig, VmConfigError};
-    use vmm_config::net::{NetworkInterfaceConfig, NetworkInterfaceConfigs, NetworkInterfaceError};
+    use vmm_config::net::{NetBuilder, NetworkInterfaceConfig, NetworkInterfaceError};
     use vmm_config::vsock::VsockDeviceConfig;
     use vmm_config::RateLimiterConfig;
     use vstate::VcpuConfig;
 
-    fn default_net_cfgs() -> NetworkInterfaceConfigs {
-        let mut net_if_cfgs = NetworkInterfaceConfigs::new();
-        net_if_cfgs
-            .insert(NetworkInterfaceConfig {
-                iface_id: "net_if1".to_string(),
-                // TempFile::new_with_prefix("") generates a random file name used as random net_if name.
-                host_dev_name: TempFile::new_with_prefix("")
-                    .unwrap()
-                    .as_path()
-                    .to_str()
-                    .unwrap()
-                    .to_string(),
-                guest_mac: Some(MacAddr::parse_str("01:23:45:67:89:0a").unwrap()),
-                rx_rate_limiter: Some(RateLimiterConfig::default()),
-                tx_rate_limiter: Some(RateLimiterConfig::default()),
-                allow_mmds_requests: false,
-            })
-            .unwrap();
+    fn default_net_cfg() -> NetworkInterfaceConfig {
+        NetworkInterfaceConfig {
+            iface_id: "net_if1".to_string(),
+            // TempFile::new_with_prefix("") generates a random file name used as random net_if name.
+            host_dev_name: TempFile::new_with_prefix("")
+                .unwrap()
+                .as_path()
+                .to_str()
+                .unwrap()
+                .to_string(),
+            guest_mac: Some(MacAddr::parse_str("01:23:45:67:89:0a").unwrap()),
+            rx_rate_limiter: Some(RateLimiterConfig::default()),
+            tx_rate_limiter: Some(RateLimiterConfig::default()),
+            allow_mmds_requests: false,
+        }
+    }
+
+    fn default_net_devs() -> NetBuilder {
+        let mut net_if_cfgs = NetBuilder::new();
+        net_if_cfgs.insert(default_net_cfg()).unwrap();
 
         net_if_cfgs
     }
@@ -303,7 +305,7 @@ mod tests {
             vm_config: VmConfig::default(),
             boot_config: Some(default_boot_cfg()),
             block: default_block_cfgs(),
-            network_interface: default_net_cfgs(),
+            network_interface: default_net_devs(),
             vsock: None,
         }
     }
@@ -538,7 +540,7 @@ mod tests {
         );
 
         match VmResources::from_json(json.as_str(), "some_version") {
-            Err(Error::NetDevice(NetworkInterfaceError::HostDeviceNameInUse { .. })) => (),
+            Err(Error::NetDevice(NetworkInterfaceError::OpenTap { .. })) => (),
             _ => unreachable!(),
         }
 
@@ -720,12 +722,7 @@ mod tests {
         let mut vm_resources = default_vm_resources();
 
         // Clone the existing net config in order to obtain a new one.
-        let mut new_net_device_cfg = vm_resources
-            .network_interface
-            .iter()
-            .next()
-            .unwrap()
-            .clone();
+        let mut new_net_device_cfg = default_net_cfg();
         new_net_device_cfg.iface_id = "new_net_if".to_string();
         new_net_device_cfg.guest_mac = Some(MacAddr::parse_str("01:23:45:67:89:0c").unwrap());
         new_net_device_cfg.host_dev_name = "dummy_path2".to_string();