refactor: remove duplicate VcpuFds from GDB

Since we store VcpuFds in VcpuHandles inside Vmm,
and GDB stub has access to Vmm, remove redundant duplication
of VcpuFds.

Signed-off-by: Egor Lazarchuk <[email protected]>

Author: ShadowCurse

src/vmm/src/builder.rs

@@ -182,18 +182,6 @@ pub fn build_microvm_for_boot(
     let entry_point = load_kernel(&boot_config.kernel_file, vm.guest_memory())?;
     let initrd = InitrdConfig::from_config(boot_config, vm.guest_memory())?;
 
-    #[cfg(feature = "gdb")]
-    let (gdb_tx, gdb_rx) = mpsc::channel();
-    #[cfg(feature = "gdb")]
-    vcpus
-        .iter_mut()
-        .for_each(|vcpu| vcpu.attach_debug_info(gdb_tx.clone()));
-    #[cfg(feature = "gdb")]
-    let vcpu_fds = vcpus
-        .iter()
-        .map(|vcpu| vcpu.copy_kvm_vcpu_fd(&vm))
-        .collect::<Result<Vec<_>, _>>()?;
-
     if vm_resources.pci_enabled {
         device_manager.enable_pci(&vm)?;
     } else {
@@ -281,7 +269,7 @@ pub fn build_microvm_for_boot(
         boot_cmdline,
     )?;
 
-    let mut vmm = Vmm {
+    let vmm = Vmm {
         instance_info: instance_info.clone(),
         shutdown_exit_code: None,
         kvm,
@@ -291,33 +279,35 @@ pub fn build_microvm_for_boot(
         vcpus_exit_evt,
         device_manager,
     };
-
-    // Move vcpus to their own threads and start their state machine in the 'Paused' state.
-    vmm.start_vcpus(
-        vcpus,
-        seccomp_filters
-            .get("vcpu")
-            .ok_or_else(|| StartMicrovmError::MissingSeccompFilters("vcpu".to_string()))?
-            .clone(),
-    )
-    .map_err(VmmError::VcpuStart)?;
-
     let vmm = Arc::new(Mutex::new(vmm));
 
     #[cfg(feature = "gdb")]
-    if let Some(gdb_socket_path) = &vm_resources.machine_config.gdb_socket_path {
-        gdb::gdb_thread(
-            vmm.clone(),
-            vcpu_fds,
-            gdb_rx,
-            entry_point.entry_addr,
-            gdb_socket_path,
-        )
-        .map_err(StartMicrovmError::GdbServer)?;
-    } else {
-        debug!("No GDB socket provided not starting gdb server.");
+    {
+        let (gdb_tx, gdb_rx) = mpsc::channel();
+        vcpus
+            .iter_mut()
+            .for_each(|vcpu| vcpu.attach_debug_info(gdb_tx.clone()));
+
+        if let Some(gdb_socket_path) = &vm_resources.machine_config.gdb_socket_path {
+            gdb::gdb_thread(vmm.clone(), gdb_rx, entry_point.entry_addr, gdb_socket_path)
+                .map_err(StartMicrovmError::GdbServer)?;
+        } else {
+            debug!("No GDB socket provided not starting gdb server.");
+        }
     }
 
+    // Move vcpus to their own threads and start their state machine in the 'Paused' state.
+    vmm.lock()
+        .unwrap()
+        .start_vcpus(
+            vcpus,
+            seccomp_filters
+                .get("vcpu")
+                .ok_or_else(|| StartMicrovmError::MissingSeccompFilters("vcpu".to_string()))?
+                .clone(),
+        )
+        .map_err(VmmError::VcpuStart)?;
+
     // Load seccomp filters for the VMM thread.
     // Execution panics if filters cannot be loaded, use --no-seccomp if skipping filters
     // altogether is the desired behaviour.

src/vmm/src/gdb/event_loop.rs

@@ -11,7 +11,6 @@ use gdbstub::conn::{Connection, ConnectionExt};
 use gdbstub::stub::run_blocking::{self, WaitForStopReasonError};
 use gdbstub::stub::{DisconnectReason, GdbStub, MultiThreadStopReason};
 use gdbstub::target::Target;
-use kvm_ioctls::VcpuFd;
 use vm_memory::GuestAddress;
 
 use super::target::{FirecrackerTarget, GdbTargetError, vcpuid_to_tid};
@@ -22,11 +21,10 @@ use crate::logger::{error, trace};
 pub fn event_loop(
     connection: UnixStream,
     vmm: Arc<Mutex<Vmm>>,
-    vcpu_fds: Vec<VcpuFd>,
     gdb_event_receiver: Receiver<usize>,
     entry_addr: GuestAddress,
 ) {
-    let target = FirecrackerTarget::new(vmm, vcpu_fds, gdb_event_receiver, entry_addr);
+    let target = FirecrackerTarget::new(vmm, gdb_event_receiver, entry_addr);
     let connection: Box<dyn ConnectionExt<Error = std::io::Error>> = { Box::new(connection) };
     let debugger = GdbStub::new(connection);
 

src/vmm/src/gdb/mod.rs

@@ -15,7 +15,6 @@ use std::sync::{Arc, Mutex};
 
 use arch::vcpu_set_debug;
 use event_loop::event_loop;
-use kvm_ioctls::VcpuFd;
 use target::GdbTargetError;
 use vm_memory::GuestAddress;
 
@@ -35,7 +34,6 @@ use crate::logger::trace;
 /// communcation to the GDB server
 pub fn gdb_thread(
     vmm: Arc<Mutex<Vmm>>,
-    vcpu_fds: Vec<VcpuFd>,
     gdb_event_receiver: Receiver<usize>,
     entry_addr: GuestAddress,
     socket_addr: &str,
@@ -44,10 +42,12 @@ pub fn gdb_thread(
     // to be stopped as it connects. This also allows us to set breakpoints before kernel starts.
     // This entry adddress is automatically used as it is not tracked inside the target state, so
     // when resumed will be removed
-    vcpu_set_debug(&vcpu_fds[0], &[entry_addr], false)?;
-
-    for vcpu_fd in &vcpu_fds[1..] {
-        vcpu_set_debug(vcpu_fd, &[], false)?;
+    {
+        let vmm = vmm.lock().unwrap();
+        vcpu_set_debug(&vmm.vcpus_handles[0].vcpu_fd, &[entry_addr], false)?;
+        for handle in &vmm.vcpus_handles[1..] {
+            vcpu_set_debug(&handle.vcpu_fd, &[], false)?;
+        }
     }
 
     let path = Path::new(socket_addr);
@@ -59,7 +59,7 @@ pub fn gdb_thread(
 
     std::thread::Builder::new()
         .name("gdb".into())
-        .spawn(move || event_loop(connection, vmm, vcpu_fds, gdb_event_receiver, entry_addr))
+        .spawn(move || event_loop(connection, vmm, gdb_event_receiver, entry_addr))
         .map_err(|_| GdbTargetError::GdbThreadError)?;
 
     Ok(())