Re: [PATCH 09/11] rust/block: Add read support for block drivers

[Paolo Bonzini <pbonzini@redhat.com>]

On 2/11/25 22:43, Kevin Wolf wrote:
> +/// A request to a block driver
> +pub enum Request {
> +    Read { offset: u64, len: u64 },
> +}
> +

Maybe add flags already?
> +#[allow(dead_code)]
> +pub enum MappingTarget {
> +    /// The described blocks are unallocated. Reading from them yields zeros.
> +    Unmapped,
> +
> +    /// The described blocks are stored in a child node.
> +    Data {
> +        /// Child node in which the data is stored
> +        node: (),

Make it already a *mut BlockDriverState, or *mut BdrvChild?  Or are you worried of
irritating the borrow checker? :)

> +        /// Offset in the child node at which the data is stored
> +        offset: u64,
> +    },
> +}
> +
> +/// A mapping for a number of contiguous guest blocks
> +pub struct Mapping {
> +    /// Offset of the mapped blocks from the perspective of the guest
> +    pub offset: u64,
> +    /// Length of the mapping in bytes
> +    pub len: u64,
> +    /// Where the data for the described blocks is stored
> +    pub target: MappingTarget,
> +}
> +
>   /// A trait for writing block drivers.
>   ///
>   /// Types that implement this trait can be registered as QEMU block drivers using the
> @@ -37,6 +72,11 @@ unsafe fn open(
>   
>       /// Returns the size of the image in bytes
>       fn size(&self) -> u64;
> +
> +    /// Returns the mapping for the first part of `req`. If the returned mapping is shorter than
> +    /// the request, the function can be called again with a shortened request to get the mapping
> +    /// for the remaining part.
> +    async fn map(&self, req: &Request) -> io::Result<Mapping>;

I am not sure I like the idea of making this the only way to do a read.
Maybe you can keep preadv_part in the trait, and add an utility function
like:

async fn bdrv_co_preadv_with_mappings<F: Future<Output = io::Result<()>>>(
     mut offset: u64,
     mut bytes: u64,
     // IIRC qiov is not changed, but perhaps you still want to require &mut
     // for it to _write_ memory?
     qiov: &mut bindings::QEMUIOVector,
     mut qiov_offset: usize,
     flags: bindings::BdrvRequestFlags,
     mut f: impl FnMut(Request) -> F) -> io::Result<()> {
     while bytes > 0 {
         let req = Request::Read { offset, len: bytes, flags };
         let mapping = f(req).await?;
         ...
     }
     Ok(())
}

Then you can implement BochsImage's trait methods as:

async fn preadv_part(
     &self,
     offset: u64,
     bytes: u64,
     qiov: &mut bindings::QEMUIOVector,
     qiov_offset: usize,
     flags: bindings::BdrvRequestFlags,
) -> io::Result<()> {
     bdrv_co_preadv_with_mappings(offset, bytes, qiov, qiov_offset, flags,
                                  |req| self.map(req)).await
}

BochsImage::map() is a private function and the outer bdrv_co_preadv_part()
only handles qemu_co_run_future() and converting the io::Result.

Paolo