refactor: Refactor monitor server

crates/vm-core/src/monitor.rs

@@ -7,7 +7,7 @@ pub enum MonitorError {
     Stream(#[from] std::io::Error),
 
     #[error("{0}")]
-    CommandHandlerConflicat(String),
+    CommandHandlerConflict(String),
 
     #[error("{0}")]
     Serde(#[from] serde_json::Error),

crates/vm-core/src/virtualization/hvp.rs

@@ -37,6 +37,6 @@ impl Hypervisor for AppleHypervisor {
         hv_unsafe_call!(hv_vm_create(vm_config))
             .map_err(|err| HypervisorError::CreateVm(err.to_string()))?;
 
-        Ok(Arc::new(AppleHypervisorVm::default()))
+        Ok(Arc::new(AppleHypervisorVm))
     }
 }

crates/vm-core/src/virtualization/hvp/vm.rs

@@ -35,7 +35,6 @@ impl From<SetUserMemoryRegionFlags> for MemPerms {
     }
 }
 
-#[derive(Default)]
 pub struct AppleHypervisorVm;
 
 impl HypervisorVm for AppleHypervisorVm {

crates/vm-vmm/src/device.rs

@@ -18,7 +18,7 @@ use vm_virtio::transport::pci::VirtioPciDevice;
 
 use crate::device::irq_allocation::IrqAllocation;
 use crate::error::Error;
-use crate::service::monitor::MonitorServerBuilder;
+use crate::service::monitor::builder::MonitorServerBuilder;
 
 mod irq_allocation;
 

crates/vm-vmm/src/lib.rs

@@ -1,4 +1,4 @@
-// #![deny(warnings)]
+#![deny(warnings)]
 
 mod device;
 mod firmware;

crates/vm-vmm/src/service/gdbstub.rs

@@ -3,9 +3,9 @@ use gdbstub_arch::aarch64::AArch64 as GdbStubArch;
 #[cfg(target_arch = "x86_64")]
 use gdbstub_arch::x86::X86_64_SSE as GdbStubArch;
 
-pub mod command;
-pub mod connection;
-pub mod error;
+pub(crate) mod command;
+pub(crate) mod connection;
+pub(crate) mod error;
 
 mod event_loop;
 mod target;

crates/vm-vmm/src/service/gdbstub/command.rs

@@ -2,12 +2,11 @@
 use gdbstub_arch::aarch64::reg::AArch64CoreRegs as ArchGdbRegs;
 #[cfg(target_arch = "x86_64")]
 use gdbstub_arch::x86::reg::X86_64CoreRegs as ArchGdbRegs;
-use thiserror::Error;
 use tokio::sync::mpsc;
 use tokio::sync::oneshot;
 
 use crate::service::gdbstub::error::VmGdbStubError;
-use crate::vmm::command::VmmCommand;
+use crate::vmm::handler::VmmCommand;
 
 pub enum GdbStubCommand {
     ReadRegisters {
@@ -52,22 +51,16 @@ pub enum GdbStubCommandResponse {
     Err,
 }
 
-#[derive(Error, Debug)]
-pub enum GdbStubCommandError {
-    #[error("Err")]
-    Err,
-}
-
 pub struct GdbStubCommandRequest {
     pub command: GdbStubCommand,
-    pub response: oneshot::Sender<Result<GdbStubCommandResponse, GdbStubCommandError>>,
+    pub response: oneshot::Sender<GdbStubCommandResponse>,
 }
 
 impl GdbStubCommand {
     pub fn send_and_then_wait(
         self,
         tx: &mpsc::Sender<VmmCommand>,
-    ) -> Result<Result<GdbStubCommandResponse, GdbStubCommandError>, VmGdbStubError> {
+    ) -> Result<GdbStubCommandResponse, VmGdbStubError> {
         let (response_tx, response_rx) = oneshot::channel();
 
         let request = VmmCommand::GdbCommand(GdbStubCommandRequest {

crates/vm-vmm/src/service/gdbstub/connection.rs

@@ -9,7 +9,7 @@ use tracing::error;
 use crate::service::gdbstub::error::VmGdbStubError;
 use crate::service::gdbstub::event_loop::VmEventLoop;
 use crate::service::gdbstub::target::VmGdbStubTarget;
-use crate::vmm::command::VmmCommand;
+use crate::vmm::handler::VmmCommand;
 
 pub struct VmGdbStubConnector {
     tx: Arc<mpsc::Sender<VmmCommand>>,

crates/vm-vmm/src/service/gdbstub/target.rs

@@ -17,7 +17,7 @@ use crate::service::gdbstub::GdbStubArch;
 use crate::service::gdbstub::command::GdbStubCommand;
 use crate::service::gdbstub::command::GdbStubCommandResponse;
 use crate::service::gdbstub::error::VmGdbStubError;
-use crate::vmm::command::VmmCommand;
+use crate::vmm::handler::VmmCommand;
 
 fn vcpu_id_to_tid(vcpu_id: usize) -> Result<Tid, VmGdbStubError> {
     Tid::new(vcpu_id + 1).ok_or(VmGdbStubError::InvalidTid)
@@ -50,17 +50,17 @@ impl MultiThreadBase for VmGdbStubTarget {
             .map_err(|_| TargetError::NonFatal)?;
 
         match response {
-            Ok(GdbStubCommandResponse::ReadRegisters { registers }) => {
+            GdbStubCommandResponse::ReadRegisters { registers } => {
                 *regs = *registers;
 
                 Ok(())
             }
-            Ok(_) => {
-                error!("Unexpected response to ReadRegisters command");
+            GdbStubCommandResponse::Err => {
+                error!("Failed to handle ReadRegisters command");
                 Err(TargetError::NonFatal)
             }
-            Err(err) => {
-                error!(?err, "Failed to read registers");
+            _ => {
+                error!("Unexpected response to ReadRegisters command");
                 Err(TargetError::NonFatal)
             }
         }
@@ -81,13 +81,13 @@ impl MultiThreadBase for VmGdbStubTarget {
         .map_err(|_| TargetError::NonFatal)?;
 
         match response {
-            Ok(GdbStubCommandResponse::WriteRegisters) => Ok(()),
-            Ok(_) => {
-                error!("Unexpected response to WriteRegisters command");
+            GdbStubCommandResponse::WriteRegisters => Ok(()),
+            GdbStubCommandResponse::Err => {
+                error!("Failed to handle command");
                 Err(TargetError::NonFatal)
             }
-            Err(err) => {
-                error!(?err, "Failed to write registers");
+            _ => {
+                error!("Unexpected response to command");
                 Err(TargetError::NonFatal)
             }
         }
@@ -110,17 +110,16 @@ impl MultiThreadBase for VmGdbStubTarget {
         .map_err(|_| TargetError::NonFatal)?;
 
         match response {
-            Ok(GdbStubCommandResponse::ReadAddrs { buf }) => {
+            GdbStubCommandResponse::ReadAddrs { buf } => {
                 data[..buf.len()].copy_from_slice(&buf);
                 Ok(data.len())
             }
-            Ok(GdbStubCommandResponse::Err) => Err(TargetError::NonFatal),
-            Ok(_) => {
-                error!("Unexpected response to ReadAddrs command");
+            GdbStubCommandResponse::Err => {
+                error!("Failed to handle command");
                 Err(TargetError::NonFatal)
             }
-            Err(err) => {
-                error!(?err, "Failed to read addresses");
+            _ => {
+                error!("Unexpected response to command");
                 Err(TargetError::NonFatal)
             }
         }
@@ -143,13 +142,13 @@ impl MultiThreadBase for VmGdbStubTarget {
         .map_err(|_| TargetError::NonFatal)?;
 
         match response {
-            Ok(GdbStubCommandResponse::WriteAddrs) => Ok(()),
-            Ok(_) => {
-                error!("Unexpected response to WriteAddrs command");
+            GdbStubCommandResponse::WriteAddrs => Ok(()),
+            GdbStubCommandResponse::Err => {
+                error!("Failed to handle command");
                 Err(TargetError::NonFatal)
             }
-            Err(err) => {
-                error!(?err, "Failed to write addresses");
+            _ => {
+                error!("Unexpected response to command");
                 Err(TargetError::NonFatal)
             }
         }
@@ -160,19 +159,22 @@ impl MultiThreadBase for VmGdbStubTarget {
         thread_is_active: &mut dyn FnMut(Tid),
     ) -> Result<(), VmGdbStubError> {
         match GdbStubCommand::ListActiveThreads.send_and_then_wait(&self.tx)? {
-            Ok(GdbStubCommandResponse::ListActiveThreads(len)) => {
+            GdbStubCommandResponse::ListActiveThreads(len) => {
                 for vcpu_id in 0..len {
                     let tid = vcpu_id_to_tid(vcpu_id)?;
                     thread_is_active(tid);
                 }
 
                 Ok(())
             }
-            Ok(_) => Err(VmGdbStubError::InvalidResponse),
-            Err(err) => {
-                error!(?err, "Failed to list active threads");
+            GdbStubCommandResponse::Err => {
+                error!("Failed to handle command");
                 Err(VmGdbStubError::ListActiveThreadsFailed)
             }
+            _ => {
+                error!("Unexpected response to command");
+                Err(VmGdbStubError::InvalidResponse)
+            }
         }
     }
 
@@ -185,12 +187,15 @@ impl MultiThreadBase for VmGdbStubTarget {
 impl MultiThreadResume for VmGdbStubTarget {
     fn resume(&mut self) -> Result<(), Self::Error> {
         match GdbStubCommand::Resume.send_and_then_wait(&self.tx)? {
-            Ok(GdbStubCommandResponse::Resume) => Ok(()),
-            Ok(_) => Err(VmGdbStubError::InvalidResponse),
-            Err(err) => {
-                error!(?err, "Failed to resume");
+            GdbStubCommandResponse::Resume => Ok(()),
+            GdbStubCommandResponse::Err => {
+                error!("Failed to handle command");
                 Err(VmGdbStubError::ResumeFailed)
             }
+            _ => {
+                error!("Unexpected response to command");
+                Err(VmGdbStubError::InvalidResponse)
+            }
         }
     }
 

crates/vm-vmm/src/service/monitor.rs

@@ -1,142 +1,3 @@
-use std::collections::HashMap;
-use std::io;
-use std::sync::Arc;
-
-use tokio::io::AsyncWriteExt;
-use tokio::net::UnixListener;
-use tokio::net::UnixStream;
-use vm_core::monitor::MonitorCommand;
-use vm_core::monitor::MonitorError;
-
-const PATH: &str = "/tmp/vm.sock";
-
-struct MonitorConnection {
-    components: Arc<HashMap<String, Box<dyn MonitorCommand>>>,
-}
-
-impl MonitorConnection {
-    fn start(&self, mut stream: UnixStream) {
-        tokio::spawn({
-            let components = self.components.clone();
-
-            async move {
-                loop {
-                    stream.readable().await?;
-
-                    let mut buf = vec![0u8; 1024];
-                    match stream.try_read(&mut buf) {
-                        Ok(0) => break,
-                        Ok(n) => {
-                            let line = match str::from_utf8(&buf[..n]) {
-                                Ok(line) => line.trim(),
-                                Err(err) => {
-                                    stream.write_all(format!("ERR {err}\n").as_bytes()).await?;
-
-                                    continue;
-                                }
-                            };
-                            if line.is_empty() {
-                                continue;
-                            }
-
-                            let mut tokens = line.split_whitespace();
-                            let command = match tokens.next() {
-                                Some(f) => f,
-                                None => continue,
-                            };
-                            let subcommands: Vec<&str> = tokens.collect();
-
-                            match components.get(command) {
-                                Some(handler) => match handler.handle_command(&subcommands).await {
-                                    Ok(resp) => {
-                                        stream.writable().await?;
-
-                                        stream.write_all(resp.as_bytes()).await?;
-                                    }
-                                    Err(e) => {
-                                        stream.write_all(format!("ERR {e}\n").as_bytes()).await?;
-                                    }
-                                },
-                                None => {
-                                    stream
-                                        .write_all(
-                                            format!("ERR unknown command {command}\n").as_bytes(),
-                                        )
-                                        .await?;
-                                }
-                            }
-                        }
-                        Err(ref e) if e.kind() == io::ErrorKind::WouldBlock => {
-                            continue;
-                        }
-                        Err(e) => {
-                            return Err(e.into());
-                        }
-                    }
-                }
-
-                Ok::<(), MonitorError>(())
-            }
-        });
-    }
-}
-
-pub struct MonitorServer {
-    components: Arc<HashMap<String, Box<dyn MonitorCommand>>>,
-}
-
-impl MonitorServer {
-    pub fn start(&self) {
-        let components = self.components.clone();
-
-        tokio::spawn(async move {
-            let Ok(listener) = UnixListener::bind(PATH) else {
-                return;
-            };
-
-            loop {
-                let stream = match listener.accept().await {
-                    Ok((stream, _)) => stream,
-                    Err(_err) => {
-                        continue;
-                    }
-                };
-
-                let monitor_connection = MonitorConnection {
-                    components: components.clone(),
-                };
-
-                monitor_connection.start(stream);
-            }
-        });
-    }
-}
-
-#[derive(Default)]
-pub struct MonitorServerBuilder {
-    components: HashMap<String, Box<dyn MonitorCommand>>,
-}
-
-impl MonitorServerBuilder {
-    pub fn register_command_handler(
-        &mut self,
-        name: &str,
-        handler: Box<dyn MonitorCommand>,
-    ) -> Result<(), MonitorError> {
-        let name = name.to_string();
-
-        if self.components.contains_key(&name) {
-            return Err(MonitorError::CommandHandlerConflicat(name));
-        }
-
-        self.components.insert(name, handler);
-
-        Ok(())
-    }
-
-    pub fn build(self) -> MonitorServer {
-        MonitorServer {
-            components: self.components.into(),
-        }
-    }
-}
+pub(crate) mod builder;
+pub(crate) mod command;
+pub(crate) mod error;