From 18ae8a991d4e65b1c203c7b28adf3403ba9da41c Mon Sep 17 00:00:00 2001 From: Harshavardhan Unnibhavi Date: Thu, 8 Jul 2021 19:30:26 +0200 Subject: Inflight I/O: Make structs and members public Signed-off-by: Harshavardhan Unnibhavi --- src/vhost_user/message.rs | 44 ++++++++++++++++---------------------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/src/vhost_user/message.rs b/src/vhost_user/message.rs index 76feca2..618940e 100644 --- a/src/vhost_user/message.rs +++ b/src/vhost_user/message.rs @@ -783,8 +783,8 @@ impl VhostUserMsgValidator for VhostUserFSSlaveMsg { /// Inflight I/O descriptor state for split virtqueues #[repr(packed)] -#[derive(Clone, Copy)] -struct DescStateSplit { +#[derive(Clone, Copy, Default)] +pub struct DescStateSplit { /// Indicate whether this descriptor (only head) is inflight or not. inflight: u8, /// Padding @@ -796,20 +796,16 @@ struct DescStateSplit { } impl DescStateSplit { - fn new() -> Self { - DescStateSplit { - inflight: 0, - padding: [0; 5], - next: 0, - counter: 0, - } + /// New instance of DescStateSplit struct + pub fn new() -> Self { + Self::default() } } /// Inflight I/O queue region for split virtqueues #[allow(safe_packed_borrows)] #[repr(packed)] -struct QueueRegionSplit { +pub struct QueueRegionSplit { /// Features flags of this region features: u64, /// Version of this region @@ -825,7 +821,8 @@ struct QueueRegionSplit { } impl QueueRegionSplit { - fn new(features: u64, queue_size: u16) -> Self { + /// New instance of QueueRegionSplit struct + pub fn new(features: u64, queue_size: u16) -> Self { QueueRegionSplit { features, version: 1, @@ -839,8 +836,8 @@ impl QueueRegionSplit { /// Inflight I/O descriptor state for packed virtqueues #[repr(packed)] -#[derive(Clone, Copy)] -struct DescStatePacked { +#[derive(Clone, Copy, Default)] +pub struct DescStatePacked { /// Indicate whether this descriptor (only head) is inflight or not. inflight: u8, /// Padding @@ -864,26 +861,16 @@ struct DescStatePacked { } impl DescStatePacked { - fn new() -> Self { - DescStatePacked { - inflight: 0, - padding: 0, - next: 0, - last: 0, - num: 0, - counter: 0, - id: 0, - flags: 0, - len: 0, - addr: 0, - } + /// New instance of DescStatePacked struct + pub fn new() -> Self { + Self::default() } } /// Inflight I/O queue region for packed virtqueues #[allow(safe_packed_borrows)] #[repr(packed)] -struct QueueRegionPacked { +pub struct QueueRegionPacked { /// Features flags of this region features: u64, /// version of this region @@ -909,7 +896,8 @@ struct QueueRegionPacked { } impl QueueRegionPacked { - fn new(features: u64, queue_size: u16) -> Self { + /// New instance of QueueRegionPacked struct + pub fn new(features: u64, queue_size: u16) -> Self { QueueRegionPacked { features, version: 1, -- cgit v1.2.3 From 99fbfc969287ea87398d3755bd2613c14d77579f Mon Sep 17 00:00:00 2001 From: Harshavardhan Unnibhavi Date: Wed, 21 Jul 2021 10:25:37 +0200 Subject: x86_64 coverage: reduce by 0.6 Signed-off-by: Harshavardhan Unnibhavi --- coverage_config_x86_64.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coverage_config_x86_64.json b/coverage_config_x86_64.json index 644f4bc..8f24950 100644 --- a/coverage_config_x86_64.json +++ b/coverage_config_x86_64.json @@ -1 +1 @@ -{"coverage_score": 82.8, "exclude_path": "src/vhost_kern/", "crate_features": "vhost-user-master,vhost-user-slave"} +{"coverage_score": 82.2, "exclude_path": "src/vhost_kern/", "crate_features": "vhost-user-master,vhost-user-slave"} -- cgit v1.2.3 From c1f77c778bc482395bb1f9d0d4961e67f30c83eb Mon Sep 17 00:00:00 2001 From: Harshavardhan Unnibhavi Date: Sun, 25 Jul 2021 21:37:17 +0200 Subject: Inflight I/O: Make struct members public Signed-off-by: Harshavardhan Unnibhavi --- src/vhost_user/message.rs | 56 +++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/src/vhost_user/message.rs b/src/vhost_user/message.rs index 618940e..f6eead2 100644 --- a/src/vhost_user/message.rs +++ b/src/vhost_user/message.rs @@ -786,13 +786,13 @@ impl VhostUserMsgValidator for VhostUserFSSlaveMsg { #[derive(Clone, Copy, Default)] pub struct DescStateSplit { /// Indicate whether this descriptor (only head) is inflight or not. - inflight: u8, + pub inflight: u8, /// Padding padding: [u8; 5], /// List of last batch of used descriptors, only when batching is used for submitting - next: u16, + pub next: u16, /// Preserve order of fetching available descriptors, only for head descriptor - counter: u64, + pub counter: u64, } impl DescStateSplit { @@ -807,17 +807,17 @@ impl DescStateSplit { #[repr(packed)] pub struct QueueRegionSplit { /// Features flags of this region - features: u64, + pub features: u64, /// Version of this region - version: u16, + pub version: u16, /// Number of DescStateSplit entries - desc_num: u16, + pub desc_num: u16, /// List to track last batch of used descriptors - last_batch_head: u16, + pub last_batch_head: u16, /// Idx value of used ring - used_idx: u16, + pub used_idx: u16, /// Pointer to an array of DescStateSplit entries - desc: u64, + pub desc: u64, } impl QueueRegionSplit { @@ -839,25 +839,25 @@ impl QueueRegionSplit { #[derive(Clone, Copy, Default)] pub struct DescStatePacked { /// Indicate whether this descriptor (only head) is inflight or not. - inflight: u8, + pub inflight: u8, /// Padding padding: u8, /// Link to next free entry - next: u16, + pub next: u16, /// Link to last entry of descriptor list, only for head - last: u16, + pub last: u16, /// Length of descriptor list, only for head - num: u16, + pub num: u16, /// Preserve order of fetching avail descriptors, only for head - counter: u64, + pub counter: u64, /// Buffer ID - id: u16, + pub id: u16, /// Descriptor flags - flags: u16, + pub flags: u16, /// Buffer length - len: u32, + pub len: u32, /// Buffer address - addr: u64, + pub addr: u64, } impl DescStatePacked { @@ -872,27 +872,27 @@ impl DescStatePacked { #[repr(packed)] pub struct QueueRegionPacked { /// Features flags of this region - features: u64, + pub features: u64, /// version of this region - version: u16, + pub version: u16, /// size of descriptor state array - desc_num: u16, + pub desc_num: u16, /// head of free DescStatePacked entry list - free_head: u16, + pub free_head: u16, /// old head of free DescStatePacked entry list - old_free_head: u16, + pub old_free_head: u16, /// used idx of descriptor ring - used_idx: u16, + pub used_idx: u16, /// old used idx of descriptor ring - old_used_idx: u16, + pub old_used_idx: u16, /// device ring wrap counter - used_wrap_counter: u8, + pub used_wrap_counter: u8, /// old device ring wrap counter - old_used_wrap_counter: u8, + pub old_used_wrap_counter: u8, /// Padding padding: [u8; 7], /// Pointer to array tracking state of each descriptor from descriptor ring - desc: u64, + pub desc: u64, } impl QueueRegionPacked { -- cgit v1.2.3 From 488b3adc2f26d150def93949d51cfdd37528a37d Mon Sep 17 00:00:00 2001 From: wanglei01 Date: Thu, 29 Jul 2021 11:30:49 +0800 Subject: fix warning: unaligned_references fix warning, when compiling with 1.53.0 ``` warning: reference to packed field is unaligned --> src/vhost_user/message.rs:252:53 | 252 | unsafe { std::mem::transmute_copy::(&self.request) } | ^^^^^^^^^^^^^ | = note: `#[warn(unaligned_references)]` on by default = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #82523 = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) ``` Signed-off-by: wanglei --- coverage_config_x86_64.json | 2 +- src/vhost_user/message.rs | 32 +++++++++++++++++++++++++++----- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/coverage_config_x86_64.json b/coverage_config_x86_64.json index 8f24950..c3e6939 100644 --- a/coverage_config_x86_64.json +++ b/coverage_config_x86_64.json @@ -1 +1 @@ -{"coverage_score": 82.2, "exclude_path": "src/vhost_kern/", "crate_features": "vhost-user-master,vhost-user-slave"} +{"coverage_score": 82.3, "exclude_path": "src/vhost_kern/", "crate_features": "vhost-user-master,vhost-user-slave"} diff --git a/src/vhost_user/message.rs b/src/vhost_user/message.rs index f6eead2..fc33e1b 100644 --- a/src/vhost_user/message.rs +++ b/src/vhost_user/message.rs @@ -223,9 +223,8 @@ bitflags! { /// Common message header for vhost-user requests and replies. /// A vhost-user message consists of 3 header fields and an optional payload. All numbers are in the /// machine native byte order. -#[allow(safe_packed_borrows)] #[repr(packed)] -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Copy)] pub(super) struct VhostUserMsgHeader { request: u32, flags: u32, @@ -233,6 +232,28 @@ pub(super) struct VhostUserMsgHeader { _r: PhantomData, } +impl Debug for VhostUserMsgHeader { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Point") + .field("request", &{ self.request }) + .field("flags", &{ self.flags }) + .field("size", &{ self.size }) + .finish() + } +} + +impl Clone for VhostUserMsgHeader { + fn clone(&self) -> VhostUserMsgHeader { + *self + } +} + +impl PartialEq for VhostUserMsgHeader { + fn eq(&self, other: &Self) -> bool { + self.request == other.request && self.flags == other.flags && self.size == other.size + } +} + impl VhostUserMsgHeader { /// Create a new instance of `VhostUserMsgHeader`. pub fn new(request: R, flags: u32, size: u32) -> Self { @@ -249,7 +270,7 @@ impl VhostUserMsgHeader { /// Get message type. pub fn get_code(&self) -> R { // It's safe because R is marked as repr(u32). - unsafe { std::mem::transmute_copy::(&self.request) } + unsafe { std::mem::transmute_copy::(&{ self.request }) } } /// Set message type. @@ -803,7 +824,6 @@ impl DescStateSplit { } /// Inflight I/O queue region for split virtqueues -#[allow(safe_packed_borrows)] #[repr(packed)] pub struct QueueRegionSplit { /// Features flags of this region @@ -868,7 +888,6 @@ impl DescStatePacked { } /// Inflight I/O queue region for packed virtqueues -#[allow(safe_packed_borrows)] #[repr(packed)] pub struct QueueRegionPacked { /// Features flags of this region @@ -994,7 +1013,10 @@ mod tests { hdr.set_version(0x1); assert!(hdr.is_valid()); + // Test Debug, Clone, PartiaEq trait assert_eq!(hdr, hdr.clone()); + assert_eq!(hdr.clone().get_code(), hdr.get_code()); + assert_eq!(format!("{:?}", hdr.clone()), format!("{:?}", hdr)); } #[test] -- cgit v1.2.3 From d65bd280d9f4e192a884f1761e4b097c11aae6de Mon Sep 17 00:00:00 2001 From: Chirantan Ekbote Date: Tue, 22 Jun 2021 19:31:54 +0900 Subject: CHROMIUM: Use File for device request fd There is nothing in the vhost-user documentation or spec that says this must be a socket for fs device requests. It's not even guaranteed that it is an fd for a UnixStream. Change the type to a File and leave it up to the backend to convert it into a more specialized type if necessary. Upstream PR: https://github.com/rust-vmm/vhost/pull/49 BUG=b:179755651 TEST=cargo test Cq-Depend: chromium:2987594 Change-Id: I52ce0c62e0627a87462be8e0a44fa534763f10ed Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/rust-vmm/vhost/+/2988140 Tested-by: Chirantan Ekbote Commit-Queue: Chirantan Ekbote Reviewed-by: Keiichi Watanabe --- src/vhost_user/slave_req_handler.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/vhost_user/slave_req_handler.rs b/src/vhost_user/slave_req_handler.rs index bbc935e..402030c 100644 --- a/src/vhost_user/slave_req_handler.rs +++ b/src/vhost_user/slave_req_handler.rs @@ -3,14 +3,13 @@ use std::fs::File; use std::mem; -use std::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; +use std::os::unix::io::{AsRawFd, RawFd}; use std::os::unix::net::UnixStream; use std::slice; use std::sync::{Arc, Mutex}; use super::connection::Endpoint; use super::message::*; -use super::slave_fs_cache::SlaveFsCacheReq; use super::{take_single_file, Error, Result}; /// Services provided to the master by the slave with interior mutability. @@ -62,7 +61,7 @@ pub trait VhostUserSlaveReqHandler { fn set_vring_enable(&self, index: u32, enable: bool) -> Result<()>; fn get_config(&self, offset: u32, size: u32, flags: VhostUserConfigFlags) -> Result>; fn set_config(&self, offset: u32, buf: &[u8], flags: VhostUserConfigFlags) -> Result<()>; - fn set_slave_req_fd(&self, _vu_req: SlaveFsCacheReq) {} + fn set_slave_req_fd(&self, _vu_req: File) {} fn get_inflight_fd(&self, inflight: &VhostUserInflight) -> Result<(VhostUserInflight, File)>; fn set_inflight_fd(&self, inflight: &VhostUserInflight, file: File) -> Result<()>; fn get_max_mem_slots(&self) -> Result; @@ -107,7 +106,7 @@ pub trait VhostUserSlaveReqHandlerMut { flags: VhostUserConfigFlags, ) -> Result>; fn set_config(&mut self, offset: u32, buf: &[u8], flags: VhostUserConfigFlags) -> Result<()>; - fn set_slave_req_fd(&mut self, _vu_req: SlaveFsCacheReq) {} + fn set_slave_req_fd(&mut self, _vu_req: File) {} fn get_inflight_fd( &mut self, inflight: &VhostUserInflight, @@ -201,7 +200,7 @@ impl VhostUserSlaveReqHandler for Mutex { self.lock().unwrap().set_config(offset, buf, flags) } - fn set_slave_req_fd(&self, vu_req: SlaveFsCacheReq) { + fn set_slave_req_fd(&self, vu_req: File) { self.lock().unwrap().set_slave_req_fd(vu_req) } @@ -639,9 +638,7 @@ impl SlaveReqHandler { fn set_slave_req_fd(&mut self, files: Option>) -> Result<()> { let file = take_single_file(files).ok_or(Error::InvalidMessage)?; - let sock = unsafe { UnixStream::from_raw_fd(file.into_raw_fd()) }; - let vu_req = SlaveFsCacheReq::from_stream(sock); - self.backend.set_slave_req_fd(vu_req); + self.backend.set_slave_req_fd(file); Ok(()) } -- cgit v1.2.3