Simplify microvm builder code and make uffd test failures easier to debug

CHANGELOG.md

@@ -31,6 +31,10 @@ and this project adheres to
 - [#4916](https://github.com/firecracker-microvm/firecracker/pull/4916): Fixed
   `IovDeque` implementation to work with any host page size. This fixes
   virtio-net device on non 4K host kernels.
+- [#4991](https://github.com/firecracker-microvm/firecracker/pull/4991): Fixed
+  `mem_size_mib` and `track_dirty_pages` being mandatory for all
+  `PATCH /machine-config` requests. Now, they can be omitted which leaves these
+  parts of the machine configuration unchanged.
 
 ## [1.10.1]
 

src/cpu-template-helper/src/main.rs

@@ -161,7 +161,7 @@ fn run(cli: Cli) -> Result<(), HelperError> {
                 let (vmm, vm_resources) = utils::build_microvm_from_config(config, template)?;
 
                 let cpu_template = vm_resources
-                    .vm_config
+                    .machine_config
                     .cpu_template
                     .get_cpu_template()?
                     .into_owned();

src/firecracker/examples/uffd/fault_all_handler.rs

@@ -24,6 +24,7 @@ fn main() {
     let (stream, _) = listener.accept().expect("Cannot listen on UDS socket");
 
     let mut runtime = Runtime::new(stream, file);
+    runtime.install_panic_hook();
     runtime.run(|uffd_handler: &mut UffdHandler| {
         // Read an event from the userfaultfd.
         let event = uffd_handler

src/firecracker/examples/uffd/uffd_utils.rs

@@ -208,6 +208,43 @@ impl Runtime {
         }
     }
 
+    fn peer_process_credentials(&self) -> libc::ucred {
+        let mut creds: libc::ucred = libc::ucred {
+            pid: 0,
+            gid: 0,
+            uid: 0,
+        };
+        let mut creds_size = size_of::<libc::ucred>() as u32;
+        let ret = unsafe {
+            libc::getsockopt(
+                self.stream.as_raw_fd(),
+                libc::SOL_SOCKET,
+                libc::SO_PEERCRED,
+                &mut creds as *mut _ as *mut _,
+                &mut creds_size as *mut libc::socklen_t,
+            )
+        };
+        if ret != 0 {
+            panic!("Failed to get peer process credentials");
+        }
+        creds
+    }
+
+    pub fn install_panic_hook(&self) {
+        let peer_creds = self.peer_process_credentials();
+
+        let default_panic_hook = std::panic::take_hook();
+        std::panic::set_hook(Box::new(move |panic_info| {
+            let r = unsafe { libc::kill(peer_creds.pid, libc::SIGKILL) };
+
+            if r != 0 {
+                eprintln!("Failed to kill Firecracker process from panic hook");
+            }
+
+            default_panic_hook(panic_info);
+        }));
+    }
+
     /// Polls the `UnixStream` and UFFD fds in a loop.
     /// When stream is polled, new uffd is retrieved.
     /// When uffd is polled, page fault is handled by

src/firecracker/examples/uffd/valid_handler.rs

@@ -24,6 +24,7 @@ fn main() {
     let (stream, _) = listener.accept().expect("Cannot listen on UDS socket");
 
     let mut runtime = Runtime::new(stream, file);
+    runtime.install_panic_hook();
     runtime.run(|uffd_handler: &mut UffdHandler| {
         // Read an event from the userfaultfd.
         let event = uffd_handler

src/firecracker/src/api_server/parsed_request.rs

@@ -163,8 +163,8 @@ impl ParsedRequest {
                     info!("The request was executed successfully. Status code: 204 No Content.");
                     Response::new(Version::Http11, StatusCode::NoContent)
                 }
-                VmmData::MachineConfiguration(vm_config) => {
-                    Self::success_response_with_data(vm_config)
+                VmmData::MachineConfiguration(machine_config) => {
+                    Self::success_response_with_data(machine_config)
                 }
                 VmmData::MmdsValue(value) => Self::success_response_with_mmds_value(value),
                 VmmData::BalloonConfig(balloon_config) => {

src/firecracker/src/api_server/request/machine_configuration.rs

@@ -31,7 +31,8 @@ pub(crate) fn parse_put_machine_config(body: &Body) -> Result<ParsedRequest, Req
     let config_update = MachineConfigUpdate::from(config);
 
     // Construct the `ParsedRequest` object.
-    let mut parsed_req = ParsedRequest::new_sync(VmmAction::UpdateVmConfiguration(config_update));
+    let mut parsed_req =
+        ParsedRequest::new_sync(VmmAction::UpdateMachineConfiguration(config_update));
     // If `cpu_template` was present, set the deprecation message in `parsing_info`.
     if let Some(msg) = deprecation_message {
         parsed_req.parsing_info().append_deprecation_message(msg);
@@ -60,7 +61,8 @@ pub(crate) fn parse_patch_machine_config(body: &Body) -> Result<ParsedRequest, R
     }
 
     // Construct the `ParsedRequest` object.
-    let mut parsed_req = ParsedRequest::new_sync(VmmAction::UpdateVmConfiguration(config_update));
+    let mut parsed_req =
+        ParsedRequest::new_sync(VmmAction::UpdateMachineConfiguration(config_update));
     // If `cpu_template` was present, set the deprecation message in `parsing_info`.
     if let Some(msg) = deprecation_message {
         parsed_req.parsing_info().append_deprecation_message(msg);
@@ -124,7 +126,7 @@ mod tests {
             };
             assert_eq!(
                 vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()),
-                VmmAction::UpdateVmConfiguration(expected_config)
+                VmmAction::UpdateMachineConfiguration(expected_config)
             );
         }
 
@@ -143,7 +145,7 @@ mod tests {
         };
         assert_eq!(
             vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()),
-            VmmAction::UpdateVmConfiguration(expected_config)
+            VmmAction::UpdateMachineConfiguration(expected_config)
         );
 
         let body = r#"{
@@ -162,7 +164,7 @@ mod tests {
         };
         assert_eq!(
             vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()),
-            VmmAction::UpdateVmConfiguration(expected_config)
+            VmmAction::UpdateMachineConfiguration(expected_config)
         );
 
         // 4. Test that applying a CPU template is successful on x86_64 while on aarch64, it is not.
@@ -185,7 +187,7 @@ mod tests {
             };
             assert_eq!(
                 vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()),
-                VmmAction::UpdateVmConfiguration(expected_config)
+                VmmAction::UpdateMachineConfiguration(expected_config)
             );
         }
         #[cfg(target_arch = "aarch64")]
@@ -210,7 +212,7 @@ mod tests {
         };
         assert_eq!(
             vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()),
-            VmmAction::UpdateVmConfiguration(expected_config)
+            VmmAction::UpdateMachineConfiguration(expected_config)
         );
 
         // 6. Test nonsense values for huge page size

src/vmm/benches/memory_access.rs

@@ -4,7 +4,7 @@
 use criterion::{criterion_group, criterion_main, BatchSize, Criterion};
 use vm_memory::GuestMemory;
 use vmm::resources::VmResources;
-use vmm::vmm_config::machine_config::{HugePageConfig, VmConfig};
+use vmm::vmm_config::machine_config::{HugePageConfig, MachineConfig};
 
 fn bench_single_page_fault(c: &mut Criterion, configuration: VmResources) {
     c.bench_function("page_fault", |b| {
@@ -33,7 +33,7 @@ pub fn bench_4k_page_fault(c: &mut Criterion) {
     bench_single_page_fault(
         c,
         VmResources {
-            vm_config: VmConfig {
+            machine_config: MachineConfig {
                 vcpu_count: 1,
                 mem_size_mib: 2,
                 ..Default::default()
@@ -47,7 +47,7 @@ pub fn bench_2m_page_fault(c: &mut Criterion) {
     bench_single_page_fault(
         c,
         VmResources {
-            vm_config: VmConfig {
+            machine_config: MachineConfig {
                 vcpu_count: 1,
                 mem_size_mib: 2,
                 huge_pages: HugePageConfig::Hugetlbfs2M,

src/vmm/src/builder.rs

@@ -67,7 +67,7 @@
 use crate::utils::u64_to_usize;
 use crate::vmm_config::boot_source::BootConfig;
 use crate::vmm_config::instance_info::InstanceInfo;
-use crate::vmm_config::machine_config::{VmConfig, VmConfigError};
+use crate::vmm_config::machine_config::{MachineConfig, MachineConfigError};
 use crate::vstate::memory::{GuestAddress, GuestMemory, GuestMemoryMmap};
 use crate::vstate::vcpu::{Vcpu, VcpuConfig, VcpuError};
 use crate::vstate::vm::Vm;
@@ -124,7 +124,7 @@
     /// Cannot restore microvm state: {0}
     RestoreMicrovmState(MicrovmStateError),
     /// Cannot set vm resources: {0}
-    SetVmResources(VmConfigError),
+    SetVmResources(MachineConfigError),
     /// Cannot create the entropy device: {0}
     CreateEntropyDevice(crate::devices::virtio::rng::EntropyError),
     /// Failed to allocate guest resource: {0}
@@ -274,15 +274,18 @@
     #[allow(unused_mut)]
     let mut boot_cmdline = boot_config.cmdline.clone();
 
-    let cpu_template = vm_resources.vm_config.cpu_template.get_cpu_template()?;
+    let cpu_template = vm_resources
+        .machine_config
+        .cpu_template
+        .get_cpu_template()?;
 
     let (mut vmm, mut vcpus) = create_vmm_and_vcpus(
         instance_info,
         event_manager,
         guest_memory,
         None,
-        vm_resources.vm_config.track_dirty_pages,
-        vm_resources.vm_config.vcpu_count,
+        vm_resources.machine_config.track_dirty_pages,
+        vm_resources.machine_config.vcpu_count,
         cpu_template.kvm_capabilities.clone(),
     )?;
 
@@ -338,7 +341,7 @@
     configure_system_for_boot(
         &mut vmm,
         vcpus.as_mut(),
-        &vm_resources.vm_config,
+        &vm_resources.machine_config,
         &cpu_template,
         entry_addr,
         &initrd,
@@ -348,7 +351,7 @@
     let vmm = Arc::new(Mutex::new(vmm));
 
     #[cfg(feature = "gdb")]
-    if let Some(gdb_socket_path) = &vm_resources.vm_config.gdb_socket_path {
+    if let Some(gdb_socket_path) = &vm_resources.machine_config.gdb_socket_path {
         gdb::gdb_thread(vmm.clone(), vcpu_fds, gdb_rx, entry_addr, gdb_socket_path)
             .map_err(GdbServer)?;
     } else {
@@ -429,7 +432,7 @@
     /// Failed to restore microVM state: {0}
     RestoreState(#[from] crate::vstate::vm::RestoreStateError),
     /// Failed to update microVM configuration: {0}
-    VmUpdateConfig(#[from] VmConfigError),
+    VmUpdateConfig(#[from] MachineConfigError),
     /// Failed to restore MMIO device: {0}
     RestoreMmioDevice(#[from] MicrovmStateError),
     /// Failed to emulate MMIO serial: {0}
@@ -469,10 +472,10 @@
     let (mut vmm, mut vcpus) = create_vmm_and_vcpus(
         instance_info,
         event_manager,
-        guest_memory.clone(),
+        guest_memory,
         uffd,
-        vm_resources.vm_config.track_dirty_pages,
-        vm_resources.vm_config.vcpu_count,
+        vm_resources.machine_config.track_dirty_pages,
+        vm_resources.machine_config.vcpu_count,
         microvm_state.vm_state.kvm_cap_modifiers.clone(),
     )?;
 
@@ -514,7 +517,7 @@
 
     // Restore devices states.
     let mmio_ctor_args = MMIODevManagerConstructorArgs {
-        mem: &guest_memory,
+        mem: &vmm.guest_memory,
         vm: vmm.vm.fd(),
         event_manager,
         resource_allocator: &mut vmm.resource_allocator,
@@ -529,7 +532,7 @@
 
     {
         let acpi_ctor_args = ACPIDeviceManagerConstructorArgs {
-            mem: &guest_memory,
+            mem: &vmm.guest_memory,
             resource_allocator: &mut vmm.resource_allocator,
             vm: vmm.vm.fd(),
         };
@@ -750,7 +753,7 @@
 pub fn configure_system_for_boot(
     vmm: &mut Vmm,
     vcpus: &mut [Vcpu],
-    vm_config: &VmConfig,
+    machine_config: &MachineConfig,
     cpu_template: &CustomCpuTemplate,
     entry_addr: GuestAddress,
     initrd: &Option<InitrdConfig>,
@@ -793,8 +796,8 @@
     let cpu_config = CpuConfiguration::apply_template(cpu_config, cpu_template)?;
 
     let vcpu_config = VcpuConfig {
-        vcpu_count: vm_config.vcpu_count,
-        smt: vm_config.smt,
+        vcpu_count: machine_config.vcpu_count,
+        smt: machine_config.smt,
         cpu_config,
     };
 

src/vmm/src/persist.rs

@@ -32,7 +32,7 @@
 use crate::utils::u64_to_usize;
 use crate::vmm_config::boot_source::BootSourceConfig;
 use crate::vmm_config::instance_info::InstanceInfo;
-use crate::vmm_config::machine_config::{HugePageConfig, MachineConfigUpdate, VmConfigError};
+use crate::vmm_config::machine_config::{HugePageConfig, MachineConfigError, MachineConfigUpdate};
 use crate::vmm_config::snapshot::{
     CreateSnapshotParams, LoadSnapshotParams, MemBackendType, SnapshotType,
 };
@@ -61,11 +61,11 @@
 impl From<&VmResources> for VmInfo {
     fn from(value: &VmResources) -> Self {
         Self {
-            mem_size_mib: value.vm_config.mem_size_mib as u64,
-            smt: value.vm_config.smt,
-            cpu_template: StaticCpuTemplate::from(&value.vm_config.cpu_template),
+            mem_size_mib: value.machine_config.mem_size_mib as u64,
+            smt: value.machine_config.smt,
+            cpu_template: StaticCpuTemplate::from(&value.machine_config.cpu_template),
             boot_source: value.boot_source.config.clone(),
-            huge_pages: value.vm_config.huge_pages,
+            huge_pages: value.machine_config.huge_pages,
         }
     }
 }
@@ -422,11 +422,11 @@
         .vcpu_states
         .len()
         .try_into()
-        .map_err(|_| VmConfigError::InvalidVcpuCount)
+        .map_err(|_| MachineConfigError::InvalidVcpuCount)
         .map_err(BuildMicrovmFromSnapshotError::VmUpdateConfig)?;
 
     vm_resources
-        .update_vm_config(&MachineConfigUpdate {
+        .update_machine_config(&MachineConfigUpdate {
             vcpu_count: Some(vcpu_count),
             mem_size_mib: Some(u64_to_usize(microvm_state.vm_info.mem_size_mib)),
             smt: Some(microvm_state.vm_info.smt),
@@ -450,7 +450,7 @@
                 mem_backend_path,
                 mem_state,
                 track_dirty_pages,
-                vm_resources.vm_config.huge_pages,
+                vm_resources.machine_config.huge_pages,
             )
             .map_err(RestoreFromSnapshotGuestMemoryError::File)?,
             None,
@@ -462,7 +462,7 @@
             // We enable the UFFD_FEATURE_EVENT_REMOVE feature only if a balloon device
             // is present in the microVM state.
             microvm_state.device_states.balloon_device.is_some(),
-            vm_resources.vm_config.huge_pages,
+            vm_resources.machine_config.huge_pages,
         )
         .map_err(RestoreFromSnapshotGuestMemoryError::Uffd)?,
     };