From 5847489855b8ce7628928d54641246db7b2bd64b Mon Sep 17 00:00:00 2001 From: Stefano Garzarella Date: Mon, 6 Nov 2023 15:53:51 +0100 Subject: [PATCH] vsock: increase test coverage Add more tests in vhost-device-vsock to increase the coverage. Some of them are really simple and perhaps nonsensical (tests for Debug, Clone, etc.), but for now we don't know how to disable this in the tool, so let's cover these cases. The vhost-device-vsock functions coverage increases from 70.73% to 90.45%: Filename Functions Missed Functions Executed ---------------------------------------------------------- main.rs 51 12 76.47% rxops.rs 8 0 100.00% rxqueue.rs 20 0 100.00% thread_backend.rs 20 3 85.00% txbuf.rs 17 0 100.00% vhu_vsock.rs 37 1 97.30% vhu_vsock_thread.rs 40 5 87.50% vsock_conn.rs 27 0 100.00% ---------------------------------------------------------- TOTAL 220 21 90.45% Closes #229 Signed-off-by: Stefano Garzarella --- Cargo.lock | 1 + coverage_config_x86_64.json | 2 +- vhost-device-vsock/Cargo.toml | 1 + vhost-device-vsock/src/main.rs | 54 +++++++++++++++++++-- vhost-device-vsock/src/rxops.rs | 8 ++++ vhost-device-vsock/src/rxqueue.rs | 7 +++ vhost-device-vsock/src/thread_backend.rs | 33 +++++++++++++ vhost-device-vsock/src/txbuf.rs | 10 ++++ vhost-device-vsock/src/vhu_vsock.rs | 24 ++++++++++ vhost-device-vsock/src/vhu_vsock_thread.rs | 56 ++++++++++++++++++++++ vhost-device-vsock/src/vsock_conn.rs | 4 +- 11 files changed, 194 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dd09f00..14806ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -963,6 +963,7 @@ dependencies = [ name = "vhost-device-vsock" version = "0.1.0" dependencies = [ + "assert_matches", "byteorder", "clap", "config", diff --git a/coverage_config_x86_64.json b/coverage_config_x86_64.json index bf64db5..57da641 100644 --- a/coverage_config_x86_64.json +++ b/coverage_config_x86_64.json @@ -1,5 +1,5 @@ { - "coverage_score": 73.42, + "coverage_score": 77.28, "exclude_path": "", "crate_features": "" } diff --git a/vhost-device-vsock/Cargo.toml b/vhost-device-vsock/Cargo.toml index ec90f38..ce76537 100644 --- a/vhost-device-vsock/Cargo.toml +++ b/vhost-device-vsock/Cargo.toml @@ -31,5 +31,6 @@ serde = { version = "1", features = ["derive"] } serde_yaml = "0.9" [dev-dependencies] +assert_matches = "1.5" virtio-queue = { version = "0.9", features = ["test-utils"] } tempfile = "3.6.0" diff --git a/vhost-device-vsock/src/main.rs b/vhost-device-vsock/src/main.rs index 1a0697e..bd757d7 100644 --- a/vhost-device-vsock/src/main.rs +++ b/vhost-device-vsock/src/main.rs @@ -320,6 +320,7 @@ fn main() { #[cfg(test)] mod tests { use super::*; + use assert_matches::assert_matches; use std::fs::File; use std::io::Write; use tempfile::tempdir; @@ -554,12 +555,19 @@ mod tests { ) .unwrap(); - let vring_workers = daemon.get_epoll_handlers(); + let mut epoll_handlers = daemon.get_epoll_handlers(); // VhostUserVsockBackend support a single thread that handles the TX and RX queues assert_eq!(backend.threads.len(), 1); - assert_eq!(vring_workers.len(), backend.threads.len()); + assert_eq!(epoll_handlers.len(), backend.threads.len()); + + for thread in backend.threads.iter() { + thread + .lock() + .unwrap() + .register_listeners(epoll_handlers.remove(0)); + } test_dir.close().unwrap(); } @@ -603,8 +611,48 @@ mod tests { ), ]; - start_backend_servers(&configs).unwrap_err(); + let error = start_backend_servers(&configs).unwrap_err(); + assert_matches!( + error, + BackendError::CouldNotCreateBackend(vhu_vsock::Error::CidAlreadyInUse) + ); + assert_eq!( + format!("{error:?}"), + "CouldNotCreateBackend(CidAlreadyInUse)" + ); test_dir.close().unwrap(); } + + #[test] + fn test_main_structs() { + let error = parse_vm_params("").unwrap_err(); + assert_matches!(error, VmArgsParseError::BadArgument); + assert_eq!(format!("{error:?}"), "BadArgument"); + + let args = VsockArgs { + param: None, + vm: None, + config: None, + }; + let error = Vec::::try_from(args).unwrap_err(); + assert_matches!(error, CliError::NoArgsProvided); + assert_eq!(format!("{error:?}"), "NoArgsProvided"); + + let args = VsockArgs::from_args(0, "", "", 0, ""); + assert_eq!(format!("{args:?}"), "VsockArgs { param: Some(VsockParam { guest_cid: 0, socket: \"\", uds_path: \"\", tx_buffer_size: 0, groups: \"\" }), vm: None, config: None }"); + + let param = args.param.unwrap().clone(); + assert_eq!(format!("{param:?}"), "VsockParam { guest_cid: 0, socket: \"\", uds_path: \"\", tx_buffer_size: 0, groups: \"\" }"); + + let config = ConfigFileVsockParam { + guest_cid: None, + socket: String::new(), + uds_path: String::new(), + tx_buffer_size: None, + groups: None, + } + .clone(); + assert_eq!(format!("{config:?}"), "ConfigFileVsockParam { guest_cid: None, socket: \"\", uds_path: \"\", tx_buffer_size: None, groups: None }"); + } } diff --git a/vhost-device-vsock/src/rxops.rs b/vhost-device-vsock/src/rxops.rs index 6f20466..76055d7 100644 --- a/vhost-device-vsock/src/rxops.rs +++ b/vhost-device-vsock/src/rxops.rs @@ -25,6 +25,14 @@ impl RxOps { mod tests { use super::*; + #[test] + fn test_rxops() { + let rx = RxOps::Request; + // Increase coverage testing Clone and Debug traits + assert_eq!(rx, rx.clone()); + assert_eq!(format!("{rx:?}"), "Request"); + } + #[test] fn test_bitmask() { assert_eq!(1, RxOps::Request.bitmask()); diff --git a/vhost-device-vsock/src/rxqueue.rs b/vhost-device-vsock/src/rxqueue.rs index 4b76727..4d6ab7f 100644 --- a/vhost-device-vsock/src/rxqueue.rs +++ b/vhost-device-vsock/src/rxqueue.rs @@ -154,4 +154,11 @@ mod tests { rxqueue.queue = 1; assert!(rxqueue.pending_rx()); } + + #[test] + fn test_debug() { + let rxqueue = RxQueue::new(); + + assert_eq!(format!("{rxqueue:?}"), "RxQueue { queue: 0 }"); + } } diff --git a/vhost-device-vsock/src/thread_backend.rs b/vhost-device-vsock/src/thread_backend.rs index 6d5e80e..2b9c4aa 100644 --- a/vhost-device-vsock/src/thread_backend.rs +++ b/vhost-device-vsock/src/thread_backend.rs @@ -419,6 +419,9 @@ mod tests { assert!(vtp.recv_pkt(&mut packet).is_ok()); + // TODO: it is a nop for now + vtp.enq_rst(); + // cleanup let _ = std::fs::remove_file(&vsock_peer_path); let _ = std::fs::remove_file(&vsock_socket_path); @@ -430,6 +433,7 @@ mod tests { fn test_vsock_thread_backend_sibling_vms() { const CID: u64 = 3; const SIBLING_CID: u64 = 4; + const SIBLING2_CID: u64 = 5; const SIBLING_LISTENING_PORT: u32 = 1234; let test_dir = tempdir().expect("Could not create a temp test directory."); @@ -449,6 +453,16 @@ mod tests { .join("test_vsock_thread_backend_sibling.vsock") .display() .to_string(); + let sibling2_vhost_socket_path = test_dir + .path() + .join("test_vsock_thread_backend_sibling2.socket") + .display() + .to_string(); + let sibling2_vsock_socket_path = test_dir + .path() + .join("test_vsock_thread_backend_sibling2.vsock") + .display() + .to_string(); let cid_map: Arc> = Arc::new(RwLock::new(HashMap::new())); @@ -463,8 +477,18 @@ mod tests { .collect(), ); + let sibling2_config = VsockConfig::new( + SIBLING2_CID, + sibling2_vhost_socket_path, + sibling2_vsock_socket_path, + CONN_TX_BUF_SIZE, + vec!["group1"].into_iter().map(String::from).collect(), + ); + let sibling_backend = Arc::new(VhostUserVsockBackend::new(sibling_config, cid_map.clone()).unwrap()); + let sibling2_backend = + Arc::new(VhostUserVsockBackend::new(sibling2_config, cid_map.clone()).unwrap()); let epoll_fd = epoll::create(false).unwrap(); @@ -513,6 +537,15 @@ mod tests { .thread_backend .pending_raw_pkts()); + packet.set_dst_cid(SIBLING2_CID); + assert!(vtp.send_pkt(&packet).is_ok()); + // packet should be discarded since sibling2 is not in the same group + assert!(!sibling2_backend.threads[0] + .lock() + .unwrap() + .thread_backend + .pending_raw_pkts()); + let mut recvd_pkt_raw = [0u8; PKT_HEADER_SIZE + DATA_LEN]; let (recvd_hdr_raw, recvd_data_raw) = recvd_pkt_raw.split_at_mut(PKT_HEADER_SIZE); diff --git a/vhost-device-vsock/src/txbuf.rs b/vhost-device-vsock/src/txbuf.rs index ef718d7..35df7f8 100644 --- a/vhost-device-vsock/src/txbuf.rs +++ b/vhost-device-vsock/src/txbuf.rs @@ -230,4 +230,14 @@ mod tests { assert_eq!(cmp_vec, data[..n]); } } + + #[test] + fn test_txbuf_debug() { + let loc_tx_buf = LocalTxBuf::new(1); + + assert_eq!( + format!("{loc_tx_buf:?}"), + "LocalTxBuf { buf: [0], head: 0, tail: 0 }" + ); + } } diff --git a/vhost-device-vsock/src/vhu_vsock.rs b/vhost-device-vsock/src/vhu_vsock.rs index 34ef99a..176fcd2 100644 --- a/vhost-device-vsock/src/vhu_vsock.rs +++ b/vhost-device-vsock/src/vhu_vsock.rs @@ -540,4 +540,28 @@ mod tests { test_dir.close().unwrap(); } + + #[test] + fn test_vhu_vsock_structs() { + let config = VsockConfig::new(0, String::new(), String::new(), 0, vec![String::new()]); + + assert_eq!(format!("{config:?}"), "VsockConfig { guest_cid: 0, socket: \"\", uds_path: \"\", tx_buffer_size: 0, groups: [\"\"] }"); + + let conn_map = ConnMapKey::new(0, 0); + assert_eq!( + format!("{conn_map:?}"), + "ConnMapKey { local_port: 0, peer_port: 0 }" + ); + assert_eq!(conn_map, conn_map.clone()); + + let virtio_config = VirtioVsockConfig::default(); + assert_eq!( + format!("{virtio_config:?}"), + "VirtioVsockConfig { guest_cid: Le64(0) }" + ); + assert_eq!(virtio_config, virtio_config.clone()); + + let error = Error::HandleEventNotEpollIn; + assert_eq!(format!("{error:?}"), "HandleEventNotEpollIn"); + } } diff --git a/vhost-device-vsock/src/vhu_vsock_thread.rs b/vhost-device-vsock/src/vhu_vsock_thread.rs index 850ad0c..c0dba70 100644 --- a/vhost-device-vsock/src/vhu_vsock_thread.rs +++ b/vhost-device-vsock/src/vhu_vsock_thread.rs @@ -730,6 +730,7 @@ impl Drop for VhostUserVsockThread { mod tests { use super::*; use std::collections::HashMap; + use std::io::Write; use tempfile::tempdir; use vm_memory::GuestAddress; use vmm_sys_util::eventfd::EventFd; @@ -806,6 +807,21 @@ mod tests { .push_back(ConnMapKey::new(0, 0)); assert!(t.process_rx(&vring, false).is_ok()); assert!(t.process_rx(&vring, true).is_ok()); + assert!(t.process_raw_pkts(&vring, false).is_ok()); + assert!(t.process_raw_pkts(&vring, true).is_ok()); + + VhostUserVsockThread::vring_handle_event(EventData { + vring: vring.clone(), + event_idx: false, + head_idx: 0, + used_len: 0, + }); + VhostUserVsockThread::vring_handle_event(EventData { + vring, + event_idx: true, + head_idx: 0, + used_len: 0, + }); dummy_fd.write(1).unwrap(); @@ -875,6 +891,46 @@ mod tests { VhostUserVsockThread::new(vsock_socket_path2, 3, CONN_TX_BUF_SIZE, groups, cid_map); assert!(t2.is_err()); + test_dir.close().unwrap(); + } + #[test] + fn test_vsock_thread_unix() { + let groups: Vec = vec![String::from("default")]; + let cid_map: Arc> = Arc::new(RwLock::new(HashMap::new())); + + let test_dir = tempdir().expect("Could not create a temp test directory."); + let vsock_path = test_dir + .path() + .join("test_vsock_thread.vsock") + .display() + .to_string(); + + let t = VhostUserVsockThread::new(vsock_path.clone(), 3, CONN_TX_BUF_SIZE, groups, cid_map); + + let mut t = t.unwrap(); + + let mem = GuestMemoryAtomic::new( + GuestMemoryMmap::<()>::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap(), + ); + + t.mem = Some(mem.clone()); + + let mut uds = UnixStream::connect(vsock_path).unwrap(); + t.process_backend_evt(EventSet::empty()); + + uds.write_all(b"CONNECT 1234\n").unwrap(); + t.process_backend_evt(EventSet::empty()); + + // Write and read something from the Unix socket + uds.write_all(b"some data").unwrap(); + + let mut buf = vec![0u8; 16]; + uds.set_nonblocking(true).unwrap(); + // There isn't any peer responding, so we don't expect data + uds.read(&mut buf).unwrap_err(); + + t.process_backend_evt(EventSet::empty()); + test_dir.close().unwrap(); } } diff --git a/vhost-device-vsock/src/vsock_conn.rs b/vhost-device-vsock/src/vsock_conn.rs index 0a766df..54e6139 100644 --- a/vhost-device-vsock/src/vsock_conn.rs +++ b/vhost-device-vsock/src/vsock_conn.rs @@ -22,7 +22,6 @@ use crate::{ vhu_vsock_thread::VhostUserVsockThread, }; -#[derive(Debug)] pub(crate) struct VsockConnection { /// Host-side stream corresponding to this vsock connection. pub stream: S, @@ -519,7 +518,8 @@ mod tests { #[test] fn test_vsock_conn_init() { // new locally inititated connection - let dummy_file = VsockDummySocket::new(); + let mut dummy_file = VsockDummySocket::new(); + assert!(dummy_file.flush().is_ok()); let mut conn_local = VsockConnection::new_local_init( dummy_file, VSOCK_HOST_CID,