patches and low-level development discussion
 help / color / mirror / code / Atom feed
blob bcfab6eace16cd33911e6045ee30faf82aee172c 2696 bytes (raw)
name: pkgs/applications/virtualization/crosvm/devices-vhost_user-loosen-expected-message-order.patch 	 # note: path name is non-authoritative(*)

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
 
From 6ac11f04ea3344ca0c3873ee9526a8863c9529ae Mon Sep 17 00:00:00 2001
From: Alyssa Ross <alyssa.ross@unikie.com>
Date: Wed, 28 Sep 2022 15:27:39 +0000
Subject: [PATCH crosvm 3/3] devices: vhost_user: loosen expected message order

Prior to this patch, the code assumed get_shared_memory_regions()
would be called before set_slave_req_fd(), since
get_shared_memory_regions() was the only place that set self.shmid,
and set_slave_req_fd() required it to be set.  There's nothing in the
vhost-user spec that enforces this ordering though, so it could fail
when used with a non-crosvm frontend.

To fix this, just don't store the shmid here at all, and fetch it from
the backend when it's required.  set_slave_req_fd() should only be
called once, and the two backends that implement
get_shared_memory_region() (Wl and Gpu) both have trivial
implementations, so there shouldn't be any performance reason to cache
shmid here.

TEST=Run cloud-hypervisor against a crosvm vhost-user backend

Change-Id: Idb44aeb1dfe1aeff70081987fe6d7d215887d2e4
---
 devices/src/virtio/vhost/user/device/handler.rs | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/devices/src/virtio/vhost/user/device/handler.rs b/devices/src/virtio/vhost/user/device/handler.rs
index 932d948959..27b4eb1619 100644
--- a/devices/src/virtio/vhost/user/device/handler.rs
+++ b/devices/src/virtio/vhost/user/device/handler.rs
@@ -408,7 +408,6 @@ where
     mem: Option<GuestMemory>,
     backend: Box<dyn VhostUserBackend>,
     ops: O,
-    shmid: Option<u8>,
 }
 
 impl DeviceRequestHandler<VhostUserRegularOps> {
@@ -436,7 +435,6 @@ where
             mem: None,
             backend,
             ops,
-            shmid: None,
         }
     }
 }
@@ -696,7 +694,12 @@ impl<O: VhostUserPlatformOps> VhostUserSlaveReqHandlerMut for DeviceRequestHandl
     }
 
     fn set_slave_req_fd(&mut self, ep: Box<dyn Endpoint<SlaveReq>>) {
-        let shmid = self.shmid.expect("unexpected slave_req_fd");
+        let shmid = self
+            .backend
+            .get_shared_memory_region()
+            .expect("unexpected slave_req_fd")
+            .id;
+
         let frontend = Slave::new(ep);
         self.backend
             .set_shared_memory_mapper(Box::new(VhostShmemMapper {
@@ -738,7 +741,6 @@ impl<O: VhostUserPlatformOps> VhostUserSlaveReqHandlerMut for DeviceRequestHandl
 
     fn get_shared_memory_regions(&mut self) -> VhostResult<Vec<VhostSharedMemoryRegion>> {
         Ok(if let Some(r) = self.backend.get_shared_memory_region() {
-            self.shmid = Some(r.id);
             vec![VhostSharedMemoryRegion::new(r.id, r.length)]
         } else {
             Vec::new()
-- 
2.37.1


debug log:

solving bcfab6eace1 ...
found bcfab6eace1 in https://inbox.spectrum-os.org/spectrum-devel/20220928170128.1583791-6-alyssa.ross@unikie.com/ ||
	https://inbox.spectrum-os.org/spectrum-devel/20220930210906.1696349-6-alyssa.ross@unikie.com/

applying [1/1] https://inbox.spectrum-os.org/spectrum-devel/20220928170128.1583791-6-alyssa.ross@unikie.com/
diff --git a/pkgs/applications/virtualization/crosvm/devices-vhost_user-loosen-expected-message-order.patch b/pkgs/applications/virtualization/crosvm/devices-vhost_user-loosen-expected-message-order.patch
new file mode 100644
index 00000000000..bcfab6eace1

1:43: trailing whitespace.
 
1:55: trailing whitespace.
 
1:68: trailing whitespace.
 
1:75: trailing whitespace.
-- 
Checking patch pkgs/applications/virtualization/crosvm/devices-vhost_user-loosen-expected-message-order.patch...
1:77: new blank line at EOF.
+
Applied patch pkgs/applications/virtualization/crosvm/devices-vhost_user-loosen-expected-message-order.patch cleanly.
warning: 5 lines add whitespace errors.

skipping https://inbox.spectrum-os.org/spectrum-devel/20220930210906.1696349-6-alyssa.ross@unikie.com/ for bcfab6eace1
index at:
100644 bcfab6eace16cd33911e6045ee30faf82aee172c	pkgs/applications/virtualization/crosvm/devices-vhost_user-loosen-expected-message-order.patch

(*) Git path names are given by the tree(s) the blob belongs to.
    Blobs themselves have no identifier aside from the hash of its contents.^

Code repositories for project(s) associated with this public inbox

	https://spectrum-os.org/git/crosvm
	https://spectrum-os.org/git/doc
	https://spectrum-os.org/git/mktuntap
	https://spectrum-os.org/git/nixpkgs
	https://spectrum-os.org/git/spectrum
	https://spectrum-os.org/git/ucspi-vsock
	https://spectrum-os.org/git/www

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).