From 34412c9b41126bb233c5cbb62b351d0e3c2d24e2 Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Wed, 11 Mar 2020 16:17:51 +0100 Subject: [PATCH] vmm: Add id option to VFIO hotplug Add a new id option to the VFIO hotplug command so that it matches the VFIO coldplug semantic. This is done by refactoring the existing code for VFIO hotplug, where VmAddDeviceData structure is replaced by DeviceConfig. This structure is the one used whenever a VFIO device is coldplugged, which is why it makes sense to reuse it for the hotplug codepath. Signed-off-by: Sebastien Boeuf --- tests/integration.rs | 6 ++---- vmm/src/api/http_endpoint.rs | 6 +++--- vmm/src/api/mod.rs | 11 +++-------- vmm/src/device_manager.rs | 12 +++--------- vmm/src/lib.rs | 8 ++++---- vmm/src/vm.rs | 13 ++++++------- 6 files changed, 21 insertions(+), 35 deletions(-) diff --git a/tests/integration.rs b/tests/integration.rs index d7bbf9026..aa29ea721 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -2409,7 +2409,7 @@ mod tests { -i \ -X PUT http://localhost/api/v1/vm.add-device \ -H 'Accept: application/json' -H 'Content-Type: application/json' \ - -d '{\"path\":\"/sys/bus/pci/devices/0000:00:07.0\"}'", + -d '{\"path\":\"/sys/bus/pci/devices/0000:00:07.0\",\"id\":\"vfio123\"}'", )?; thread::sleep(std::time::Duration::new(10, 0)); @@ -2444,15 +2444,13 @@ mod tests { // Let's now verify that we can correctly remove the virtio-net // device through the "remove-device" command responsible for // unplugging VFIO devices. - // "vfio2" is used here because the two previous devices will be - // affected with the values "vfio0" and "vfio1" by default. guest.ssh_command_l1( "sudo curl \ --unix-socket /tmp/ch_api.sock \ -i \ -X PUT http://localhost/api/v1/vm.remove-device \ -H 'Accept: application/json' -H 'Content-Type: application/json' \ - -d '{\"id\":\"vfio2\"}'", + -d '{\"id\":\"vfio123\"}'", )?; thread::sleep(std::time::Duration::new(10, 0)); diff --git a/vmm/src/api/http_endpoint.rs b/vmm/src/api/http_endpoint.rs index 7c0b46b91..019ab8f39 100644 --- a/vmm/src/api/http_endpoint.rs +++ b/vmm/src/api/http_endpoint.rs @@ -7,7 +7,7 @@ use crate::api::http::EndpointHandler; use crate::api::{ vm_add_device, vm_boot, vm_create, vm_delete, vm_info, vm_pause, vm_reboot, vm_remove_device, vm_resize, vm_resume, vm_shutdown, vmm_ping, vmm_shutdown, ApiError, ApiRequest, ApiResult, - VmAction, VmAddDeviceData, VmConfig, VmRemoveDeviceData, VmResizeData, + DeviceConfig, VmAction, VmConfig, VmRemoveDeviceData, VmResizeData, }; use micro_http::{Body, Method, Request, Response, StatusCode, Version}; use serde_json::Error as SerdeError; @@ -283,8 +283,8 @@ impl EndpointHandler for VmAddDevice { Method::Put => { match &req.body { Some(body) => { - // Deserialize into a VmAddDeviceData - let vm_add_device_data: VmAddDeviceData = + // Deserialize into a DeviceConfig + let vm_add_device_data: DeviceConfig = match serde_json::from_slice(body.raw()) .map_err(HttpError::SerdeJsonDeserialize) { diff --git a/vmm/src/api/mod.rs b/vmm/src/api/mod.rs index 6c5e3e6f0..24a48e9c0 100644 --- a/vmm/src/api/mod.rs +++ b/vmm/src/api/mod.rs @@ -36,7 +36,7 @@ pub use self::http::start_http_thread; pub mod http; pub mod http_endpoint; -use crate::config::VmConfig; +use crate::config::{DeviceConfig, VmConfig}; use crate::vm::{Error as VmError, VmState}; use std::io; use std::sync::mpsc::{channel, RecvError, SendError, Sender}; @@ -125,11 +125,6 @@ pub struct VmResizeData { pub desired_ram: Option, } -#[derive(Clone, Deserialize, Serialize)] -pub struct VmAddDeviceData { - pub path: String, -} - #[derive(Clone, Deserialize, Serialize)] pub struct VmRemoveDeviceData { pub id: String, @@ -199,7 +194,7 @@ pub enum ApiRequest { VmResize(Arc, Sender), /// Add a device to the VM. - VmAddDevice(Arc, Sender), + VmAddDevice(Arc, Sender), /// Remove a device from the VM. VmRemoveDevice(Arc, Sender), @@ -359,7 +354,7 @@ pub fn vm_resize( pub fn vm_add_device( api_evt: EventFd, api_sender: Sender, - data: Arc, + data: Arc, ) -> ApiResult<()> { let (response_sender, response_receiver) = channel(); diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 3552c5ea3..4f89a0d85 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -1796,13 +1796,7 @@ impl DeviceManager { } #[cfg(feature = "pci_support")] - pub fn add_device(&mut self, path: String) -> DeviceManagerResult { - let mut device_cfg = DeviceConfig { - path: PathBuf::from(path), - iommu: false, - id: None, - }; - + pub fn add_device(&mut self, device_cfg: &mut DeviceConfig) -> DeviceManagerResult<()> { let pci = if let Some(pci_bus) = &self.pci_bus { Arc::clone(&pci_bus) } else { @@ -1827,13 +1821,13 @@ impl DeviceManager { &mut pci.lock().unwrap(), &interrupt_manager, &device_fd, - &mut device_cfg, + device_cfg, )?; // Update the PCIU bitmap self.pci_devices_up |= 1 << (device_id >> 3); - Ok(device_cfg) + Ok(()) } #[cfg(feature = "pci_support")] diff --git a/vmm/src/lib.rs b/vmm/src/lib.rs index 0b2fc73b6..c462979f0 100644 --- a/vmm/src/lib.rs +++ b/vmm/src/lib.rs @@ -16,7 +16,7 @@ extern crate tempfile; extern crate vmm_sys_util; use crate::api::{ApiError, ApiRequest, ApiResponse, ApiResponsePayload, VmInfo, VmmPingResponse}; -use crate::config::VmConfig; +use crate::config::{DeviceConfig, VmConfig}; use crate::vm::{Error as VmError, Vm, VmState}; use libc::EFD_NONBLOCK; use std::io; @@ -372,9 +372,9 @@ impl Vmm { } } - fn vm_add_device(&mut self, path: String) -> result::Result<(), VmError> { + fn vm_add_device(&mut self, device_cfg: DeviceConfig) -> result::Result<(), VmError> { if let Some(ref mut vm) = self.vm { - if let Err(e) = vm.add_device(path) { + if let Err(e) = vm.add_device(device_cfg) { error!("Error when adding new device to the VM: {:?}", e); Err(e) } else { @@ -555,7 +555,7 @@ impl Vmm { } ApiRequest::VmAddDevice(add_device_data, sender) => { let response = self - .vm_add_device(add_device_data.path.clone()) + .vm_add_device(add_device_data.as_ref().clone()) .map_err(ApiError::VmAddDevice) .map(|_| ApiResponsePayload::Empty); sender.send(response).map_err(Error::ApiResponseSend)?; diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index eece98817..e5165b6ba 100755 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -24,7 +24,7 @@ extern crate vm_allocator; extern crate vm_memory; extern crate vm_virtio; -use crate::config::VmConfig; +use crate::config::{DeviceConfig, VmConfig}; use crate::cpu; use crate::device_manager::{get_win_size, Console, DeviceManager, DeviceManagerError}; use crate::memory_manager::{get_host_cpu_phys_bits, Error as MemoryManagerError, MemoryManager}; @@ -543,15 +543,14 @@ impl Vm { Ok(()) } - pub fn add_device(&mut self, _path: String) -> Result<()> { + pub fn add_device(&mut self, mut _device_cfg: DeviceConfig) -> Result<()> { if cfg!(feature = "pci_support") { #[cfg(feature = "pci_support")] { - let device_cfg = self - .device_manager + self.device_manager .lock() .unwrap() - .add_device(_path) + .add_device(&mut _device_cfg) .map_err(Error::DeviceManager)?; // Update VmConfig by adding the new device. This is important to @@ -559,9 +558,9 @@ impl Vm { { let mut config = self.config.lock().unwrap(); if let Some(devices) = config.devices.as_mut() { - devices.push(device_cfg); + devices.push(_device_cfg); } else { - config.devices = Some(vec![device_cfg]); + config.devices = Some(vec![_device_cfg]); } }