Skip to content

Commit e0959d2

Browse files
committed
feat(vmm): migrate all log callsites to rate-limited macros
Replace every error, warn, and info macro invocation in the vmm crate with its rate-limited counterpart. This covers all subsystems: virtio devices, legacy devices, PCI, ACPI, vCPU handling, signal handlers, device manager, snapshot paths, RPC interface, GDB support, and the rate limiter itself. debug-level calls are left unchanged as they are filtered out in production builds. Signed-off-by: Nikita Kalyazin <kalyazin@amazon.com>
1 parent 7d67c69 commit e0959d2

62 files changed

Lines changed: 562 additions & 464 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

src/vmm/src/acpi/mod.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
use acpi_tables::fadt::{FADT_F_HW_REDUCED_ACPI, FADT_F_PWR_BUTTON, FADT_F_SLP_BUTTON};
55
use acpi_tables::{Aml, Dsdt, Fadt, Madt, Mcfg, Rsdp, Sdt, Xsdt, aml};
6-
use log::{debug, error};
6+
use log::debug;
77
use vm_allocator::AllocPolicy;
88

99
use crate::Vcpu;
@@ -12,6 +12,7 @@ use crate::acpi::x86_64::{
1212
};
1313
use crate::arch::x86_64::layout;
1414
use crate::device_manager::DeviceManager;
15+
use crate::logger::error_rate_limited;
1516
use crate::vstate::memory::{GuestAddress, GuestMemoryMmap};
1617
use crate::vstate::resources::ResourceAllocator;
1718

@@ -68,7 +69,9 @@ impl AcpiTableWriter<'_> {
6869

6970
table
7071
.write_to_guest(self.mem, GuestAddress(addr))
71-
.inspect_err(|err| error!("acpi: Could not write table in guest memory: {err}"))?;
72+
.inspect_err(|err| {
73+
error_rate_limited!("acpi: Could not write table in guest memory: {err}")
74+
})?;
7275

7376
debug!(
7477
"acpi: Wrote table ({} bytes) at address: {:#010x}",
@@ -179,7 +182,9 @@ impl AcpiTableWriter<'_> {
179182
fn build_rsdp(&mut self, xsdt_addr: u64) -> Result<(), AcpiError> {
180183
let mut rsdp = Rsdp::new(OEM_ID, xsdt_addr);
181184
rsdp.write_to_guest(self.mem, rsdp_addr())
182-
.inspect_err(|err| error!("acpi: Could not write RSDP in guest memory: {err}"))?;
185+
.inspect_err(|err| {
186+
error_rate_limited!("acpi: Could not write RSDP in guest memory: {err}")
187+
})?;
183188

184189
debug!(
185190
"acpi: Wrote RSDP ({} bytes) at address: {:#010x}",

src/vmm/src/arch/aarch64/cache_info.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
use std::path::{Path, PathBuf};
55
use std::{fs, io};
66

7-
use crate::logger::warn;
7+
use crate::logger::warn_rate_limited;
88

99
// Based on https://elixir.free-electrons.com/linux/v4.9.62/source/arch/arm64/kernel/cacheinfo.c#L29.
1010
const MAX_CACHE_LEVEL: u8 = 7;
@@ -304,7 +304,7 @@ pub(crate) fn read_cache_config(
304304
let level = cache.level;
305305
append_cache_level(cache_l1, cache_non_l1, cache);
306306
if !msg.is_empty() && !logged_missing_attr {
307-
warn!("Could not read the {msg} for cache level {level}.");
307+
warn_rate_limited!("Could not read the {msg} for cache level {level}.");
308308
logged_missing_attr = true;
309309
}
310310
}

src/vmm/src/arch/aarch64/mod.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use crate::cpu_config::templates::CustomCpuTemplate;
3030
use crate::initrd::InitrdConfig;
3131
use zerocopy::IntoBytes;
3232

33-
use crate::logger::warn;
33+
use crate::logger::warn_rate_limited;
3434
use crate::utils::{align_up, u64_to_usize, usize_to_u64};
3535
use crate::vmm_config::machine_config::MachineConfig;
3636
use crate::vstate::memory::{
@@ -66,7 +66,7 @@ pub fn arch_memory_regions(size: usize) -> Vec<(GuestAddress, usize)> {
6666
let dram_size = min(size, layout::DRAM_MEM_MAX_SIZE);
6767

6868
if dram_size != size {
69-
logger::warn!(
69+
logger::warn_rate_limited!(
7070
"Requested memory size {} exceeds architectural maximum (1022GiB). Size has been \
7171
truncated to {}",
7272
size,
@@ -176,7 +176,7 @@ fn override_clidr(vcpus: &[Vcpu]) -> Result<(), ConfigurationError> {
176176
// Writing an all-zero CLIDR would tell the guest there are no caches,
177177
// which is worse than whatever KVM fabricated. Leave it alone.
178178
if l1_caches.is_empty() {
179-
warn!("No L1 caches found in sysfs, skipping CLIDR override");
179+
warn_rate_limited!("No L1 caches found in sysfs, skipping CLIDR override");
180180
return Ok(());
181181
}
182182

@@ -191,7 +191,7 @@ fn override_clidr(vcpus: &[Vcpu]) -> Result<(), ConfigurationError> {
191191
.fd
192192
.get_one_reg(regs::CLIDR_EL1, cur_clidr.as_mut_bytes())
193193
{
194-
warn!("Failed to read CLIDR_EL1, skipping override: {e}");
194+
warn_rate_limited!("Failed to read CLIDR_EL1, skipping override: {e}");
195195
return Ok(());
196196
}
197197

@@ -204,9 +204,10 @@ fn override_clidr(vcpus: &[Vcpu]) -> Result<(), ConfigurationError> {
204204
.fd
205205
.set_one_reg(regs::CLIDR_EL1, new_clidr.as_bytes())
206206
{
207-
warn!(
207+
warn_rate_limited!(
208208
"Failed to set CLIDR_EL1 to {:#x} on vCPU {}, skipping override: {e}",
209-
new_clidr, vcpu.kvm_vcpu.index
209+
new_clidr,
210+
vcpu.kvm_vcpu.index
210211
);
211212
return Ok(());
212213
}

src/vmm/src/arch/aarch64/vcpu.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use crate::arch::aarch64::kvm::OptionalCapabilities;
2121
use crate::arch::aarch64::regs::{Aarch64RegisterVec, KVM_REG_ARM64_SVE_VLS};
2222
use crate::cpu_config::aarch64::custom_cpu_template::VcpuFeatures;
2323
use crate::cpu_config::templates::CpuConfiguration;
24-
use crate::logger::{IncMetric, METRICS, error};
24+
use crate::logger::{IncMetric, METRICS, error_rate_limited};
2525
use crate::vcpu::{VcpuConfig, VcpuError};
2626
use crate::vstate::bus::Bus;
2727
use crate::vstate::memory::{Address, GuestMemoryMmap};
@@ -503,7 +503,7 @@ impl Peripherals {
503503
METRICS.vcpu.failures.inc();
504504
// TODO: Are we sure we want to finish running a vcpu upon
505505
// receiving a vm exit that is not necessarily an error?
506-
error!("Unexpected exit reason on vcpu run: {:?}", exit);
506+
error_rate_limited!("Unexpected exit reason on vcpu run: {:?}", exit);
507507
Err(VcpuError::UnhandledKvmExit(format!("{:?}", exit)))
508508
}
509509
}

src/vmm/src/arch/mod.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
use std::fmt;
55
use std::sync::LazyLock;
66

7-
use log::warn;
7+
use crate::logger::warn_rate_limited;
88
use serde::{Deserialize, Serialize};
99
use vm_memory::GuestAddress;
1010

@@ -66,7 +66,9 @@ pub fn host_page_size() -> usize {
6666
// # Safety: Value always valid
6767
let r = unsafe { libc::sysconf(libc::_SC_PAGESIZE) };
6868
usize::try_from(r).unwrap_or_else(|_| {
69-
warn!("Could not get host page size with sysconf, assuming default 4K host pages");
69+
warn_rate_limited!(
70+
"Could not get host page size with sysconf, assuming default 4K host pages"
71+
);
7072
4096
7173
})
7274
});

src/vmm/src/arch/x86_64/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ pub fn arch_memory_regions(size: usize) -> Vec<(GuestAddress, usize)> {
117117
);
118118

119119
if dram_size != size {
120-
logger::warn!(
120+
logger::warn_rate_limited!(
121121
"Requested memory size {} exceeds architectural maximum (1022GiB). Size has been \
122122
truncated to {}",
123123
size,

src/vmm/src/arch/x86_64/vcpu.rs

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ use kvm_bindings::{
1414
kvm_mp_state, kvm_regs, kvm_sregs, kvm_vcpu_events, kvm_xcrs, kvm_xsave, kvm_xsave2,
1515
};
1616
use kvm_ioctls::{VcpuExit, VcpuFd};
17-
use log::{error, warn};
1817
use serde::{Deserialize, Serialize};
1918
use vmm_sys_util::fam::{self, FamStruct};
2019

@@ -24,7 +23,7 @@ use crate::arch::x86_64::interrupts;
2423
use crate::arch::x86_64::msr::{MsrError, create_boot_msr_entries};
2524
use crate::arch::x86_64::regs::{SetupFpuError, SetupRegistersError, SetupSpecialRegistersError};
2625
use crate::cpu_config::x86_64::{CpuConfiguration, cpuid};
27-
use crate::logger::{IncMetric, METRICS};
26+
use crate::logger::{IncMetric, METRICS, error_rate_limited, warn_rate_limited};
2827
use crate::vstate::bus::Bus;
2928
use crate::vstate::memory::GuestMemoryMmap;
3029
use crate::vstate::vcpu::{VcpuConfig, VcpuEmulation, VcpuError};
@@ -281,7 +280,7 @@ impl KvmVcpu {
281280
// https://elixir.bootlin.com/linux/v6.17.5/source/arch/x86/kvm/x86.c#L5736-L5737
282281
if let Err(err) = self.fd.kvmclock_ctrl() {
283282
METRICS.vcpu.kvmclock_ctrl_fails.inc();
284-
warn!("KVM_KVMCLOCK_CTRL call failed {}", err);
283+
warn_rate_limited!("KVM_KVMCLOCK_CTRL call failed {}", err);
285284
}
286285
}
287286

@@ -403,7 +402,7 @@ impl KvmVcpu {
403402
.flat_map(|msrs| msrs.as_mut_slice())
404403
.filter(|msr| msr.index == MSR_IA32_TSC_DEADLINE && msr.data == 0)
405404
.for_each(|msr| {
406-
warn!(
405+
warn_rate_limited!(
407406
"MSR_IA32_TSC_DEADLINE is 0, replacing with {:#x}.",
408407
tsc_value
409408
);
@@ -430,9 +429,10 @@ impl KvmVcpu {
430429
deferred_msrs
431430
.push(*msr)
432431
.inspect_err(|err| {
433-
error!(
432+
error_rate_limited!(
434433
"Failed to move MSR {} into later chunk: {:?}",
435-
msr.index, err
434+
msr.index,
435+
err
436436
)
437437
})
438438
.is_err()
@@ -589,7 +589,9 @@ impl KvmVcpu {
589589
// v0.25 and newer snapshots without TSC will only work on
590590
// the same CPU model as the host on which they were taken.
591591
// TODO: Add negative test for this warning failure.
592-
warn!("TSC freq not available. Snapshot cannot be loaded on a different CPU model.");
592+
warn_rate_limited!(
593+
"TSC freq not available. Snapshot cannot be loaded on a different CPU model."
594+
);
593595
None
594596
});
595597
let cpuid = self.get_cpuid()?;
@@ -729,7 +731,10 @@ impl Peripherals {
729731
if let Some(pio_bus) = &self.pio_bus {
730732
let _metric = METRICS.vcpu.exit_io_in_agg.record_latency_metrics();
731733
if let Err(err) = pio_bus.read(u64::from(addr), data) {
732-
warn!("vcpu: IO read @ {addr:#x}:{:#x} failed: {err}", data.len());
734+
warn_rate_limited!(
735+
"vcpu: IO read @ {addr:#x}:{:#x} failed: {err}",
736+
data.len()
737+
);
733738
}
734739
METRICS.vcpu.exit_io_in.inc();
735740
}
@@ -739,15 +744,18 @@ impl Peripherals {
739744
if let Some(pio_bus) = &self.pio_bus {
740745
let _metric = METRICS.vcpu.exit_io_out_agg.record_latency_metrics();
741746
if let Err(err) = pio_bus.write(u64::from(addr), data) {
742-
warn!("vcpu: IO write @ {addr:#x}:{:#x} failed: {err}", data.len());
747+
warn_rate_limited!(
748+
"vcpu: IO write @ {addr:#x}:{:#x} failed: {err}",
749+
data.len()
750+
);
743751
}
744752
METRICS.vcpu.exit_io_out.inc();
745753
}
746754
Ok(VcpuEmulation::Handled)
747755
}
748756
unexpected_exit => {
749757
METRICS.vcpu.failures.inc();
750-
error!("Unexpected exit reason on vcpu run: {:?}", unexpected_exit);
758+
error_rate_limited!("Unexpected exit reason on vcpu run: {:?}", unexpected_exit);
751759
Err(VcpuError::UnhandledKvmExit(format!(
752760
"{:?}",
753761
unexpected_exit

src/vmm/src/arch/x86_64/xstate.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
use vmm_sys_util::syscall::SyscallReturnCode;
55

66
use crate::arch::x86_64::generated::arch_prctl;
7-
use crate::logger::info;
7+
use crate::logger::info_rate_limited;
88

99
const INTEL_AMX_MASK: u64 = 1u64 << arch_prctl::ARCH_XCOMP_TILEDATA;
1010

@@ -76,7 +76,7 @@ fn get_supported_xfeatures() -> Result<Option<u64>, std::io::Error> {
7676
// version prior to v5.16).
7777
// https://github.com/torvalds/linux/commit/db8268df0983adc2bb1fb48c9e5f7bfbb5f617f3
7878
Err(err) if err.raw_os_error() == Some(libc::EINVAL) => {
79-
info!("Dynamic XSTATE feature enabling is not supported.");
79+
info_rate_limited!("Dynamic XSTATE feature enabling is not supported.");
8080
Ok(None)
8181
}
8282
Err(err) => Err(err),
@@ -117,7 +117,7 @@ fn request_xfeature_permission(xfeature: u32) -> Result<(), std::io::Error> {
117117
// https://github.com/torvalds/linux/blame/f443e374ae131c168a065ea1748feac6b2e76613/arch/x86/kvm/x86.c#L8850-L8853
118118
// https://github.com/firecracker-microvm/firecracker/pull/5065
119119
Err(err) if err.raw_os_error() == Some(libc::EINVAL) => {
120-
info!("Dynamic XSTATE feature enabling is not supported for guest.");
120+
info_rate_limited!("Dynamic XSTATE feature enabling is not supported for guest.");
121121
Ok(())
122122
}
123123
Err(err) => Err(err),

src/vmm/src/builder.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ use crate::devices::virtio::vsock::{Vsock, VsockUnixBackend};
4141
use crate::gdb;
4242
use crate::initrd::{InitrdConfig, InitrdError};
4343
use crate::logger::debug;
44+
#[cfg(target_arch = "aarch64")]
45+
use crate::logger::warn_rate_limited;
4446
use crate::persist::{MicrovmState, MicrovmStateError};
4547
use crate::resources::VmResources;
4648
use crate::seccomp::BpfThreadMap;
@@ -294,7 +296,7 @@ pub fn build_microvm_for_boot(
294296
if vcpus[0].kvm_vcpu.supports_pvtime() {
295297
setup_pvtime(&mut vm.resource_allocator(), &mut vcpus)?;
296298
} else {
297-
log::warn!("Vcpus do not support pvtime, steal time will not be reported to guest");
299+
warn_rate_limited!("Vcpus do not support pvtime, steal time will not be reported to guest");
298300
}
299301

300302
configure_system_for_boot(

src/vmm/src/cpu_config/x86_64/cpuid/normalize.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
use crate::cpu_config::x86_64::cpuid::{
55
CpuidEntry, CpuidKey, CpuidRegisters, CpuidTrait, KvmCpuidFlags, cpuid,
66
};
7-
use crate::logger::warn;
7+
use crate::logger::warn_rate_limited;
88
use crate::vmm_config::machine_config::MAX_SUPPORTED_VCPUS;
99

1010
/// Error type for [`super::Cpuid::normalize`].
@@ -379,7 +379,7 @@ impl super::Cpuid {
379379
// However, we intentionally leave Firecracker not fail for unsupported
380380
// kernels to keep working. Note that we can detect KVM regression thanks
381381
// to the test that compares a fingerprint with its baseline.
382-
warn!("Subleaf {index} not expected for CPUID leaf 0xB.");
382+
warn_rate_limited!("Subleaf {index} not expected for CPUID leaf 0xB.");
383383
subleaf.result.ecx = index;
384384
}
385385
}

0 commit comments

Comments
 (0)