fix: move volatile memory access methods to Bytes trait

The `read_[exact_]_volatile_from` and `write_[all_]volatile_to`
functions were intended to be replacements for their `Read` and `Write`
based counterparts. However, those used to live in the `Bytes` trait,
not the `GuestMemory` trait. Fix up this goof on my part by moving them
to the `Bytes` trait.

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>

Author: roypat

CHANGELOG.md

@@ -6,6 +6,11 @@
 
 - \[[#311](https://github.com/rust-vmm/vm-memory/pull/311)\] Allow compiling without the ReadVolatile and WriteVolatile implementations
 
+### Changed
+
+- \[[#307](https://github.com/rust-vmm/vm-memory/pull/304)\] Move `read_volatile_from`, `read_exact_volatile_from`,
+  `write_volatile_to` and `write_all_volatile_to` functions from the `GuestMemory` trait to the `Bytes` trait.
+
 ### Removed
 
 - \[[#307](https://github.com/rust-vmm/vm-memory/pull/304)\] Remove deprecated functions `Bytes::read_from`, `Bytes::read_exact_from`,

src/bytes.rs

@@ -18,6 +18,7 @@ use std::sync::atomic::Ordering;
 
 use crate::atomic_integer::AtomicInteger;
 use crate::volatile_memory::VolatileSlice;
+use crate::{ReadVolatile, WriteVolatile};
 
 /// Types for which it is safe to initialize from raw data.
 ///
@@ -276,6 +277,102 @@ pub trait Bytes<A> {
         self.read_slice(result.as_mut_slice(), addr).map(|_| result)
     }
 
+    /// Reads up to `count` bytes from `src` and writes them into the container at `addr`.
+    /// Unlike `VolatileRead::read_volatile`, this function retries on `EINTR` being returned from
+    /// the underlying I/O `read` operation.
+    ///
+    /// Returns the number of bytes written into the container.
+    ///
+    /// # Arguments
+    /// * `addr` - Begin writing at this address.
+    /// * `src` - Copy from `src` into the container.
+    /// * `count` - Copy `count` bytes from `src` into the container.
+    ///
+    /// # Examples
+    ///
+    /// * Read bytes from /dev/urandom (uses the `backend-mmap` feature)
+    ///
+    /// ```
+    /// # #[cfg(all(feature = "backend-mmap", feature = "rawfd"))]
+    /// # {
+    /// # use vm_memory::{Address, GuestMemory, Bytes, GuestAddress, GuestMemoryMmap};
+    /// # use std::fs::File;
+    /// # use std::path::Path;
+    /// #
+    /// # let start_addr = GuestAddress(0x1000);
+    /// # let gm = GuestMemoryMmap::<()>::from_ranges(&vec![(start_addr, 0x400)])
+    /// #    .expect("Could not create guest memory");
+    /// # let addr = GuestAddress(0x1010);
+    /// # let mut file = if cfg!(unix) {
+    /// let mut file = File::open(Path::new("/dev/urandom")).expect("Could not open /dev/urandom");
+    /// #   file
+    /// # } else {
+    /// #   File::open(Path::new("c:\\Windows\\system32\\ntoskrnl.exe"))
+    /// #       .expect("Could not open c:\\Windows\\system32\\ntoskrnl.exe")
+    /// # };
+    ///
+    /// gm.read_volatile_from(addr, &mut file, 128)
+    ///     .expect("Could not read from /dev/urandom into guest memory");
+    ///
+    /// let read_addr = addr.checked_add(8).expect("Could not compute read address");
+    /// let rand_val: u32 = gm
+    ///     .read_obj(read_addr)
+    ///     .expect("Could not read u32 val from /dev/urandom");
+    /// # }
+    /// ```
+    fn read_volatile_from<F>(&self, addr: A, src: &mut F, count: usize) -> Result<usize, Self::E>
+    where
+        F: ReadVolatile;
+
+    /// Reads exactly `count` bytes from an object and writes them into the container at `addr`.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if `count` bytes couldn't have been copied from `src` to the container.
+    /// Part of the data may have been copied nevertheless.
+    ///
+    /// # Arguments
+    /// * `addr` - Begin writing at this address.
+    /// * `src` - Copy from `src` into the container.
+    /// * `count` - Copy exactly `count` bytes from `src` into the container.
+    fn read_exact_volatile_from<F>(
+        &self,
+        addr: A,
+        src: &mut F,
+        count: usize,
+    ) -> Result<(), Self::E>
+    where
+        F: ReadVolatile;
+
+    /// Reads up to `count` bytes from the container at `addr` and writes them into `dst`.
+    /// Unlike `VolatileWrite::write_volatile`, this function retries on `EINTR` being returned by
+    /// the underlying I/O `write` operation.
+    ///
+    /// Returns the number of bytes written into the object.
+    ///
+    /// # Arguments
+    /// * `addr` - Begin reading from this address.
+    /// * `dst` - Copy from the container to `dst`.
+    /// * `count` - Copy `count` bytes from the container to `dst`.
+    fn write_volatile_to<F>(&self, addr: A, dst: &mut F, count: usize) -> Result<usize, Self::E>
+    where
+        F: WriteVolatile;
+
+    /// Reads exactly `count` bytes from the container at `addr` and writes them into an object.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if `count` bytes couldn't have been copied from the container to `dst`.
+    /// Part of the data may have been copied nevertheless.
+    ///
+    /// # Arguments
+    /// * `addr` - Begin reading from this address.
+    /// * `dst` - Copy from the container to `dst`.
+    /// * `count` - Copy exactly `count` bytes from the container to `dst`.
+    fn write_all_volatile_to<F>(&self, addr: A, dst: &mut F, count: usize) -> Result<(), Self::E>
+    where
+        F: WriteVolatile;
+
     /// Atomically store a value at the specified address.
     fn store<T: AtomicAccess>(&self, val: T, addr: A, order: Ordering) -> Result<(), Self::E>;
 
@@ -414,6 +511,54 @@ pub(crate) mod tests {
             Ok(())
         }
 
+        fn read_volatile_from<F>(
+            &self,
+            _addr: usize,
+            _src: &mut F,
+            _count: usize,
+        ) -> Result<usize, Self::E>
+        where
+            F: ReadVolatile,
+        {
+            unimplemented!()
+        }
+
+        fn read_exact_volatile_from<F>(
+            &self,
+            _addr: usize,
+            _src: &mut F,
+            _count: usize,
+        ) -> Result<(), Self::E>
+        where
+            F: ReadVolatile,
+        {
+            unimplemented!()
+        }
+
+        fn write_volatile_to<F>(
+            &self,
+            _addr: usize,
+            _dst: &mut F,
+            _count: usize,
+        ) -> Result<usize, Self::E>
+        where
+            F: WriteVolatile,
+        {
+            unimplemented!()
+        }
+
+        fn write_all_volatile_to<F>(
+            &self,
+            _addr: usize,
+            _dst: &mut F,
+            _count: usize,
+        ) -> Result<(), Self::E>
+        where
+            F: WriteVolatile,
+        {
+            unimplemented!()
+        }
+
         fn store<T: AtomicAccess>(
             &self,
             _val: T,

src/guest_memory.rs

@@ -54,7 +54,6 @@ use crate::bitmap::{Bitmap, BS, MS};
 use crate::bytes::{AtomicAccess, Bytes};
 use crate::io::{ReadVolatile, WriteVolatile};
 use crate::volatile_memory::{self, VolatileSlice};
-use crate::GuestMemoryError;
 
 /// Errors associated with handling guest memory accesses.
 #[allow(missing_docs)]
@@ -538,137 +537,6 @@ pub trait GuestMemory {
         }
     }
 
-    /// Reads up to `count` bytes from an object and writes them into guest memory at `addr`.
-    ///
-    /// Returns the number of bytes written into guest memory.
-    ///
-    /// # Arguments
-    /// * `addr` - Begin writing at this address.
-    /// * `src` - Copy from `src` into the container.
-    /// * `count` - Copy `count` bytes from `src` into the container.
-    ///
-    /// # Examples
-    ///
-    /// * Read bytes from /dev/urandom (uses the `backend-mmap` feature)
-    ///
-    /// ```
-    /// # #[cfg(all(feature = "backend-mmap", feature = "rawfd"))]
-    /// # {
-    /// # use vm_memory::{Address, GuestMemory, Bytes, GuestAddress, GuestMemoryMmap};
-    /// # use std::fs::File;
-    /// # use std::path::Path;
-    /// #
-    /// # let start_addr = GuestAddress(0x1000);
-    /// # let gm = GuestMemoryMmap::<()>::from_ranges(&vec![(start_addr, 0x400)])
-    /// #    .expect("Could not create guest memory");
-    /// # let addr = GuestAddress(0x1010);
-    /// # let mut file = if cfg!(unix) {
-    /// let mut file = File::open(Path::new("/dev/urandom")).expect("Could not open /dev/urandom");
-    /// #   file
-    /// # } else {
-    /// #   File::open(Path::new("c:\\Windows\\system32\\ntoskrnl.exe"))
-    /// #       .expect("Could not open c:\\Windows\\system32\\ntoskrnl.exe")
-    /// # };
-    ///
-    /// gm.read_volatile_from(addr, &mut file, 128)
-    ///     .expect("Could not read from /dev/urandom into guest memory");
-    ///
-    /// let read_addr = addr.checked_add(8).expect("Could not compute read address");
-    /// let rand_val: u32 = gm
-    ///     .read_obj(read_addr)
-    ///     .expect("Could not read u32 val from /dev/urandom");
-    /// # }
-    /// ```
-    fn read_volatile_from<F>(&self, addr: GuestAddress, src: &mut F, count: usize) -> Result<usize>
-    where
-        F: ReadVolatile,
-    {
-        self.try_access(count, addr, |_, len, caddr, region| -> Result<usize> {
-            let mut vslice = region.get_slice(caddr, len)?;
-
-            src.read_volatile(&mut vslice)
-                .map_err(GuestMemoryError::from)
-        })
-    }
-
-    /// Reads up to `count` bytes from guest memory at `addr` and writes them it into an object.
-    ///
-    /// Returns the number of bytes copied from guest memory.
-    ///
-    /// # Arguments
-    /// * `addr` - Begin reading from this address.
-    /// * `dst` - Copy from guest memory to `dst`.
-    /// * `count` - Copy `count` bytes from guest memory to `dst`.
-    fn write_volatile_to<F>(&self, addr: GuestAddress, dst: &mut F, count: usize) -> Result<usize>
-    where
-        F: WriteVolatile,
-    {
-        self.try_access(count, addr, |_, len, caddr, region| -> Result<usize> {
-            let vslice = region.get_slice(caddr, len)?;
-
-            // For a non-RAM region, reading could have side effects, so we
-            // must use write_all().
-            dst.write_all_volatile(&vslice)?;
-
-            Ok(len)
-        })
-    }
-
-    /// Reads exactly `count` bytes from an object and writes them into guest memory at `addr`.
-    ///
-    /// # Errors
-    ///
-    /// Returns an error if `count` bytes couldn't have been copied from `src` to guest memory.
-    /// Part of the data may have been copied nevertheless.
-    ///
-    /// # Arguments
-    /// * `addr` - Begin writing at this address.
-    /// * `src` - Copy from `src` into guest memory.
-    /// * `count` - Copy exactly `count` bytes from `src` into guest memory.
-    fn read_exact_volatile_from<F>(
-        &self,
-        addr: GuestAddress,
-        src: &mut F,
-        count: usize,
-    ) -> Result<()>
-    where
-        F: ReadVolatile,
-    {
-        let res = self.read_volatile_from(addr, src, count)?;
-        if res != count {
-            return Err(Error::PartialBuffer {
-                expected: count,
-                completed: res,
-            });
-        }
-        Ok(())
-    }
-
-    /// Reads exactly `count` bytes from guest memory at `addr` and writes them into an object.
-    ///
-    /// # Errors
-    ///
-    /// Returns an error if `count` bytes couldn't have been copied from guest memory to `dst`.
-    /// Part of the data may have been copied nevertheless.
-    ///
-    /// # Arguments
-    /// * `addr` - Begin reading from this address.
-    /// * `dst` - Copy from guest memory to `dst`.
-    /// * `count` - Copy exactly `count` bytes from guest memory to `dst`.
-    fn write_all_volatile_to<F>(&self, addr: GuestAddress, dst: &mut F, count: usize) -> Result<()>
-    where
-        F: WriteVolatile,
-    {
-        let res = self.write_volatile_to(addr, dst, count)?;
-        if res != count {
-            return Err(Error::PartialBuffer {
-                expected: count,
-                completed: res,
-            });
-        }
-        Ok(())
-    }
-
     /// Get the host virtual address corresponding to the guest address.
     ///
     /// Some [`GuestMemory`](trait.GuestMemory.html) implementations, like `GuestMemoryMmap`,
@@ -796,6 +664,59 @@ impl<T: GuestMemory + ?Sized> Bytes<GuestAddress> for T {
         Ok(())
     }
 
+    fn read_volatile_from<F>(&self, addr: GuestAddress, src: &mut F, count: usize) -> Result<usize>
+    where
+        F: ReadVolatile,
+    {
+        self.try_access(count, addr, |_, len, caddr, region| -> Result<usize> {
+            region.read_volatile_from(caddr, src, len)
+        })
+    }
+
+    fn read_exact_volatile_from<F>(
+        &self,
+        addr: GuestAddress,
+        src: &mut F,
+        count: usize,
+    ) -> Result<()>
+    where
+        F: ReadVolatile,
+    {
+        let res = self.read_volatile_from(addr, src, count)?;
+        if res != count {
+            return Err(Error::PartialBuffer {
+                expected: count,
+                completed: res,
+            });
+        }
+        Ok(())
+    }
+
+    fn write_volatile_to<F>(&self, addr: GuestAddress, dst: &mut F, count: usize) -> Result<usize>
+    where
+        F: WriteVolatile,
+    {
+        self.try_access(count, addr, |_, len, caddr, region| -> Result<usize> {
+            // For a non-RAM region, reading could have side effects, so we
+            // must use write_all().
+            region.write_all_volatile_to(caddr, dst, len).map(|()| len)
+        })
+    }
+
+    fn write_all_volatile_to<F>(&self, addr: GuestAddress, dst: &mut F, count: usize) -> Result<()>
+    where
+        F: WriteVolatile,
+    {
+        let res = self.write_volatile_to(addr, dst, count)?;
+        if res != count {
+            return Err(Error::PartialBuffer {
+                expected: count,
+                completed: res,
+            });
+        }
+        Ok(())
+    }
+
     fn store<O: AtomicAccess>(&self, val: O, addr: GuestAddress, order: Ordering) -> Result<()> {
         // `find_region` should really do what `to_region_addr` is doing right now, except
         // it should keep returning a `Result`.