fix: Update logging

Adds additional `std::fmt::Debug` implementations, reduces dynamic
dispatch, removes the `BusDevice` trait and adds
`missing_debug_implementations`.

Signed-off-by: Jonathan Woollett-Light <[email protected]>

Author: Jonathan Woollett-Light

.cargo/config

@@ -5,7 +5,8 @@ rustflags = [
   "-Dclippy::ptr_as_ptr",
   "-Dclippy::undocumented_unsafe_blocks",
   "-Dclippy::cast_lossless",
-  "-Dclippy::cast_sign_loss"
+  "-Dclippy::cast_sign_loss",
+  "-Dmissing_debug_implementations"
 ]
 
 [net]

Cargo.lock

@@ -10,6 +10,7 @@
 mod parsed_request;
 mod request;
 
+use std::fmt::Debug;
 use std::path::PathBuf;
 use std::sync::mpsc;
 
@@ -44,6 +45,7 @@ pub enum Error {
 type Result<T> = std::result::Result<T, Error>;
 
 /// Structure associated with the API server implementation.
+#[derive(Debug)]
 pub struct ApiServer {
     /// Sender which allows passing messages to the VMM.
     api_request_sender: mpsc::Sender<ApiRequest>,
@@ -113,7 +115,7 @@ impl ApiServer {
     /// let (api_request_sender, _from_api) = channel();
     /// let (to_api, vmm_response_receiver) = channel();
     /// let time_reporter = ProcessTimeReporter::new(Some(1), Some(1), Some(1));
-    /// let seccomp_filters = get_filters(SeccompConfig::None).unwrap();
+    /// let seccomp_filters = get_filters(SeccompConfig::<std::io::Empty>::None).unwrap();
     /// let payload_limit = HTTP_MAX_PAYLOAD_SIZE;
     /// let (socket_ready_sender, socket_ready_receiver): (Sender<bool>, Receiver<bool>) = channel();
     ///
@@ -291,13 +293,13 @@ impl ApiServer {
     }
 
     /// An HTTP response which also includes a body.
-    pub(crate) fn json_response<T: Into<String>>(status: StatusCode, body: T) -> Response {
+    pub(crate) fn json_response<T: Into<String> + Debug>(status: StatusCode, body: T) -> Response {
         let mut response = Response::new(Version::Http11, status);
         response.set_body(Body::new(body.into()));
         response
     }
 
-    fn json_fault_message<T: AsRef<str> + serde::Serialize>(msg: T) -> String {
+    fn json_fault_message<T: AsRef<str> + serde::Serialize + Debug>(msg: T) -> String {
         json!({ "fault_message": msg }).to_string()
     }
 }
@@ -443,7 +445,7 @@ mod tests {
         let to_vmm_fd = EventFd::new(libc::EFD_NONBLOCK).unwrap();
         let (api_request_sender, _from_api) = channel();
         let (to_api, vmm_response_receiver) = channel();
-        let seccomp_filters = get_filters(SeccompConfig::Advanced).unwrap();
+        let seccomp_filters = get_filters(SeccompConfig::<std::io::Empty>::Advanced).unwrap();
         let (socket_ready_sender, socket_ready_receiver) = channel();
 
         thread::Builder::new()
@@ -491,7 +493,7 @@ mod tests {
         let to_vmm_fd = EventFd::new(libc::EFD_NONBLOCK).unwrap();
         let (api_request_sender, _from_api) = channel();
         let (_to_api, vmm_response_receiver) = channel();
-        let seccomp_filters = get_filters(SeccompConfig::Advanced).unwrap();
+        let seccomp_filters = get_filters(SeccompConfig::<std::io::Empty>::Advanced).unwrap();
         let (socket_ready_sender, socket_ready_receiver) = channel();
 
         thread::Builder::new()

src/api_server/src/lib.rs

@@ -1,6 +1,8 @@
 // Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
+use std::fmt::Debug;
+
 use logger::{error, info};
 use micro_http::{Body, Method, Request, Response, StatusCode, Version};
 use serde::ser::Serialize;
@@ -27,14 +29,13 @@ use crate::request::version::parse_get_version;
 use crate::request::vsock::parse_put_vsock;
 use crate::ApiServer;
 
-#[cfg_attr(test, derive(Debug))]
+#[derive(Debug)]
 pub(crate) enum RequestAction {
     Sync(Box<VmmAction>),
     ShutdownInternal, // !!! not an API, used by shutdown to thread::join the API thread
 }
 
-#[derive(Default)]
-#[cfg_attr(test, derive(Debug, PartialEq))]
+#[derive(Debug, Default, PartialEq)]
 pub(crate) struct ParsingInfo {
     deprecation_message: Option<String>,
 }
@@ -52,7 +53,7 @@ impl ParsingInfo {
     }
 }
 
-#[cfg_attr(test, derive(Debug))]
+#[derive(Debug)]
 pub(crate) struct ParsedRequest {
     action: RequestAction,
     parsing_info: ParsingInfo,
@@ -141,7 +142,7 @@ impl ParsedRequest {
 
     pub(crate) fn success_response_with_data<T>(body_data: &T) -> Response
     where
-        T: ?Sized + Serialize,
+        T: ?Sized + Serialize + Debug,
     {
         info!("The request was executed successfully. Status code: 200 OK.");
         let mut response = Response::new(Version::Http11, StatusCode::OK);

src/api_server/src/parsed_request.rs

@@ -100,7 +100,7 @@ pub fn build_microvm_from_config(config: &str) -> Result<(Arc<Mutex<Vmm>>, VmRes
     let vm_resources = VmResources::from_json(config, &instance_info, HTTP_MAX_PAYLOAD_SIZE, None)
         .map_err(Error::CreateVmResources)?;
     let mut event_manager = EventManager::new().unwrap();
-    let seccomp_filters = get_filters(SeccompConfig::None).unwrap();
+    let seccomp_filters = get_filters(SeccompConfig::<std::io::Empty>::None).unwrap();
 
     // Build a microVM.
     let vmm = build_microvm_for_boot(

src/cpu-template-helper/src/utils/mod.rs

@@ -48,17 +48,19 @@ impl ByteBuffer for [u8] {
 
 #[cfg(test)]
 mod tests {
+    use std::fmt::Debug;
+
     use super::*;
 
-    fn bb_len<T: ByteBuffer + ?Sized>(buf: &T) -> usize {
+    fn bb_len<T: ByteBuffer + ?Sized + Debug>(buf: &T) -> usize {
         buf.len()
     }
 
-    fn bb_is_empty<T: ByteBuffer + ?Sized>(buf: &T) -> bool {
+    fn bb_is_empty<T: ByteBuffer + ?Sized + Debug>(buf: &T) -> bool {
         buf.len() == 0
     }
 
-    fn bb_read_from_1<T: ByteBuffer + ?Sized>(src: &T, dst: &mut [u8]) {
+    fn bb_read_from_1<T: ByteBuffer + ?Sized + Debug>(src: &T, dst: &mut [u8]) {
         src.read_to_slice(1, dst);
     }
 

src/dumbo/src/lib.rs

@@ -65,6 +65,7 @@ pub enum Error {
 /// ARP is a generic protocol as far as data
 /// link layer and network layer protocols go, but this particular implementation is concerned with
 /// ARP frames related to IPv4 over Ethernet.
+#[derive(Debug)]
 pub struct EthIPv4ArpFrame<'a, T: 'a> {
     bytes: InnerBytes<'a, T>,
 }
@@ -338,23 +339,17 @@ pub fn test_speculative_tpa(buf: &[u8], addr: Ipv4Addr) -> bool {
 
 #[cfg(test)]
 mod tests {
-    use std::fmt;
+    use std::str::FromStr;
 
     use super::*;
 
-    impl<'a, T: NetworkBytes> fmt::Debug for EthIPv4ArpFrame<'a, T> {
-        fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-            write!(f, "(EthIPv4ArpFrame frame)")
-        }
-    }
-
     #[test]
     fn test_eth_ipv4_arp_frame() {
         let mut a = [0u8; 1000];
         let mut bad_array = [0u8; 1];
 
-        let sha = MacAddr::parse_str("01:23:45:67:89:ab").unwrap();
-        let tha = MacAddr::parse_str("cd:ef:01:23:45:67").unwrap();
+        let sha = MacAddr::from_str("01:23:45:67:89:ab").unwrap();
+        let tha = MacAddr::from_str("cd:ef:01:23:45:67").unwrap();
         let spa = Ipv4Addr::new(10, 1, 2, 3);
         let tpa = Ipv4Addr::new(10, 4, 5, 6);
 

src/dumbo/src/pdu/arp.rs

@@ -151,6 +151,7 @@ impl<'a> NetworkBytesMut for &'a mut [u8] {}
 // This struct is used as a convenience for any type which contains a generic member implementing
 // NetworkBytes with a lifetime, so we don't have to also add the PhantomData member each time. We
 // use pub(super) here because we only want this to be usable by the child modules of `pdu`.
+#[derive(Debug)]
 pub(super) struct InnerBytes<'a, T: 'a> {
     bytes: T,
     phantom: PhantomData<&'a T>,

src/dumbo/src/pdu/bytes.rs

@@ -4,6 +4,7 @@
 //! Contains support for parsing and writing Ethernet frames. Does not currently offer support for
 //! 802.1Q tags.
 
+use std::fmt::Debug;
 use std::result::Result;
 
 use super::bytes::{InnerBytes, NetworkBytes, NetworkBytesMut};
@@ -33,6 +34,7 @@ pub enum Error {
 }
 
 /// Interprets the inner bytes as an Ethernet frame.
+#[derive(Debug)]
 pub struct EthernetFrame<'a, T: 'a> {
     bytes: InnerBytes<'a, T>,
 }
@@ -99,7 +101,7 @@ impl<'a, T: NetworkBytes> EthernetFrame<'a, T> {
     }
 }
 
-impl<'a, T: NetworkBytesMut> EthernetFrame<'a, T> {
+impl<'a, T: NetworkBytesMut + Debug> EthernetFrame<'a, T> {
     /// Attempts to write an Ethernet frame using the given header fields to `buf`.
     fn new_with_header(
         buf: T,
@@ -183,23 +185,17 @@ impl<'a, T: NetworkBytes> Incomplete<EthernetFrame<'a, T>> {
 
 #[cfg(test)]
 mod tests {
-    use std::fmt;
+    use std::str::FromStr;
 
     use super::*;
 
-    impl<'a, T: NetworkBytes> fmt::Debug for EthernetFrame<'a, T> {
-        fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-            write!(f, "(Ethernet frame)")
-        }
-    }
-
     #[test]
     fn test_ethernet_frame() {
         let mut a = [0u8; 10000];
         let mut bad_array = [0u8; 1];
 
-        let dst_mac = MacAddr::parse_str("01:23:45:67:89:ab").unwrap();
-        let src_mac = MacAddr::parse_str("cd:ef:01:23:45:67").unwrap();
+        let dst_mac = MacAddr::from_str("01:23:45:67:89:ab").unwrap();
+        let src_mac = MacAddr::from_str("cd:ef:01:23:45:67").unwrap();
         let ethertype = 1289;
 
         assert_eq!(

src/dumbo/src/pdu/ethernet.rs

@@ -8,6 +8,7 @@
 //! [here]: https://en.wikipedia.org/wiki/IPv4#Packet_structure
 
 use std::convert::From;
+use std::fmt::Debug;
 use std::net::Ipv4Addr;
 use std::result::Result;
 
@@ -55,6 +56,7 @@ pub enum Error {
 }
 
 /// Interprets the inner bytes as an IPv4 packet.
+#[derive(Debug)]
 pub struct IPv4Packet<'a, T: 'a> {
     bytes: InnerBytes<'a, T>,
 }
@@ -247,7 +249,7 @@ impl<'a, T: NetworkBytes> IPv4Packet<'a, T> {
     }
 }
 
-impl<'a, T: NetworkBytesMut> IPv4Packet<'a, T> {
+impl<'a, T: NetworkBytesMut + Debug> IPv4Packet<'a, T> {
     /// Attempts to write an IPv4 packet header to `buf`, making sure there is enough space.
     ///
     /// This method returns an incomplete packet, because the size of the payload might be unknown
@@ -381,7 +383,7 @@ impl<'a, T: NetworkBytesMut> IPv4Packet<'a, T> {
 /// It can be transformed into an `IPv4Packet` by specifying the size of the payload, and
 /// shrinking the inner byte sequence to be as large as the packet itself (this includes setting
 /// the `total length` header field).
-impl<'a, T: NetworkBytesMut> Incomplete<IPv4Packet<'a, T>> {
+impl<'a, T: NetworkBytesMut + Debug> Incomplete<IPv4Packet<'a, T>> {
     /// Transforms `self` into an `IPv4Packet` based on the supplied header and payload length. May
     /// panic for invalid values of the input parameters.
     ///
@@ -466,25 +468,11 @@ pub fn test_speculative_dst_addr(buf: &[u8], addr: Ipv4Addr) -> bool {
 
 #[cfg(test)]
 mod tests {
-    use std::fmt;
-
     use super::*;
     use crate::MacAddr;
 
     const MAX_HEADER_LEN: usize = 60;
 
-    impl<'a, T: NetworkBytes> fmt::Debug for IPv4Packet<'a, T> {
-        fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-            write!(f, "(IPv4 packet)")
-        }
-    }
-
-    impl<'a, T: NetworkBytes> fmt::Debug for Incomplete<IPv4Packet<'a, T>> {
-        fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-            write!(f, "(Incomplete IPv4 packet)")
-        }
-    }
-
     #[test]
     fn test_set_get() {
         let mut a = [0u8; 100];