rust-vmm / vm-memory

Virtual machine's guest memory crate
Apache License 2.0
312 stars 104 forks source link

Normalize the way to create GuestMemoryMmap instance #66

Closed jiangliu closed 4 years ago

jiangliu commented 4 years ago

With code evolves, now we have several constructors for GuestMemoryMmap:

pub fn new(ranges: &[(GuestAddress, usize)]) -> result::Result<Self, Error>
pub fn with_files<A, T>(ranges: T) -> result::Result<Self, Error>
pub fn from_regions(mut regions: Vec<GuestRegionMmap>) -> result::Result<Self, Error>;
pub fn from_arc_regions(regions: Vec<Arc<GuestRegionMmap>>) -> result::Result<Self, Error>

With these four constructors, we still have no constructors to create an empty GuestMemoryMmap instance. All these constructors assume that memory regions are static and created at boot time. And memory hotplug breaks this assumption.

So how about refining the constructors as:

pub fn new() -> Self
pub fn from_ranges(ranges: &[(GuestAddress, usize)]) -> result::Result<Self, Error>
pub fn from_ranges_with_files<A, T>(ranges: T) -> result::Result<Self, Error>
pub fn from_regions(mut regions: Vec<GuestRegionMmap>) -> result::Result<Self, Error>;
pub fn from_arc_regions(regions: Vec<Arc<GuestRegionMmap>>) -> result::Result<Self, Error>

Note, these changes may break existing code in CrosVM, firecracker and cloud hypervisor projects.

bonzini commented 4 years ago

No objection from me. However from_regions and from_arc_regions could be changed into implementation of the From<> trait instead of being associated functions.

jiangliu commented 4 years ago

No objection from me. However from_regions and from_arc_regions could be changed into implementation of the From<> trait instead of being associated functions.

The from_arc_regions() has some sanity-check logics, so can't be convert to From trait.