On Tue, Feb 18, 2025 at 07:20:17PM +0100, Kevin Wolf wrote: > +/// A request to a block driver > +pub enum Request { > + Read { offset: u64, len: u64 }, > +} > + > +/// The target for a number of guest blocks, e.g. a location in a child node > or the information > +/// that the described blocks are unmapped. > +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: Arc<BdrvChild>, > + > + /// 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. > /// > @@ -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>;
Here are my thoughts about how the interface could evolve as more functionality is added (beyond the scope of this patch). I'm sure you have your own ideas that aren't in this patch series, so take it for what it's worth. The main point: The current interface has Request, which tells the BlockDriver that this is a Read. I think this interface is still based on I/O requests rather than being about mappings. The caller should handle read/write/discard/etc request logic themselves based on the mapping information from the BlockDriver. Then the BlockDriver just worries about mapping blocks rather than how to treat different types of requests. A sketch of the interface: The interface consists of fetch(), alloc(), and update(). 1. Fetch mappings located around a given range: fetch(offset, len) -> [Mapping] The returned array of mappings must cover <offset, len>, but it can also be larger. For example, it could return the entire extent that includes <offset, len>. Or it could even contain the <offset, len> mapping along with a bunch of other mappings. If the BlockDriver pays the cost of a metadata read operation to load mappings, then let's report all mappings even if they precede or follow the <offset, len> range. The exact amount of extra mappings reported depends on the BlockDriver implementation and the metadata layout, but in qcow2 it might be an L2 table, for example. The idea is that if the BlockDriver volunteers extra mapping information, then the caller can perform future operations without calling into the BlockDriver again. If it turns out that certain BlockDriver implementations don't benefit, that's okay, but let's include the flexibility to volunteer more mapping information than just <offset, len> in the interface. 2. Allocate new unmapped blocks: alloc(num_blocks) -> Result<(BdrvChild, Offset)> (A choice needs to be made whether the blocks are contiguous or not, I'm showing the contiguous function prototype here because it's simpler, but it could also support non-contiguous allocations.) These blocks are not mapped yet and the caller is expected to write to them eventually, populating them with data. I'm assuming the qcow2 crash consistency model where leaking clusters is acceptable here. Not sure whether this API makes sense for all other image formats, but please bear with me for the final part of the interface where everything fits together. 3. Update mapping: update(offset, len, MappingTarget) -> Result<()> This adds or changes mappings. It's the most complicated function and there are a lot of details about which inputs should be allowed/forbidden (e.g. creating completely new mappings, splitting existing mappings, toggling MappingTarget on an existing mapping). Perhaps in practice only a few valid combinations of inputs will be allowed rather than full ability to manipulate mappings. Anyway, here is how it can be used: - Allocating write. After blocks allocated with alloc() have been written with data, call update() to establish a mapping. - Discard. Change the MappingTarget of an existing mapping to unmap blocks. This frees blocks unless they should be anchored (aka pre-allocated). - Write to anchored blocks. If the mapping had blocks allocated but they were not mapped, then update() should be called after data has been written to the blocks to restore the mapping from anchored to normal mapped blocks. The overall idea is that I/O requests are not part of the interface, only mappings. The caller decides how to perform I/O requests and calls the appropriate combination of fetch(), alloc(), and update() APIs to ensure the necessary mapping state is present. It will probably be necessary to add additional mapping information indicating permissions, compression/encryption info, etc so that the caller can process I/O requests rather than sending them into the BlockDriver. Stefan
signature.asc
Description: PGP signature