Skip to content

Commit f9e065d

Browse files
committed
refactor: Stop using AsFd hack in MaybeBounce
We put the bounds as AsFd to work around limitations in vm-memory about not implementing Read/WriteVolatile for &File and &mut File, but it ended up just causing even more workarounds being required elsewhere (vsock unittests, where AsFd cannot meaningfully be implemented for mocks). So just bite the bullet and clone some files until vm-memory gets some more impls. Signed-off-by: Patrick Roy <[email protected]>
1 parent d6cfe19 commit f9e065d

File tree

3 files changed

+21
-20
lines changed

3 files changed

+21
-20
lines changed

src/vmm/src/builder.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
66
use std::fmt::Debug;
77
use std::io::{self};
8+
use std::os::fd::AsFd;
89
use std::os::unix::fs::MetadataExt;
910
#[cfg(feature = "gdb")]
1011
use std::sync::mpsc;
@@ -274,7 +275,7 @@ pub fn build_microvm_for_boot(
274275
}
275276

276277
let entry_point = load_kernel(
277-
MaybeBounce::new(&boot_config.kernel_file, secret_free),
278+
MaybeBounce::new(boot_config.kernel_file.try_clone().unwrap(), secret_free),
278279
vmm.vm.guest_memory(),
279280
)?;
280281
let initrd = match &boot_config.initrd_file {
@@ -286,7 +287,7 @@ pub fn build_microvm_for_boot(
286287

287288
Some(InitrdConfig::from_reader(
288289
vmm.vm.guest_memory(),
289-
MaybeBounce::new(initrd_file, secret_free),
290+
MaybeBounce::new(initrd_file.as_fd(), secret_free),
290291
u64_to_usize(size),
291292
)?)
292293
}

src/vmm/src/vstate/memory.rs

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
use std::fs::File;
99
use std::io::{Read, Seek, SeekFrom, Write};
1010
use std::mem::ManuallyDrop;
11-
use std::os::fd::{AsFd, AsRawFd};
11+
use std::os::fd::AsRawFd;
1212
use std::ptr::null_mut;
1313
use std::sync::Arc;
1414

@@ -106,7 +106,7 @@ impl<T, const N: usize> MaybeBounce<T, N> {
106106
}
107107

108108
// FIXME: replace AsFd with ReadVolatile once &File: ReadVolatile in vm-memory.
109-
impl<T: Read + AsFd, const N: usize> ReadVolatile for MaybeBounce<T, N> {
109+
impl<T: ReadVolatile, const N: usize> ReadVolatile for MaybeBounce<T, N> {
110110
fn read_volatile<B: BitmapSlice>(
111111
&mut self,
112112
buf: &mut VolatileSlice<B>,
@@ -121,8 +121,7 @@ impl<T: Read + AsFd, const N: usize> ReadVolatile for MaybeBounce<T, N> {
121121
let how_much = buf.len().min(bbuf.len());
122122
let n = self
123123
.target
124-
.read(&mut bbuf[..how_much])
125-
.map_err(VolatileMemoryError::IOError)?;
124+
.read_volatile(&mut VolatileSlice::from(&mut bbuf[..how_much]))?;
126125
buf.copy_from(&bbuf[..n]);
127126

128127
buf = buf.offset(n)?;
@@ -135,12 +134,12 @@ impl<T: Read + AsFd, const N: usize> ReadVolatile for MaybeBounce<T, N> {
135134

136135
Ok(total)
137136
} else {
138-
self.target.as_fd().read_volatile(buf)
137+
self.target.read_volatile(buf)
139138
}
140139
}
141140
}
142141

143-
impl<T: Write + AsFd, const N: usize> WriteVolatile for MaybeBounce<T, N> {
142+
impl<T: WriteVolatile, const N: usize> WriteVolatile for MaybeBounce<T, N> {
144143
fn write_volatile<B: BitmapSlice>(
145144
&mut self,
146145
buf: &VolatileSlice<B>,
@@ -155,8 +154,7 @@ impl<T: Write + AsFd, const N: usize> WriteVolatile for MaybeBounce<T, N> {
155154
let how_much = buf.copy_to(bbuf);
156155
let n = self
157156
.target
158-
.write(&bbuf[..how_much])
159-
.map_err(VolatileMemoryError::IOError)?;
157+
.write_volatile(&VolatileSlice::from(&mut bbuf[..how_much]))?;
160158
buf = buf.offset(n)?;
161159
total += n;
162160

@@ -167,7 +165,7 @@ impl<T: Write + AsFd, const N: usize> WriteVolatile for MaybeBounce<T, N> {
167165

168166
Ok(total)
169167
} else {
170-
self.target.as_fd().write_volatile(buf)
168+
self.target.write_volatile(buf)
171169
}
172170
}
173171
}
@@ -652,6 +650,7 @@ mod tests {
652650

653651
use std::collections::HashMap;
654652
use std::io::{Read, Seek};
653+
use std::os::fd::AsFd;
655654

656655
use itertools::Itertools;
657656
use vmm_sys_util::tempfile::TempFile;
@@ -1030,13 +1029,13 @@ mod tests {
10301029

10311030
let mut data = (0..=255).collect_vec();
10321031

1033-
MaybeBounce::new(file_direct.as_file(), false)
1032+
MaybeBounce::new(file_direct.as_file().as_fd(), false)
10341033
.write_all_volatile(&VolatileSlice::from(data.as_mut_slice()))
10351034
.unwrap();
1036-
MaybeBounce::new(file_bounced.as_file(), true)
1035+
MaybeBounce::new(file_bounced.as_file().as_fd(), true)
10371036
.write_all_volatile(&VolatileSlice::from(data.as_mut_slice()))
10381037
.unwrap();
1039-
MaybeBounce::<_, 7>::new_persistent(file_persistent_bounced.as_file(), true)
1038+
MaybeBounce::<_, 7>::new_persistent(file_persistent_bounced.as_file().as_fd(), true)
10401039
.write_all_volatile(&VolatileSlice::from(data.as_mut_slice()))
10411040
.unwrap();
10421041

@@ -1051,13 +1050,13 @@ mod tests {
10511050
.seek(SeekFrom::Start(0))
10521051
.unwrap();
10531052

1054-
MaybeBounce::new(file_direct.as_file(), false)
1053+
MaybeBounce::new(file_direct.as_file().as_fd(), false)
10551054
.read_exact_volatile(&mut VolatileSlice::from(data_direct.as_mut_slice()))
10561055
.unwrap();
1057-
MaybeBounce::new(file_bounced.as_file(), true)
1056+
MaybeBounce::new(file_bounced.as_file().as_fd(), true)
10581057
.read_exact_volatile(&mut VolatileSlice::from(data_bounced.as_mut_slice()))
10591058
.unwrap();
1060-
MaybeBounce::<_, 7>::new_persistent(file_persistent_bounced.as_file(), true)
1059+
MaybeBounce::<_, 7>::new_persistent(file_persistent_bounced.as_file().as_fd(), true)
10611060
.read_exact_volatile(&mut VolatileSlice::from(
10621061
data_persistent_bounced.as_mut_slice(),
10631062
))

src/vmm/src/vstate/vm.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
use std::collections::HashMap;
99
use std::fs::{File, OpenOptions};
1010
use std::io::Write;
11-
use std::os::fd::FromRawFd;
11+
use std::os::fd::{AsFd, FromRawFd};
1212
use std::path::Path;
1313
use std::sync::Arc;
1414

@@ -465,9 +465,10 @@ impl Vm {
465465
.guest_memory()
466466
.iter()
467467
.any(|r| r.inner().guest_memfd != 0);
468+
let mut maybe_bounce = MaybeBounce::new(file.as_fd(), secret_hidden);
468469
self.guest_memory()
469-
.dump(&mut MaybeBounce::new(&file, secret_hidden))
470-
.and_then(|_| self.swiotlb_regions().dump(&mut file))?;
470+
.dump(&mut maybe_bounce)
471+
.and_then(|_| self.swiotlb_regions().dump(&mut maybe_bounce))?;
471472
self.reset_dirty_bitmap();
472473
self.guest_memory().reset_dirty();
473474
}

0 commit comments

Comments
 (0)