Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #36999 - Drop libvirt CPU model option for VM creation #9963

Merged
merged 1 commit into from
Jul 30, 2024

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Dec 19, 2023

Since fog-libvirt 0.12.0 the default CPU model was some abstract type with a low compatibility level. Since 0.12.0 it defaults to CPU passthrough, as recommended by qemu if live migration is not required (https://www.qemu.org/docs/master/system/qemu-cpu-models.html). That's not something we support today, so it should be safe to drop the CPU model selection.

RFC open for this: https://community.theforeman.org/t/drop-cpu-type-option-from-libvirt-compute-resource-provisioning/36267

Comment on lines -14 to -21
def cpu_mode
attributes[:cpu][:mode]
end

def cpu_mode=(cpumode)
attributes[:cpu][:mode] = (cpumode == 'default') ? nil : cpumode
end

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be that this was also how it was previously set for the API, but I'm not sure it was ever exposed properly.

@ekohl ekohl marked this pull request as ready for review April 10, 2024 09:00
@ekohl ekohl requested a review from a team as a code owner April 10, 2024 09:00
Since fog-libvirt 0.12.0 the default CPU model was some abstract type
with a low compatibility level. Since 0.12.0 it defaults to CPU
passthrough, as recommended by qemu if live migration is not required
(https://www.qemu.org/docs/master/system/qemu-cpu-models.html). That's
not something we support today, so it should be safe to drop the CPU
model selection.
@ekohl ekohl force-pushed the 36999-drop-libvirt-cpu-model branch from f96ddfb to 8097b38 Compare April 10, 2024 09:00
@ares
Copy link
Member

ares commented Jul 8, 2024

LGTM, could you please also update https://github.com/theforeman/hammer-cli-foreman/blob/master/doc/host_create.md. Given this changes the API, it should probably be mentioned in the release notes.

@ekohl
Copy link
Member Author

ekohl commented Jul 10, 2024

LGTM, could you please also update https://github.com/theforeman/hammer-cli-foreman/blob/master/doc/host_create.md. Given this changes the API, it should probably be mentioned in the release notes.

That states:

Sample help output for hammer-cli-foreman 0.15.0 and Foreman 1.20 follows:

Which is ancient.

Anyway, theforeman/hammer-cli-foreman#629.

@ehelms ehelms merged commit d7b17c5 into theforeman:develop Jul 30, 2024
39 checks passed
@ekohl ekohl deleted the 36999-drop-libvirt-cpu-model branch July 30, 2024 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants