Skip to content

Commit

Permalink
vsock: avoid circular references
Browse files Browse the repository at this point in the history
We have the following circular references found by Li Zebin:
    VhostUserBackend ==> VhostUserVsockThread ==> VringEpollHandler

In addition to causing a resource leak, this causes also an error
after we merged commit 38caab2 ("vsock: Don't allow duplicate CIDs").
When the VM reboot or shutdown, the application exits with the
following error:

    [ERROR vhost_device_vsock] Could not create backend:
        CID already in use by another vsock device

This happened because we have these circular references and
VhostUserVsockThread::drop() is never invoked. So, we don't remove
the cid from the map.

Let's fix this problem by simply removing the reference to
VringEpollHandler from VhostUserVsockThread. In fact, we do not
need to keep the reference for the lifetime of VhostUserVsockThread,
as we only need to add the handlers once.

Let's also rename the fields to follow the current VhostUserDaemon
API.

Closes #438

Reported-by: Li Zebin <cutelizebin@gmail.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
  • Loading branch information
stefano-garzarella authored and vireshk committed Oct 27, 2023
1 parent 1921ec2 commit 09e9da3
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 15 deletions.
4 changes: 2 additions & 2 deletions vhost-device-vsock/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,13 +234,13 @@ pub(crate) fn start_backend_server(
)
.map_err(BackendError::CouldNotCreateDaemon)?;

let mut vring_workers = daemon.get_epoll_handlers();
let mut epoll_handlers = daemon.get_epoll_handlers();

for thread in backend.threads.iter() {
thread
.lock()
.unwrap()
.set_vring_worker(Some(vring_workers.remove(0)));
.register_listeners(epoll_handlers.remove(0));
}

daemon.start(listener).unwrap();
Expand Down
18 changes: 5 additions & 13 deletions vhost-device-vsock/src/vhu_vsock_thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ pub(crate) struct VhostUserVsockThread {
host_sock_path: String,
/// Listener listening for new connections on the host.
host_listener: UnixListener,
/// Instance of VringWorker.
vring_worker: Option<Arc<VringEpollHandler<ArcVhostBknd, VringRwLock, ()>>>,
/// epoll fd to which new host connections are added.
epoll_file: File,
/// VsockThreadBackend instance.
Expand Down Expand Up @@ -151,7 +149,6 @@ impl VhostUserVsockThread {
host_sock: host_sock.as_raw_fd(),
host_sock_path: uds_path,
host_listener: host_sock,
vring_worker: None,
epoll_file,
thread_backend,
guest_cid,
Expand Down Expand Up @@ -242,20 +239,15 @@ impl VhostUserVsockThread {
self.epoll_file.as_raw_fd()
}

/// Set self's VringWorker.
pub fn set_vring_worker(
/// Register our listeners in the VringEpollHandler
pub fn register_listeners(
&mut self,
vring_worker: Option<Arc<VringEpollHandler<ArcVhostBknd, VringRwLock, ()>>>,
epoll_handler: Arc<VringEpollHandler<ArcVhostBknd, VringRwLock, ()>>,
) {
self.vring_worker = vring_worker;
self.vring_worker
.as_ref()
.unwrap()
epoll_handler
.register_listener(self.get_epoll_fd(), EventSet::IN, u64::from(BACKEND_EVENT))
.unwrap();
self.vring_worker
.as_ref()
.unwrap()
epoll_handler
.register_listener(
self.sibling_event_fd.as_raw_fd(),
EventSet::IN,
Expand Down

0 comments on commit 09e9da3

Please sign in to comment.