From a738808604d9a6f4fae25438170a462d807f89ff Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Fri, 6 Aug 2021 17:43:53 +0200 Subject: [PATCH] virtio-devices: vhost_user: Make vhost-user handle optional This anticipates the need for creating a new Blk, Fs or Net object without having performed the connection with the vhost-user backend yet. Signed-off-by: Sebastien Boeuf --- virtio-devices/src/vhost_user/blk.rs | 186 +++++++++++++++------------ virtio-devices/src/vhost_user/fs.rs | 167 +++++++++++++----------- virtio-devices/src/vhost_user/net.rs | 167 +++++++++++++----------- 3 files changed, 295 insertions(+), 225 deletions(-) diff --git a/virtio-devices/src/vhost_user/blk.rs b/virtio-devices/src/vhost_user/blk.rs index 9ca1299e9..26d0a7328 100644 --- a/virtio-devices/src/vhost_user/blk.rs +++ b/virtio-devices/src/vhost_user/blk.rs @@ -53,7 +53,7 @@ impl VhostUserMasterReqHandler for SlaveReqHandler {} pub struct Blk { common: VirtioCommon, id: String, - vu: Arc>, + vu: Option>>, config: VirtioBlockConfig, guest_memory: Option>, acked_protocol_features: u64, @@ -140,7 +140,7 @@ impl Blk { ..Default::default() }, id, - vu: Arc::new(Mutex::new(vu)), + vu: Some(Arc::new(Mutex::new(vu))), config, guest_memory: None, acked_protocol_features, @@ -211,15 +211,16 @@ impl VirtioDevice for Blk { } self.config.writeback = data[0]; - if let Err(e) = self - .vu - .lock() - .unwrap() - .socket_handle() - .set_config(offset as u32, VhostUserConfigFlags::WRITABLE, data) - .map_err(Error::VhostUserSetConfig) - { - error!("Failed setting vhost-user-blk configuration: {:?}", e); + if let Some(vu) = &self.vu { + if let Err(e) = vu + .lock() + .unwrap() + .socket_handle() + .set_config(offset as u32, VhostUserConfigFlags::WRITABLE, data) + .map_err(Error::VhostUserSetConfig) + { + error!("Failed setting vhost-user-blk configuration: {:?}", e); + } } } @@ -249,8 +250,12 @@ impl VirtioDevice for Blk { None }; - self.vu - .lock() + if self.vu.is_none() { + error!("Missing vhost-user handle"); + return Err(ActivateError::BadActivate); + } + let vu = self.vu.as_ref().unwrap(); + vu.lock() .unwrap() .setup_vhost_user( &mem.memory(), @@ -268,7 +273,7 @@ impl VirtioDevice for Blk { let (kill_evt, pause_evt) = self.common.dup_eventfds(); let mut handler: VhostUserEpollHandler = VhostUserEpollHandler { - vu: self.vu.clone(), + vu: vu.clone(), mem, kill_evt, pause_evt, @@ -308,14 +313,15 @@ impl VirtioDevice for Blk { self.common.resume().ok()?; } - if let Err(e) = self - .vu - .lock() - .unwrap() - .reset_vhost_user(self.common.queue_sizes.len()) - { - error!("Failed to reset vhost-user daemon: {:?}", e); - return None; + if let Some(vu) = &self.vu { + if let Err(e) = vu + .lock() + .unwrap() + .reset_vhost_user(self.common.queue_sizes.len()) + { + error!("Failed to reset vhost-user daemon: {:?}", e); + return None; + } } if let Some(kill_evt) = self.common.kill_evt.take() { @@ -330,41 +336,46 @@ impl VirtioDevice for Blk { } fn shutdown(&mut self) { - let _ = unsafe { libc::close(self.vu.lock().unwrap().socket_handle().as_raw_fd()) }; + if let Some(vu) = &self.vu { + let _ = unsafe { libc::close(vu.lock().unwrap().socket_handle().as_raw_fd()) }; + } } fn add_memory_region( &mut self, region: &Arc, ) -> std::result::Result<(), crate::Error> { - if self.acked_protocol_features & VhostUserProtocolFeatures::CONFIGURE_MEM_SLOTS.bits() != 0 - { - self.vu - .lock() - .unwrap() - .add_memory_region(region) - .map_err(crate::Error::VhostUserAddMemoryRegion) - } else if let Some(guest_memory) = &self.guest_memory { - self.vu - .lock() - .unwrap() - .update_mem_table(guest_memory.memory().deref()) - .map_err(crate::Error::VhostUserUpdateMemory) - } else { - Ok(()) + if let Some(vu) = &self.vu { + if self.acked_protocol_features & VhostUserProtocolFeatures::CONFIGURE_MEM_SLOTS.bits() + != 0 + { + return vu + .lock() + .unwrap() + .add_memory_region(region) + .map_err(crate::Error::VhostUserAddMemoryRegion); + } else if let Some(guest_memory) = &self.guest_memory { + return vu + .lock() + .unwrap() + .update_mem_table(guest_memory.memory().deref()) + .map_err(crate::Error::VhostUserUpdateMemory); + } } + Ok(()) } } impl Pausable for Blk { fn pause(&mut self) -> result::Result<(), MigratableError> { - self.vu - .lock() - .unwrap() - .pause_vhost_user(self.vu_num_queues) - .map_err(|e| { - MigratableError::Pause(anyhow!("Error pausing vhost-user-blk backend: {:?}", e)) - })?; + if let Some(vu) = &self.vu { + vu.lock() + .unwrap() + .pause_vhost_user(self.vu_num_queues) + .map_err(|e| { + MigratableError::Pause(anyhow!("Error pausing vhost-user-blk backend: {:?}", e)) + })?; + } self.common.pause() } @@ -376,13 +387,19 @@ impl Pausable for Blk { epoll_thread.thread().unpark(); } - self.vu - .lock() - .unwrap() - .resume_vhost_user(self.vu_num_queues) - .map_err(|e| { - MigratableError::Resume(anyhow!("Error resuming vhost-user-blk backend: {:?}", e)) - }) + if let Some(vu) = &self.vu { + vu.lock() + .unwrap() + .resume_vhost_user(self.vu_num_queues) + .map_err(|e| { + MigratableError::Resume(anyhow!( + "Error resuming vhost-user-blk backend: {:?}", + e + )) + }) + } else { + Ok(()) + } } } @@ -404,49 +421,56 @@ impl Transportable for Blk {} impl Migratable for Blk { fn start_dirty_log(&mut self) -> std::result::Result<(), MigratableError> { - if let Some(guest_memory) = &self.guest_memory { - let last_ram_addr = guest_memory.memory().last_addr().raw_value(); - self.vu - .lock() - .unwrap() - .start_dirty_log(last_ram_addr) - .map_err(|e| { - MigratableError::StartDirtyLog(anyhow!( - "Error starting migration for vhost-user-blk backend: {:?}", - e - )) - }) + if let Some(vu) = &self.vu { + if let Some(guest_memory) = &self.guest_memory { + let last_ram_addr = guest_memory.memory().last_addr().raw_value(); + vu.lock() + .unwrap() + .start_dirty_log(last_ram_addr) + .map_err(|e| { + MigratableError::StartDirtyLog(anyhow!( + "Error starting migration for vhost-user-blk backend: {:?}", + e + )) + }) + } else { + Err(MigratableError::StartDirtyLog(anyhow!( + "Missing guest memory" + ))) + } } else { - Err(MigratableError::StartDirtyLog(anyhow!( - "Missing guest memory" - ))) + Ok(()) } } fn stop_dirty_log(&mut self) -> std::result::Result<(), MigratableError> { - self.vu.lock().unwrap().stop_dirty_log().map_err(|e| { - MigratableError::StopDirtyLog(anyhow!( - "Error stopping migration for vhost-user-blk backend: {:?}", - e - )) - }) + if let Some(vu) = &self.vu { + vu.lock().unwrap().stop_dirty_log().map_err(|e| { + MigratableError::StopDirtyLog(anyhow!( + "Error stopping migration for vhost-user-blk backend: {:?}", + e + )) + }) + } else { + Ok(()) + } } fn dirty_log(&mut self) -> std::result::Result { - if let Some(guest_memory) = &self.guest_memory { - let last_ram_addr = guest_memory.memory().last_addr().raw_value(); - self.vu - .lock() - .unwrap() - .dirty_log(last_ram_addr) - .map_err(|e| { + if let Some(vu) = &self.vu { + if let Some(guest_memory) = &self.guest_memory { + let last_ram_addr = guest_memory.memory().last_addr().raw_value(); + vu.lock().unwrap().dirty_log(last_ram_addr).map_err(|e| { MigratableError::DirtyLog(anyhow!( "Error retrieving dirty ranges from vhost-user-blk backend: {:?}", e )) }) + } else { + Err(MigratableError::DirtyLog(anyhow!("Missing guest memory"))) + } } else { - Err(MigratableError::DirtyLog(anyhow!("Missing guest memory"))) + Ok(MemoryRangeTable::default()) } } } diff --git a/virtio-devices/src/vhost_user/fs.rs b/virtio-devices/src/vhost_user/fs.rs index da2372a3b..b9f7952b1 100644 --- a/virtio-devices/src/vhost_user/fs.rs +++ b/virtio-devices/src/vhost_user/fs.rs @@ -298,7 +298,7 @@ unsafe impl ByteValued for VirtioFsConfig {} pub struct Fs { common: VirtioCommon, id: String, - vu: Arc>, + vu: Option>>, config: VirtioFsConfig, // Hold ownership of the memory that is allocated for the device // which will be automatically dropped when the device is dropped @@ -388,7 +388,7 @@ impl Fs { ..Default::default() }, id, - vu: Arc::new(Mutex::new(vu)), + vu: Some(Arc::new(Mutex::new(vu))), config, cache, slave_req_support, @@ -493,8 +493,12 @@ impl VirtioDevice for Fs { None }; - self.vu - .lock() + if self.vu.is_none() { + error!("Missing vhost-user handle"); + return Err(ActivateError::BadActivate); + } + let vu = self.vu.as_ref().unwrap(); + vu.lock() .unwrap() .setup_vhost_user( &mem.memory(), @@ -511,7 +515,7 @@ impl VirtioDevice for Fs { // the backend as well as requests initiated by the backend. let (kill_evt, pause_evt) = self.common.dup_eventfds(); let mut handler: VhostUserEpollHandler = VhostUserEpollHandler { - vu: self.vu.clone(), + vu: vu.clone(), mem, kill_evt, pause_evt, @@ -558,14 +562,15 @@ impl VirtioDevice for Fs { self.common.resume().ok()?; } - if let Err(e) = self - .vu - .lock() - .unwrap() - .reset_vhost_user(self.common.queue_sizes.len()) - { - error!("Failed to reset vhost-user daemon: {:?}", e); - return None; + if let Some(vu) = &self.vu { + if let Err(e) = vu + .lock() + .unwrap() + .reset_vhost_user(self.common.queue_sizes.len()) + { + error!("Failed to reset vhost-user daemon: {:?}", e); + return None; + } } if let Some(kill_evt) = self.common.kill_evt.take() { @@ -580,7 +585,9 @@ impl VirtioDevice for Fs { } fn shutdown(&mut self) { - let _ = unsafe { libc::close(self.vu.lock().unwrap().socket_handle().as_raw_fd()) }; + if let Some(vu) = &self.vu { + let _ = unsafe { libc::close(vu.lock().unwrap().socket_handle().as_raw_fd()) }; + } } fn get_shm_regions(&self) -> Option { @@ -603,22 +610,24 @@ impl VirtioDevice for Fs { &mut self, region: &Arc, ) -> std::result::Result<(), crate::Error> { - if self.acked_protocol_features & VhostUserProtocolFeatures::CONFIGURE_MEM_SLOTS.bits() != 0 - { - self.vu - .lock() - .unwrap() - .add_memory_region(region) - .map_err(crate::Error::VhostUserAddMemoryRegion) - } else if let Some(guest_memory) = &self.guest_memory { - self.vu - .lock() - .unwrap() - .update_mem_table(guest_memory.memory().deref()) - .map_err(crate::Error::VhostUserUpdateMemory) - } else { - Ok(()) + if let Some(vu) = &self.vu { + if self.acked_protocol_features & VhostUserProtocolFeatures::CONFIGURE_MEM_SLOTS.bits() + != 0 + { + return vu + .lock() + .unwrap() + .add_memory_region(region) + .map_err(crate::Error::VhostUserAddMemoryRegion); + } else if let Some(guest_memory) = &self.guest_memory { + return vu + .lock() + .unwrap() + .update_mem_table(guest_memory.memory().deref()) + .map_err(crate::Error::VhostUserUpdateMemory); + } } + Ok(()) } fn userspace_mappings(&self) -> Vec { @@ -639,13 +648,14 @@ impl VirtioDevice for Fs { impl Pausable for Fs { fn pause(&mut self) -> result::Result<(), MigratableError> { - self.vu - .lock() - .unwrap() - .pause_vhost_user(self.vu_num_queues) - .map_err(|e| { - MigratableError::Pause(anyhow!("Error pausing vhost-user-fs backend: {:?}", e)) - })?; + if let Some(vu) = &self.vu { + vu.lock() + .unwrap() + .pause_vhost_user(self.vu_num_queues) + .map_err(|e| { + MigratableError::Pause(anyhow!("Error pausing vhost-user-fs backend: {:?}", e)) + })?; + } self.common.pause() } @@ -657,13 +667,19 @@ impl Pausable for Fs { epoll_thread.thread().unpark(); } - self.vu - .lock() - .unwrap() - .resume_vhost_user(self.vu_num_queues) - .map_err(|e| { - MigratableError::Resume(anyhow!("Error resuming vhost-user-fs backend: {:?}", e)) - }) + if let Some(vu) = &self.vu { + vu.lock() + .unwrap() + .resume_vhost_user(self.vu_num_queues) + .map_err(|e| { + MigratableError::Resume(anyhow!( + "Error resuming vhost-user-fs backend: {:?}", + e + )) + }) + } else { + Ok(()) + } } } @@ -685,49 +701,56 @@ impl Transportable for Fs {} impl Migratable for Fs { fn start_dirty_log(&mut self) -> std::result::Result<(), MigratableError> { - if let Some(guest_memory) = &self.guest_memory { - let last_ram_addr = guest_memory.memory().last_addr().raw_value(); - self.vu - .lock() - .unwrap() - .start_dirty_log(last_ram_addr) - .map_err(|e| { - MigratableError::StartDirtyLog(anyhow!( - "Error starting migration for vhost-user-fs backend: {:?}", - e - )) - }) + if let Some(vu) = &self.vu { + if let Some(guest_memory) = &self.guest_memory { + let last_ram_addr = guest_memory.memory().last_addr().raw_value(); + vu.lock() + .unwrap() + .start_dirty_log(last_ram_addr) + .map_err(|e| { + MigratableError::StartDirtyLog(anyhow!( + "Error starting migration for vhost-user-fs backend: {:?}", + e + )) + }) + } else { + Err(MigratableError::StartDirtyLog(anyhow!( + "Missing guest memory" + ))) + } } else { - Err(MigratableError::StartDirtyLog(anyhow!( - "Missing guest memory" - ))) + Ok(()) } } fn stop_dirty_log(&mut self) -> std::result::Result<(), MigratableError> { - self.vu.lock().unwrap().stop_dirty_log().map_err(|e| { - MigratableError::StopDirtyLog(anyhow!( - "Error stopping migration for vhost-user-fs backend: {:?}", - e - )) - }) + if let Some(vu) = &self.vu { + vu.lock().unwrap().stop_dirty_log().map_err(|e| { + MigratableError::StopDirtyLog(anyhow!( + "Error stopping migration for vhost-user-fs backend: {:?}", + e + )) + }) + } else { + Ok(()) + } } fn dirty_log(&mut self) -> std::result::Result { - if let Some(guest_memory) = &self.guest_memory { - let last_ram_addr = guest_memory.memory().last_addr().raw_value(); - self.vu - .lock() - .unwrap() - .dirty_log(last_ram_addr) - .map_err(|e| { + if let Some(vu) = &self.vu { + if let Some(guest_memory) = &self.guest_memory { + let last_ram_addr = guest_memory.memory().last_addr().raw_value(); + vu.lock().unwrap().dirty_log(last_ram_addr).map_err(|e| { MigratableError::DirtyLog(anyhow!( "Error retrieving dirty ranges from vhost-user-fs backend: {:?}", e )) }) + } else { + Err(MigratableError::DirtyLog(anyhow!("Missing guest memory"))) + } } else { - Err(MigratableError::DirtyLog(anyhow!("Missing guest memory"))) + Ok(MemoryRangeTable::default()) } } } diff --git a/virtio-devices/src/vhost_user/net.rs b/virtio-devices/src/vhost_user/net.rs index da7e35afe..3ce2865fc 100644 --- a/virtio-devices/src/vhost_user/net.rs +++ b/virtio-devices/src/vhost_user/net.rs @@ -106,7 +106,7 @@ impl EpollHelperHandler for NetCtrlEpollHandler { pub struct Net { common: VirtioCommon, id: String, - vu: Arc>, + vu: Option>>, config: VirtioNetConfig, guest_memory: Option>, acked_protocol_features: u64, @@ -198,7 +198,7 @@ impl Net { min_queues: DEFAULT_QUEUE_NUMBER as u16, ..Default::default() }, - vu: Arc::new(Mutex::new(vu)), + vu: Some(Arc::new(Mutex::new(vu))), config, guest_memory: None, acked_protocol_features, @@ -327,8 +327,12 @@ impl VirtioDevice for Net { None }; - self.vu - .lock() + if self.vu.is_none() { + error!("Missing vhost-user handle"); + return Err(ActivateError::BadActivate); + } + let vu = self.vu.as_ref().unwrap(); + vu.lock() .unwrap() .setup_vhost_user( &mem.memory(), @@ -346,7 +350,7 @@ impl VirtioDevice for Net { let (kill_evt, pause_evt) = self.common.dup_eventfds(); let mut handler: VhostUserEpollHandler = VhostUserEpollHandler { - vu: self.vu.clone(), + vu: vu.clone(), mem, kill_evt, pause_evt, @@ -386,14 +390,15 @@ impl VirtioDevice for Net { self.common.resume().ok()?; } - if let Err(e) = self - .vu - .lock() - .unwrap() - .reset_vhost_user(self.common.queue_sizes.len()) - { - error!("Failed to reset vhost-user daemon: {:?}", e); - return None; + if let Some(vu) = &self.vu { + if let Err(e) = vu + .lock() + .unwrap() + .reset_vhost_user(self.common.queue_sizes.len()) + { + error!("Failed to reset vhost-user daemon: {:?}", e); + return None; + } } if let Some(kill_evt) = self.common.kill_evt.take() { @@ -408,7 +413,9 @@ impl VirtioDevice for Net { } fn shutdown(&mut self) { - let _ = unsafe { libc::close(self.vu.lock().unwrap().socket_handle().as_raw_fd()) }; + if let Some(vu) = &self.vu { + let _ = unsafe { libc::close(vu.lock().unwrap().socket_handle().as_raw_fd()) }; + } // Remove socket path if needed if self.server { @@ -420,34 +427,37 @@ impl VirtioDevice for Net { &mut self, region: &Arc, ) -> std::result::Result<(), crate::Error> { - if self.acked_protocol_features & VhostUserProtocolFeatures::CONFIGURE_MEM_SLOTS.bits() != 0 - { - self.vu - .lock() - .unwrap() - .add_memory_region(region) - .map_err(crate::Error::VhostUserAddMemoryRegion) - } else if let Some(guest_memory) = &self.guest_memory { - self.vu - .lock() - .unwrap() - .update_mem_table(guest_memory.memory().deref()) - .map_err(crate::Error::VhostUserUpdateMemory) - } else { - Ok(()) + if let Some(vu) = &self.vu { + if self.acked_protocol_features & VhostUserProtocolFeatures::CONFIGURE_MEM_SLOTS.bits() + != 0 + { + return vu + .lock() + .unwrap() + .add_memory_region(region) + .map_err(crate::Error::VhostUserAddMemoryRegion); + } else if let Some(guest_memory) = &self.guest_memory { + return vu + .lock() + .unwrap() + .update_mem_table(guest_memory.memory().deref()) + .map_err(crate::Error::VhostUserUpdateMemory); + } } + Ok(()) } } impl Pausable for Net { fn pause(&mut self) -> result::Result<(), MigratableError> { - self.vu - .lock() - .unwrap() - .pause_vhost_user(self.vu_num_queues) - .map_err(|e| { - MigratableError::Pause(anyhow!("Error pausing vhost-user-net backend: {:?}", e)) - })?; + if let Some(vu) = &self.vu { + vu.lock() + .unwrap() + .pause_vhost_user(self.vu_num_queues) + .map_err(|e| { + MigratableError::Pause(anyhow!("Error pausing vhost-user-net backend: {:?}", e)) + })?; + } self.common.pause() } @@ -463,13 +473,19 @@ impl Pausable for Net { ctrl_queue_epoll_thread.thread().unpark(); } - self.vu - .lock() - .unwrap() - .resume_vhost_user(self.vu_num_queues) - .map_err(|e| { - MigratableError::Resume(anyhow!("Error resuming vhost-user-net backend: {:?}", e)) - }) + if let Some(vu) = &self.vu { + vu.lock() + .unwrap() + .resume_vhost_user(self.vu_num_queues) + .map_err(|e| { + MigratableError::Resume(anyhow!( + "Error resuming vhost-user-net backend: {:?}", + e + )) + }) + } else { + Ok(()) + } } } @@ -491,49 +507,56 @@ impl Transportable for Net {} impl Migratable for Net { fn start_dirty_log(&mut self) -> std::result::Result<(), MigratableError> { - if let Some(guest_memory) = &self.guest_memory { - let last_ram_addr = guest_memory.memory().last_addr().raw_value(); - self.vu - .lock() - .unwrap() - .start_dirty_log(last_ram_addr) - .map_err(|e| { - MigratableError::StartDirtyLog(anyhow!( - "Error starting migration for vhost-user-net backend: {:?}", - e - )) - }) + if let Some(vu) = &self.vu { + if let Some(guest_memory) = &self.guest_memory { + let last_ram_addr = guest_memory.memory().last_addr().raw_value(); + vu.lock() + .unwrap() + .start_dirty_log(last_ram_addr) + .map_err(|e| { + MigratableError::StartDirtyLog(anyhow!( + "Error starting migration for vhost-user-net backend: {:?}", + e + )) + }) + } else { + Err(MigratableError::StartDirtyLog(anyhow!( + "Missing guest memory" + ))) + } } else { - Err(MigratableError::StartDirtyLog(anyhow!( - "Missing guest memory" - ))) + Ok(()) } } fn stop_dirty_log(&mut self) -> std::result::Result<(), MigratableError> { - self.vu.lock().unwrap().stop_dirty_log().map_err(|e| { - MigratableError::StopDirtyLog(anyhow!( - "Error stopping migration for vhost-user-net backend: {:?}", - e - )) - }) + if let Some(vu) = &self.vu { + vu.lock().unwrap().stop_dirty_log().map_err(|e| { + MigratableError::StopDirtyLog(anyhow!( + "Error stopping migration for vhost-user-net backend: {:?}", + e + )) + }) + } else { + Ok(()) + } } fn dirty_log(&mut self) -> std::result::Result { - if let Some(guest_memory) = &self.guest_memory { - let last_ram_addr = guest_memory.memory().last_addr().raw_value(); - self.vu - .lock() - .unwrap() - .dirty_log(last_ram_addr) - .map_err(|e| { + if let Some(vu) = &self.vu { + if let Some(guest_memory) = &self.guest_memory { + let last_ram_addr = guest_memory.memory().last_addr().raw_value(); + vu.lock().unwrap().dirty_log(last_ram_addr).map_err(|e| { MigratableError::DirtyLog(anyhow!( "Error retrieving dirty ranges from vhost-user-net backend: {:?}", e )) }) + } else { + Err(MigratableError::DirtyLog(anyhow!("Missing guest memory"))) + } } else { - Err(MigratableError::DirtyLog(anyhow!("Missing guest memory"))) + Ok(MemoryRangeTable::default()) } } }