Maintenance 3

.buildkite/common.py

@@ -273,7 +273,7 @@ def __init__(self, with_build_step=True, **kwargs):
         if not args.binary_dir and with_build_step:
             build_cmds, self.shared_build = shared_build()
             self.build_group_per_arch(
-                "🏗️ Build", build_cmds, depends_on_build=False, set_key=self.shared_build
+                "build", build_cmds, depends_on_build=False, set_key=self.shared_build
             )
         else:
             self.shared_build = None

.buildkite/pipeline_coverage.py

@@ -9,7 +9,7 @@
 pipeline = BKPipeline(with_build_step=False)
 
 pipeline.build_group(
-    ":coverage: Coverage",
+    "coverage",
     pipeline.devtool_test(
         devtool_opts="--no-build",
         pytest_opts="integration_tests/build/test_coverage.py",

.buildkite/pipeline_cpu_template.py

@@ -25,7 +25,7 @@ class BkStep(str, Enum):
         BkStep.COMMAND: [
             "tools/devtool -y test --no-build -- -m no_block_pr -n4 --dist worksteal integration_tests/functional/test_cpu_features_x86_64.py -k 'test_cpu_rdmsr' "
         ],
-        BkStep.LABEL: "📖 rdmsr",
+        BkStep.LABEL: "rdmsr",
         "instances": [
             "m5n.metal",
             "m6i.metal",
@@ -39,7 +39,7 @@ class BkStep(str, Enum):
         BkStep.COMMAND: [
             "tools/devtool -y test --no-build -- -m no_block_pr integration_tests/functional/test_cpu_template_helper.py -k test_guest_cpu_config_change",
         ],
-        BkStep.LABEL: "🖐️ fingerprint",
+        BkStep.LABEL: "fingerprint",
     },
     "cpuid_wrmsr": {
         "snapshot": {
@@ -48,7 +48,7 @@ class BkStep(str, Enum):
                 "mkdir -pv tests/snapshot_artifacts_upload/{instance}_{os}_{kv}",
                 "sudo mv tests/snapshot_artifacts/* tests/snapshot_artifacts_upload/{instance}_{os}_{kv}",
             ],
-            BkStep.LABEL: "📸 create snapshots",
+            BkStep.LABEL: "snapshot-create",
             BkStep.ARTIFACTS: "tests/snapshot_artifacts_upload/**/*",
             BkStep.TIMEOUT: 30,
         },
@@ -58,7 +58,7 @@ class BkStep(str, Enum):
                 "mv tests/snapshot_artifacts_upload/{instance}_{os}_{kv} tests/snapshot_artifacts",
                 "tools/devtool -y test --no-build -- -m nonci -n4 --dist worksteal integration_tests/functional/test_cpu_features_x86_64.py -k 'test_cpu_wrmsr_restore or test_cpu_cpuid_restore'",
             ],
-            BkStep.LABEL: "📸 load snapshot artifacts created on {instance} {snapshot_os} {snapshot_kv} to {restore_instance} {restore_os} {restore_kv}",
+            BkStep.LABEL: "snapshot-restore-src-{instance}-{snapshot_os}-{snapshot_kv}-dst-{restore_instance}-{restore_os}-{restore_kv}",
             BkStep.TIMEOUT: 30,
         },
         "cross_instances": {
@@ -137,7 +137,7 @@ def group_snapshot_restore(test_step):
         )
 
     groups.append(
-        {"group": "📸 restores snapshots", "steps": steps, "depends_on": "snapshot"}
+        {"group": "snapshot-restore", "steps": steps, "depends_on": "snapshot"}
     )
     return groups
 

.buildkite/pipeline_cross.py

@@ -36,7 +36,7 @@
         "tar cSvf snapshots/{instance}_{kv}.tar snapshot_artifacts",
     ]
     pipeline.build_group(
-        "📸 create snapshots",
+        "snapshot-create",
         commands,
         timeout=30,
         artifact_paths="snapshots/**/*",
@@ -93,13 +93,13 @@
                     pytest_opts=f"-m nonci -n8 --dist worksteal {k_val} integration_tests/functional/test_snapshot_restore_cross_kernel.py",
                 ),
             ],
-            "label": f"🎬 {src_instance} {src_kv} ➡️ {dst_instance} {dst_kv}",
+            "label": f"snapshot-restore-src-{src_instance}-{src_kv}-dst-{dst_instance}-{dst_kv}",
             "timeout": 30,
             "agents": {"instance": dst_instance, "kv": dst_kv, "os": dst_os},
             **per_instance,
         }
         steps.append(step)
     pipeline.add_step(
-        {"group": "🎬 restore across instances and kernels", "steps": steps}
+        {"group": "snapshot-restore-across-instances-and-kernels", "steps": steps}
     )
     print(pipeline.to_json())

.buildkite/pipeline_docker_popular.py

@@ -13,7 +13,7 @@
 ROOTFS_TAR = f"rootfs_$(uname -m)_{random_str(k=8)}.tar.gz"
 
 pipeline.build_group_per_arch(
-    ":ship: Rootfs build",
+    "rootfs-build",
     [
         "sudo yum install -y systemd-container",
         "cd tools/test-popular-containers",
@@ -26,7 +26,7 @@
 )
 
 pipeline.build_group(
-    ":whale: Docker Popular Containers",
+    "docker-popular-containers",
     [
         "./tools/devtool download_ci_artifacts",
         f'buildkite-agent artifact download "{ROOTFS_TAR}" .',

.buildkite/pipeline_pr.py

@@ -27,15 +27,15 @@
 pipeline.add_step(
     {
         "command": "./tools/devtool -y checkstyle",
-        "label": "🪶 Style",
+        "label": "style",
     },
     depends_on_build=False,
 )
 
 # run sanity build of devtool if Dockerfile is changed
 if any(x.parent.name == "devctr" for x in changed_files):
     pipeline.build_group_per_arch(
-        "🐋 Dev Container Sanity Build",
+        "dev-container-sanity-build",
         "./tools/devtool -y build_devctr && DEVCTR_IMAGE_TAG=latest ./tools/devtool test --no-build -- integration_tests/functional/test_api.py",
     )
 
@@ -44,7 +44,7 @@
     for x in changed_files
 ):
     pipeline.build_group_per_arch(
-        "📦 Release Sanity Build",
+        "release-sanity-build",
         "./tools/devtool -y make_release",
         depends_on_build=False,
     )
@@ -56,7 +56,7 @@
     or any(x.name == "test_kani.py" for x in changed_files)
 ):
     kani_grp = pipeline.build_group(
-        "🔍 Kani",
+        "kani",
         "./tools/devtool -y test --no-build -- ../tests/integration_tests/test_kani.py -n auto",
         # Kani step default
         # Kani runs fastest on m6a.metal
@@ -66,26 +66,23 @@
         **DEFAULTS_PERF,
         depends_on_build=False,
     )
-    # modify Kani steps' label
-    for step in kani_grp["steps"]:
-        step["label"] = "🔍 Kani"
 
 if run_all_tests(changed_files):
     pipeline.build_group(
-        "📦 Build",
+        "build",
         pipeline.devtool_test(pytest_opts="integration_tests/build/"),
         depends_on_build=False,
     )
 
     pipeline.build_group(
-        "⚙ Functional and security 🔒",
+        "functional-and-security",
         pipeline.devtool_test(
             pytest_opts="-n 16 --dist worksteal integration_tests/{{functional,security}}",
         ),
     )
 
     pipeline.build_group(
-        "⏱ Performance",
+        "performance",
         pipeline.devtool_test(
             devtool_opts="--performance -c 1-10 -m 0",
             pytest_opts="../tests/integration_tests/performance/",

.buildkite/pipeline_pr_no_block.py

@@ -19,7 +19,7 @@
 )
 
 pipeline.build_group(
-    "❓ Optional",
+    "optional",
     pipeline.devtool_test(
         devtool_opts="--performance -c 1-10 -m 0",
         pytest_opts="integration_tests/ -m no_block_pr --log-cli-level=INFO",

.buildkite/pipeline_release_qa.py

@@ -14,7 +14,7 @@
 
 pipeline.add_step(
     {
-        "label": "Download release",
+        "label": "download-release",
         "if": 'build.env("VERSION") != "dev"',
         "command": [
             "aws s3 sync --no-sign-request s3://spec.ccfc.min/firecracker-ci/firecracker/$$VERSION release-$$VERSION",
@@ -25,7 +25,7 @@
 )
 
 pipeline.build_group_per_arch(
-    ":building_construction: Make release",
+    "make-release",
     # if is a keyword for python, so we need this workaround to expand it as a kwarg
     **{"if": 'build.env("VERSION") == "dev"'},
     command=[
@@ -48,7 +48,7 @@
 # The devtool expects the examples to be in the same folder as the binaries to run some tests
 # (for example, uffd handler tests). Build them and upload them in the same folder.
 pipeline.build_group_per_arch(
-    ":hammer_and_wrench: Build examples",
+    "build-examples",
     command=[
         "CARGO_TARGET=$$(uname -m)-unknown-linux-musl",
         "./tools/devtool -y sh cargo build --target $$CARGO_TARGET --release --examples",
@@ -63,7 +63,7 @@
 
 pipeline.add_step(
     {
-        "label": ":pipeline: PR",
+        "label": "pr",
         "command": (
             ".buildkite/pipeline_pr.py --binary-dir release-$$VERSION "
             "| jq '(..|select(.priority? != null).priority) += 100' "

rustfmt.toml

@@ -0,0 +1,8 @@
+comment_width = 100
+wrap_comments = true
+format_code_in_doc_comments = true
+format_strings = true
+imports_granularity = "Module"
+normalize_comments = true
+normalize_doc_attributes = true
+group_imports = "StdExternalCrate"

src/cpu-template-helper/src/utils/x86_64.rs

@@ -194,7 +194,6 @@ mod tests {
     use vmm::cpu_config::x86_64::custom_cpu_template::CpuidRegisterModifier;
 
     use super::*;
-    use crate::utils::x86_64::{cpuid_leaf_modifier, cpuid_reg_modifier, msr_modifier};
 
     macro_rules! cpuid_modifier_map {
         ($leaf:expr, $subleaf:expr, $flags:expr, $register:expr, $value:expr) => {

src/firecracker/src/api_server/parsed_request.rs

@@ -406,12 +406,12 @@ pub mod tests {
             "{} {} HTTP/1.1\r\nContent-Type: application/json\r\n",
             request_type, endpoint
         );
-        if body.is_some() {
+        if let Some(body) = body {
             return format!(
                 "{}Content-Length: {}\r\n\r\n{}",
                 req_no_body,
-                body.unwrap().len(),
-                body.unwrap()
+                body.len(),
+                body
             );
         }
         format!("{}\r\n", req_no_body,)

src/vmm/src/arch/x86_64/interrupts.rs

@@ -111,7 +111,7 @@ mod tests {
 
         v.iter_mut()
             .for_each(|x| *x = set_apic_delivery_mode(*x, 2));
-        let after: Vec<u32> = v.iter().map(|x| ((*x & !0x700) | ((2) << 8))).collect();
+        let after: Vec<u32> = v.iter().map(|x| (*x & !0x700) | ((2) << 8)).collect();
         assert_eq!(v, after);
     }
 

src/vmm/src/builder.rs

@@ -31,6 +31,7 @@ use crate::device_manager::{
 };
 use crate::devices::virtio::balloon::Balloon;
 use crate::devices::virtio::block::device::Block;
+use crate::devices::virtio::device::VirtioDevice;
 use crate::devices::virtio::mem::{VIRTIO_MEM_DEFAULT_SLOT_SIZE_MIB, VirtioMem};
 use crate::devices::virtio::net::Net;
 use crate::devices::virtio::pmem::device::Pmem;
@@ -682,7 +683,7 @@ fn attach_net_devices<'a, I: Iterator<Item = &'a Arc<Mutex<Net>>> + Debug>(
     event_manager: &mut EventManager,
 ) -> Result<(), StartMicrovmError> {
     for net_device in net_devices {
-        let id = net_device.lock().expect("Poisoned lock").id().clone();
+        let id = net_device.lock().expect("Poisoned lock").id().to_string();
         event_manager.add_subscriber(net_device.clone());
         // The device mutex mustn't be locked here otherwise it will deadlock.
         device_manager.attach_virtio_device(vm, id, net_device.clone(), cmdline, false)?;
@@ -753,7 +754,7 @@ pub(crate) mod tests {
     use super::*;
     use crate::device_manager::tests::default_device_manager;
     use crate::devices::virtio::block::CacheType;
-    use crate::devices::virtio::generated::virtio_ids;
+    use crate::devices::virtio::device::VirtioDeviceType;
     use crate::devices::virtio::rng::device::ENTROPY_DEV_ID;
     use crate::devices::virtio::vsock::VSOCK_DEV_ID;
     use crate::mmds::data_store::{Mmds, MmdsVersion};
@@ -955,7 +956,7 @@ pub(crate) mod tests {
 
         assert!(
             vmm.device_manager
-                .get_virtio_device(virtio_ids::VIRTIO_ID_VSOCK, &vsock_dev_id)
+                .get_virtio_device(VirtioDeviceType::Vsock, &vsock_dev_id)
                 .is_some()
         );
     }
@@ -980,7 +981,7 @@ pub(crate) mod tests {
 
         assert!(
             vmm.device_manager
-                .get_virtio_device(virtio_ids::VIRTIO_ID_RNG, ENTROPY_DEV_ID)
+                .get_virtio_device(VirtioDeviceType::Rng, ENTROPY_DEV_ID)
                 .is_some()
         );
     }
@@ -1044,7 +1045,7 @@ pub(crate) mod tests {
 
         assert!(
             vmm.device_manager
-                .get_virtio_device(virtio_ids::VIRTIO_ID_BALLOON, BALLOON_DEV_ID)
+                .get_virtio_device(VirtioDeviceType::Balloon, BALLOON_DEV_ID)
                 .is_some()
         );
     }
@@ -1095,7 +1096,7 @@ pub(crate) mod tests {
             assert!(cmdline_contains(&cmdline, "root=/dev/vda ro"));
             assert!(
                 vmm.device_manager
-                    .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str())
+                    .get_virtio_device(VirtioDeviceType::Block, drive_id.as_str())
                     .is_some()
             );
         }
@@ -1116,7 +1117,7 @@ pub(crate) mod tests {
             assert!(cmdline_contains(&cmdline, "root=PARTUUID=0eaa91a0-01 rw"));
             assert!(
                 vmm.device_manager
-                    .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str())
+                    .get_virtio_device(VirtioDeviceType::Block, drive_id.as_str())
                     .is_some()
             );
         }
@@ -1138,7 +1139,7 @@ pub(crate) mod tests {
             assert!(!cmdline_contains(&cmdline, "root=/dev/vda"));
             assert!(
                 vmm.device_manager
-                    .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str())
+                    .get_virtio_device(VirtioDeviceType::Block, drive_id.as_str())
                     .is_some()
             );
         }
@@ -1175,17 +1176,17 @@ pub(crate) mod tests {
             assert!(cmdline_contains(&cmdline, "root=PARTUUID=0eaa91a0-01 rw"));
             assert!(
                 vmm.device_manager
-                    .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, "root")
+                    .get_virtio_device(VirtioDeviceType::Block, "root")
                     .is_some()
             );
             assert!(
                 vmm.device_manager
-                    .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, "secondary")
+                    .get_virtio_device(VirtioDeviceType::Block, "secondary")
                     .is_some()
             );
             assert!(
                 vmm.device_manager
-                    .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, "third")
+                    .get_virtio_device(VirtioDeviceType::Block, "third")
                     .is_some()
             );
 
@@ -1214,7 +1215,7 @@ pub(crate) mod tests {
             assert!(cmdline_contains(&cmdline, "root=/dev/vda rw"));
             assert!(
                 vmm.device_manager
-                    .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str())
+                    .get_virtio_device(VirtioDeviceType::Block, drive_id.as_str())
                     .is_some()
             );
         }
@@ -1235,7 +1236,7 @@ pub(crate) mod tests {
             assert!(cmdline_contains(&cmdline, "root=PARTUUID=0eaa91a0-01 ro"));
             assert!(
                 vmm.device_manager
-                    .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str())
+                    .get_virtio_device(VirtioDeviceType::Block, drive_id.as_str())
                     .is_some()
             );
         }
@@ -1256,7 +1257,7 @@ pub(crate) mod tests {
             assert!(cmdline_contains(&cmdline, "root=/dev/vda rw"));
             assert!(
                 vmm.device_manager
-                    .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str())
+                    .get_virtio_device(VirtioDeviceType::Block, drive_id.as_str())
                     .is_some()
             );
         }
@@ -1279,7 +1280,7 @@ pub(crate) mod tests {
         assert!(cmdline_contains(&cmdline, "root=/dev/pmem0 ro"));
         assert!(
             vmm.device_manager
-                .get_virtio_device(virtio_ids::VIRTIO_ID_PMEM, id.as_str())
+                .get_virtio_device(VirtioDeviceType::Pmem, id.as_str())
                 .is_some()
         );
     }