Skip to content

Commit 3793b99

Browse files
roypatroypat
committed
refactor: Get rid of VmConfig
This struct is almost a 1:1 duplication of `struct MachineConfig`, with only a single deviation when it comes to CPU template handling (see below). This makes it very annoying to add new fields to the /machine-config endpoint, because counter-intuitively we have to hand-edit at least 3 structs to add new fields to. We can get rid of this duplication by merging VmConfig and MachineConfig into just `MachineConfig` (that's what the endpoint is called, so having the struct be the same makes sense). We now need to handle a bit of nasty-ness when it comes to CPU templates, because /machine-config can only be used for specifying static cpu templates, while a VmConfig is used to hold whatever CPU template is stored, in whatever way. However, we can handle this at the serde layer, by making serialize/deserialize ignore the field if it doesnt contain a static template. This is ugly, but since static templates are deprecated, we have line of sight to getting rid of this weirdness when we release 2.0. While we're at it, opportunistically rename functions etc to uniformly call this thing a "machine config" instead of a "vm config". Signed-off-by: Patrick Roy <[email protected]>
1 parent 9cf1e6d commit 3793b99

File tree

11 files changed

+234
-166
lines changed

11 files changed

+234
-166
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ fn run(cli: Cli) -> Result<(), HelperError> {
161161
let (vmm, vm_resources) = utils::build_microvm_from_config(config, template)?;
162162

163163
let cpu_template = vm_resources
164-
.vm_config
164+
.machine_config
165165
.cpu_template
166166
.get_cpu_template()?
167167
.into_owned();

‎src/firecracker/src/api_server/parsed_request.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,8 @@ impl ParsedRequest {
163163
info!("The request was executed successfully. Status code: 204 No Content.");
164164
Response::new(Version::Http11, StatusCode::NoContent)
165165
}
166-
VmmData::MachineConfiguration(vm_config) => {
167-
Self::success_response_with_data(vm_config)
166+
VmmData::MachineConfiguration(machine_config) => {
167+
Self::success_response_with_data(machine_config)
168168
}
169169
VmmData::MmdsValue(value) => Self::success_response_with_mmds_value(value),
170170
VmmData::BalloonConfig(balloon_config) => {

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ pub(crate) fn parse_put_machine_config(body: &Body) -> Result<ParsedRequest, Req
3131
let config_update = MachineConfigUpdate::from(config);
3232

3333
// Construct the `ParsedRequest` object.
34-
let mut parsed_req = ParsedRequest::new_sync(VmmAction::UpdateVmConfiguration(config_update));
34+
let mut parsed_req =
35+
ParsedRequest::new_sync(VmmAction::UpdateMachineConfiguration(config_update));
3536
// If `cpu_template` was present, set the deprecation message in `parsing_info`.
3637
if let Some(msg) = deprecation_message {
3738
parsed_req.parsing_info().append_deprecation_message(msg);
@@ -60,7 +61,8 @@ pub(crate) fn parse_patch_machine_config(body: &Body) -> Result<ParsedRequest, R
6061
}
6162

6263
// Construct the `ParsedRequest` object.
63-
let mut parsed_req = ParsedRequest::new_sync(VmmAction::UpdateVmConfiguration(config_update));
64+
let mut parsed_req =
65+
ParsedRequest::new_sync(VmmAction::UpdateMachineConfiguration(config_update));
6466
// If `cpu_template` was present, set the deprecation message in `parsing_info`.
6567
if let Some(msg) = deprecation_message {
6668
parsed_req.parsing_info().append_deprecation_message(msg);
@@ -124,7 +126,7 @@ mod tests {
124126
};
125127
assert_eq!(
126128
vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()),
127-
VmmAction::UpdateVmConfiguration(expected_config)
129+
VmmAction::UpdateMachineConfiguration(expected_config)
128130
);
129131
}
130132

@@ -143,7 +145,7 @@ mod tests {
143145
};
144146
assert_eq!(
145147
vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()),
146-
VmmAction::UpdateVmConfiguration(expected_config)
148+
VmmAction::UpdateMachineConfiguration(expected_config)
147149
);
148150

149151
let body = r#"{
@@ -162,7 +164,7 @@ mod tests {
162164
};
163165
assert_eq!(
164166
vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()),
165-
VmmAction::UpdateVmConfiguration(expected_config)
167+
VmmAction::UpdateMachineConfiguration(expected_config)
166168
);
167169

168170
// 4. Test that applying a CPU template is successful on x86_64 while on aarch64, it is not.
@@ -185,7 +187,7 @@ mod tests {
185187
};
186188
assert_eq!(
187189
vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()),
188-
VmmAction::UpdateVmConfiguration(expected_config)
190+
VmmAction::UpdateMachineConfiguration(expected_config)
189191
);
190192
}
191193
#[cfg(target_arch = "aarch64")]
@@ -210,7 +212,7 @@ mod tests {
210212
};
211213
assert_eq!(
212214
vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()),
213-
VmmAction::UpdateVmConfiguration(expected_config)
215+
VmmAction::UpdateMachineConfiguration(expected_config)
214216
);
215217

216218
// 6. Test nonsense values for huge page size

‎src/vmm/benches/memory_access.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
use criterion::{criterion_group, criterion_main, BatchSize, Criterion};
55
use vm_memory::GuestMemory;
66
use vmm::resources::VmResources;
7-
use vmm::vmm_config::machine_config::{HugePageConfig, VmConfig};
7+
use vmm::vmm_config::machine_config::{HugePageConfig, MachineConfig};
88

99
fn bench_single_page_fault(c: &mut Criterion, configuration: VmResources) {
1010
c.bench_function("page_fault", |b| {
@@ -33,7 +33,7 @@ pub fn bench_4k_page_fault(c: &mut Criterion) {
3333
bench_single_page_fault(
3434
c,
3535
VmResources {
36-
vm_config: VmConfig {
36+
machine_config: MachineConfig {
3737
vcpu_count: 1,
3838
mem_size_mib: 2,
3939
..Default::default()
@@ -47,7 +47,7 @@ pub fn bench_2m_page_fault(c: &mut Criterion) {
4747
bench_single_page_fault(
4848
c,
4949
VmResources {
50-
vm_config: VmConfig {
50+
machine_config: MachineConfig {
5151
vcpu_count: 1,
5252
mem_size_mib: 2,
5353
huge_pages: HugePageConfig::Hugetlbfs2M,

‎src/vmm/src/builder.rs‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ use crate::snapshot::Persist;
6767
use crate::utils::u64_to_usize;
6868
use crate::vmm_config::boot_source::BootConfig;
6969
use crate::vmm_config::instance_info::InstanceInfo;
70-
use crate::vmm_config::machine_config::{VmConfig, VmConfigError};
70+
use crate::vmm_config::machine_config::{MachineConfig, MachineConfigError};
7171
use crate::vstate::memory::{GuestAddress, GuestMemory, GuestMemoryMmap};
7272
use crate::vstate::vcpu::{Vcpu, VcpuConfig, VcpuError};
7373
use crate::vstate::vm::Vm;
@@ -124,7 +124,7 @@ pub enum StartMicrovmError {
124124
/// Cannot restore microvm state: {0}
125125
RestoreMicrovmState(MicrovmStateError),
126126
/// Cannot set vm resources: {0}
127-
SetVmResources(VmConfigError),
127+
SetVmResources(MachineConfigError),
128128
/// Cannot create the entropy device: {0}
129129
CreateEntropyDevice(crate::devices::virtio::rng::EntropyError),
130130
/// Failed to allocate guest resource: {0}
@@ -274,15 +274,18 @@ pub fn build_microvm_for_boot(
274274
#[allow(unused_mut)]
275275
let mut boot_cmdline = boot_config.cmdline.clone();
276276

277-
let cpu_template = vm_resources.vm_config.cpu_template.get_cpu_template()?;
277+
let cpu_template = vm_resources
278+
.machine_config
279+
.cpu_template
280+
.get_cpu_template()?;
278281

279282
let (mut vmm, mut vcpus) = create_vmm_and_vcpus(
280283
instance_info,
281284
event_manager,
282285
guest_memory,
283286
None,
284-
vm_resources.vm_config.track_dirty_pages,
285-
vm_resources.vm_config.vcpu_count,
287+
vm_resources.machine_config.track_dirty_pages,
288+
vm_resources.machine_config.vcpu_count,
286289
cpu_template.kvm_capabilities.clone(),
287290
)?;
288291

@@ -338,7 +341,7 @@ pub fn build_microvm_for_boot(
338341
configure_system_for_boot(
339342
&mut vmm,
340343
vcpus.as_mut(),
341-
&vm_resources.vm_config,
344+
&vm_resources.machine_config,
342345
&cpu_template,
343346
entry_addr,
344347
&initrd,
@@ -348,7 +351,7 @@ pub fn build_microvm_for_boot(
348351
let vmm = Arc::new(Mutex::new(vmm));
349352

350353
#[cfg(feature = "gdb")]
351-
if let Some(gdb_socket_path) = &vm_resources.vm_config.gdb_socket_path {
354+
if let Some(gdb_socket_path) = &vm_resources.machine_config.gdb_socket_path {
352355
gdb::gdb_thread(vmm.clone(), vcpu_fds, gdb_rx, entry_addr, gdb_socket_path)
353356
.map_err(GdbServer)?;
354357
} else {
@@ -429,7 +432,7 @@ pub enum BuildMicrovmFromSnapshotError {
429432
/// Failed to restore microVM state: {0}
430433
RestoreState(#[from] crate::vstate::vm::RestoreStateError),
431434
/// Failed to update microVM configuration: {0}
432-
VmUpdateConfig(#[from] VmConfigError),
435+
VmUpdateConfig(#[from] MachineConfigError),
433436
/// Failed to restore MMIO device: {0}
434437
RestoreMmioDevice(#[from] MicrovmStateError),
435438
/// Failed to emulate MMIO serial: {0}
@@ -471,8 +474,8 @@ pub fn build_microvm_from_snapshot(
471474
event_manager,
472475
guest_memory.clone(),
473476
uffd,
474-
vm_resources.vm_config.track_dirty_pages,
475-
vm_resources.vm_config.vcpu_count,
477+
vm_resources.machine_config.track_dirty_pages,
478+
vm_resources.machine_config.vcpu_count,
476479
microvm_state.vm_state.kvm_cap_modifiers.clone(),
477480
)?;
478481

@@ -750,7 +753,7 @@ fn create_vcpus(vm: &Vm, vcpu_count: u8, exit_evt: &EventFd) -> Result<Vec<Vcpu>
750753
pub fn configure_system_for_boot(
751754
vmm: &mut Vmm,
752755
vcpus: &mut [Vcpu],
753-
vm_config: &VmConfig,
756+
machine_config: &MachineConfig,
754757
cpu_template: &CustomCpuTemplate,
755758
entry_addr: GuestAddress,
756759
initrd: &Option<InitrdConfig>,
@@ -793,8 +796,8 @@ pub fn configure_system_for_boot(
793796
let cpu_config = CpuConfiguration::apply_template(cpu_config, cpu_template)?;
794797

795798
let vcpu_config = VcpuConfig {
796-
vcpu_count: vm_config.vcpu_count,
797-
smt: vm_config.smt,
799+
vcpu_count: machine_config.vcpu_count,
800+
smt: machine_config.smt,
798801
cpu_config,
799802
};
800803

‎src/vmm/src/persist.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ use crate::snapshot::Snapshot;
3232
use crate::utils::u64_to_usize;
3333
use crate::vmm_config::boot_source::BootSourceConfig;
3434
use crate::vmm_config::instance_info::InstanceInfo;
35-
use crate::vmm_config::machine_config::{HugePageConfig, MachineConfigUpdate, VmConfigError};
35+
use crate::vmm_config::machine_config::{HugePageConfig, MachineConfigError, MachineConfigUpdate};
3636
use crate::vmm_config::snapshot::{
3737
CreateSnapshotParams, LoadSnapshotParams, MemBackendType, SnapshotType,
3838
};
@@ -61,11 +61,11 @@ pub struct VmInfo {
6161
impl From<&VmResources> for VmInfo {
6262
fn from(value: &VmResources) -> Self {
6363
Self {
64-
mem_size_mib: value.vm_config.mem_size_mib as u64,
65-
smt: value.vm_config.smt,
66-
cpu_template: StaticCpuTemplate::from(&value.vm_config.cpu_template),
64+
mem_size_mib: value.machine_config.mem_size_mib as u64,
65+
smt: value.machine_config.smt,
66+
cpu_template: StaticCpuTemplate::from(&value.machine_config.cpu_template),
6767
boot_source: value.boot_source.config.clone(),
68-
huge_pages: value.vm_config.huge_pages,
68+
huge_pages: value.machine_config.huge_pages,
6969
}
7070
}
7171
}
@@ -422,11 +422,11 @@ pub fn restore_from_snapshot(
422422
.vcpu_states
423423
.len()
424424
.try_into()
425-
.map_err(|_| VmConfigError::InvalidVcpuCount)
425+
.map_err(|_| MachineConfigError::InvalidVcpuCount)
426426
.map_err(BuildMicrovmFromSnapshotError::VmUpdateConfig)?;
427427

428428
vm_resources
429-
.update_vm_config(&MachineConfigUpdate {
429+
.update_machine_config(&MachineConfigUpdate {
430430
vcpu_count: Some(vcpu_count),
431431
mem_size_mib: Some(u64_to_usize(microvm_state.vm_info.mem_size_mib)),
432432
smt: Some(microvm_state.vm_info.smt),
@@ -450,7 +450,7 @@ pub fn restore_from_snapshot(
450450
mem_backend_path,
451451
mem_state,
452452
track_dirty_pages,
453-
vm_resources.vm_config.huge_pages,
453+
vm_resources.machine_config.huge_pages,
454454
)
455455
.map_err(RestoreFromSnapshotGuestMemoryError::File)?,
456456
None,
@@ -462,7 +462,7 @@ pub fn restore_from_snapshot(
462462
// We enable the UFFD_FEATURE_EVENT_REMOVE feature only if a balloon device
463463
// is present in the microVM state.
464464
microvm_state.device_states.balloon_device.is_some(),
465-
vm_resources.vm_config.huge_pages,
465+
vm_resources.machine_config.huge_pages,
466466
)
467467
.map_err(RestoreFromSnapshotGuestMemoryError::Uffd)?,
468468
};

0 commit comments

Comments
 (0)