Skip to content

Commit 92a0e25

Browse files
committed
deprecate GuestMemory::try_access()
Deprecate 'GuestMemory::try_access()` in favor of the external iterator provided by `GuestMemory::get_slices()`. Rewrite all internal usages of `try_access()` in terms of `get_slices()`. Signed-off-by: Patrick Roy <[email protected]>
1 parent e3b5bba commit 92a0e25

File tree

3 files changed

+44
-35
lines changed

3 files changed

+44
-35
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@
3939

4040
- \[[#339](https://github.com/rust-vmm/vm-memory/pull/339)\] Fix `Bytes::read()` and `Bytes::write()` not to ignore `try_access()`'s `count` parameter
4141

42+
### Deprecated
43+
44+
- \[[#349](https://github.com/rust-vmm/vm-memory/pull/349)\] Deprecate `GuestMemory::try_access()`. Use `GuestMemory::get_slices()` instead.
45+
4246
## \[v0.16.1\]
4347

4448
### Added

src/bitmap/mod.rs

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -288,16 +288,10 @@ pub(crate) mod tests {
288288

289289
// Finally, let's invoke the generic tests for `Bytes`.
290290
let check_range_closure = |m: &M, start: usize, len: usize, clean: bool| -> bool {
291-
let mut check_result = true;
292-
m.try_access(len, GuestAddress(start as u64), |_, size, reg_addr, reg| {
293-
if !check_range(&reg.bitmap(), reg_addr.0 as usize, size, clean) {
294-
check_result = false;
295-
}
296-
Ok(size)
291+
m.get_slices(GuestAddress(start as u64), len).all(|r| {
292+
let slice = r.unwrap();
293+
check_range(slice.bitmap(), 0, slice.len(), clean)
297294
})
298-
.unwrap();
299-
300-
check_result
301295
};
302296

303297
test_bytes(

src/guest_memory.rs

Lines changed: 37 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -355,10 +355,10 @@ pub trait GuestMemory {
355355

356356
/// Check whether the range [base, base + len) is valid.
357357
fn check_range(&self, base: GuestAddress, len: usize) -> bool {
358-
match self.try_access(len, base, |_, count, _, _| -> Result<usize> { Ok(count) }) {
359-
Ok(count) => count == len,
360-
_ => false,
361-
}
358+
self.get_slices(base, len)
359+
.map(|r| r.map(|slice| slice.len()).unwrap_or(0))
360+
.sum::<usize>()
361+
== len
362362
}
363363

364364
/// Returns the address plus the offset if it is present within the memory of the guest.
@@ -376,6 +376,10 @@ pub trait GuestMemory {
376376
/// - the error code returned by the callback 'f'
377377
/// - the size of the already handled data when encountering the first hole
378378
/// - the size of the already handled data when the whole range has been handled
379+
#[deprecated(
380+
since = "0.17.0",
381+
note = "supplemented by external iterator `get_slices()`"
382+
)]
379383
fn try_access<F>(&self, count: usize, addr: GuestAddress, mut f: F) -> Result<usize>
380384
where
381385
F: FnMut(usize, usize, MemoryRegionAddress, &Self::R) -> Result<usize>,
@@ -527,6 +531,14 @@ impl<'a, M: GuestMemory + ?Sized> GuestMemorySliceIterator<'a, M> {
527531

528532
Some(region.get_slice(start, len as usize))
529533
}
534+
535+
/// Adapts this [`GuestMemorySliceIterator`] to return `None` (e.g. gracefully terminate)
536+
/// instead of `Err(Error::InvalidGuestAddress)` when iteration hits an invalid [`GuestAddress`].
537+
pub fn partial(self) -> impl Iterator<Item = Result<VolatileSlice<'a, MS<'a, M>>>> {
538+
self.enumerate()
539+
.filter(|(i, r)| !(*i > 0 && matches!(r, Err(Error::InvalidGuestAddress(_)))))
540+
.map(|(_, r)| r)
541+
}
530542
}
531543

532544
impl<'a, M: GuestMemory + ?Sized> Iterator for GuestMemorySliceIterator<'a, M> {
@@ -556,23 +568,15 @@ impl<T: GuestMemory + ?Sized> Bytes<GuestAddress> for T {
556568
type E = Error;
557569

558570
fn write(&self, buf: &[u8], addr: GuestAddress) -> Result<usize> {
559-
self.try_access(
560-
buf.len(),
561-
addr,
562-
|offset, count, caddr, region| -> Result<usize> {
563-
region.write(&buf[offset..(offset + count)], caddr)
564-
},
565-
)
571+
self.get_slices(addr, buf.len())
572+
.partial()
573+
.try_fold(0, |acc, slice| Ok(acc + slice?.write(&buf[acc..], 0)?))
566574
}
567575

568576
fn read(&self, buf: &mut [u8], addr: GuestAddress) -> Result<usize> {
569-
self.try_access(
570-
buf.len(),
571-
addr,
572-
|offset, count, caddr, region| -> Result<usize> {
573-
region.read(&mut buf[offset..(offset + count)], caddr)
574-
},
575-
)
577+
self.get_slices(addr, buf.len())
578+
.partial()
579+
.try_fold(0, |acc, slice| Ok(acc + slice?.read(&mut buf[acc..], 0)?))
576580
}
577581

578582
/// # Examples
@@ -636,9 +640,12 @@ impl<T: GuestMemory + ?Sized> Bytes<GuestAddress> for T {
636640
where
637641
F: ReadVolatile,
638642
{
639-
self.try_access(count, addr, |_, len, caddr, region| -> Result<usize> {
640-
region.read_volatile_from(caddr, src, len)
641-
})
643+
self.get_slices(addr, count)
644+
.partial()
645+
.try_fold(0, |acc, r| {
646+
let slice = r?;
647+
Ok(acc + slice.read_volatile_from(0, src, slice.len())?)
648+
})
642649
}
643650

644651
fn read_exact_volatile_from<F>(
@@ -664,11 +671,15 @@ impl<T: GuestMemory + ?Sized> Bytes<GuestAddress> for T {
664671
where
665672
F: WriteVolatile,
666673
{
667-
self.try_access(count, addr, |_, len, caddr, region| -> Result<usize> {
668-
// For a non-RAM region, reading could have side effects, so we
669-
// must use write_all().
670-
region.write_all_volatile_to(caddr, dst, len).map(|()| len)
671-
})
674+
self.get_slices(addr, count)
675+
.partial()
676+
.try_fold(0, |acc, r| {
677+
let slice = r?;
678+
// For a non-RAM region, reading could have side effects, so we
679+
// must use write_all().
680+
slice.write_all_volatile_to(0, dst, slice.len())?;
681+
Ok(acc + slice.len())
682+
})
672683
}
673684

674685
fn write_all_volatile_to<F>(&self, addr: GuestAddress, dst: &mut F, count: usize) -> Result<()>

0 commit comments

Comments
 (0)