Skip to content

Commit

Permalink
vmm: Deprecate static CPU templates
Browse files Browse the repository at this point in the history
Deprecate the `cpu_template` field in the `/machine-config` API, which
is used to set static CPU templates. Users can use custom CPU templates
as an alternative.

To differentiate between explicitly specifying `None` and not specifying
anything, wrap the `cpu_template` of the `MachineConfig` with `Option`.
Thanks to this, it can notify the deprecation of only users using the
field.

Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
  • Loading branch information
zulinx86 committed Sep 22, 2023
1 parent 4186c25 commit 1a6ba57
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 12 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@
- Changed the dump feature of `cpu-template-helper` tool not to enumerate program
counter (PC) on ARM because it is determined by the given kernel image and
it is useless in the custom CPU template context.
- Deprecated `cpu_template` field in `PUT` and `PATCH` request of `/machine-config`
API, which is used to set static CPU templates. Users can use custom CPU templates
as an alternative. For more details, please refer here (link to GitHub discussion
to be added).

### Fixed

Expand Down
6 changes: 6 additions & 0 deletions docs/cpu_templates/cpu-templates.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ Firecracker supports two types of CPU templates:
- Custom CPU templates - users can create their own CPU templates in json
format and pass them to Firecracker

> **Note**
Staitc CPU templates are deprecated starting with v1.5.0. It will be removed in
accordance with our policy. Even after dropping the support, users can use
custom CPU templates as an alternative. For more details, please refer here (
link to the GitHub discussion to be added).

> **Note**
CPU templates for ARM (both static and custom) require the following patch
to be available in the host kernel: [Support writable CPU ID registers from userspace](https://lore.kernel.org/kvm/20230212215830.2975485-1-jingzhangos@google.com/#t).
Expand Down
61 changes: 53 additions & 8 deletions src/api_server/src/request/machine_configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,25 @@ pub(crate) fn parse_put_machine_config(body: &Body) -> Result<ParsedRequest, Err
err
})?;

// Check for the presence of deprecated `cpu_template` field.
let mut deprecation_message = None;
if config.cpu_template.is_some() {
// `cpu_template` field in request is deprecated.
METRICS.deprecated_api.deprecated_http_api_calls.inc();
deprecation_message = Some("PUT /machine-config: cpu_template field is deprecated.");
}

// Convert `MachineConfig` to `MachineConfigUpdate`.
let config_update = MachineConfigUpdate::from(config);

Ok(ParsedRequest::new_sync(VmmAction::UpdateVmConfiguration(
config_update,
)))
// Construct the `ParsedRequest` object.
let mut parsed_req = ParsedRequest::new_sync(VmmAction::UpdateVmConfiguration(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);
}

Ok(parsed_req)
}

pub(crate) fn parse_patch_machine_config(body: &Body) -> Result<ParsedRequest, Error> {
Expand All @@ -39,9 +53,22 @@ pub(crate) fn parse_patch_machine_config(body: &Body) -> Result<ParsedRequest, E
return method_to_error(Method::Patch);
}

Ok(ParsedRequest::new_sync(VmmAction::UpdateVmConfiguration(
config_update,
)))
// Check for the presence of deprecated `cpu_template` field.
let mut deprecation_message = None;
if config_update.cpu_template.is_some() {
// `cpu_template` field in request is deprecated.
METRICS.deprecated_api.deprecated_http_api_calls.inc();
deprecation_message = Some("PATCH /machine-config: cpu_template is deprecated.");
}

// Construct the `ParsedRequest` object.
let mut parsed_req = ParsedRequest::new_sync(VmmAction::UpdateVmConfiguration(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);
}

Ok(parsed_req)
}

#[cfg(test)]
Expand Down Expand Up @@ -79,6 +106,24 @@ mod tests {
"vcpu_count": 8,
"mem_size_mib": 1024
}"#;
let expected_config = MachineConfigUpdate {
vcpu_count: Some(8),
mem_size_mib: Some(1024),
smt: Some(false),
cpu_template: None,
track_dirty_pages: Some(false),
};

match vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()) {
VmmAction::UpdateVmConfiguration(config) => assert_eq!(config, expected_config),
_ => panic!("Test failed."),
}

let body = r#"{
"vcpu_count": 8,
"mem_size_mib": 1024,
"cpu_template": "None"
}"#;
let expected_config = MachineConfigUpdate {
vcpu_count: Some(8),
mem_size_mib: Some(1024),
Expand All @@ -102,7 +147,7 @@ mod tests {
vcpu_count: Some(8),
mem_size_mib: Some(1024),
smt: Some(false),
cpu_template: Some(StaticCpuTemplate::None),
cpu_template: None,
track_dirty_pages: Some(true),
};

Expand Down Expand Up @@ -155,7 +200,7 @@ mod tests {
vcpu_count: Some(8),
mem_size_mib: Some(1024),
smt: Some(true),
cpu_template: Some(StaticCpuTemplate::None),
cpu_template: None,
track_dirty_pages: Some(true),
};

Expand Down
9 changes: 9 additions & 0 deletions src/vmm/src/cpu_config/templates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,15 @@ impl From<&Option<CpuTemplateType>> for StaticCpuTemplate {
}
}

impl From<&CpuTemplateType> for StaticCpuTemplate {
fn from(value: &CpuTemplateType) -> Self {
match value {
CpuTemplateType::Static(template) => *template,
CpuTemplateType::Custom(_) => StaticCpuTemplate::None,

Check warning on line 79 in src/vmm/src/cpu_config/templates.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/cpu_config/templates.rs#L79

Added line #L79 was not covered by tests
}
}
}

impl<'a> TryFrom<&'a [u8]> for CustomCpuTemplate {
type Error = serde_json::Error;

Expand Down
8 changes: 4 additions & 4 deletions src/vmm/src/vmm_config/machine_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ pub struct MachineConfig {
#[serde(default, deserialize_with = "deserialize_smt")]
pub smt: bool,
/// A CPU template that it is used to filter the CPU features exposed to the guest.
#[serde(default, skip_serializing_if = "StaticCpuTemplate::is_none")]
pub cpu_template: StaticCpuTemplate,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub cpu_template: Option<StaticCpuTemplate>,
/// Enables or disables dirty page tracking. Enabling allows incremental snapshots.
#[serde(default)]
pub track_dirty_pages: bool,
Expand Down Expand Up @@ -122,7 +122,7 @@ impl From<MachineConfig> for MachineConfigUpdate {
vcpu_count: Some(cfg.vcpu_count),
mem_size_mib: Some(cfg.mem_size_mib),
smt: Some(cfg.smt),
cpu_template: Some(cfg.cpu_template),
cpu_template: cfg.cpu_template,
track_dirty_pages: Some(cfg.track_dirty_pages),
}
}
Expand Down Expand Up @@ -212,7 +212,7 @@ impl From<&VmConfig> for MachineConfig {
vcpu_count: value.vcpu_count,
mem_size_mib: value.mem_size_mib,
smt: value.smt,
cpu_template: (&value.cpu_template).into(),
cpu_template: value.cpu_template.as_ref().map(|template| template.into()),
track_dirty_pages: value.track_dirty_pages,
}
}
Expand Down

0 comments on commit 1a6ba57

Please sign in to comment.