Skip to content

Commit 0296646

Browse files
roypatroypat
committed
refactor: hoist up hugepages/file-based restore incompatibility check
Move this check out of the memory allocation code and up into the snapshot restoration code. This allows us to encode the impossibility of mmap-ing a snapshot file with hugepages at the type level (by simply not giving an option to pass a `HugePageConfig` to `GuestMemoryMmap::create` if `MemoryKind::Snapshot(_)` is passed). Signed-off-by: Patrick Roy <[email protected]>
1 parent b6f266a commit 0296646

File tree

6 files changed

+45
-88
lines changed

6 files changed

+45
-88
lines changed

‎src/vmm/src/devices/virtio/block/virtio/io/mod.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,9 +232,8 @@ pub mod tests {
232232
fn create_mem() -> GuestMemoryMmap {
233233
GuestMemoryMmap::create(
234234
[(GuestAddress(0), MEM_LEN)].into_iter(),
235-
MemoryKind::MapAnonymous,
235+
MemoryKind::MapAnonymous(HugePageConfig::None),
236236
true,
237-
HugePageConfig::None,
238237
)
239238
.unwrap()
240239
}

‎src/vmm/src/devices/virtio/vhost_user.rs‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,6 @@ mod tests {
466466

467467
use super::*;
468468
use crate::test_utils::create_tmp_socket;
469-
use crate::vmm_config::machine_config::HugePageConfig;
470469
use crate::vstate::memory::{GuestAddress, GuestMemoryExtension, MemoryKind};
471470

472471
#[test]
@@ -764,13 +763,9 @@ mod tests {
764763
(GuestAddress(0x10000), region_size),
765764
];
766765

767-
let guest_memory = GuestMemoryMmap::create(
768-
regions.into_iter(),
769-
MemoryKind::Snapshot(file),
770-
false,
771-
HugePageConfig::None,
772-
)
773-
.unwrap();
766+
let guest_memory =
767+
GuestMemoryMmap::create(regions.into_iter(), MemoryKind::Snapshot(file), false)
768+
.unwrap();
774769

775770
vuh.update_mem_table(&guest_memory).unwrap();
776771

@@ -887,7 +882,6 @@ mod tests {
887882
[(GuestAddress(0), region_size)].into_iter(),
888883
MemoryKind::Snapshot(file),
889884
false,
890-
HugePageConfig::None,
891885
)
892886
.unwrap();
893887

‎src/vmm/src/persist.rs‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -449,16 +449,20 @@ pub fn restore_from_snapshot(
449449
let mem_state = &microvm_state.memory_state;
450450

451451
let (guest_memory, uffd) = match params.mem_backend.backend_type {
452-
MemBackendType::File => (
453-
guest_memory_from_file(
454-
mem_backend_path,
455-
mem_state,
456-
track_dirty_pages,
457-
vm_resources.machine_config.huge_pages,
452+
MemBackendType::File => {
453+
if vm_resources.machine_config.huge_pages.is_hugetlbfs() {
454+
return Err(RestoreFromSnapshotGuestMemoryError::File(
455+
GuestMemoryFromFileError::HugetlbfsSnapshot,
456+
)
457+
.into());
458+
}
459+
460+
(
461+
guest_memory_from_file(mem_backend_path, mem_state, track_dirty_pages)
462+
.map_err(RestoreFromSnapshotGuestMemoryError::File)?,
463+
None,
458464
)
459-
.map_err(RestoreFromSnapshotGuestMemoryError::File)?,
460-
None,
461-
),
465+
}
462466
MemBackendType::Uffd => guest_memory_from_uffd(
463467
mem_backend_path,
464468
mem_state,
@@ -514,14 +518,15 @@ pub enum GuestMemoryFromFileError {
514518
File(#[from] std::io::Error),
515519
/// Failed to restore guest memory: {0}
516520
Restore(#[from] MemoryError),
521+
/// Cannot restore hugetlbfs backed snapshot by mapping the memory file. Please use uffd.
522+
HugetlbfsSnapshot,
517523
}
518524

519525
/// Creates a [`GuestMemoryMmap`] from a snapshot file
520526
pub fn guest_memory_from_file(
521527
mem_file_path: &Path,
522528
mem_state: &GuestMemoryState,
523529
track_dirty_pages: bool,
524-
huge_pages: HugePageConfig,
525530
) -> Result<GuestMemoryMmap, GuestMemoryFromFileError> {
526531
let mem_file = File::open(mem_file_path)?;
527532
let guest_mem = GuestMemoryMmap::create(
@@ -531,7 +536,6 @@ pub fn guest_memory_from_file(
531536
.map(|region| (GuestAddress(region.base_address), region.size)),
532537
MemoryKind::Snapshot(mem_file),
533538
track_dirty_pages,
534-
huge_pages,
535539
)?;
536540
Ok(guest_mem)
537541
}
@@ -596,9 +600,8 @@ fn create_guest_memory(
596600
.regions
597601
.iter()
598602
.map(|region| (GuestAddress(region.base_address), region.size)),
599-
MemoryKind::MapAnonymous,
603+
MemoryKind::MapAnonymous(huge_pages),
600604
track_dirty_pages,
601-
huge_pages,
602605
)?;
603606
let mut backend_mappings = Vec::with_capacity(guest_memory.num_regions());
604607
for (mem_region, state_region) in guest_memory.iter().zip(mem_state.regions.iter()) {

‎src/vmm/src/resources.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -466,16 +466,15 @@ impl VmResources {
466466
// that would not be worth the effort.
467467

468468
let memory_kind = if vhost_user_device_used {
469-
MemoryKind::Memfd
469+
MemoryKind::Memfd(self.machine_config.huge_pages)
470470
} else {
471-
MemoryKind::MapAnonymous
471+
MemoryKind::MapAnonymous(self.machine_config.huge_pages)
472472
};
473473

474474
GuestMemoryMmap::create(
475475
crate::arch::arch_memory_regions(self.machine_config.mem_size_mib << 20).into_iter(),
476476
memory_kind,
477477
self.machine_config.track_dirty_pages,
478-
self.machine_config.huge_pages,
479478
)
480479
}
481480
}

‎src/vmm/src/test_utils/mod.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,8 @@ pub fn single_region_mem_at(at: u64, size: usize) -> GuestMemoryMmap {
3636
pub fn multi_region_mem(regions: &[(GuestAddress, usize)]) -> GuestMemoryMmap {
3737
GuestMemoryMmap::create(
3838
regions.iter().copied(),
39-
MemoryKind::MapAnonymous,
39+
MemoryKind::MapAnonymous(HugePageConfig::None),
4040
false,
41-
HugePageConfig::None,
4241
)
4342
.expect("Cannot initialize memory")
4443
}

‎src/vmm/src/vstate/memory.rs‎

Lines changed: 22 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -51,27 +51,27 @@ pub enum MemoryError {
5151
Memfd(memfd::Error),
5252
/// Cannot resize memfd file: {0}
5353
MemfdSetLen(std::io::Error),
54-
/// Cannot restore hugetlbfs backed snapshot by mapping the memory file. Please use uffd.
55-
HugetlbfsSnapshot,
5654
}
5755

5856
/// Enum representing the type of memory backing the virtual machine
5957
#[derive(Debug)]
6058
pub enum MemoryKind {
6159
/// MAP_PRIVATE | MAP_ANONYMOUS should be used
62-
MapAnonymous,
60+
MapAnonymous(HugePageConfig),
6361
/// A memfd mapped as MAP_SHARED should be used (in case guest memory needs to be
6462
/// shared with other processes)
65-
Memfd,
63+
Memfd(HugePageConfig),
6664
/// The given snapshot file should be mapped into the geust
6765
Snapshot(File),
6866
}
6967

7068
impl MemoryKind {
7169
fn mmap_flags(&self) -> libc::c_int {
7270
match self {
73-
MemoryKind::MapAnonymous => libc::MAP_ANONYMOUS | libc::MAP_PRIVATE,
74-
MemoryKind::Memfd => libc::MAP_SHARED,
71+
MemoryKind::MapAnonymous(hp_cfg) => {
72+
libc::MAP_ANONYMOUS | libc::MAP_PRIVATE | hp_cfg.mmap_flags()
73+
}
74+
MemoryKind::Memfd(hp_cfg) => libc::MAP_SHARED | hp_cfg.mmap_flags(),
7575
MemoryKind::Snapshot(_) => libc::MAP_PRIVATE,
7676
}
7777
}
@@ -87,7 +87,6 @@ where
8787
regions: impl Iterator<Item = (GuestAddress, usize)>,
8888
kind: MemoryKind,
8989
track_dirty_pages: bool,
90-
huge_pages: HugePageConfig,
9190
) -> Result<Self, MemoryError>;
9291

9392
/// Describes GuestMemoryMmap through a GuestMemoryState struct.
@@ -138,27 +137,22 @@ impl GuestMemoryExtension for GuestMemoryMmap {
138137
regions: impl Iterator<Item = (GuestAddress, usize)>,
139138
kind: MemoryKind,
140139
track_dirty_pages: bool,
141-
huge_pages: HugePageConfig,
142140
) -> Result<GuestMemoryMmap, MemoryError> {
143141
let mut offset = 0;
144142

145-
if huge_pages.is_hugetlbfs() && matches!(kind, MemoryKind::Snapshot(_)) {
146-
return Err(MemoryError::HugetlbfsSnapshot);
147-
}
148-
149143
let regions = regions
150144
.map(|(start, size)| {
151145
let mut builder = MmapRegionBuilder::new_with_bitmap(
152146
size,
153147
track_dirty_pages.then(|| AtomicBitmap::with_len(size)),
154148
)
155149
.with_mmap_prot(libc::PROT_READ | libc::PROT_WRITE)
156-
.with_mmap_flags(libc::MAP_NORESERVE | kind.mmap_flags() | huge_pages.mmap_flags());
150+
.with_mmap_flags(libc::MAP_NORESERVE | kind.mmap_flags());
157151

158152
let file_offset = match kind {
159-
MemoryKind::MapAnonymous => None,
160-
MemoryKind::Memfd => Some(FileOffset::new(
161-
create_memfd(size, huge_pages.into())?.into_file(),
153+
MemoryKind::MapAnonymous(_) => None,
154+
MemoryKind::Memfd(hp_config) => Some(FileOffset::new(
155+
create_memfd(size, hp_config.into())?.into_file(),
162156
0,
163157
)),
164158
MemoryKind::Snapshot(ref f) => Some(FileOffset::new(
@@ -368,9 +362,8 @@ mod tests {
368362

369363
let guest_memory = GuestMemoryMmap::create(
370364
regions.into_iter(),
371-
MemoryKind::MapAnonymous,
365+
MemoryKind::MapAnonymous(HugePageConfig::None),
372366
false,
373-
HugePageConfig::None,
374367
)
375368
.unwrap();
376369
guest_memory.iter().for_each(|region| {
@@ -390,9 +383,8 @@ mod tests {
390383

391384
let guest_memory = GuestMemoryMmap::create(
392385
regions.into_iter(),
393-
MemoryKind::MapAnonymous,
386+
MemoryKind::MapAnonymous(HugePageConfig::None),
394387
true,
395-
HugePageConfig::None,
396388
)
397389
.unwrap();
398390
guest_memory.iter().for_each(|region| {
@@ -422,7 +414,6 @@ mod tests {
422414
regions.into_iter(),
423415
MemoryKind::Snapshot(file.try_clone().unwrap()),
424416
false,
425-
HugePageConfig::None,
426417
)
427418
.unwrap();
428419
guest_memory.iter().for_each(|region| {
@@ -438,7 +429,6 @@ mod tests {
438429
regions.into_iter(),
439430
MemoryKind::Snapshot(file.try_clone().unwrap()),
440431
false,
441-
HugePageConfig::None,
442432
)
443433
.unwrap();
444434
guest_memory.iter().for_each(|region| {
@@ -452,7 +442,6 @@ mod tests {
452442
regions.into_iter(),
453443
MemoryKind::Snapshot(file.try_clone().unwrap()),
454444
true,
455-
HugePageConfig::None,
456445
)
457446
.unwrap();
458447
guest_memory.iter().for_each(|region| {
@@ -461,22 +450,6 @@ mod tests {
461450
}
462451
}
463452

464-
#[test]
465-
fn test_from_state() {
466-
let file = TempFile::new().unwrap().into_file();
467-
468-
// No mapping of snapshots that were taken with hugetlbfs enabled
469-
let err = GuestMemoryMmap::create(
470-
[(GuestAddress(0), 4096)].into_iter(),
471-
MemoryKind::Snapshot(file),
472-
false,
473-
HugePageConfig::Hugetlbfs2M,
474-
)
475-
.unwrap_err();
476-
477-
assert!(matches!(err, MemoryError::HugetlbfsSnapshot), "{:?}", err);
478-
}
479-
480453
#[test]
481454
fn test_mark_dirty() {
482455
let page_size = get_page_size().unwrap();
@@ -489,9 +462,8 @@ mod tests {
489462
];
490463
let guest_memory = GuestMemoryMmap::create(
491464
regions.into_iter(),
492-
MemoryKind::MapAnonymous,
465+
MemoryKind::MapAnonymous(HugePageConfig::None),
493466
true,
494-
HugePageConfig::None,
495467
)
496468
.unwrap();
497469

@@ -550,9 +522,8 @@ mod tests {
550522
// Test with a single region
551523
let guest_memory = GuestMemoryMmap::create(
552524
[(GuestAddress(0), region_size)].into_iter(),
553-
MemoryKind::MapAnonymous,
525+
MemoryKind::MapAnonymous(HugePageConfig::None),
554526
false,
555-
HugePageConfig::None,
556527
)
557528
.unwrap();
558529
check_serde(&guest_memory);
@@ -565,9 +536,8 @@ mod tests {
565536
];
566537
let guest_memory = GuestMemoryMmap::create(
567538
regions.into_iter(),
568-
MemoryKind::MapAnonymous,
539+
MemoryKind::MapAnonymous(HugePageConfig::None),
569540
false,
570-
HugePageConfig::None,
571541
)
572542
.unwrap();
573543
check_serde(&guest_memory);
@@ -584,9 +554,8 @@ mod tests {
584554
];
585555
let guest_memory = GuestMemoryMmap::create(
586556
mem_regions.into_iter(),
587-
MemoryKind::MapAnonymous,
557+
MemoryKind::MapAnonymous(HugePageConfig::None),
588558
true,
589-
HugePageConfig::None,
590559
)
591560
.unwrap();
592561

@@ -615,9 +584,8 @@ mod tests {
615584
];
616585
let guest_memory = GuestMemoryMmap::create(
617586
mem_regions.into_iter(),
618-
MemoryKind::MapAnonymous,
587+
MemoryKind::MapAnonymous(HugePageConfig::None),
619588
true,
620-
HugePageConfig::None,
621589
)
622590
.unwrap();
623591

@@ -654,9 +622,8 @@ mod tests {
654622
];
655623
let guest_memory = GuestMemoryMmap::create(
656624
mem_regions.into_iter(),
657-
MemoryKind::MapAnonymous,
625+
MemoryKind::MapAnonymous(HugePageConfig::None),
658626
true,
659-
HugePageConfig::None,
660627
)
661628
.unwrap();
662629
// Check that Firecracker bitmap is clean.
@@ -683,8 +650,7 @@ mod tests {
683650
.dump(&mut tmp_file.as_file().try_clone().unwrap())
684651
.unwrap();
685652

686-
let restored_guest_memory =
687-
guest_memory_from_file(&path, &memory_state, false, HugePageConfig::None).unwrap();
653+
let restored_guest_memory = guest_memory_from_file(&path, &memory_state, false).unwrap();
688654

689655
// Check that the region contents are the same.
690656
let mut restored_region = vec![0u8; page_size * 2];
@@ -713,9 +679,8 @@ mod tests {
713679
];
714680
let guest_memory = GuestMemoryMmap::create(
715681
mem_regions.into_iter(),
716-
MemoryKind::MapAnonymous,
682+
MemoryKind::MapAnonymous(HugePageConfig::None),
717683
true,
718-
HugePageConfig::None,
719684
)
720685
.unwrap();
721686
// Check that Firecracker bitmap is clean.
@@ -749,8 +714,7 @@ mod tests {
749714
.unwrap();
750715

751716
// We can restore from this because this is the first dirty dump.
752-
let restored_guest_memory =
753-
guest_memory_from_file(&path, &memory_state, false, HugePageConfig::None).unwrap();
717+
let restored_guest_memory = guest_memory_from_file(&path, &memory_state, false).unwrap();
754718

755719
// Check that the region contents are the same.
756720
let mut restored_region = vec![0u8; region_size];
@@ -808,9 +772,8 @@ mod tests {
808772
];
809773
let guest_memory = GuestMemoryMmap::create(
810774
mem_regions.into_iter(),
811-
MemoryKind::MapAnonymous,
775+
MemoryKind::MapAnonymous(HugePageConfig::None),
812776
true,
813-
HugePageConfig::None,
814777
)
815778
.unwrap();
816779

0 commit comments

Comments
 (0)