Skip to content

Added snapshot feature for vhost-device-vsock. #827

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion vhost-device-vsock/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ env_logger = "0.11"
epoll = "4.3.2"
log = "0.4"
thiserror = "2.0"
vhost = { version = "0.13", features = ["vhost-user-backend"] }
vhost = { git = "https://github.com/WeiChungHsu/vhost.git", branch = "main", features = ["vhost-user-backend"] }
vhost-user-backend = "0.17"
virtio-bindings = "0.2.5"
virtio-queue = "0.14"
Expand Down
68 changes: 65 additions & 3 deletions vhost-device-vsock/src/vhu_vsock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,19 @@

use std::{
collections::{HashMap, HashSet},
io::{self, Result as IoResult},
fs::File,
io::{self, BufReader, Read, Result as IoResult},
path::PathBuf,
sync::{Arc, Mutex, RwLock},
thread::{self, JoinHandle},
};

use log::warn;
use thiserror::Error as ThisError;
use vhost::vhost_user::message::{VhostUserProtocolFeatures, VhostUserVirtioFeatures};
use vhost::vhost_user::message::{
VhostTransferStateDirection, VhostTransferStatePhase, VhostUserProtocolFeatures,
VhostUserVirtioFeatures
};
use vhost_user_backend::{VhostUserBackend, VringRwLock};
use virtio_bindings::bindings::{
virtio_config::VIRTIO_F_NOTIFY_ON_EMPTY, virtio_config::VIRTIO_F_VERSION_1,
Expand Down Expand Up @@ -258,6 +263,7 @@ pub(crate) struct VhostUserVsockBackend {
pub threads: Vec<Mutex<VhostUserVsockThread>>,
queues_per_thread: Vec<u64>,
pub exit_event: EventFd,
pub handler_thread: Mutex<Option<JoinHandle<()>>>,
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this whole abstraction is needed, because if a thread is created, then Option<JoinHandle<()>> is not None. Plus, created field does not guard against handle being None because even if they are guarded under the same mutex, they are accessed separately (!). Change the handler_thread field to Mutex<Option<JoinHandle<()>>> and in check_device_state function you can lock the mutex and use Option::take to move the thread handle and join it if it's not None.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just fixed the codes based on the suggestion. Please let me know if it meets your expectation.


impl VhostUserVsockBackend {
Expand All @@ -279,6 +285,7 @@ impl VhostUserVsockBackend {
threads: vec![thread],
queues_per_thread,
exit_event: EventFd::new(EFD_NONBLOCK).map_err(Error::EventFdCreate)?,
handler_thread: Mutex::new(None),
})
}
}
Expand All @@ -303,7 +310,62 @@ impl VhostUserBackend for VhostUserVsockBackend {
}

fn protocol_features(&self) -> VhostUserProtocolFeatures {
VhostUserProtocolFeatures::MQ | VhostUserProtocolFeatures::CONFIG
VhostUserProtocolFeatures::MQ
| VhostUserProtocolFeatures::CONFIG
| VhostUserProtocolFeatures::DEVICE_STATE
}

#[allow(unused_variables)]
fn set_device_state_fd(
&self,
direction: VhostTransferStateDirection,
phase: VhostTransferStatePhase,
file: File,
) -> IoResult<Option<File>> {
let handle = thread::spawn(move || {
match direction {
VhostTransferStateDirection::SAVE => {
// save
// No device state to save yet, just close the FD.
drop(file);
}
VhostTransferStateDirection::LOAD => {
// load
// No device state to load yet, just verify it is empty.
let mut data = Vec::new();
let mut reader = BufReader::new(file);
if reader.read_to_end(&mut data).is_err() {
println!("vhost-device-vsock loaded device state read failed");
return;
}

if data.len() > 0 {
println!("vhost-device-vsock loaded device state is non-empty. BUG!");
return;
}
}
_ => {
println!("invalid transfer_direction");
return;
}
}
});
*self.handler_thread.lock().unwrap() = Some(handle);
Ok(None)
}

fn check_device_state(&self) -> IoResult<()> {
if !self.handler_thread.lock().unwrap().is_none() {
self.handler_thread
.lock()
.unwrap()
.take()
.unwrap()
.join()
.unwrap();
*self.handler_thread.lock().unwrap() = None;
}
Ok(())
}

fn set_event_idx(&self, enabled: bool) {
Expand Down
16 changes: 14 additions & 2 deletions vhost-device-vsock/src/vhu_vsock_thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,10 @@ impl VhostUserVsockThread {

let mut vring_mut = vring.get_mut();

if !vring_mut.get_enabled() {
return Ok(());
}

let queue = vring_mut.get_queue_mut();

while let Some(mut avail_desc) = queue
Expand Down Expand Up @@ -657,6 +661,8 @@ impl VhostUserVsockThread {
loop {
if !self.thread_backend.pending_rx() {
break;
} else if !vring.get_enabled() {
break;
}
vring.disable_notification().unwrap();

Expand All @@ -677,6 +683,8 @@ impl VhostUserVsockThread {
loop {
if !self.thread_backend.pending_raw_pkts() {
break;
} else if !vring.get_enabled() {
break;
}
vring.disable_notification().unwrap();

Expand Down Expand Up @@ -722,8 +730,12 @@ impl VhostUserVsockThread {
None => return Err(Error::NoMemoryConfigured),
};

while let Some(mut avail_desc) = vring
.get_mut()
let mut vring_mut = vring.get_mut();
if !vring_mut.get_enabled() {
return Ok(());
}

while let Some(mut avail_desc) = vring_mut
.get_queue_mut()
.iter(atomic_mem.memory())
.map_err(|_| Error::IterateQueue)?
Expand Down